Repository navigation
SIGSEGV with generators after direct change in gi_frame #125038
Description
Crash report
What happened?
There is a segmentation fault with simple code snippet:
g = (x for x in range(10))
g.gi_frame.f_locals['.0'] = range(20)
list(g)
print("No segfault")There is a SIGSEGV on my linux machine (Debian GNU/Linux 10) with both main branch Python and 3.13 version.
Message "No segfault" is printed on Python 3.7.3 (default, Oct 31 2022, 14:04:00).
CPython versions tested on:
3.13, CPython main branch
Operating systems tested on:
Linux
Output from running 'python -VV' on the command line:
Python 3.14.0a0 (heads/peg_parser_remove_redundant_functions:e1b62e5cf79, Oct 3 2024, 14:38:46) [GCC 8.3.0]
Linked PRs
- gh-125038: Iterator checks are added for some FOR_ITER bytecodes, crash fixed #125051
- gh-125038: Crash after genexpr.gi_frame.f_locals manipulations is fixed #125178
- [3.13] gh-125038: Fix crash after genexpr.gi_frame.f_locals manipulations (GH-125178) #125846
- gh-125038: redundant GET_ITER instructions are removed from genexpr code #126408
Activity
JelleZijlstra commented on Oct 7, 2024
Backtrace on MacOS:
(lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) * frame #0: 0x0000000000000000 frame #1: 0x00000001001d0c5c python.exe`_PyEval_EvalFrameDefault(tstate=<unavailable>, frame=0x0000000100bcf898, throwflag=0) at generated_cases.c.h:3628:36 [opt] frame #2: 0x00000001000a48e0 python.exe`gen_send_ex2 [inlined] _PyEval_EvalFrame(tstate=0x00000001005a8738, frame=<unavailable>, throwflag=0) at pycore_ceval.h:119:16 [opt] frame #3: 0x00000001000a48d4 python.exe`gen_send_ex2(gen=0x0000000100bcf850, arg=0x0000000000000000, presult=0x000000016fdfe378, exc=0, closing=<unavailable>) at genobject.c:245:24 [opt] frame #4: 0x00000001000a2f28 python.exe`gen_iternext(self=<unavailable>) at genobject.c:612:9 [opt] frame #5: 0x00000001000ba0a4 python.exe`list_extend_iter_lock_held(self=0x00000001010ca490, iterable=0x00000001010ca490) at listobject.c:1194:26 [opt] frame #6: 0x00000001000b6fb8 python.exe`_list_extend(self=<unavailable>, iterable=<unavailable>) at listobject.c:1367:15 [opt] [artificial] frame #7: 0x00000001000bd578 python.exe`list___init___impl(self=0x00000001010ca490, iterable=0x0000000100bcf850) at listobject.c:3391:13 [opt] frame #8: 0x00000001000b8ef0 python.exe`list_vectorcall(type=<unavailable>, args=0x00000001009dc690, nargsf=<unavailable>, kwnames=<unavailable>) at listobject.c:3415:13 [opt] frame #9: 0x00000001000855f8 python.exe`_PyObject_VectorcallTstate(tstate=0x00000001005a8738, callable=0x0000000100530f30, args=0x00000001009dc690, nargsf=9223372036854775809, kwnames=0x0000000000000000) at pycore_call.h:167:11 [opt]The crash is in the FOR_ITER instruction; it blindly accesses tp_iternext without verifying that the object being iterated over has this attribute.
The repro case is a bit dubious as it's mutating f_locals, but probably we should indeed support this. The fix would be as simple as inserting a NULL check.
pythongh-125038: PyIter_Checks are added for some FOR_ITER bytecodes
efimov-mikhail commented on Oct 7, 2024
Thank you for such a concrete comment!
It seems like I made fix by myself after it.
pythongh-125038: Test on gi_frame.f_locals change for a generator
pythongh-125038: News entry is added.
pythongh-125038: Raise TypeError on such case instead of silently sto…
pythongh-125038: PyIter_Checks are added for some FOR_ITER bytecodes
pythongh-125038: Test on gi_frame.f_locals change for a generator
pythongh-125038: News entry is added.
pythongh-125038: Raise TypeError on such case instead of silently sto…
pythongh-125038: Test improvement, assertRaisesRegex is used
18 remaining items
JelleZijlstra commented on Oct 22, 2024
The initial fix is merged into main, though we'll want to eventually implement a different fix.
I thought it was worth investigating whether a similar crash was possible with async genexps. However, it seems that the GET_ANEXT impression is implemented more robustly and I couldn't find a crash by manipulating the locals of an asyncgen frame.
[3.13] gh-125038: Fix crash after genexpr.gi_frame.f_locals manipulat…
efimov-mikhail commented on Oct 23, 2024
Yes, it seems so.
Actually, function _PyEval_GetANext from Python/ceval.c contains NULL check for type->tp_as_async->am_anext method. TypeError will be raised if no such method presents.
This code looks very similar to one of the previous variants of fixing crash in my PR.
Maybe, we can remove NULL check from the GET_ANEXT bytecode implementation.
And place GET_AITER instruction before GET_ANEXT, in similarity with "sync" generators.
This change could possible improve perfomance a little bit.
But I'm not sure that it's worth it.
On the other side, TypeErrors in GET_AITER and GET_ANEXT slightly differs in case of am_anext == NULL.
Maybe we should use the same text in those functions.
cc @JelleZijlstra, @markshannon
pythongh-125038: redundant GET_ITER and GET_AITER are removed from ge…
pythongh-125038: redundant GET_ITER and GET_AITER are removed from ge…
pythongh-125038: some changes after review
pythongh-125038: typo fix in previous commit
pythongh-125038: some tests on CHECK_ITERABLE are added
pythongh-125038: generated file is updated, see python#126222
ncoghlan commented on Nov 8, 2024
See #125723 for another generator f_locals related crash that may be confounding the investigation attempts here (I don't think it's a duplicate, but it's a segfault where we have no idea of the root cause, so that assessment is purely based on the reproducer having a very different structure from the ones described here)
serhiy-storchaka commented on Dec 25, 2024
I do not think there was a bug. Bytecode manipulation can lead to crash -- this is expected. Addind a duplicated GET_ITER is just a pessimization. It slows down everyones code. I think this change should be reverted.
pythongh-125038: Fix crash after genexpr.gi_frame.f_locals manipulati…
efimov-mikhail commented on May 25, 2025
IMO, this issue should be close because of #132351
efimov-mikhail commented on Jun 22, 2025
I've started related d.p.o. thread here: https://discuss.python.org/t/generator-expressions-behavior-with-non-iterable-objects/96329.