-
-
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
Changes from 4 commits
41e9404
1e7ae53
1de4a56
7dcdf04
b28b230
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Only raise a ``RecursionError`` or trigger a fatal error if the stack | ||
| pointer is both below the limit pointer *and* above the stack base. If | ||
| outside of these bounds assume that it is OK. This prevents false positives | ||
| when user-space threads swap stacks. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -362,9 +362,11 @@ _Py_ReachedRecursionLimitWithMargin(PyThreadState *tstate, int margin_count) | |
| _Py_InitializeRecursionLimits(tstate); | ||
| } | ||
| #if _Py_STACK_GROWS_DOWN | ||
| return here_addr <= _tstate->c_stack_soft_limit + margin_count * _PyOS_STACK_MARGIN_BYTES; | ||
| return here_addr <= _tstate->c_stack_soft_limit + margin_count * _PyOS_STACK_MARGIN_BYTES && | ||
| here_addr >= _tstate->c_stack_soft_limit - 2 * _PyOS_STACK_MARGIN_BYTES; | ||
| #else | ||
| return here_addr > _tstate->c_stack_soft_limit - margin_count * _PyOS_STACK_MARGIN_BYTES; | ||
| return here_addr > _tstate->c_stack_soft_limit - margin_count * _PyOS_STACK_MARGIN_BYTES && | ||
| here_addr <= _tstate->c_stack_soft_limit + 2 * _PyOS_STACK_MARGIN_BYTES; | ||
| #endif | ||
| } | ||
|
|
||
|
|
@@ -455,7 +457,7 @@ int pthread_attr_destroy(pthread_attr_t *a) | |
| #endif | ||
|
|
||
| static void | ||
| hardware_stack_limits(uintptr_t *base, uintptr_t *top) | ||
| hardware_stack_limits(uintptr_t *base, uintptr_t *top, uintptr_t sp) | ||
| { | ||
| #ifdef WIN32 | ||
| ULONG_PTR low, high; | ||
|
|
@@ -491,10 +493,19 @@ hardware_stack_limits(uintptr_t *base, uintptr_t *top) | |
| return; | ||
| } | ||
| # endif | ||
| uintptr_t here_addr = _Py_get_machine_stack_pointer(); | ||
| uintptr_t top_addr = _Py_SIZE_ROUND_UP(here_addr, 4096); | ||
| // Add some space for caller function then round to minimum page size | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the comment.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to avoid calculation, and use
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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*), 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*), SYSTEM_PAGE_SIZE); | ||
| *base = base_addr; | ||
| *top = base_addr + Py_C_STACK_SIZE; | ||
| #endif | ||
| #endif | ||
| } | ||
|
|
||
|
|
@@ -543,7 +554,8 @@ void | |
| _Py_InitializeRecursionLimits(PyThreadState *tstate) | ||
| { | ||
| uintptr_t base, top; | ||
| hardware_stack_limits(&base, &top); | ||
| uintptr_t here_addr = _Py_get_machine_stack_pointer(); | ||
| hardware_stack_limits(&base, &top, here_addr); | ||
| assert(top != 0); | ||
|
|
||
| tstate_set_stack(tstate, base, top); | ||
|
|
@@ -587,7 +599,7 @@ PyUnstable_ThreadState_ResetStackProtection(PyThreadState *tstate) | |
|
|
||
|
|
||
| /* The function _Py_EnterRecursiveCallTstate() only calls _Py_CheckRecursiveCall() | ||
| if the recursion_depth reaches recursion_limit. */ | ||
| if the stack pointer is between the stack base and c_stack_hard_limit. */ | ||
| int | ||
| _Py_CheckRecursiveCall(PyThreadState *tstate, const char *where) | ||
| { | ||
|
|
@@ -596,10 +608,12 @@ _Py_CheckRecursiveCall(PyThreadState *tstate, const char *where) | |
| assert(_tstate->c_stack_soft_limit != 0); | ||
encukou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| assert(_tstate->c_stack_hard_limit != 0); | ||
| #if _Py_STACK_GROWS_DOWN | ||
| assert(here_addr >= _tstate->c_stack_hard_limit - _PyOS_STACK_MARGIN_BYTES); | ||
| if (here_addr < _tstate->c_stack_hard_limit) { | ||
| /* Overflowing while handling an overflow. Give up. */ | ||
| int kbytes_used = (int)(_tstate->c_stack_top - here_addr)/1024; | ||
| #else | ||
| assert(here_addr <= _tstate->c_stack_hard_limit + _PyOS_STACK_MARGIN_BYTES); | ||
| if (here_addr > _tstate->c_stack_hard_limit) { | ||
| /* Overflowing while handling an overflow. Give up. */ | ||
| int kbytes_used = (int)(here_addr - _tstate->c_stack_top)/1024; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.