Upgrade to Cython 0.19

[jdemeyer]

Upgrade to Cython 0.19.

spkg: http://boxen.math.washington.edu/home/jdemeyer/spkg/cython-0.19.p0.spkg (spkg diff)

upstream:

1. Bug report about segfaults in libGAP: http://trac.cython.org/cython_trac/ticket/808 (fixed in Cython 0.19)
2. Bug with g++ on Solaris: http://trac.cython.org/cython_trac/ticket/809 (patch in spkg and submitted upstream)
3. Bug with g++ on Solaris: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57025 (deny it's a bug)
4. Bug with cpdef keyword arguments in lazy_import.pyx: http://trac.cython.org/cython_trac/ticket/810

With this spkg, all doctests pass, but the docbuilder (in particular the command ./sage --docbuild reference/combinat html) crashes due to a problem with lazy_import: attachment: sage_crash_W7eVMu.log The Sage library patch works around this problem.

On Itanium, there is a doctest failure

sage -t --long devel/sage/sage/structure/element.pyx
**********************************************************************
File "devel/sage/sage/structure/element.pyx", line 1733, in sage.structure.element.RingElement.__pow__
Failed example:
    p(a,2,1)
Expected:
    Traceback (most recent call last):
    ...
    RuntimeError: __pow__ dummy argument not used
Got:
    202
**********************************************************************

This is likely a bug in GCC-4.7.3, but no further investigation has been done. Compiling sage/structure/element.pyx with -Os works around the problem, see patch.

apply attachment: 14452_cython_0.19.patch

Depends on #13826

Upstream: None of the above - read trac for reasoning.

CC: @robertwb

Component: packages: standard

Author: Jeroen Demeyer

Reviewer: Robert Bradshaw

Merged: sage-5.9.rc0

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

[jdemeyer]

comment:2

Robert, I think there is a serious Cython bug: the __new__ method of a class might actually use the wrong class: in devel/sage/sage/libs/gap/element.pyx, the code

def GapElement_Record r = GapElement_Record.__new__(GapElement_Record)

generates

__pyx_t_1 = __pyx_tp_new_4sage_4libs_3gap_7element_GapElement(((PyTypeObject *)((PyObject*)__pyx_ptype_4sage_4libs_3gap_7element_GapElement_Record)), ((PyObject *)__pyx_empty_tuple), NULL);

Note the call to the tp_new function of GapElement (a child class), not GapElement_Record.

[jdemeyer]

Upstream: Reported upstream. No feedback yet.

[robertwb]

comment:4

cython/cython#214

[robertwb]

Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.

[jdemeyer]

comment:5

Attachment: sage_crash_W7eVMu.log

[jdemeyer]

comment:7

Here is what I found about the docbuilder crash: it works with Cython 0.18, not with current master at 0ee15b1829125b6844ac13f437c2d9bd61a4ceca.

In lazy_import.pyx, somehow self._object becomes NULL in the line

setattr(cls, alias, self._object)

which is compiled to

           __pyx_t_1 = __pyx_v_self->_object;                                      
           __Pyx_INCREF(__pyx_t_1);
           __pyx_t_10 = PyObject_SetAttr(__pyx_v_cls, __pyx_v_alias, __pyx_t_1); if (unlikely(__pyx_t_10 == -1)) {__pyx_filename = __pyx_f[0]; __pyx_lineno = 169; __py
           __Pyx_DECREF(__pyx_t_1); __pyx_t_1 = 0;                                 

I don't see anything suspicious here.

[jdemeyer]

comment:8

Generating just lazy_import.c with Cython-0.18 makes the documentation build.

[jdemeyer]

comment:9

Robert, I feel like I can not do much more to debug the docbuilding since I don't really understand the mechanics of lazy_import.

[robertwb]

comment:13

I don't see anything suspicious there either. Can you try compiling lazy_import.pyx with -O0? What's the diff of this file between 0.18 and 0.19?

[jdemeyer]

comment:14

Replying to @robertwb:

Can you try compiling lazy_import.pyx with -O0?

I did, and it didn't help. I could try to bisect the Cython git repo for the bug.

[jdemeyer]

comment:15

Results of bisecting:

• commit 19c0408476515c89a60655b753f08a3bd18eec83 is good

• commit d44a0845fc91bf7ade61dee431358f375c580f44 is bad

All commits in between simply fail to compile.

[jdemeyer]

comment:16

The relevant change seems to be the line

obj = self._get_object(owner=owner)

which was compiled in the good way as

  __pyx_t_1 = PyObject_GetAttr(((PyObject *)__pyx_v_self), __pyx_n_s___get_object); if (unlikely(!__pyx_t_1)) {__pyx_filename = __pyx_f[0]; __pyx_lineno = 401; __pyx_clineno = __LINE__; goto __pyx_L1_error;}
  __Pyx_GOTREF(__pyx_t_1);
  __pyx_t_2 = PyDict_New(); if (unlikely(!__pyx_t_2)) {__pyx_filename = __pyx_f[0]; __pyx_lineno = 401; __pyx_clineno = __LINE__; goto __pyx_L1_error;}
  __Pyx_GOTREF(((PyObject *)__pyx_t_2));
  if (PyDict_SetItem(__pyx_t_2, ((PyObject *)__pyx_n_s__owner), __pyx_v_owner) < 0) {__pyx_filename = __pyx_f[0]; __pyx_lineno = 401; __pyx_clineno = __LINE__; goto __pyx_L1_error;}
  __pyx_t_3 = PyObject_Call(__pyx_t_1, ((PyObject *)__pyx_empty_tuple), ((PyObject *)__pyx_t_2)); if (unlikely(!__pyx_t_3)) {__pyx_filename = __pyx_f[0]; __pyx_lineno = 401; __pyx_clineno = __LINE__; goto __pyx_L1_error;}
  __Pyx_GOTREF(__pyx_t_3);
  __Pyx_DECREF(__pyx_t_1); __pyx_t_1 = 0;
  __Pyx_DECREF(((PyObject *)__pyx_t_2)); __pyx_t_2 = 0;
  __pyx_v_obj = __pyx_t_3;
  __pyx_t_3 = 0;

and in the bad way as

  __pyx_t_2.__pyx_n = 1;
  __pyx_t_2.owner = __pyx_v_owner;
  __pyx_t_1 = ((struct __pyx_vtabstruct_4sage_4misc_11lazy_import_LazyImport *)__pyx_v_self->__pyx_vtab)->_get_object(__pyx_v_self, 0, &__pyx_t_2); if (unlikely(!__pyx_t_1)) {__pyx_filename = __pyx_f[0]; __pyx_lineno = 401; __pyx_clineno = __LINE__; goto __pyx_L1_error;}
  __Pyx_GOTREF(__pyx_t_1);
  __pyx_v_obj = __pyx_t_1;
  __pyx_t_1 = 0;

[jdemeyer]

comment:17

Alternatively, changing the line

cpdef _get_object(self, owner=None):

to

def _get_object(self, owner=None):

also fixes the problem. I am applying this workaround in attachment: 14452_cython_0.19.patch.

[jdemeyer]

Changed upstream from Fixed upstream, but not in a stable release. to None of the above - read trac for reasoning.

[jdemeyer]

spkg diff

[jdemeyer]

comment:21

Attachment: cython-0.19.p0.diff.gz

[jdemeyer]

comment:23

With these patches, all fine on the buildbot.

[jdemeyer]

Attachment: 14452_cython_0.19.patch.gz

[jdemeyer]

Dependencies: #13826

[jdemeyer]

comment:24

Rebased to #13826.

[robertwb]

comment:26

Thanks for debugging this! I filed http://trac.cython.org/cython_trac/ticket/810 about (what I think is) the underlying bug you uncovered with the lazy import stuff.

[jdemeyer]

Merged: sage-5.9.rc0

[jdemeyer]

Reviewer: Robert Bradshaw

[jdemeyer]

comment:28

Replying to @robertwb:

Thanks for debugging this! I filed http://trac.cython.org/cython_trac/ticket/810 about (what I think is) the underlying bug you uncovered with the lazy import stuff.

Do you have a better idea about what precisely is wrong? I isolated the problem, but I don't know why it segfaults.