Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
GH-139653: Only raise an exception (or fatal error) when the stack pointer is about to overflow the stack.#141711
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
markshannon commented Nov 18, 2025 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
…out to overflow the stack. Only raises if the stack pointer is both below the limit *and* above the stack base. This prevents false positives for user-space threads, as the stack pointer will be outside those bounds if the stack has been swapped.
| # endif | ||
| uintptr_there_addr=_Py_get_machine_stack_pointer(); | ||
| uintptr_ttop_addr=_Py_SIZE_ROUND_UP(here_addr, 4096); | ||
| // Add some space for caller function then round to minimum page size |
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.
Thanks for the comment.
Both “space for caller function” and “minimum page size” are guesses; what happens if they're wrong?
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.
Nothing bad. We only use the upper limit (for stack growing down) for reporting stack use in case of an overflow.
If we are wrong the report value will be off by a few kb, but it is already an estimate. We are assuming that there isn't much stack above the call to create the thread state.
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.
Would it make sense to avoid calculation, and use here_addr as is? With the Py_C_STACK_SIZE guesses, 4k indeed doesn't matter.
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.
It's not a perfect guess, but I don't see any reason to make it worse.
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.
What I see is unnecessary complexity: adding 60 bytes of faux precision to a guess that's in the order of megabytes is rather confusing. These look like more numbers to tweak if you get spurious overflow check failures
If this needs to go in, could you add a comment that this guess can be wildly wrong?
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.
Will do.
Uh oh!
There was an error while loading. Please reload this page.
bedevere-bot commented Nov 18, 2025
🤖 New build scheduled with the buildbot fleet by @encukou for commit 41e9404 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F141711%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
efimov-mikhail left a comment • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
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.
I've noticed some typos.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Python/ceval.c Outdated
| /* The function _Py_EnterRecursiveCallTstate() only calls _Py_CheckRecursiveCall() | ||
| if the recursion_depth reaches recursion_limit. */ | ||
| if the stack poineter is between the stack base and c_stack_hard_limit. */ |
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.
| ifthestackpoineterisbetweenthestackbaseandc_stack_hard_limit. */ | |
| ifthestackpointerisbetweenthestackbaseandc_stack_hard_limit. */ |
| # endif | ||
| uintptr_there_addr=_Py_get_machine_stack_pointer(); | ||
| uintptr_ttop_addr=_Py_SIZE_ROUND_UP(here_addr, 4096); | ||
| // Add some space for caller function then round to minimum page size |
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.
What I see is unnecessary complexity: adding 60 bytes of faux precision to a guess that's in the order of megabytes is rather confusing. These look like more numbers to tweak if you get spurious overflow check failures
If this needs to go in, could you add a comment that this guess can be wildly wrong?
encukou left a comment
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.
Thank you!
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
c25a070 into python:mainUh oh!
There was an error while loading. Please reload this page.
Thanks @markshannon for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Sorry, @markshannon, I could not cleanly backport this to |
…ack pointer is about to overflow the stack. (pythonGH-141711) Only raises if the stack pointer is both below the limit *and* above the stack base. This prevents false positives for user-space threads, as the stack pointer will be outside those bounds if the stack has been swapped. (cherry picked from commit c25a070)
GH-141892 is a backport of this pull request to the 3.14 branch. |
… the stack pointer is about to overflow the stack. (pythonGH-141711) Only raises if the stack pointer is both below the limit *and* above the stack base. This prevents false positives for user-space threads, as the stack pointer will be outside those bounds if the stack has been swapped. Cherry-picked from commit c25a070
… the stack pointer is about to overflow the stack. (pythonGH-141711) Only raises if the stack pointer is both below the limit *and* above the stack base. This prevents false positives for user-space threads, as the stack pointer will be outside those bounds if the stack has been swapped. Cherry-picked from commit c25a070
GH-141944 is a backport of this pull request to the 3.14 branch. |
…tack pointer is about to overflow the stack. (GH-141711) (GH-141944) Only raises if the stack pointer is both below the limit *and* above the stack base. This prevents false positives for user-space threads, as the stack pointer will be outside those bounds if the stack has been swapped. Cherry-picked from commit c25a070 Co-authored-by: Mark Shannon <[email protected]>
…ack pointer is about to overflow the stack. (pythonGH-141711) Only raises if the stack pointer is both below the limit *and* above the stack base. This prevents false positives for user-space threads, as the stack pointer will be outside those bounds if the stack has been swapped.
…ack pointer is about to overflow the stack. (pythonGH-141711) Only raises if the stack pointer is both below the limit *and* above the stack base. This prevents false positives for user-space threads, as the stack pointer will be outside those bounds if the stack has been swapped.
Only raises if the stack pointer is both below the limit and above the stack base. This prevents false positives for user-space threads, as the stack pointer will be outside those bounds if the stack has been swapped.