Skip to content

SIGSEGV with generators after direct change in gi_frame #125038

Closed
Closed
SIGSEGV with generators after direct change in gi_frame#125038
Labels
3.13only security fixes3.14bugs and security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-crashA hard crash of the interpreter, possibly with a core dump

Description

@efimov-mikhail
efimov-mikhail
opened on Oct 7, 2024

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

Activity

  1. efimov-mikhail
    added
    type-crashA hard crash of the interpreter, possibly with a core dump
    on Oct 7, 2024
  2. AA-Turner
    added
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    3.13only security fixes
    3.14bugs and security fixes
    on Oct 7, 2024

  3. JelleZijlstra commented on Oct 7, 2024

    @JelleZijlstra
    JelleZijlstra
    on Oct 7, 2024
    Member

    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.

  4. efimov-mikhail
    added a commit that references this issue on Oct 7, 2024

    pythongh-125038: PyIter_Checks are added for some FOR_ITER bytecodes

    ffc0958

  5. efimov-mikhail commented on Oct 7, 2024

    @efimov-mikhail
    efimov-mikhail
    on Oct 7, 2024
    MemberAuthor

    Thank you for such a concrete comment!
    It seems like I made fix by myself after it.

  6. efimov-mikhail
    added 8 commits that reference this issue on Oct 7, 2024

    pythongh-125038: Test on gi_frame.f_locals change for a generator

    4f52d20

    pythongh-125038: News entry is added.

    3b6e814

    pythongh-125038: Raise TypeError on such case instead of silently sto…

    f056908

    pythongh-125038: PyIter_Checks are added for some FOR_ITER bytecodes

    523f765

    pythongh-125038: Test on gi_frame.f_locals change for a generator

    e316cb4

    pythongh-125038: News entry is added.

    09df328

    pythongh-125038: Raise TypeError on such case instead of silently sto…

    d3d5b39

    pythongh-125038: Test improvement, assertRaisesRegex is used

    c353cf1

  7. 18 remaining items

  8. JelleZijlstra commented on Oct 22, 2024

    @JelleZijlstra
    JelleZijlstra
    on Oct 22, 2024
    Member

    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.

  9. JelleZijlstra
    added a commit that references this issue on Oct 23, 2024

    [3.13] gh-125038: Fix crash after genexpr.gi_frame.f_locals manipulat…

    bcc7227

  10. efimov-mikhail commented on Oct 23, 2024

    @efimov-mikhail
    efimov-mikhail
    on Oct 23, 2024
    MemberAuthor

    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.

    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

  11. efimov-mikhail
    added 6 commits that reference this issue on Nov 4, 2024

    pythongh-125038: redundant GET_ITER and GET_AITER are removed from ge…

    9bbdf16

    pythongh-125038: redundant GET_ITER and GET_AITER are removed from ge…

    affb1f8

    pythongh-125038: some changes after review

    b9e2b38

    pythongh-125038: typo fix in previous commit

    30060e5

    pythongh-125038: some tests on CHECK_ITERABLE are added

    ee94916

    pythongh-125038: generated file is updated, see python#126222

    6065d84

  12. ncoghlan commented on Nov 8, 2024

    @ncoghlan
    ncoghlan
    on Nov 8, 2024
    Contributor

    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)

  13. serhiy-storchaka commented on Dec 25, 2024

    @serhiy-storchaka
    serhiy-storchaka
    on Dec 25, 2024
    Member

    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.

  14. ebonnal
    added a commit that references this issue on Jan 12, 2025

    pythongh-125038: Fix crash after genexpr.gi_frame.f_locals manipulati…

    99b2b8e

  15. efimov-mikhail commented on May 25, 2025

    @efimov-mikhail
    efimov-mikhail
    on May 25, 2025
    MemberAuthor

    IMO, this issue should be close because of #132351

  16. efimov-mikhail
    closed this as completedon May 25, 2025

  17. efimov-mikhail commented on Jun 22, 2025

    @efimov-mikhail
    efimov-mikhail
    on Jun 22, 2025
    MemberAuthor

    I've started related d.p.o. thread here: https://discuss.python.org/t/generator-expressions-behavior-with-non-iterable-objects/96329.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    3.13only security fixes3.14bugs and security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-crashA hard crash of the interpreter, possibly with a core dump

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions