Opened 14 months ago

Closed 3 months ago

#28538 closed defect (fixed)

Segfault for boolean evaluation of expression with assumptions

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

Description (last modified by tmonteil)

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 14 months ago by charpent

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 14 months ago by charpent (previous) (diff)

comment:2 Changed 14 months ago by tmonteil

  • Description modified (diff)

comment:3 Changed 14 months ago by tmonteil

  • Cc rws kcrisman added

comment:4 Changed 14 months ago by kcrisman

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 14 months ago by nbruin

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 14 months ago by tmonteil

  • Description modified (diff)
  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

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

comment:7 Changed 13 months ago by tmonteil

  • Report Upstream changed from Reported upstream. No feedback yet. to Fixed upstream, in a later stable release.

comment:8 Changed 11 months ago by embray

  • Milestone changed from sage-9.0 to sage-9.1

Ticket retargeted after milestone closed

comment:9 Changed 7 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

comment:10 follow-up: Changed 4 months ago by tmonteil

  • Dependencies set to #30063

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

comment:11 in reply to: ↑ 10 ; follow-up: Changed 4 months ago by 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 :

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 ; follow-up: Changed 4 months ago by tmonteil

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 ; follow-up: Changed 4 months ago by 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.

comment:14 Changed 3 months ago by tmonteil

  • Branch set to u/tmonteil/segfault_for_boolean_evaluation_of_expression_with_assumptions

comment:15 Changed 3 months ago by tmonteil

  • Authors set to Thierry Monteil
  • Commit set to fe71c17ac3270f68eae91648f90cadcabaa48d47
  • Status changed from new to needs_review

New commits:

fe71c17#28538 : add doctest for #28538

comment:16 in reply to: ↑ 13 Changed 3 months ago by tmonteil

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 3 months ago by kcrisman

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 3 months ago by slabbe

  • Status changed from needs_review to needs_work

Typo: Chack -> Check

comment:19 Changed 3 months ago by git

  • Commit changed from fe71c17ac3270f68eae91648f90cadcabaa48d47 to cb701710ac20b6a1b74505b44d06ca5c15412fd2

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

cb70171#28538 : typo

comment:20 Changed 3 months ago by tmonteil

  • Status changed from needs_work to needs_review

Fixed, thanks for pointing this !

comment:21 Changed 3 months ago by mkoeppe

  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

comment:22 Changed 3 months ago by vbraun

  • Branch changed from u/tmonteil/segfault_for_boolean_evaluation_of_expression_with_assumptions to cb701710ac20b6a1b74505b44d06ca5c15412fd2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.