Skip to content

Type propagation does not account for the setting local variables via frame.f_locals in a debugger or similar. #115709

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

Closed
Tracked by #115419
markshannon opened this issue Feb 20, 2024 · 5 comments
Labels
3.13 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) release-blocker type-bug An unexpected behavior, bug, or error

Comments

@markshannon
Copy link
Member

markshannon commented Feb 20, 2024

Bug report

Bug description:

Consider

def testfunc(loops):
    for _ in range(loops):
        a = 1
        a + a
        a + a
        change_a()
        a + a

and suppose that change_a manages to change the value of the local a to "a".
Specialization will have converted the subsequent a + a to integer addition, and type propagation will have stripped the guard, resulting in a crash.

This is incredibly unlikely, and very hard to come up with a test for but it is theoretically possible.
It may be become more likely (and easier to test for) if PEP 667 is accepted.

Linked PRs

@markshannon
Copy link
Member Author

The fix is relatively straightforward:
Invalidate all executors depending on a code object should any locals associated with that code object get changed via frame.f_locals

@Eclips4 Eclips4 added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Feb 20, 2024
@encukou
Copy link
Member

encukou commented Feb 27, 2024

Is this on track for 3.13.0a5 (on 2024-03-12), or should I look into it?

@markshannon
Copy link
Member Author

markshannon commented Feb 27, 2024

It's on track for the release, but probably not for 3.13.0a5.

The fix will be easier with PEP 667, so I'm waiting to see if that is accepted first.
Identifying if a local variable has changed is currently awkward with PyFrame_LocalsToFast and PyFrame_FastToLocals, but with PEP 667 it is almost trivial.

@Yhg1s
Copy link
Member

Yhg1s commented May 5, 2024

Since PEP 667 was accepted, is this on track to be finished before beta 1 (next Tuesday)?

gvanrossum pushed a commit that referenced this issue May 6, 2024
…frame.f_locals (#118639)

Also fix unrelated assert in debug Tier2/JIT builds.
@gvanrossum
Copy link
Member

I believe this is fixed now by gh-118639 and definitely no longer blocking the release. Waiting for @markshannon to close it.

SonicField pushed a commit to SonicField/cpython that referenced this issue May 8, 2024
…d via frame.f_locals (python#118639)

Also fix unrelated assert in debug Tier2/JIT builds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) release-blocker type-bug An unexpected behavior, bug, or error
Projects
Development

No branches or pull requests

5 participants