Opened 11 years ago
Closed 8 years ago
#11918 closed defect (fixed)
Sage should ship its Valgrind suppressions file, or not insist on its presence
Reported by: | leif | Owned by: | tbd |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | scripts | Keywords: | --valgrind --memcheck sage.supp suppressions |
Cc: | roed, SimonKing | Merged in: | |
Authors: | Reviewers: | Volker Braun | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
As reported on the IRC, running sage --valgrind
(alias sage --memcheck
) is apparently broken because the suppressions file is missing (and its directory, $SAGE_LOCAL/lib/valgrind/
, doesn't exist).
Both sage-valgrind
and sage-doctest
seem to hardcode $SAGE_LOCAL/lib/valgrind/sage.supp
.
Unless we again(?) create and ship this file, we could at least add
if [[ ! -f "$SAGE_LOCAL"/lib/valgrind/sage.supp ]]; then mkdir -p "$SAGE_LOCAL"/lib/valgrind touch "$SAGE_LOCAL"/lib/valgrind/sage.supp fi
to sage-valgrind
.
Using variables (perhaps also specifiable by the user) for both the directory and the filename would be better of course.
And / or only pass --suppressions=...
if the file really exists.
Attachments (1)
Change History (21)
comment:1 Changed 11 years ago by
- Owner changed from leif to tbd
comment:2 Changed 11 years ago by
- Description modified (diff)
comment:3 follow-up: ↓ 4 Changed 11 years ago by
comment:4 in reply to: ↑ 3 Changed 11 years ago by
Replying to aapitzsch:
sage.supp is part of the optional valgrind package which BTW is out of date.
Yes, but it's IMHO pointless to install an (almost always) obsolete spkg just to get a file which consists of only a few lines. It should be part of the standard distribution.
(More recent valgrind
versions are either already installed or at least available as a "native" package in any reasonable distro. Moreover, I don't think one would only use it for Sage. Note also that usually valgrind
has to be updated each time new processor instructions / ISA extensions come up.)
Not testing for its presence (btw. without any meaningful error message from Sage's side) is certainly a bug.
"The suppression file gets rid of a bunch of annoying issues introduced by zlib and Cython."
Is this still necessary?
I think so, but it should be kept up-to-date anyway.
comment:5 Changed 11 years ago by
- Keywords --valgrind added
- Summary changed from `sage --valgrind` etc. apparently broken to Sage should ship its Valgrind suppressions file, or not insist on its presence
comment:6 Changed 11 years ago by
P.S.: Thanks for bringing this ticket back to my mind. We recently had some discussion on the IRC regarding the spkg, but I completely forgot about the ticket... ;-)
comment:7 Changed 10 years ago by
- Cc roed added
comment:8 Changed 10 years ago by
- Cc SimonKing added
Changed 10 years ago by
sage-valgrind suppression file (store as $SAGE_LOCAL/lib/valgrind/sage.supp)
comment:9 Changed 10 years ago by
comment:10 Changed 10 years ago by
Hi Nils,
thank you for the sage.supp at #11918! When I run the tests of sage/rings/polynomial/infinite_polynomial_ring.py (which make Volker's patchbot at #12876 segfault, even though they are fine for me, I do not get a SIGILL. But I do get a considerable amount of lost memory:
==13541== 13,936 bytes in 1 blocks are definitely lost in loss record 8,673 of 8,997 ==13541== at 0x4C244E8: malloc (vg_replace_malloc.c:236) ==13541== by 0x21E8984F: omAllocFromSystem (omAllocSystem.c:184) ==13541== by 0x21E89A21: omAllocLarge (omAllocSystem.c:39) ==13541== by 0x21BB3A00: iiAllStart(procinfo*, char*, feBufferTypes, int) (omalloc.h:2432) ==13541== by 0x21BB3B95: iiPStart(idrec*, sleftv*) (iplib.cc:360) ==13541== by 0x21BB4148: iiMake_proc(idrec*, sip_package*, sleftv*) (iplib.cc:482) ==13541== by 0x2239B64D: __pyx_f_4sage_4libs_8singular_8function_call_function(__pyx_obj_4sage_4libs_8singular_8function_SingularFunction*, _object*, _object*, __pyx_opt_args_4sage_4libs_8singular_8f unction_call_function*) (function.cpp:13241) ==13541== by 0x2239CBA8: __pyx_pw_4sage_4libs_8singular_8function_16SingularFunction_5__call__(_object*, _object*, _object*) (function.cpp:11924) ==13541== by 0x4E742C2: PyObject_Call (abstract.c:2529) ==13541== by 0x4F160FC: PyEval_EvalFrameEx (ceval.c:4239) ==13541== by 0x4F19124: PyEval_EvalCodeEx (ceval.c:3253) ==13541== by 0x4E9C122: function_call (funcobject.c:526) ==13541== by 0x4E742C2: PyObject_Call (abstract.c:2529) ==13541== by 0x4F14C59: PyEval_EvalFrameEx (ceval.c:4334) ==13541== by 0x4F19124: PyEval_EvalCodeEx (ceval.c:3253) ==13541== by 0x4E9C122: function_call (funcobject.c:526) ==13541== by 0x4E742C2: PyObject_Call (abstract.c:2529) ==13541== by 0x4F14C59: PyEval_EvalFrameEx (ceval.c:4334) ==13541== by 0x4F19124: PyEval_EvalCodeEx (ceval.c:3253) ==13541== by 0x4E9C122: function_call (funcobject.c:526) ==13541== by 0x4E742C2: PyObject_Call (abstract.c:2529) ==13541== by 0xB29B841: __pyx_pw_4sage_4misc_9cachefunc_12CachedMethod_3_instance_call (cachefunc.c:9733) ==13541== by 0x4E742C2: PyObject_Call (abstract.c:2529) ==13541== by 0xB29C7D4: __pyx_pw_4sage_4misc_9cachefunc_18CachedMethodCaller_7__call__ (cachefunc.c:7254) ==13541== by 0x4E742C2: PyObject_Call (abstract.c:2529)
Is there a way to find out what singular_function or what cached method are involved?
comment:11 follow-up: ↓ 12 Changed 10 years ago by
Isn't it telling you?
==13541== by 0x2239B64D: __pyx_f_4sage_4libs_8singular_8function_call_function(__pyx_obj_4sage_4libs_8singular_8function_SingularFunction*, _object*, _object*, __pyx_f_4sage_4libs_8singular_8function_call_function(__pyx_obj_4sage_4libs_8singular_8function_SingularFunction*, _object*, _object*, __pyx_opt_args_4sage_4libs_8singular_8f unction_call_function*) (function.cpp:13241) ==13541== by 0x2239CBA8: __pyx_pw_4sage_4libs_8singular_8function_16SingularFunction_5__call__(_object*, _object*, _object*) (function.cpp:11924)
You should be able to look that line up: (function.cpp:11924)
. I suppose that's a cython generated file, so the context there will tell you which function this is in the corresponding function.pyx
file.
I have no experience with valgrind myself. However, I think Python's memory management can confuse valgrind quite a bit. I was actually more hoping for a "double free" or "access to freed memory block" type error (i.e., illegal use of a pointer value.)
It may well be that my SIGILL is indeed a matter of mpfr on Corei7 compiling to something that's too fancy for valgrind and not a pointer to an error.
comment:12 in reply to: ↑ 11 ; follow-up: ↓ 13 Changed 10 years ago by
Replying to nbruin:
Isn't it telling you?
No, it isn't. It just tells that it is a singular_function (as defined in sage.libs.singular.function), but it could be any function of Singular (std, slimgb, reduce, system, ...)
comment:13 in reply to: ↑ 12 Changed 10 years ago by
Replying to SimonKing:
Replying to nbruin:
Isn't it telling you?
No, it isn't. It just tells that it is a singular_function (as defined in sage.libs.singular.function), but it could be any function of Singular (std, slimgb, reduce, system, ...)
Oh, right. That's going to just as opaque as debugging python code with gdb then. I guess you could try and set a breakpoint at the function and then investigate the arguments? It's triggering iiPStart
though. That might tell you something?
Anyway, given that there's a good chance this is a false positive anyway, perhaps this call sequence might not be the one to concentrate on. I'd imagine omalloc
plays tricks that would confuse valgrind (it's an advanced memory manager after all!), so malloc
"losing" memory doesn't sound particularly worrisome to me.
comment:14 Changed 9 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:15 Changed 9 years ago by
Ping! Is anybody inclined to fix this? Sorry that my knowledge of valgrind is too limited for doing this myself.
comment:16 Changed 9 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:17 Changed 8 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:18 Changed 8 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:19 Changed 8 years ago by
- Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
- Status changed from new to needs_review
This has been fixed with #15586.
comment:20 Changed 8 years ago by
- Resolution set to fixed
- Reviewers set to Volker Braun
- Status changed from needs_review to closed
sage.supp is part of the optional valgrind package which BTW is out of date.
According to http://groups.google.com/group/sage-devel/browse_thread/thread/1657cccac33c9dd7
Is this still necessary?