Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sage should ship its Valgrind suppressions file, or not insist on its presence #11918

Closed
nexttime mannequin opened this issue Oct 13, 2011 · 14 comments
Closed

Sage should ship its Valgrind suppressions file, or not insist on its presence #11918

nexttime mannequin opened this issue Oct 13, 2011 · 14 comments

Comments

@nexttime
Copy link
Mannequin

nexttime mannequin commented Oct 13, 2011

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.

CC: @roed314 @simon-king-jena

Component: scripts

Keywords: --valgrind --memcheck sage.supp suppressions

Reviewer: Volker Braun

Issue created by migration from https://trac.sagemath.org/ticket/11918

@nexttime nexttime mannequin added this to the sage-5.11 milestone Oct 13, 2011
@nexttime nexttime mannequin added c: scripts labels Oct 13, 2011
@nexttime nexttime mannequin assigned nexttime and unassigned nexttime Oct 13, 2011
@nexttime

This comment has been minimized.

@a-andre
Copy link

a-andre commented Oct 26, 2011

comment:3

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
The suppression file gets rid of a bunch of annoying issues introduced by zlib and Cython.

Is this still necessary?

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Oct 26, 2011

comment:4

Replying to @a-andre:

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.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Oct 26, 2011

Changed keywords from --memcheck sage.supp suppressions to --valgrind --memcheck sage.supp suppressions

@nexttime nexttime mannequin changed the title sage --valgrind etc. apparently broken Sage should ship its Valgrind suppressions file, or not insist on its presence Oct 26, 2011
@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Oct 26, 2011

comment:6

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... ;-)

@nbruin
Copy link
Contributor

nbruin commented Aug 30, 2012

Attachment: sage.supp.gz

sage-valgrind suppression file (store as $SAGE_LOCAL/lib/valgrind/sage.supp)

@nbruin
Copy link
Contributor

nbruin commented Aug 30, 2012

comment:9

Until this ticket is fixed, I've attached attachment: sage.supp, which might help people with using valgrind in the mean time. File should be at
$SAGE_LOCAL/lib/valgrind/sage.supp to be picked up. NOTE: This is sage-liberal.supp from the spkg.

@simon-king-jena
Copy link
Member

comment:10

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?

@nbruin
Copy link
Contributor

nbruin commented Aug 30, 2012

comment:11

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.

@simon-king-jena
Copy link
Member

comment:12

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, ...)

@nbruin
Copy link
Contributor

nbruin commented Aug 30, 2012

comment:13

Replying to @simon-king-jena:

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.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@simon-king-jena
Copy link
Member

comment:15

Ping! Is anybody inclined to fix this? Sorry that my knowledge of valgrind is too limited for doing this myself.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@a-andre
Copy link

a-andre commented Oct 12, 2014

comment:19

This has been fixed with #15586.

@a-andre a-andre removed this from the sage-6.4 milestone Oct 12, 2014
@vbraun
Copy link
Member

vbraun commented Oct 25, 2014

Reviewer: Volker Braun

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants