Opened 5 years ago

Closed 5 years ago

#22068 closed defect (fixed)

Segfault when substituting NaN in symbolic expression

Reported by: jdemeyer Owned by:
Priority: critical Milestone: sage-7.5
Component: symbolics Keywords:
Cc: rws 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 rws)

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 5 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 5 years ago by rws

  • Description modified (diff)

comment:4 Changed 5 years ago by rws

  • Branch set to u/rws/segfault_when_substituting_nan_in_symbolic_expression

comment:5 Changed 5 years ago by rws

  • Authors set to Ralf Stephan
  • Commit set to 30d0199625147d630c3d1784206df03e02f160cd
  • Status changed from new to needs_review

New commits:

30d019922068: Segfault when substituting NaN in symbolic expression

comment:6 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_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 5 years ago by jdemeyer

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

comment:8 Changed 5 years ago by git

  • Commit changed from 30d0199625147d630c3d1784206df03e02f160cd to 3c6f0415fa5ea190e9251733fa6c9d58679d9899

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

3c6f04122068: address reviewer's comments

comment:9 Changed 5 years ago by rws

  • Status changed from needs_work to needs_review

comment:10 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Same problem for

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

comment:11 Changed 5 years ago by jdemeyer

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 5 years ago by git

  • Commit changed from 3c6f0415fa5ea190e9251733fa6c9d58679d9899 to d0fba8412612599cbcb22deb49c24541b40a0dbe

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

d0fba8422068: address reviewer's comments

comment:13 Changed 5 years ago by rws

  • Status changed from needs_work to needs_review

New commits:

d0fba8422068: address reviewer's comments

comment:14 Changed 5 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_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 5 years ago by vbraun

  • Branch changed from u/rws/segfault_when_substituting_nan_in_symbolic_expression to d0fba8412612599cbcb22deb49c24541b40a0dbe
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.