Opened 3 years ago

Closed 2 years ago

#28538 closed defect (fixed)

Segfault for boolean evaluation of expression with assumptions

Reported by: Thierry Monteil Owned by:
Priority: critical Milestone: sage-9.2
Component: symbolics Keywords:
Cc: Ralf Stephan, Karl-Dieter Crisman Merged in:
Authors: Thierry Monteil Reviewers: Matthias Koeppe
Report Upstream: Fixed upstream, in a later stable release. Work issues:
Branch: cb70171 (Commits, GitHub, GitLab) Commit: cb701710ac20b6a1b74505b44d06ca5c15412fd2
Dependencies: #30063 Stopgaps:

Status badges

Description (last modified by Thierry Monteil)

As reported on this ask question:

sage: x, y = var('x, y')
sage: assume(x>0)
sage: assume(y>0)
sage: bool(y*(x-y)==0)

leads to (on my computer 8.9.rc1) a sequence of:

;;;
;;; Detected access to protected memory, also kwown as 'bus or segmentation fault'.
;;; Jumping to the outermost toplevel prompt
;;;

followed by a Segmentation fault crash of Sage.

Or (as reported, on 8.8):

RuntimeError: ECL says: C-STACK overflow at size 1048576. Stack can probably be resized. Proceed with caution.

Exchanging x and y works correctly:

sage: x, y = var('x, y')
sage: assume(x>0)
sage: assume(y>0)
sage: bool(x*(y-x)==0)
False

Upstream ticket: https://sourceforge.net/p/maxima/bugs/3583/

Change History (22)

comment:1 Changed 3 years ago by Emmanuel Charpentier

On 8.9.rc1+#28534 (Python 3-based), I get a lot of

;;;
;;; Detected access to protected memory, also kwown as 'bus or segmentation fault'.
;;; Jumping to the outermost toplevel prompt
;;;

and a Sage crash:

Process Sage erreur de segmentation

Nice one...

Last edited 3 years ago by Emmanuel Charpentier (previous) (diff)

comment:2 Changed 3 years ago by Thierry Monteil

Description: modified (diff)

comment:3 Changed 3 years ago by Thierry Monteil

Cc: Ralf Stephan Karl-Dieter Crisman added

comment:4 Changed 3 years ago by Karl-Dieter Crisman

I'm so sorry I just don't have time any more to track down as many of these (though once in a while I somehow make the time). But I think the best thing to do is to do whatever bool does in Maxima-in-sage sage -maxima and then load exactly the packages preloaded by Sage - the complex domain is usually the most suspicious one on these fronts, though I have to say this is really puzzling. I imagine bool calls a comparison with zero at some point in Maxima, though I don't recall any more because I wasn't involved with the comparison-with-zero code much.

comment:5 Changed 3 years ago by Nils Bruin

In maxima:

domain: complex;
assume(x>0,y>0);
is(equal(y*(x-y),0));

replicates the crash. That's sufficient to report upstream. Perhaps they can fix it.

comment:6 Changed 3 years ago by Thierry Monteil

Description: modified (diff)
Report Upstream: N/AReported upstream. No feedback yet.

Thanks for tracking, this is now tracked upstream as https://sourceforge.net/p/maxima/bugs/3583/

comment:7 Changed 3 years ago by Thierry Monteil

Report Upstream: Reported upstream. No feedback yet.Fixed upstream, in a later stable release.

comment:8 Changed 3 years ago by Erik Bray

Milestone: sage-9.0sage-9.1

Ticket retargeted after milestone closed

comment:9 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.1sage-9.2

comment:10 Changed 2 years ago by Thierry Monteil

Dependencies: #30063

Once #30063 will be merged, i will make a patch to doctest that ticket.

comment:11 in reply to:  10 ; Changed 2 years ago by Emmanuel Charpentier

Replying to tmonteil:

Once #30063 will be merged, i will make a patch to doctest that ticket.

The original problem seems fixed by #30063 :

sage: x, y = var('x, y') 
....: assume(x>0) 
....: assume(y>0) 
....: bool(y*(x-y)==0)                                                          
False

Testing it properly might be a bit tricky, tough...

comment:12 in reply to:  11 ; Changed 2 years ago by Thierry Monteil

Replying to charpent:

Replying to tmonteil:

Once #30063 will be merged, i will make a patch to doctest that ticket.

The original problem seems fixed by #30063 :

This is why i am waiting #30063 to be merged to add a doctest.

sage: x, y = var('x, y') 
....: assume(x>0) 
....: assume(y>0) 
....: bool(y*(x-y)==0)                                                          
False

Testing it properly might be a bit tricky, tough...

What is wrong with using the raw example ?

comment:13 in reply to:  12 ; Changed 2 years ago by Emmanuel Charpentier

Replying to tmonteil:

[ Snip... ]

Testing it properly might be a bit tricky, tough...

What is wrong with using the raw example ?

Too special case... I am not sure what the original problem was.

comment:14 Changed 2 years ago by Thierry Monteil

Branch: u/tmonteil/segfault_for_boolean_evaluation_of_expression_with_assumptions

comment:15 Changed 2 years ago by Thierry Monteil

Authors: Thierry Monteil
Commit: fe71c17ac3270f68eae91648f90cadcabaa48d47
Status: newneeds_review

New commits:

fe71c17#28538 : add doctest for #28538

comment:16 in reply to:  13 Changed 2 years ago by Thierry Monteil

Replying to charpent:

Replying to tmonteil:

[ Snip... ]

Testing it properly might be a bit tricky, tough...

What is wrong with using the raw example ?

Too special case... I am not sure what the original problem was.

This is a Maxima bug, it was reported and fixed upstream. I do not have the skill to inspect further within Maxima source code, so this doctest is the best i can provide, and it corresponds to the reported bug. If someone could provide more doctests to surround the original problem more securely, i am all for it.

comment:17 Changed 2 years ago by Karl-Dieter Crisman

I think that testing the original problem no longer leads to a segfault is fine. I agree that this is all we can do if Maxima upstream fixed it and we don't really know what the issue was. (Though it looks like it was, again, our use of domain:complex that triggered it.)

comment:18 Changed 2 years ago by Sébastien Labbé

Status: needs_reviewneeds_work

Typo: Chack -> Check

comment:19 Changed 2 years ago by git

Commit: fe71c17ac3270f68eae91648f90cadcabaa48d47cb701710ac20b6a1b74505b44d06ca5c15412fd2

Branch pushed to git repo; I updated commit sha1. New commits:

cb70171#28538 : typo

comment:20 Changed 2 years ago by Thierry Monteil

Status: needs_workneeds_review

Fixed, thanks for pointing this !

comment:21 Changed 2 years ago by Matthias Köppe

Reviewers: Matthias Koeppe
Status: needs_reviewpositive_review

comment:22 Changed 2 years ago by Volker Braun

Branch: u/tmonteil/segfault_for_boolean_evaluation_of_expression_with_assumptionscb701710ac20b6a1b74505b44d06ca5c15412fd2
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.