Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#158 closed defect (fixed)

out-of-order assignment with strings causes segfault

Reported by: hoytak Owned by: Kurt Smith
Priority: critical Milestone: 0.11.1
Component: Code Generation Keywords:



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


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


Change History (12)

comment:1 Changed 7 years ago by scoder

  • Milestone changed from wishlist to 0.11

comment:2 Changed 6 years ago by scoder

There are two problems involved here: one is the missing None check that #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...

comment:3 Changed 6 years ago by dagss

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.

comment:4 Changed 6 years ago by robertwb

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

comment:5 Changed 6 years ago by robertwb

  • Milestone changed from 0.11 to 0.11.1

comment:6 Changed 6 years ago by robertwb

  • Priority changed from minor to critical

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.

comment:7 Changed 6 years ago by dagss

  • Owner changed from somebody to Kurt Smith

comment:9 Changed 6 years ago by dagss

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

comment:10 Changed 6 years ago by dagss

  • Resolution set to fixed
  • Status changed from new to closed

comment:11 Changed 6 years ago by dagss

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

Note: See TracTickets for help on using tickets.