Skip to content
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

GH-91079: Implement C stack limits using addresses, not counters. #130007

Merged
merged 46 commits into from
Feb 19, 2025
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
fc910a3
Hide that C recursion protection is implemented with a counter. There…
markshannon Feb 10, 2025
afeb866
Implement C recursion protection with limit pointers
markshannon Feb 11, 2025
22ca169
Use uintptr_t instead of char * to avoid warnings and UB
markshannon Feb 11, 2025
e8d8c4b
Merge branch 'main' into c-recursion-limit
markshannon Feb 11, 2025
b3638a5
Fix typo and update stable ABI
markshannon Feb 11, 2025
774efb5
Tweak AST test numbers
markshannon Feb 11, 2025
151c88f
Improve logic handling trial C stack overflow
markshannon Feb 11, 2025
350f8ec
Remove calls to PyOS_CheckStack
markshannon Feb 11, 2025
428c46a
Up the limits for recursion tests
markshannon Feb 11, 2025
a9be141
Use deeper stack for test
markshannon Feb 12, 2025
03fc52e
Remove exceeds_recursion_limit and get_c_recursion_limit. Use platfor…
markshannon Feb 12, 2025
afac1e6
Do fewer probes when growing stack limit
markshannon Feb 12, 2025
9da904d
Tweak depths
markshannon Feb 12, 2025
a802ff6
Merge branch 'main' into c-recursion-limit
markshannon Feb 12, 2025
dbcf6f0
Perform lazy initialization of c recursion check
markshannon Feb 12, 2025
2cc3287
Post merge fixup
markshannon Feb 12, 2025
e697926
Up depth again
markshannon Feb 12, 2025
f8a9143
Drop 'failing' depth
markshannon Feb 12, 2025
7d6d77f
Add news
markshannon Feb 12, 2025
31a83dc
Increase headroom
markshannon Feb 12, 2025
47c50aa
Update test
markshannon Feb 12, 2025
495c4ea
Tweak some more thresholds and tests
markshannon Feb 12, 2025
9e0cc67
Add stack protection to parser
markshannon Feb 13, 2025
857a7bb
Make tests more robust to low stacks
markshannon Feb 13, 2025
9c9326a
Improve error messages for stack overflow
markshannon Feb 13, 2025
75d3219
Merge branch 'main' into c-recursion-limit
markshannon Feb 13, 2025
3e41b46
Fix formatting
markshannon Feb 13, 2025
158401a
Halve size of WASI stack
markshannon Feb 13, 2025
c1eb229
Reduce webassembly 'stack size' by 10
markshannon Feb 13, 2025
7407d2b
Halve size of WASI stack, again
markshannon Feb 13, 2025
978b5e7
Change WASI stack back to 100k
markshannon Feb 13, 2025
7b36f59
Add many skip tests for WASI due to stack issues
markshannon Feb 13, 2025
5e5db03
Probe all pages when extending stack limits
markshannon Feb 14, 2025
82173ed
Fix compiler warnings
markshannon Feb 14, 2025
64cfd86
Use GetCurrentThreadStackLimits instead of probing with alloca
markshannon Feb 14, 2025
e52137f
Refactor a bit
markshannon Feb 14, 2025
704c336
Merge branch 'main' into c-recursion-limit
markshannon Feb 14, 2025
21366c3
Fix logic error in test
markshannon Feb 17, 2025
7761d31
Make ABI function needed for Py_TRASHCAN_BEGIN private
markshannon Feb 17, 2025
b067e3e
Move new fields to _PyThreadStateImpl to avoid any potential API brea…
markshannon Feb 17, 2025
b0e695f
Fix missing cast
markshannon Feb 17, 2025
c4cd68f
yet another missing _
markshannon Feb 17, 2025
9654790
Tidy up a bit
markshannon Feb 18, 2025
2cef96f
Restore use of exceeds_recursion_limit
markshannon Feb 18, 2025
c5d8a40
Address remaining review comments
markshannon Feb 18, 2025
33e11c8
Merge branch 'main' into c-recursion-limit
markshannon Feb 19, 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
1 change: 1 addition & 0 deletions Doc/data/stable_abi.dat

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Include/ceval.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ PyAPI_FUNC(int) Py_GetRecursionLimit(void);
PyAPI_FUNC(int) Py_EnterRecursiveCall(const char *where);
PyAPI_FUNC(void) Py_LeaveRecursiveCall(void);

PyAPI_FUNC(int) Py_ReachedRecursionLimit(PyThreadState *tstate, int margin_count);

PyAPI_FUNC(const char *) PyEval_GetFuncName(PyObject *);
PyAPI_FUNC(const char *) PyEval_GetFuncDesc(PyObject *);

Expand Down
8 changes: 3 additions & 5 deletions Include/cpython/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -490,15 +490,13 @@ PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(PyThreadState *tstate);
#define Py_TRASHCAN_BEGIN(op, dealloc) \
do { \
PyThreadState *tstate = PyThreadState_Get(); \
if (tstate->c_recursion_remaining <= Py_TRASHCAN_HEADROOM && Py_TYPE(op)->tp_dealloc == (destructor)dealloc) { \
if (Py_ReachedRecursionLimit(tstate, 1) && Py_TYPE(op)->tp_dealloc == (destructor)dealloc) { \
_PyTrash_thread_deposit_object(tstate, (PyObject *)op); \
break; \
} \
tstate->c_recursion_remaining--;
}
/* The body of the deallocator is here. */
#define Py_TRASHCAN_END \
tstate->c_recursion_remaining++; \
if (tstate->delete_later && tstate->c_recursion_remaining > (Py_TRASHCAN_HEADROOM*2)) { \
if (tstate->delete_later && !Py_ReachedRecursionLimit(tstate, 2)) { \
_PyTrash_thread_destroy_chain(tstate); \
} \
} while (0);
Expand Down
37 changes: 4 additions & 33 deletions Include/cpython/pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ struct _ts {
int py_recursion_remaining;
int py_recursion_limit;

int c_recursion_remaining;
// These are addresses, but we need to convert to ints to avoid UB.
iritkatriel marked this conversation as resolved.
Show resolved Hide resolved
uintptr_t c_stack_top;
uintptr_t c_stack_soft_limit;
uintptr_t c_stack_hard_limit;
int recursion_headroom; /* Allow 50 more calls to handle any errors. */

/* 'tracing' keeps track of the execution depth when tracing/profiling.
Expand Down Expand Up @@ -202,37 +205,6 @@ struct _ts {
PyObject *threading_local_sentinel;
};

#ifdef Py_DEBUG
// A debug build is likely built with low optimization level which implies
// higher stack memory usage than a release build: use a lower limit.
# define Py_C_RECURSION_LIMIT 500
#elif defined(__s390x__)
# define Py_C_RECURSION_LIMIT 800
#elif defined(_WIN32) && defined(_M_ARM64)
# define Py_C_RECURSION_LIMIT 1000
#elif defined(_WIN32)
# define Py_C_RECURSION_LIMIT 3000
#elif defined(__ANDROID__)
// On an ARM64 emulator, API level 34 was OK with 10000, but API level 21
// crashed in test_compiler_recursion_limit.
# define Py_C_RECURSION_LIMIT 3000
#elif defined(_Py_ADDRESS_SANITIZER)
# define Py_C_RECURSION_LIMIT 4000
#elif defined(__sparc__)
// test_descr crashed on sparc64 with >7000 but let's keep a margin of error.
# define Py_C_RECURSION_LIMIT 4000
#elif defined(__wasi__)
// Based on wasmtime 16.
# define Py_C_RECURSION_LIMIT 5000
#elif defined(__hppa__) || defined(__powerpc64__)
// test_descr crashed with >8000 but let's keep a margin of error.
# define Py_C_RECURSION_LIMIT 5000
#else
// This value is duplicated in Lib/test/support/__init__.py
# define Py_C_RECURSION_LIMIT 10000
#endif


/* other API */

/* Similar to PyThreadState_Get(), but don't issue a fatal error
Expand All @@ -246,7 +218,6 @@ _PyThreadState_UncheckedGet(void)
return PyThreadState_GetUnchecked();
}


// Disable tracing and profiling.
PyAPI_FUNC(void) PyThreadState_EnterTracing(PyThreadState *tstate);

Expand Down
39 changes: 19 additions & 20 deletions Include/internal/pycore_ceval.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,18 +193,11 @@ extern void _PyEval_DeactivateOpCache(void);

/* --- _Py_EnterRecursiveCall() ----------------------------------------- */

#ifdef USE_STACKCHECK
/* With USE_STACKCHECK macro defined, trigger stack checks in
_Py_CheckRecursiveCall() on every 64th call to _Py_EnterRecursiveCall. */
static inline int _Py_MakeRecCheck(PyThreadState *tstate) {
return (tstate->c_recursion_remaining-- < 0
|| (tstate->c_recursion_remaining & 63) == 0);
char here;
uintptr_t here_addr = (uintptr_t)&here;
return here_addr < tstate->c_stack_soft_limit;
}
#else
static inline int _Py_MakeRecCheck(PyThreadState *tstate) {
return tstate->c_recursion_remaining-- < 0;
}
#endif

// Export for '_json' shared extension, used via _Py_EnterRecursiveCall()
// static inline function.
Expand All @@ -220,23 +213,30 @@ static inline int _Py_EnterRecursiveCallTstate(PyThreadState *tstate,
return (_Py_MakeRecCheck(tstate) && _Py_CheckRecursiveCall(tstate, where));
}

static inline void _Py_EnterRecursiveCallTstateUnchecked(PyThreadState *tstate) {
assert(tstate->c_recursion_remaining > 0);
tstate->c_recursion_remaining--;
}

static inline int _Py_EnterRecursiveCall(const char *where) {
PyThreadState *tstate = _PyThreadState_GET();
return _Py_EnterRecursiveCallTstate(tstate, where);
}

static inline void _Py_LeaveRecursiveCallTstate(PyThreadState *tstate) {
tstate->c_recursion_remaining++;
static inline void _Py_LeaveRecursiveCallTstate(PyThreadState *tstate) {
(void)tstate;
}

PyAPI_FUNC(void) _Py_InitializeRecursionLimits(PyThreadState *tstate);

static inline int _Py_ReachedRecursionLimit(PyThreadState *tstate, int margin_count) {
char here;
uintptr_t here_addr = (uintptr_t)&here;
if (here_addr > tstate->c_stack_soft_limit + margin_count * PYOS_STACK_MARGIN_BYTES) {
return 0;
}
if (tstate->c_stack_hard_limit == 0) {
_Py_InitializeRecursionLimits(tstate);
}
return here_addr <= tstate->c_stack_soft_limit + margin_count * PYOS_STACK_MARGIN_BYTES;
}

static inline void _Py_LeaveRecursiveCall(void) {
PyThreadState *tstate = _PyThreadState_GET();
_Py_LeaveRecursiveCallTstate(tstate);
}

extern struct _PyInterpreterFrame* _PyEval_GetFrame(void);
Expand Down Expand Up @@ -327,7 +327,6 @@ void _Py_unset_eval_breaker_bit_all(PyInterpreterState *interp, uintptr_t bit);

PyAPI_FUNC(PyObject *) _PyFloat_FromDouble_ConsumeInputs(_PyStackRef left, _PyStackRef right, double value);


#ifdef __cplusplus
}
#endif
Expand Down
2 changes: 0 additions & 2 deletions Include/internal/pycore_symtable.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ struct symtable {
PyObject *st_private; /* name of current class or NULL */
_PyFutureFeatures *st_future; /* module's future features that affect
the symbol table */
int recursion_depth; /* current recursion depth */
int recursion_limit; /* recursion limit */
};

typedef struct _symtable_entry {
Expand Down
16 changes: 11 additions & 5 deletions Include/pythonrun.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,18 @@ PyAPI_FUNC(void) PyErr_DisplayException(PyObject *);
/* Stuff with no proper home (yet) */
PyAPI_DATA(int) (*PyOS_InputHook)(void);

/* Stack size, in "pointers" (so we get extra safety margins
on 64-bit platforms). On a 32-bit platform, this translates
to an 8k margin. */
#define PYOS_STACK_MARGIN 2048
/* Stack size, in "pointers". This must be large enough, so
* no two calls to check recursion depth are more than this far
* apart. In practice, that means it must be larger than the C
* stack consumption of PyEval_EvalDefault */
#if defined(Py_DEBUG) && defined(WIN32)
# define PYOS_STACK_MARGIN 3072
#else
# define PYOS_STACK_MARGIN 2048
#endif
#define PYOS_STACK_MARGIN_BYTES (PYOS_STACK_MARGIN * sizeof(void *))

#if defined(WIN32) && !defined(MS_WIN64) && !defined(_M_ARM) && defined(_MSC_VER) && _MSC_VER >= 1300
#if defined(WIN32)
encukou marked this conversation as resolved.
Show resolved Hide resolved
/* Enable stack checking under Microsoft C */
// When changing the platforms, ensure PyOS_CheckStack() docs are still correct
#define USE_STACKCHECK
Expand Down
6 changes: 4 additions & 2 deletions Lib/test/list_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
from functools import cmp_to_key

from test import seq_tests
from test.support import ALWAYS_EQ, NEVER_EQ, get_c_recursion_limit, skip_emscripten_stack_overflow
from test.support import ALWAYS_EQ, NEVER_EQ
from test.support import skip_emscripten_stack_overflow, skip_wasi_stack_overflow


class CommonTest(seq_tests.CommonTest):
Expand Down Expand Up @@ -59,10 +60,11 @@ def test_repr(self):
self.assertEqual(str(a2), "[0, 1, 2, [...], 3]")
self.assertEqual(repr(a2), "[0, 1, 2, [...], 3]")

@skip_wasi_stack_overflow()
@skip_emscripten_stack_overflow()
def test_repr_deep(self):
a = self.type2test([])
for i in range(get_c_recursion_limit() + 1):
for i in range(100_000):
encukou marked this conversation as resolved.
Show resolved Hide resolved
a = self.type2test([a])
self.assertRaises(RecursionError, repr, a)

Expand Down
5 changes: 3 additions & 2 deletions Lib/test/mapping_tests.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# tests common to dict and UserDict
import unittest
import collections
from test.support import get_c_recursion_limit, skip_emscripten_stack_overflow
from test.support import skip_emscripten_stack_overflow, skip_wasi_stack_overflow


class BasicTestMappingProtocol(unittest.TestCase):
Expand Down Expand Up @@ -622,10 +622,11 @@ def __repr__(self):
d = self._full_mapping({1: BadRepr()})
self.assertRaises(Exc, repr, d)

@skip_wasi_stack_overflow()
@skip_emscripten_stack_overflow()
def test_repr_deep(self):
d = self._empty_mapping()
for i in range(get_c_recursion_limit() + 1):
for i in range(100_000):
d0 = d
d = self._empty_mapping()
d[1] = d0
Expand Down
1 change: 0 additions & 1 deletion Lib/test/pythoninfo.py
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,6 @@ def collect_testcapi(info_add):
for name in (
'LONG_MAX', # always 32-bit on Windows, 64-bit on 64-bit Unix
'PY_SSIZE_T_MAX',
'Py_C_RECURSION_LIMIT',
'SIZEOF_TIME_T', # 32-bit or 64-bit depending on the platform
'SIZEOF_WCHAR_T', # 16-bit or 32-bit depending on the platform
):
Expand Down
20 changes: 4 additions & 16 deletions Lib/test/support/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@
"run_with_tz", "PGO", "missing_compiler_executable",
"ALWAYS_EQ", "NEVER_EQ", "LARGEST", "SMALLEST",
"LOOPBACK_TIMEOUT", "INTERNET_TIMEOUT", "SHORT_TIMEOUT", "LONG_TIMEOUT",
"Py_DEBUG", "exceeds_recursion_limit", "get_c_recursion_limit",
"skip_on_s390x",
"Py_DEBUG", "skip_on_s390x",
"requires_jit_enabled",
"requires_jit_disabled",
"force_not_colorized",
Expand Down Expand Up @@ -558,6 +557,9 @@ def skip_android_selinux(name):
def skip_emscripten_stack_overflow():
return unittest.skipIf(is_emscripten, "Exhausts limited stack on Emscripten")

def skip_wasi_stack_overflow():
return unittest.skipIf(is_wasi, "Exhausts stack on WASI")

is_apple_mobile = sys.platform in {"ios", "tvos", "watchos"}
is_apple = is_apple_mobile or sys.platform == "darwin"

Expand Down Expand Up @@ -2623,20 +2625,6 @@ def adjust_int_max_str_digits(max_digits):
finally:
sys.set_int_max_str_digits(current)


def get_c_recursion_limit():
try:
import _testcapi
return _testcapi.Py_C_RECURSION_LIMIT
except ImportError:
raise unittest.SkipTest('requires _testcapi')


def exceeds_recursion_limit():
"""For recursion tests, easily exceeds default recursion limit."""
return get_c_recursion_limit() * 3


# Windows doesn't have os.uname() but it doesn't support s390x.
is_s390x = hasattr(os, 'uname') and os.uname().machine == 's390x'
skip_on_s390x = unittest.skipIf(is_s390x, 'skipped on s390x')
Expand Down
23 changes: 12 additions & 11 deletions Lib/test/test_ast/test_ast.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
_testinternalcapi = None

from test import support
from test.support import os_helper, script_helper, skip_emscripten_stack_overflow
from test.support import os_helper, script_helper
from test.support import skip_emscripten_stack_overflow, skip_wasi_stack_overflow
from test.support.ast_helper import ASTTestMixin
from test.test_ast.utils import to_tuple
from test.test_ast.snippets import (
Expand Down Expand Up @@ -745,25 +746,25 @@ def next(self):
enum._test_simple_enum(_Precedence, ast._Precedence)

@support.cpython_only
@skip_wasi_stack_overflow()
@skip_emscripten_stack_overflow()
def test_ast_recursion_limit(self):
fail_depth = support.exceeds_recursion_limit()
crash_depth = 100_000
success_depth = int(support.get_c_recursion_limit() * 0.8)
crash_depth = 200_000
success_depth = 200
if _testinternalcapi is not None:
remaining = _testinternalcapi.get_c_recursion_remaining()
success_depth = min(success_depth, remaining)

def check_limit(prefix, repeated):
expect_ok = prefix + repeated * success_depth
ast.parse(expect_ok)
for depth in (fail_depth, crash_depth):
broken = prefix + repeated * depth
details = "Compiling ({!r} + {!r} * {})".format(
prefix, repeated, depth)
with self.assertRaises(RecursionError, msg=details):
with support.infinite_recursion():
ast.parse(broken)

broken = prefix + repeated * crash_depth
details = "Compiling ({!r} + {!r} * {})".format(
prefix, repeated, crash_depth)
with self.assertRaises(RecursionError, msg=details):
with support.infinite_recursion():
ast.parse(broken)

check_limit("a", "()")
check_limit("a", ".b")
Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_call.py
Original file line number Diff line number Diff line change
Expand Up @@ -1064,10 +1064,10 @@ def c_py_recurse(m):
recurse(90_000)
with self.assertRaises(RecursionError):
recurse(101_000)
c_recurse(100)
c_recurse(50)
with self.assertRaises(RecursionError):
c_recurse(90_000)
c_py_recurse(90)
c_py_recurse(50)
with self.assertRaises(RecursionError):
c_py_recurse(100_000)

Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_capi/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ def test_trashcan_subclass(self):
# activated when its tp_dealloc is being called by a subclass
from _testcapi import MyList
L = None
for i in range(1000):
for i in range(100):
L = MyList((L,))

@support.requires_resource('cpu')
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ def test_odd_sizes(self):
self.assertEqual(Dot(1)._replace(d=999), (999,))
self.assertEqual(Dot(1)._fields, ('d',))

n = support.exceeds_recursion_limit()
n = 100_000
names = list(set(''.join([choice(string.ascii_letters)
for j in range(10)]) for i in range(n)))
n = len(names)
Expand Down
Loading
Loading