Opened 8 months ago
Closed 6 months ago
#32602 closed defect (fixed)
Magma interface throws segfault
Reported by:  kedlaya  Owned by:  

Priority:  major  Milestone:  sage9.5 
Component:  interfaces  Keywords:  Magma, booleans 
Cc:  Merged in:  
Authors:  Kiran Kedlaya  Reviewers:  Nils Bruin, Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  46d7eac (Commits, GitHub, GitLab)  Commit:  46d7eac7c6b2ca798bcaa958043478c31b18b05a 
Dependencies:  Stopgaps: 
Description
The following call to the Magma interface seems to be triggering a segfault on the Python side. Traceback is from 9.2, but I actually first hit this in 9.5beta1, so I assume nothing changed in between.
sage: Q.<x> = PolynomialRing(GF(3)) sage: u = x^6+x^4+2*x^3+2*x+1 sage: F0 = magma.FunctionField(GF(3)) sage: S = magma.PolynomialRing(F0) sage: F = magma.FunctionField(S.1^2  u(F0.1)) Fatal Python error: Cannot recover from stack overflow. Python runtime state: initialized Thread 0x00007f79c3d44700 (most recent call first): File "/usr/lib/python3.8/threading.py", line 302 in wait File "/usr/lib/python3.8/threading.py", line 558 in wait File "/scratch/sage/sage9.2/local/lib/python3.8/sitepackages/IPython/core/history.py", line 829 in run File "/scratch/sage/sage9.2/local/lib/python3.8/sitepackages/IPython/core/history.py", line 58 in needs_sqlite File "</scratch/sage/sage9.2/local/lib/python3.8/sitepackages/decorator.py:decoratorgen24>", line 2 in run File "/usr/lib/python3.8/threading.py", line 932 in _bootstrap_inner File "/usr/lib/python3.8/threading.py", line 890 in _bootstrap Current thread 0x00007f79c5cbf740 (most recent call first): File "/usr/lib/python3.8/contextlib.py", line 86 in __init__ File "/usr/lib/python3.8/contextlib.py", line 240 in helper ...
Change History (13)
comment:1 Changed 8 months ago by
comment:2 Changed 8 months ago by
For comparison, this works just fine:
sage: Q.<x,y> = PolynomialRing(GF(3), 2) sage: u = x^6+x^4+2*x^3+2*x+1 sage: F0 = magma.FunctionField(GF(3)) sage: u(F0.1,0) $.1^6 + $.1^4 + 2*$.1^3 + 2*$.1 + 1
so it is possible the underlying issue isn't in the interface at all.
comment:3 Changed 8 months ago by
The GDB attachment doesn't quite work for me, but it can print a traceback that gives some information:
...[loads of repeats START] #11115 0x00007f618907563a in PyObject_IsTrue.part.0 () from /lib64/libpython3.9.so.1.0 No symbol table info available. #11116 0x00007f618910f939 in bool_new () from /lib64/libpython3.9.so.1.0 No symbol table info available. #11117 0x00007f6189068e8b in type_call () from /lib64/libpython3.9.so.1.0 No symbol table info available. #11118 0x00007f6189068c73 in _PyObject_MakeTpCall () from /lib64/libpython3.9.so.1.0 No symbol table info available. #11119 0x00007f61890650dd in _PyEval_EvalFrameDefault () from /lib64/libpython3.9.so.1.0 No symbol table info available. #11120 0x00007f618906d8db in function_code_fastcall () from /lib64/libpython3.9.so.1.0 No symbol table info available. #11121 0x00007f61890764d4 in method_vectorcall () from /lib64/libpython3.9.so.1.0 No symbol table info available. #11122 0x00007f6189064fce in _PyEval_EvalFrameDefault () from /lib64/libpython3.9.so.1.0 No symbol table info available. #11123 0x00007f618906d8db in function_code_fastcall () from /lib64/libpython3.9.so.1.0 No symbol table info available. #11124 0x00007f6189097063 in PyObject_CallOneArg.lto_priv.13 () from /lib64/libpython3.9.so.1.0 No symbol table info available. #11125 0x00007f61890ce131 in slot_nb_bool () from /lib64/libpython3.9.so.1.0 No symbol table info available. ...[loads of repeats END] #11138 0x00007f61791f949a in __pyx_pf_4sage_9structure_7element_7Element_60is_zero (__pyx_v_self=<optimized out>) at build/cythonized/sage/structure/element.c:10177 ... #11142 0x00007f60159e8b08 in __pyx_pf_4sage_5rings_10polynomial_18polynomial_element_10Polynomial_18__call__ (__pyx_v_kwds=0x7f5fc9b275c0, __pyx_v_args=0x7f5fc9b10f70, __pyx_v_self=<optimized out>) at build/cythonized/sage/rings/polynomial/polynomial_element.c:9783 ... #11143 __pyx_pw_4sage_5rings_10polynomial_18polynomial_element_10Polynomial_19__call__ (__pyx_v_self=<optimized out>, __pyx_args=0x7f5fc9b10f70, __pyx_kwds=<optimized out>) at build/cythonized/sage/rings/polynomial/polynomial_element.c:8807 ... #11146 __pyx_pf_4sage_5rings_10polynomial_21polynomial_zmod_flint_21Polynomial_zmod_flint_4__call__ (__pyx_v_kwds=0x7f5fc9ad4fc0, __pyx_v_x=0x7f5fc9b10a90, __pyx_v_self=<optimized out>) at build/cythonized/sage/rings/polynomial/polynomial_zmod_flint.cpp:15823 .... #11147 __pyx_pw_4sage_5rings_10polynomial_21polynomial_zmod_flint_21Polynomial_zmod_flint_5__call__ (__pyx_v_self=<optimized out>, __pyx_args=0x7f5fc9b10a90, __pyx_kwds=<optimized out>) at build/cythonized/sage/rings/polynomial/polynomial_zmod_flint.cpp:15409 ...
It looks like this traceback goes innerattop, so it seems there is some zmod_flint that leads to an infinite recursion in determining if something is true.
In the python traceback there is a pretty good indication of where the infinite recursion happens:
... Current thread 0x00007f6dbad68740 (most recent call first): File "/usr/lib64/python3.9/re.py", line 291 in _compile File "/usr/lib64/python3.9/re.py", line 252 in compile File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/pexpect/spawnbase.py", line 232 in compile_pattern_list File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/pexpect/spawnbase.py", line 342 in expect File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/sage/interfaces/expect.py", line 983 in _eval_line File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/sage/interfaces/expect.py", line 1381 in <listcomp> File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/sage/interfaces/expect.py", line 1381 in eval File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/sage/interfaces/magma.py", line 558 in eval File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/sage/interfaces/magma.py", line 1080 in _next_var_name File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/sage/interfaces/interface.py", line 512 in _create File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/sage/interfaces/expect.py", line 1469 in __init__ File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/sage/interfaces/interface.py", line 295 in __call__ File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/sage/interfaces/magma.py", line 791 in __call__ ...[The following repeat a LOT] File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/sage/interfaces/magma.py", line 2631 in __bool__ File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/sage/interfaces/interface.py", line 1308 in bool File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/sage/interfaces/magma.py", line 2631 in __bool__ File "/usr/local/sage/sagegit/local/lib64/python3.9/sitepackages/sage/interfaces/interface.py", line 1308 in bool
so this looks like a __bool__
call on a magma interface element leading to exactly the same again (probably with the same argument). That's consistent with the GDB traceback part too (at least the fact that PyObject_IsTrue
is involved)
comment:4 Changed 8 months ago by
Indeed,
sage: Q.<x> = PolynomialRing(GF(3)) sage: u = x^6+x^4+2*x^3+2*x+1 sage: F0 = magma.FunctionField(GF(3)) sage: bool(F0.1)
already blows up.
In fact:
sage: magma("1 eq 0").bool()
blows up: the magma interface cannot determine the truth value of magma's "false" object.
The same happens for
sage: magma("1 eq 1").bool()
On can see why: looking into the the code, we're executing:
try: return not self.parent()("%s eq 0" % self.name()).bool() except TypeError: # comparing with 0 didn't work; try comparing with try: return not self.parent()("%s eq false" % self.name()).bool()
so in order to determine the truth value of "true", we first try "true eq 0" (which raises TypeError?) and then try "true eq false" which will produce "false". We then try "false eq 0" and then "false eq false" which will produce "true", and now the circle is complete.
This has never worked. I suspect someone was a bit overzealous in changing a "x == 0" to a "bool(x)", because that's something that python allows but isn't always a better expression of intent and, as this shows, may not actually be an improvement in code because it's not actually equivalent!!! F0.1 == 0
works just fine.
comment:5 Changed 8 months ago by
Looks like this goes back to https://trac.sagemath.org/ticket/7504 . I suspect that with the transition from Py2 to Py3 and the corresponding change from __nonzero__
to __bool__
a delicate balance was disturbed that previously allowed boolean objects from escaping this infinite recursion. Clearly, the results of these "%s eq 0" and "%s eq false" tests should be examined for their string value and not depend on reentering the interface code to reapply "bool".
comment:6 Changed 8 months ago by
Well, that explains my feeling of deja vu anyway!
comment:7 Changed 7 months ago by
 Branch set to u/kedlaya/magma_interface_throws_segfault
comment:8 Changed 7 months ago by
 Commit set to c896669b1f2303da42e33bbb50d664bd16c7124c
 Keywords booleans added
 Status changed from new to needs_review
Patch to change the __bool__
method as nbruin suggests, doing the comparison at the level of the Magma string representation. I'm a bit nervous that this is too simpleminded, but it doesn't seem to break any of the doctests.
Speaking of which, I tweaked a couple of unrelated doctests which were slightly off (possibly dependent on Magma version; I tried this with 2.255).
comment:9 Changed 7 months ago by
It looks like the branch linked presently is just 9.5.beta3. Did you forget to push?
comment:10 Changed 7 months ago by
 Commit changed from c896669b1f2303da42e33bbb50d664bd16c7124c to 46d7eac7c6b2ca798bcaa958043478c31b18b05a
Branch pushed to git repo; I updated commit sha1. New commits:
46d7eac  Change __bool__ in Magma interface

comment:11 Changed 7 months ago by
I had remembered to push but not to commit. :)
comment:12 Changed 7 months ago by
 Reviewers set to Nils Bruin, Travis Scrimshaw
 Status changed from needs_review to positive_review
The fix seems reasonable to me. Nils, if you have any further comments, feel free to add them (including reverting the positive review).
comment:13 Changed 6 months ago by
 Branch changed from u/kedlaya/magma_interface_throws_segfault to 46d7eac7c6b2ca798bcaa958043478c31b18b05a
 Resolution set to fixed
 Status changed from positive_review to closed
In the last line, already
triggers a