-
Notifications
You must be signed in to change notification settings - Fork 46
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
Use _Py_DebugOffsets instead of hardcoded offsets when possible #206
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
godlygeek
force-pushed
the
use_py_debug_offsets_v3
branch
9 times, most recently
from
August 17, 2024 05:33
d53a64f
to
18a44a3
Compare
pablogsal
reviewed
Aug 23, 2024
pablogsal
reviewed
Aug 23, 2024
pablogsal
reviewed
Aug 23, 2024
pablogsal
reviewed
Aug 23, 2024
pablogsal
reviewed
Aug 23, 2024
pablogsal
approved these changes
Aug 23, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo the things that we discussed offline
pablogsal
reviewed
Aug 23, 2024
Add a new `Structure` class that encapsulates reading and interpreting the memory of the various Python data structures we need to work with. This is both easier to work with, and lays the groundwork for us to be able to handle reading structures without knowing their maximum size at compile time. This also fixes several places where we read more bytes than necessary, which could have resulted in spurious memory read failures due to reading past the end of a mapping. Signed-off-by: Matt Wozniski <[email protected]>
Now that we're using `Structure` everywhere, we no longer need to know the largest possible static footprint for each type of structure, so we can drop the unions, and the typedefs that referenced them. Signed-off-by: Matt Wozniski <[email protected]>
Previously we were handling different offsets between versions purely through version checks on the manager, but this won't work going forward, because free-threading builds of 3.13 use a different structure layout. Handle this by switching to offsets. Signed-off-by: Matt Wozniski <[email protected]>
Now that we plan to actually make use of the debug offsets, instead of just checking against them, it's no longer an exceptional or erroneous case when they don't match the compiled-in offsets. Drop this down to a warning, so that it shows up when debugging a problem but isn't in the way when using PyStack as an end user. Signed-off-by: Matt Wozniski <[email protected]>
The `End()` address of each map is exclusive, not inclusive, so a map where `addr == map.End()` does not contain that address. Signed-off-by: Matt Wozniski <[email protected]>
Flexible array members are a C feature that doesn't exist in standard C++. While g++ allows them, not being able to call `sizeof()` on the field gets in the way of some macro shenanigans I need to do. Signed-off-by: Matt Wozniski <[email protected]>
And in particular, allow `PYTHON_TEST_VERSION=3.13t` for requesting to test only this one Python version. Signed-off-by: Matt Wozniski <[email protected]>
This test makes some assumptions about the layout of some Python objects that don't hold in free-threading interpreters. Tweak the assumptions a bit so that they do hold. Signed-off-by: Matt Wozniski <[email protected]>
In free-threading builds the GIL may be enabled or disabled at runtime. In order to test that we can accurately report whether a thread in the free threading interpreter is holding the GIL, we need to run these tests with the GIL enabled. Signed-off-by: Matt Wozniski <[email protected]>
The free-threading build adds some extra text in the middle of the Py_GetVersion string that we need to account for. Signed-off-by: Matt Wozniski <[email protected]>
If we're able to locate and validate the _Py_DebugOffsets structure, use the offsets contained in it for the rest of our work. Signed-off-by: Matt Wozniski <[email protected]>
If we've found the `_Py_DebugOffsets` structure, we can use this to find the interpreter state quickly and efficiently: the debug offsets are at the start of the `_PyRuntime` structure, which contains a reference to the interpreter state, at an offset identified by the debug offsets. Signed-off-by: Matt Wozniski <[email protected]>
Signed-off-by: Matt Wozniski <[email protected]>
Signed-off-by: Matt Wozniski <[email protected]>
By default pytest keeps temp files for the last 3 runs of the test suite around, but this can quickly fill up a disk due to the number of core files we create. Configure it to only keep temp files for failed tests. Signed-off-by: Matt Wozniski <[email protected]>
godlygeek
force-pushed
the
use_py_debug_offsets_v3
branch
from
August 23, 2024 22:29
18a44a3
to
48ec209
Compare
@pablogsal I've addressed all of your suggestions. I've also rebased this on main, and fixed conflicts that arose due to the fix for handling shim frames. Take another look when you get a chance, please. |
pablogsal
approved these changes
Aug 28, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Python 3.13 adds a new
_Py_DebugOffsets
structure to_PyRuntimeState
which tells us more or less everything that we need to know in order to decode the interpreter's data structures. Look for this structure and, if we find it, use the offsets contained in it rather than the offsets compiled into pystack at build time.This allows us to decode stacks for interpreters with different build time sizes of different structures, including those running in a
python3.13t
free-threaded interpreter.