Skip to content

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.

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

Merged
markshannon merged 5 commits into from
Nov 19, 2025
+38 −10
Merged

GH-139653: Only raise an exception (or fatal error) when the stack pointer is about to overflow the stack. #141711

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
41e9404
Only raise an exception (or fatal error) when the stack pointer is ab…
markshannon Nov 17, 2025
1e7ae53
Fix typo and clarify
markshannon Nov 18, 2025
1de4a56
A couple more typos
markshannon Nov 19, 2025
7dcdf04
Update comment
markshannon Nov 20, 2025
b28b230
Fix typos
markshannon Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix typo and clarify
  • Loading branch information
@markshannon
markshannon committed Nov 18, 2025
commit 1e7ae53afe1a4457e75fff7dc167f9c9dc9f091c
9 changes: 6 additions & 3 deletions Python/ceval.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,12 +494,15 @@ hardware_stack_limits(uintptr_t *base, uintptr_t *top, uintptr_t sp)
}
# endif
// Add some space for caller function then round to minimum page size
Copy link
Copy Markdown
Member

@encukou encukou Nov 18, 2025

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?

Copy link
Copy Markdown
Member Author

@markshannon markshannon Nov 18, 2025

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.

Copy link
Copy Markdown
Member

@encukou encukou Nov 18, 2025

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.

Copy link
Copy Markdown
Member Author

@markshannon markshannon Nov 18, 2025

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.

Copy link
Copy Markdown
Member

@encukou encukou Nov 18, 2025

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?

Copy link
Copy Markdown
Member Author

@markshannon markshannon Nov 18, 2025

Choose a reason for hiding this comment

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

Will do.

// This is a guess at the top of the stack, but should be a reasonably
// good guess if called from _PyThreadState_Attach when creating a thread.
// If the thread is attached deep in a call stack, then the guess will be poor.
#if _Py_STACK_GROWS_DOWN
uintptr_t top_addr = _Py_SIZE_ROUND_UP(sp + 8*sizeof(void*), 4096);
uintptr_t top_addr = _Py_SIZE_ROUND_UP(sp + 8*sizeof(void*), SYSTEM_PAGE_SIZE);
*top = top_addr;
*base = top_addr - Py_C_STACK_SIZE;
# else
uintptr_t base_addr = _Py_SIZE_ROUND_DOWN(sp - 8*sizeof(void*), 4096);
uintptr_t base_addr = _Py_SIZE_ROUND_DOWN(sp - 8*sizeof(void*), SYSTEM_PAGE_SIZE);
*base = base_addr;
*top = base_addr + Py_C_STACK_SIZE;
#endif
Expand Down Expand Up @@ -596,7 +599,7 @@ PyUnstable_ThreadState_ResetStackProtection(PyThreadState *tstate)


/* The function _Py_EnterRecursiveCallTstate() only calls _Py_CheckRecursiveCall()
if the stack poineter is between the stack base and c_stack_hard_limit. */
if the stack pointer is between the stack base and c_stack_hard_limit. */
int
_Py_CheckRecursiveCall(PyThreadState *tstate, const char *where)
{
Expand Down
Loading