-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
Fix assert Crash for certain Cython generated functions #135306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
I encountered a crash from the asertion line (e.g. 1240) when running a Cython generated extension module (grpc-python, which creates async def functions) under PYTHONDEBUGMODE. It appears that the co object created by Cython in this case has a zero length code bytes -- which probably made sense because there is no python bytecode. Assuming Cython is correct in reporting a non-zero sized co object in this case, I think we can short circuit the Addr* functions for zero sized co objects.
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
I don't know why we should assume this. Why would they report a non-zero sized co object if the co object created by Cython does not exist? maybe I misunderstand the terminology, but do you mean that Please open an issue (either here or on Cython) to ask for experts. This needs to be discussed more in details. Also, can we have some reproducer first? |
Sorry I updated the message and the title after the new data came back (my reproducer was not very clean). It was addrq == 8 and PyCode_NBYTES(co) == 8. This is a bit out of my depth. My gut feeling is maybe a minimal of 8 bytes is required for a Cython produced function (to represent calling the binary extension)? If it is addrq == PyCode_NBYTES(co) that looks suspicious then I can try to dig in why Cython is producing that length and or the line number information that addrq == 8; but probably @scoder from Cython already knew the answer? |
AFAICT, the issue is with cc @markshannon And again, I think we need an issue for that. Even if the change looks small, the rationale isn't necessarily simple (and we can even have a NEWS entry as it can cause a crash). |
Thanks for the quick initial responses. At the moment I am not quite sure if an issue shall go to Python or to go to Cython. aka how addrq could become 8 in this case: I guess the criteria for such is "does an extension module in general the ability to control the value of addrq?" Would you enlighten me on this? The only information on |
I encountered a crash from the asertion line (e.g. 1240) when running a Cython generated extension module (grpc-python, which creates async def functions) under PYTHONDEBUGMODE.
It appears that the co object created by Cython in this case has nbytes == 8 and somehow the addrq is also 8. This is probably some sort of empty code object convention but I am out of my depth here.
Without this patch the assertion below crashes the interpreter. If I simply comment out the assertion the line numbers shows up as None; but I worry some of the returning value being -1 is used as an indication of errors. In any case there is no real byte code so I think we should return something like 0 to indicate such.