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
Fix failing assertion in linbox/matrix/permutation-matrix.h #13878
Comments
comment:1
The gdb backtrace is as follows:
Question: Is that an upstream bug or a bug in the Sage library? |
comment:2
It looks like trivial spans in large ZZ-modules are to blame:
runs OK, but with |
comment:3
I have commented on sage-devel, this is totally a DNDEBUG problem. We got slammed by it in sage-on-gentoo when linbox was updated since our python doesn't add DNDEBUG either. See cschwan/sage-on-gentoo#166. |
comment:4
Offending code is this:
This gets called with |
Attachment: trac_13878-preliminary.patch.gz preliminary fix |
comment:5
preliminary patch attached. It does make the annoying SIGABRTs go away, so it allows testing for other issues. I'm not particularly promoting this patch for general use (it'll be a bit of a speed penalty and I'm not so sure it's required for proper functioning under normal conditions) |
comment:6
I think it would be interesting to be able to use linbox without having to add DNDEBUG, it is a good thing in my book. Your fix seem minimal to me. Since sage-on-gentoo do not compile with DNDEBUG will I be able to open bugs each time I get a signal from linbox if we go that route :) |
comment:7
Hooray! With you preliminary patch and MALLOC_CHECK_=3, I get
What is needed to do to finalise your preliminary patch? Adding the above example as a doc test would be half cheating, because this used only to fail in debug mode. But still, one could include this example and comment on it accordingly. |
comment:8
Replying to @simon-king-jena:
At first blush it seems this is an upstream bug, so this would only be a workaround, not a proper fix. Furthermore, it seems this code seems to work in an NDEBUG linbox, so perhaps the test is just overzealous. I haven't checked linbox's documentation to see if they declare this input illegal. Other than that, there already are doctests that test this issue, so a comment should be enough documentation for this fix. |
This comment has been minimized.
This comment has been minimized.
Author: Nils Bruin |
comment:9
Attachment: trac_13878-reviewer.patch.gz |
comment:10
It might ultimately be an upstream bug. But I think fixing it on our end is fine. I suggest to get this in, so that Sage finally has a working debug version. Positive review. I documented the change in a comment, in my reviewer patch. |
Reviewer: Simon King |
This comment has been minimized.
This comment has been minimized.
Merged: sage-5.7.beta4 |
With the Sage debug version of #13864, one gets the following crash, as reported by Nils:
APPLY
CC: @nbruin @vbraun @jpflori @malb
Component: linear algebra
Keywords: debug, linbox, assertion
Author: Nils Bruin
Reviewer: Simon King
Merged: sage-5.7.beta4
Issue created by migration from https://trac.sagemath.org/ticket/13878
The text was updated successfully, but these errors were encountered: