out-of-order assignment with strings causes segfault

[robertwb]

Hello,

In cython 1474:7f1b96cad687, the following code results in a segfault. It seems a relevant check for None is omitted because of the assignment, even though the assignment comes later.

def crash():
    print "%s" % s
    cdef str s = "Test"

When crash() is run, the program segfaults. However, the following code segments behave correctly, printing None.

def nocrash():
    print "%s" % s
    cdef str s

and

def nocrash():
    cdef str s = None
    print "%s" % s

Furthermore, assigning None to the s afterwards also causes it to crash (compare above).

def crash(): 
    print "%s" % s
    cdef str s = None

Thanks!
--Hoyt

Migrated from http://trac.cython.org/ticket/158

[robertwb]

scoder changed milestone from wishlist to 0.11
commented

[robertwb]

scoder commented

There are two problems involved here: one is the missing None check that http://trac.cython.org/ticket/166 describes, the other problem is that initial assignments of a cdef apparently fail to take previous usages of the name into account. This is a bit tricky to fix due to Cython's scoping rules: variables exist inside the entire scope (i.e. module/function), regardless of their point of declaration.

Another bug that flow control analysis could potentially fix...

[robertwb]

@dagss commented

Would it be help to simply disable the ability to declare variables after they are used? This is rather easily checked without full flow control analysis.

[robertwb]

@robertwb commented

Yes, I think we should disallow variable declarations after they are used.

[robertwb]

@robertwb changed milestone from 0.11 to 0.11.1
commented

[robertwb]

@robertwb changed priority from minor to critical
commented

This is a pretty bad bug, but it'll be more than a quick fix and it's not a regression so I don't want to hold up 0.11 for it.

[robertwb]

@dagss changed owner from somebody to Kurt Smith
commented

[robertwb]

@dagss commented

http://thread.gmane.org/gmane.comp.python.cython.devel/4775

[robertwb]

@dagss commented

Note that this is almost done, there's a patch on the mailing list.

[robertwb]

@dagss changed resolution to fixed
status from new to closed
commented

http://hg.cython.org/cython-devel/rev/478a5cfba350

[robertwb]

@dagss commented

To be clear, this was fixed by making Hoyt's example raise a compilation error.

[robertwb]

@robertwb commented

See also http://hg.cython.org/cython-devel/rev/38fe28886d7c