8000 bpo-32280: Store `_PyRuntime` in a named section by maxbelanger · Pull Request #4802 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-32280: Store _PyRuntime in a named section #4802

8000
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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 3, 2021
Merged

bpo-32280: Store _PyRuntime in a named section #4802

merged 1 commit into from
Jun 3, 2021

Conversation

maxbelanger
Copy link
Contributor
@maxbelanger maxbelanger commented Dec 11, 2017 •
edited by bedevere-bot
Loading

This patch stores the _PyRuntime structure in a section of the same name (on Windows, macOS or Linux; otherwise untested in our configurations). This allows a debugging or crash reporting tool to quickly locate this structure at runtime without requiring the symbol table.

https://bugs.python.org/issue32280

@brettcannon brettcannon added the type-feature A feature request or enhancement label Apr 1, 2019
@ericsnowcurrently
Copy link
Member

@zooba, I figure you'd know about "named sections" well enough to have some insight here. The point of the PR may be moot (see the BPO issue), but let's assume we want to accommodate the use case described in the issue. Is a named section the right way to go? Is the proposed change the approach we'd want in CPython?

Note that the question of the fate of the _Py_Runtime static global is discussed separately in the BPO issue. :)

@ericsnowcurrently
Copy link
Member

ping, @zooba

@csabella csabella requested a review from vstinner February 4, 2020 12:08
@vstinner
Copy link
Member
vstinner commented Feb 6, 2020

csabella requested a review from vstinner 2 days ago

I don't know anything about the Windows and macOS specific pagma. So I have no opinion on this PR.

@csabella csabella requested a review from zooba May 25, 2020 14:48
@pablogsal pablogsal force-pushed the impl-runtime-section branch 2 times, most recently from db906f5 to 10f79fe Compare June 3, 2021 22:30
@pablogsal
Copy link
Member

This can be very useful to debuggers so I think this is a good idea but I think we should just have it as non-stable detail that could change if we need to so I am removing the NEWS item. People implementing debuggers and runtime state tools will already read the source to reverse engineer the information needed so they will find this in any case.

I have also updated the patch to include this on Linux for gcc/clang

@pablogsal pablogsal force-pushed the impl-runtime-section branch 2 times, most recently from 7f48393 to 8ca4970 Compare June 3, 2021 22:40
@pablogsal
Copy link
Member
pablogsal commented Jun 3, 2021 •
edited
Loading

Example usage:

✦ ❯ readelf -t ./python | grep PyRuntime -A 2
  [27] .PyRuntime
       PROGBITS         00000000005a0cd0  000000000059fcd0  0
       00000000000002a0 0000000000000000  0                 8

✦ ❯ ./python
Python 3.11.0a0 (heads/impl-runtime-section-dirty:7f48393536, Jun  3 2021, 23:39:39) [Clang 11.1.0 ] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>>
[2]  + 107638 suspended  ./python

✦2 ❯ head /proc/107638/maps
56268ebed000-56268ec48000 r--p 00000000 08:12 5247400                    /home/pablogsal/github/python/master/python

❯ sudo gdb -p 107638
gdb) x/b (0x56268ebed000+0x00000000005a0cd0)
0x56268f18dcd0 <_PyRuntime>:    0

obviously, the real advantage here is to other tools much less powerful than GDB :)

@pablogsal pablogsal force-pushed the impl-runtime-section branch from 8ca4970 to 44b352a Compare June 3, 2021 22:50
@pablogsal pablogsal force-pushed the impl-runtime-section branch from 44b352a to ffcd44c Compare June 3, 2021 23:14
@pablogsal pablogsal merged commit 35002aa into python:main Jun 3, 2021
@pablogsal pablogsal added the needs backport to 3.10 only security fixes label Jul 29, 2021
@miss-islington
Copy link
Contributor

Thanks @maxbelanger for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 29, 2021
@maxbelanger @miss-islington
bpo-32280: Store _PyRuntime in a named section (pythonGH-4802)
9232dbe 
This commit stores the _PyRuntime structure in a section of the same name. This allows a debugging or crash reporting tool to quickly locate this structure at runtime without requiring the symbol table.

Co-authored-by: Pablo Galindo <pablogsal@gmail.com>
(cherry picked from commit 35002aa)

Co-authored-by: Max Bélanger <aeromax@gmail.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jul 29, 2021
@bedevere-bot
Copy link

GH-27448 is a backport of this pull request to the 3.10 branch.

pablogsal pushed a commit that referenced this pull request Jul 29, 2021
@miss-islington @maxbelanger
bpo-32280: Store _PyRuntime in a named section (GH-4802) (#27448)
ebba286 
This commit stores the _PyRuntime structure in a section of the same name. This allows a debugging or crash reporting tool to quickly locate this structure at runtime without requiring the symbol table.

Co-authored-by: Pablo Galindo <pablogsal@gmail.com>
(cherry picked from commit 35002aa)

Co-authored-by: Max Bélanger <aeromax@gmail.com>

Co-authored-by: Max Bélanger <aeromax@gmail.com>
This was referenced Oct 31, 2024
Closed
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rezduan83 Rezduan83 Rezduan83 approved these changes

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

@vstinner vstinner Awaiting requested review from vstinner

@zooba zooba Awaiting requested review from zooba

Assignees
No one assigned
Labels
skip news type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@maxbelanger @ericsnowcurrently @vstinner @pablogsal @miss-islington @bedevere-bot @Rezduan83 @brettcannon @the-knights-who-say-ni
0