Deal with hooked tp_* functions when using a debug build of Python

[jpflori]

Python debug checks are confused by the fast tp_* functions we hook in integer.pyx and real_double.pyx

Apply attachment: trac_13868_tp_hooks_and_debug-typo.patch.

CC: @vbraun @simon-king-jena @jdemeyer

Component: build

Keywords: hook debug

Author: Volker Braun

Reviewer: Jean-Pierre Flori

Merged: sage-5.6.beta3

Issue created by migration from https://trac.sagemath.org/ticket/13868

[vbraun]

comment:1

In fact, thats commented out:

        # This line is only needed if Python is compiled in debugging
        # mode './configure --with-pydebug'. If that is the case a Python
        # object has a bunch of debugging fields which are initialized
        # with this macro. For speed reasons, we don't call it if Python
        # is not compiled in debug mode. So uncomment the following line
        # if you are debugging Python.

        #PyObject_INIT(new, (<RichPyObject*>global_dummy_Integer).ob_type)

[jpflori]

comment:2

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.

[vbraun]

comment:3

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.

[vbraun]

Attachment: trac_13868_tp_hooks_and_debug.patch.gz

Initial patch

[simon-king-jena]

comment:4

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?

[simon-king-jena]

comment:5

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!).

[simon-king-jena]

comment:6

Replying to @simon-king-jena:

But on #13867 you said that for this one needs a new Singular spkg

Found it! #13876, right?

[vbraun]

comment:7

Yes!

[vbraun]

Author: Volker Braun

[simon-king-jena]

comment:8

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.

[vbraun]

comment:9

Did you compile everything from scratch? You definitely need to recompile pynac with debugging, maybe more?

[simon-king-jena]

comment:10

I did sage -ba after installing all stuff on top of an existing sage-5.5.

[vbraun]

comment:11

If you start on sage-5.5 you at least need the patch from #13740, maybe more...

[simon-king-jena]

comment:12

Replying to @vbraun:

If you start on sage-5.5 you at least need the patch from #13740, maybe more...

I of course have that patch, since I have the new cython spkg from #13832.

[jdemeyer]

comment:13

I don't know whether indented preprocessor directives like

#if ...
    #define ...
#endif

are standard C. Maybe they are (GCC accepts them and documents this), but usually this is written as

#if ...
#define ...
#endif

[vbraun]

comment:14

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.

[vbraun]

comment:15

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

[simon-king-jena]

comment:16

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?

[jpflori]

Initial patch + typo fixed.

[jpflori]

comment:17

Attachment: trac_13868_tp_hooks_and_debug-typo.patch.gz

Works fine and seems correct to me.
I just added a missing u in some comment.

[jpflori]

Reviewer: Jean-Pierre Flori

[jdemeyer]

Merged: sage-5.6.beta3