Skip to content

Commit 255026d

Browse files
markshannonmarkshannon
authored
GH-126910: Make _Py_get_machine_stack_pointer return the stack pointer (#147945)
* Make _Py_get_machine_stack_pointer return the stack pointer (or close to it), not the frame pointer * Make ``_Py_ReachedRecursionLimit`` inline again * Remove ``_Py_MakeRecCheck`` relacing its use with ``_Py_ReachedRecursionLimit`` * Move stack swtiching check into ``_Py_CheckRecursiveCall``
1 parent 9e5b838 commit 255026d

File tree

7 files changed

+37
-55
lines changed

7 files changed

+37
-55
lines changed

‎Include/internal/pycore_ceval.h‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -211,16 +211,16 @@ extern void _PyEval_DeactivateOpCache(void);
211211

212212
/* --- _Py_EnterRecursiveCall() ----------------------------------------- */
213213

214-
static inline int _Py_MakeRecCheck(PyThreadState *tstate) {
214+
static inline int _Py_ReachedRecursionLimit(PyThreadState *tstate) {
215215
uintptr_t here_addr = _Py_get_machine_stack_pointer();
216216
_PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
217-
// Overflow if stack pointer is between soft limit and the base of the hardware stack.
218-
// If it is below the hardware stack base, assume that we have the wrong stack limits, and do nothing.
219-
// We could have the wrong stack limits because of limited platform support, or user-space threads.
217+
// Possible overflow if stack pointer is beyond the soft limit.
218+
// _Py_CheckRecursiveCall will check for corner cases and
219+
// report an error if there is an overflow.
220220
#if _Py_STACK_GROWS_DOWN
221-
return here_addr < _tstate->c_stack_soft_limit && here_addr >= _tstate->c_stack_soft_limit - 2 * _PyOS_STACK_MARGIN_BYTES;
221+
return here_addr < _tstate->c_stack_soft_limit;
222222
#else
223-
return here_addr > _tstate->c_stack_soft_limit && here_addr <= _tstate->c_stack_soft_limit + 2 * _PyOS_STACK_MARGIN_BYTES;
223+
return here_addr > _tstate->c_stack_soft_limit;
224224
#endif
225225
}
226226

@@ -235,7 +235,7 @@ PyAPI_FUNC(int) _Py_CheckRecursiveCallPy(
235235

236236
static inline int _Py_EnterRecursiveCallTstate(PyThreadState *tstate,
237237
const char *where) {
238-
return (_Py_MakeRecCheck(tstate) && _Py_CheckRecursiveCall(tstate, where));
238+
return (_Py_ReachedRecursionLimit(tstate) && _Py_CheckRecursiveCall(tstate, where));
239239
}
240240

241241
static inline int _Py_EnterRecursiveCall(const char *where) {
@@ -249,8 +249,6 @@ static inline void _Py_LeaveRecursiveCallTstate(PyThreadState *tstate) {
249249

250250
PyAPI_FUNC(void) _Py_InitializeRecursionLimits(PyThreadState *tstate);
251251

252-
PyAPI_FUNC(int) _Py_ReachedRecursionLimit(PyThreadState *tstate);
253-
254252
// Export for test_peg_generator
255253
PyAPI_FUNC(int) _Py_ReachedRecursionLimitWithMargin(
256254
PyThreadState *tstate,

‎Include/internal/pycore_pystate.h‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -306,23 +306,23 @@ _Py_AssertHoldsTstateFunc(const char *func)
306306
#define _Py_AssertHoldsTstate()
307307
#endif
308308

309-
#if !_Py__has_builtin(__builtin_frame_address) && !defined(__GNUC__) && !defined(_MSC_VER)
310-
static uintptr_t return_pointer_as_int(char* p) {
311-
return (uintptr_t)p;
312-
}
313-
#endif
314309

315310
static inline uintptr_t
316311
_Py_get_machine_stack_pointer(void) {
317-
#if _Py__has_builtin(__builtin_frame_address) || defined(__GNUC__)
318-
return (uintptr_t)__builtin_frame_address(0);
319-
#elif defined(_MSC_VER)
320-
return (uintptr_t)_AddressOfReturnAddress();
312+
uintptr_t result;
313+
#if !defined(_MSC_VER) && defined(_M_ARM64)
314+
result = __getReg(31);
315+
#elif defined(_MSC_VER) && defined(_M_X64)
316+
result = (uintptr_t)_AddressOfReturnAddress();
317+
#elif defined(__aarch64__)
318+
__asm__ ("mov %0, sp" : "=r" (result));
319+
#elif defined(__x86_64__)
320+
__asm__("{movq %%rsp, %0" : "=r" (result));
321321
#else
322322
char here;
323-
/* Avoid compiler warning about returning stack address */
324-
return return_pointer_as_int(&here);
323+
result = (uintptr_t)&here;
325324
#endif
325+
return result;
326326
}
327327

328328
static inline intptr_t

‎Include/internal/pycore_pythonrun.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ extern PyObject * _Py_CompileStringObjectWithModule(
4646
* stack consumption of PyEval_EvalDefault */
4747
#if (defined(Py_DEBUG) \
4848
|| defined(_Py_ADDRESS_SANITIZER) \
49-
|| defined(_Py_THREAD_SANITIZER))
49+
|| defined(_Py_THREAD_SANITIZER)) \
50+
|| defined(_Py_UNDEFINED_BEHAVIOR_SANITIZER)
5051
# define _PyOS_LOG2_STACK_MARGIN 12
5152
#else
5253
# define _PyOS_LOG2_STACK_MARGIN 11

‎Include/pyport.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,11 @@ extern "C" {
598598
# define _Py_NO_SANITIZE_THREAD __attribute__((no_sanitize_thread))
599599
# endif
600600
# endif
601+
# if __has_feature(undefined_behavior_sanitizer)
602+
# if !defined(_Py_UNDEFINED_BEHAVIOR_SANITIZER)
603+
# define _Py_UNDEFINED_BEHAVIOR_SANITIZER
604+
# endif
605+
# endif
601606
#elif defined(__GNUC__)
602607
# if defined(__SANITIZE_ADDRESS__)
603608
# define _Py_ADDRESS_SANITIZER

‎Lib/test/test_pyexpat.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -707,7 +707,7 @@ def test_trigger_leak(self):
707707
def test_deeply_nested_content_model(self):
708708
# This should raise a RecursionError and not crash.
709709
# See https://github.com/python/cpython/issues/145986.
710-
N = 500_000
710+
N = 800_000
711711
data = (
712712
b'<!DOCTYPE root [\n<!ELEMENT root '
713713
+ b'(a, ' * N + b'a' + b')' * N

‎Python/ceval.c‎

Lines changed: 10 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -49,20 +49,6 @@ _Py_ReachedRecursionLimitWithMargin(PyThreadState *tstate, int margin_count)
4949
#endif
5050
}
5151

52-
void
53-
_Py_EnterRecursiveCallUnchecked(PyThreadState *tstate)
54-
{
55-
uintptr_t here_addr = _Py_get_machine_stack_pointer();
56-
_PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
57-
#if _Py_STACK_GROWS_DOWN
58-
if (here_addr < _tstate->c_stack_hard_limit) {
59-
#else
60-
if (here_addr > _tstate->c_stack_hard_limit) {
61-
#endif
62-
Py_FatalError("Unchecked stack overflow.");
63-
}
64-
}
65-
6652
#if defined(__s390x__)
6753
# define Py_C_STACK_SIZE 320000
6854
#elif defined(_WIN32)
@@ -278,7 +264,7 @@ PyUnstable_ThreadState_ResetStackProtection(PyThreadState *tstate)
278264

279265

280266
/* The function _Py_EnterRecursiveCallTstate() only calls _Py_CheckRecursiveCall()
281-
if the stack pointer is between the stack base and c_stack_hard_limit. */
267+
if the stack pointer is beyond c_stack_soft_limit. */
282268
int
283269
_Py_CheckRecursiveCall(PyThreadState *tstate, const char *where)
284270
{
@@ -287,16 +273,21 @@ _Py_CheckRecursiveCall(PyThreadState *tstate, const char *where)
287273
assert(_tstate->c_stack_soft_limit != 0);
288274
assert(_tstate->c_stack_hard_limit != 0);
289275
#if _Py_STACK_GROWS_DOWN
290-
assert(here_addr >= _tstate->c_stack_hard_limit - _PyOS_STACK_MARGIN_BYTES);
291276
if (here_addr < _tstate->c_stack_hard_limit) {
292-
/* Overflowing while handling an overflow. Give up. */
277+
if (here_addr < _tstate->c_stack_hard_limit - _PyOS_STACK_MARGIN_BYTES) {
278+
// Far out of bounds -- Assume stack switching has occurred
279+
return 0;
280+
}
293281
int kbytes_used = (int)(_tstate->c_stack_top - here_addr)/1024;
294282
#else
295-
assert(here_addr <= _tstate->c_stack_hard_limit + _PyOS_STACK_MARGIN_BYTES);
296283
if (here_addr > _tstate->c_stack_hard_limit) {
297-
/* Overflowing while handling an overflow. Give up. */
284+
if (here_addr > _tstate->c_stack_hard_limit + _PyOS_STACK_MARGIN_BYTES) {
285+
// Far out of bounds -- Assume stack switching has occurred
286+
return 0;
287+
}
298288
int kbytes_used = (int)(here_addr - _tstate->c_stack_top)/1024;
299289
#endif
290+
/* Too much stack used to safely raise an exception. Give up. */
300291
char buffer[80];
301292
snprintf(buffer, 80, "Unrecoverable stack overflow (used %d kB)%s", kbytes_used, where);
302293
Py_FatalError(buffer);
@@ -1201,19 +1192,6 @@ _PyEval_GetIter(_PyStackRef iterable, _PyStackRef *index_or_null, int yield_from
12011192
return PyStackRef_FromPyObjectSteal(iter_o);
12021193
}
12031194

1204-
Py_NO_INLINE int
1205-
_Py_ReachedRecursionLimit(PyThreadState *tstate) {
1206-
uintptr_t here_addr = _Py_get_machine_stack_pointer();
1207-
_PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
1208-
assert(_tstate->c_stack_hard_limit != 0);
1209-
#if _Py_STACK_GROWS_DOWN
1210-
return here_addr <= _tstate->c_stack_soft_limit;
1211-
#else
1212-
return here_addr >= _tstate->c_stack_soft_limit;
1213-
#endif
1214-
}
1215-
1216-
12171195
#if (defined(__GNUC__) && __GNUC__ >= 10 && !defined(__clang__)) && defined(__x86_64__)
12181196
/*
12191197
* gh-129987: The SLP autovectorizer can cause poor code generation for

‎Python/jit.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ _PyJIT_Compile(_PyExecutorObject *executor, const _PyUOpInstruction trace[], siz
734734
return 0;
735735
}
736736

737-
/* One-off compilation of the jit entry shim
737+
/* One-off compilation of the jit entry shim.
738738
* We compile this once only as it effectively a normal
739739
* function, but we need to use the JIT because it needs
740740
* to understand the jit-specific calling convention.

0 commit comments

Comments
 (0)