Opened 7 years ago
Closed 7 years ago
#13868 closed enhancement (fixed)
Deal with hooked tp_* functions when using a debug build of Python
Reported by: | jpflori | Owned by: | GeorgSWeber |
---|---|---|---|
Priority: | major | Milestone: | sage-5.6 |
Component: | build | Keywords: | hook debug |
Cc: | vbraun, SimonKing, jdemeyer | Merged in: | sage-5.6.beta3 |
Authors: | Volker Braun | Reviewers: | Jean-Pierre Flori |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Python debug checks are confused by the fast tp_* functions we hook in integer.pyx and real_double.pyx
Attachments (2)
Change History (20)
comment:1 Changed 7 years ago by
comment:2 Changed 7 years ago by
I tried to uncomment both these lines in integer.pyx and real_double.pyx but this lead to segfaults when starting Sage.
If I prevent the use of the fast tp_*, Sage starts (although I get a TypeError? in sage/libs/gen/late_import, I've not recompiled everything after rebuilding Python)
So this will need more work.
comment:3 follow-up: ↓ 4 Changed 7 years ago by
I found the bug, the PyObject_INIT
was only called when a new integer / real object was created and not when a recycled object from the pool was used. You have to call it in both branches, of course. I'll work out a fix where the debug preprocessor macro is used to conditionally compile it in.
comment:4 in reply to: ↑ 3 Changed 7 years ago by
Replying to vbraun:
I found the bug, the
PyObject_INIT
was only called when a new integer / real object was created and not when a recycled object from the pool was used. You have to call it in both branches, of course. I'll work out a fix where the debug preprocessor macro is used to conditionally compile it in.
I am a bit puzzled. Why is this only done when the debug preprocessor macro is used? Do you claim that this is a bug only in debug mode? Why could it not create trouble without debug mode?
comment:5 follow-up: ↓ 6 Changed 7 years ago by
Anyway, using your patch plus the new cython plus the latest version of the python spkg in debug mode, I am now able to start Sage without a crash. It "only" reports
ImportError: /home/simon/SAGE/debug/sage-5.5.rc0/local/lib/python2.7/site-packages/sage/rings/polynomial/multi_polynomial_libsingular.so: undefined symbol: _Z7_p_TestP8spolyrecP9sip_sringi
But on #13867 you said that for this one needs a new Singular spkg (hopefully one that still allows for using xalloc!).
comment:6 in reply to: ↑ 5 Changed 7 years ago by
comment:8 Changed 7 years ago by
With the patches listed at #13877, I get:
/home/simon/SAGE/debug/sage-5.5.rc0/local/lib/python2.7/site-packages/sage/all.py:76: RuntimeWarning: tp_compare didn't return -1 or -2 for exception import sage.symbolic.pynac --------------------------------------------------------------------------- SystemError Traceback (most recent call last) /home/simon/SAGE/debug/sage-5.5.rc0/local/lib/python2.7/site-packages/IPython/ipmaker.py in force_import(modname, force_reload) 61 reload(sys.modules[modname]) 62 else: ---> 63 __import__(modname) 64 65 /home/simon/SAGE/debug/sage-5.5.rc0/local/bin/ipy_profile_sage.py in <module>() 5 preparser(True) 6 ----> 7 import sage.all_cmdline 8 sage.all_cmdline._init_cmdline(globals()) 9 /home/simon/SAGE/debug/sage-5.5.rc0/local/lib/python2.7/site-packages/sage/all_cmdline.py in <module>() 12 try: 13 ---> 14 from sage.all import * 15 from sage.calculus.predefined import x 16 preparser(on=True) /home/simon/SAGE/debug/sage-5.5.rc0/local/lib/python2.7/site-packages/sage/all.py in <module>() 74 75 # This must come before Calculus -- it initializes the Pynac library. ---> 76 import sage.symbolic.pynac 77 78 from sage.modules.all import * SystemError: Objects/object.c:854: bad argument to internal function Error importing ipy_profile_sage - perhaps you should run %upgrade? WARNING: Loading of ipy_profile_sage failed.
So, tp_compare does something unexpected here.
comment:9 Changed 7 years ago by
Did you compile everything from scratch? You definitely need to recompile pynac with debugging, maybe more?
comment:10 Changed 7 years ago by
I did sage -ba after installing all stuff on top of an existing sage-5.5.
comment:11 follow-up: ↓ 12 Changed 7 years ago by
If you start on sage-5.5 you at least need the patch from #13740, maybe more...
comment:12 in reply to: ↑ 11 Changed 7 years ago by
comment:13 Changed 7 years ago by
I don't know whether indented preprocessor directives like
#if ... #define ... #endif
are standard C. Maybe they are, but usually this is written as
#if ... #define ... #endif
comment:14 Changed 7 years ago by
Pre-ANSI C did not allow for space, but those compilers are long dead (even by skynet standards). Its legal in all C versions that will at least start building Sage. I prefer the Cython-style indentation, then you don't have to mentally switch when reading it.
comment:15 follow-up: ↓ 16 Changed 7 years ago by
It would be nice if we could review and merge this ticket as it is the cause for the segfault during Sage "make" with SAGE_DEBUG=yes
comment:16 in reply to: ↑ 15 Changed 7 years ago by
Replying to vbraun:
It would be nice if we could review and merge this ticket as it is the cause for the segfault during Sage "make" with
SAGE_DEBUG=yes
Is this still the case? With all the new spkgs mentioned in the ticket description of #13864 (in particular, with cython-0.17.4), sage builds and starts fine on bsd.math, with SAGE_DEBUG=yes. Or has that only been a problem on other systems?
comment:17 Changed 7 years ago by
- Description modified (diff)
- Reviewers set to Jean-Pierre Flori
- Status changed from needs_review to positive_review
Works fine and seems correct to me. I just added a missing u in some comment.
comment:18 Changed 7 years ago by
- Merged in set to sage-5.6.beta3
- Resolution set to fixed
- Status changed from positive_review to closed
In fact, thats commented out: