Opened 6 years ago

Closed 6 years ago

#22068 closed defect (fixed)

Segfault when substituting NaN in symbolic expression

Reported by: Jeroen Demeyer Owned by:
Priority: critical Milestone: sage-7.5
Component: symbolics Keywords:
Cc: Ralf Stephan Merged in:
Authors: Ralf Stephan Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: d0fba84 (Commits, GitHub, GitLab) Commit: d0fba8412612599cbcb22deb49c24541b40a0dbe
Dependencies: Stopgaps:

Status badges

Description (last modified by Ralf Stephan)

sage: sin(x).subs(x=RR('NaN'))
Segmentation fault

GDB shows an infinite recursion (leading to stack overflow):

#0  0x00007fff59b974d5 in info (inf=16, this=<optimized out>) at ex.h:145
#1  GiNaC::sin_eval (x=...) at inifcns_trig.cpp:68
#2  0x00007fff59b56570 in GiNaC::function::eval (this=0x7fffff800490, level=<optimized out>) at function.cpp:876
#3  0x00007fff59b3f13e in GiNaC::ex::construct_from_basic (other=...) at ex.cpp:639
#4  0x00007fff59b97899 in ex (other=..., this=0x7fffff800440) at ex.h:297
#5  GiNaC::sin_eval (x=...) at inifcns_trig.cpp:77
#6  0x00007fff59b56570 in GiNaC::function::eval (this=0x7fffff801470, level=<optimized out>) at function.cpp:876
#7  0x00007fff59b3f13e in GiNaC::ex::construct_from_basic (other=...) at ex.cpp:639
#8  0x00007fff59b97899 in ex (other=..., this=0x7fffff801420) at ex.h:297
#9  GiNaC::sin_eval (x=...) at inifcns_trig.cpp:77
#10 0x00007fff59b56570 in GiNaC::function::eval (this=0x7fffff802450, level=<optimized out>) at function.cpp:876
#11 0x00007fff59b3f13e in GiNaC::ex::construct_from_basic (other=...) at ex.cpp:639
#12 0x00007fff59b97899 in ex (other=..., this=0x7fffff802400) at ex.h:297
#13 GiNaC::sin_eval (x=...) at inifcns_trig.cpp:77
#14 0x00007fff59b56570 in GiNaC::function::eval (this=0x7fffff803430, level=<optimized out>) at function.cpp:876
#15 0x00007fff59b3f13e in GiNaC::ex::construct_from_basic (other=...) at ex.cpp:639
#16 0x00007fff59b97899 in ex (other=..., this=0x7fffff8033e0) at ex.h:297
#17 GiNaC::sin_eval (x=...) at inifcns_trig.cpp:77
#18 0x00007fff59b56570 in GiNaC::function::eval (this=0x7fffff804410, level=<optimized out>) at function.cpp:876
#19 0x00007fff59b3f13e in GiNaC::ex::construct_from_basic (other=...) at ex.cpp:639
#20 0x00007fff59b97899 in ex (other=..., this=0x7fffff8043c0) at ex.h:297
#21 GiNaC::sin_eval (x=...) at inifcns_trig.cpp:77
[...]

Reason one is that for the negativity check of a PyObject Pynac checks if it's real and if so, call a Python LT compare with 0. The reality check already should fail.

sage: RR('NaN').is_real()    # bug
True
sage: NaN.is_real()
False

Secondly, coercion of RR('NaN') into SR should return the symbolic constant NaN which does not have these problems. The ticket should fix both.

Change History (15)

comment:1 Changed 6 years ago by Jeroen Demeyer

Description: modified (diff)

comment:2 Changed 6 years ago by Jeroen Demeyer

Description: modified (diff)

comment:3 Changed 6 years ago by Ralf Stephan

Description: modified (diff)

comment:4 Changed 6 years ago by Ralf Stephan

Branch: u/rws/segfault_when_substituting_nan_in_symbolic_expression

comment:5 Changed 6 years ago by Ralf Stephan

Authors: Ralf Stephan
Commit: 30d0199625147d630c3d1784206df03e02f160cd
Status: newneeds_review

New commits:

30d019922068: Segfault when substituting NaN in symbolic expression

comment:6 Changed 6 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

I'm not convinced about

if x == RR('NaN')

I would prefer to see a type-specific check. For example, Integer can never be NaN. And for float or complex, it seems overkill to involve the coercion model just to check whether the value is NaN. And for elements of RR, you could use the is_NaN() method.

comment:7 Changed 6 years ago by Jeroen Demeyer

You should add a doctest for sin(x).subs(x=RR('NaN')) which is the original issue of this ticket.

comment:8 Changed 6 years ago by git

Commit: 30d0199625147d630c3d1784206df03e02f160cd3c6f0415fa5ea190e9251733fa6c9d58679d9899

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

3c6f04122068: address reviewer's comments

comment:9 Changed 6 years ago by Ralf Stephan

Status: needs_workneeds_review

comment:10 Changed 6 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

Same problem for

sin(x).subs(x=float('NaN'))

comment:11 Changed 6 years ago by Jeroen Demeyer

By the way, a useful trick to check for NaN is a != a: that should be true exactly when a is NaN. Unfortunately, this does not currently work correctly for RR but it does work for float and complex.

comment:12 Changed 6 years ago by git

Commit: 3c6f0415fa5ea190e9251733fa6c9d58679d9899d0fba8412612599cbcb22deb49c24541b40a0dbe

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

d0fba8422068: address reviewer's comments

comment:13 Changed 6 years ago by Ralf Stephan

Status: needs_workneeds_review

New commits:

d0fba8422068: address reviewer's comments

comment:14 Changed 6 years ago by Jeroen Demeyer

Reviewers: Jeroen Demeyer
Status: needs_reviewpositive_review

I find the output sin(NaN) a bit strange (surely just NaN would be better), but anything is better than a segfault.

comment:15 Changed 6 years ago by Volker Braun

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