Skip to content

gh-132775: Add _PyCode_VerifyStateless() #133221

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

Merged

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Apr 30, 2025

"Stateless" code is a function or code object which does not rely on external state or internal state. It may rely on arguments and
builtins, but not globals or a closure. I've left a comment in pycore_code.h that provides more detail.

We also add _PyFunction_VerifyStateless(). The new functions will be used in several later changes that facilitate "sharing" functions and code objects between interpreters.

@ericsnowcurrently
Copy link
Member Author

!buildbot s390x RHEL8 LTO + PGO

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @ericsnowcurrently for commit d45c0e0 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133221%2Fmerge

The command will test the builders whose names match following regular expression: s390x RHEL8 LTO \+ PGO

The builders matched are:

  • s390x RHEL8 LTO + PGO PR


// The last instruction either returns or raises. We can take advantage
// of that for a quick exit.
_Py_CODEUNIT final = _Py_GetBaseCodeUnit(co, len-1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the warning on Windows fixable on this PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@ericsnowcurrently ericsnowcurrently enabled auto-merge (squash) May 5, 2025 21:29
@ericsnowcurrently ericsnowcurrently merged commit d270bb5 into python:main May 5, 2025
38 checks passed
@ericsnowcurrently ericsnowcurrently deleted the add-pycode-verify-stateless branch May 5, 2025 21:49
@nascheme
Copy link
Member

nascheme commented May 6, 2025

This looks to be failing on the iOS buildbot.

test_code (test)
Doctest: test.test_code ... ok
======================================================================
ERROR: test_stateless (test.test_code.CodeTest.test_stateless) [(<function spam_N at 0x10fab5b50>, '(code)')]
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/buildbot/Library/Developer/XCTestDevices/3749308D-3FDE-41E4-B27C-3A4DD1FCF87A/data/Containers/Bundle/Application/40457EBF-BF74-4E58-B5DB-F52F8BC085A9/iOSTestbed.app/python/lib/python3.14/test/test_code.py", line 1049, in test_stateless
    _testinternalcapi.verify_stateless_code(func.__code__)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^
ValueError: only basic code objects are supported
======================================================================
ERROR: test_stateless (test.test_code.CodeTest.test_stateless) [(<function spam_C at 0x10fab4a10>, '(code)')]
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/buildbot/Library/Developer/XCTestDevices/3749308D-3FDE-41E4-B27C-3A4DD1FCF87A/data/Containers/Bundle/Application/40457EBF-BF74-4E58-B5DB-F52F8BC085A9/iOSTestbed.app/python/lib/python3.14/test/test_code.py", line 1049, in test_stateless
    _testinternalcapi.verify_stateless_code(func.__code__)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^
ValueError: only basic code objects are supported

https://buildbot.python.org/#/builders/1380/builds/3582/

@encukou
Copy link
Member

encukou commented May 6, 2025

The test fails when it's run twice:

./python -m test test_code test_code

@encukou
Copy link
Member

encukou commented May 6, 2025

Looks like setting sys.settrace with opcode tracing causes all code to be instrumented when it's next run.

import sys
import _testinternalcapi

def foo():
    pass

def trace_temporarily():
    sys._getframe().f_trace_opcodes = True
    sys.settrace(print)
    sys.settrace(None)
    sys._getframe().f_trace_opcodes = False
trace_temporarily()

_testinternalcapi.verify_stateless_code(foo.__code__)
foo()
_testinternalcapi.verify_stateless_code(foo.__code__)  # fails

In this case, test_code_equal_with_instrumentation does this. If it runs first, then some test that exepcutes the tested functions (to get the closures I guess), then test_stateless will fail.

@encukou
Copy link
Member

encukou commented May 6, 2025

I'm not sure what exactly "statelessness" is used for in the subinterpreters code. But, it looks like if it is to be useful, it either

  • needs to be treated as an important property that can't be broken by optimizations, or
  • the function should be _PyCode_IsStateless, returning a bool, with typical users needing to implement a fallback (copying the code object should be enough?)

Unfortunately, given the timing, I think this needs to be reverted for 3.14 :(

@hugovk
Copy link
Member

hugovk commented May 6, 2025

@encukou Please could you open a revert PR?

@encukou
Copy link
Member

encukou commented May 6, 2025

Will do. Resolving conflicts.

encukou added a commit to encukou/cpython that referenced this pull request May 6, 2025
…)"

This reverts commit d270bb5.

Note that the test functions in _code_definitions are not reverted.
@hugovk
Copy link
Member

hugovk commented May 6, 2025

Reverted in #133497.

@ericsnowcurrently
Copy link
Member Author

Thanks for jumping on this. I'll sort it out. FYI, the relevant code is _PyCode_CheckNoInternalState(). It isn't clear to me how test_code_equal_with_instrumentation is affecting the functions used in test_stateless, but it probably doesn't matter. :)

ericsnowcurrently added a commit to ericsnowcurrently/cpython that referenced this pull request May 6, 2025
ericsnowcurrently added a commit that referenced this pull request May 8, 2025
This reverts commit 3c73cf5 (gh-133497), which itself reverted
the original commit d270bb5 (gh-133221).

We reverted the original change due to failing android tests.
The checks in _PyCode_CheckNoInternalState() were too strict,
so we've relaxed them.
ericsnowcurrently added a commit that referenced this pull request May 8, 2025
This reverts commit 3c73cf5 (gh-133497), which itself reverted
the original commit d270bb5 (gh-133221).

We reverted the original change due to failing android tests.
The checks in _PyCode_CheckNoInternalState() were too strict,
so we've relaxed them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants