pythonGH-91079: Revert "pythonGH-91079: Implement C stack limits using addresses, not counters. (pythonGH-130007)" for now (GH130413)

Revert "pythonGH-91079: Implement C stack limits using addresses, not counters. (pythonGH-130007)" for now

Unfortunatlely, the change broke some buildbots.

This reverts commit 2498c22.

Author: encukou

Doc/c-api/exceptions.rst

@@ -921,7 +921,11 @@ because the :ref:`call protocol <call>` takes care of recursion handling.
 
    Marks a point where a recursive C-level call is about to be performed.
 
-   The function then checks if the stack limit is reached.  If this is the
+   If :c:macro:`!USE_STACKCHECK` is defined, this function checks if the OS
+   stack overflowed using :c:func:`PyOS_CheckStack`.  If this is the case, it
+   sets a :exc:`MemoryError` and returns a nonzero value.
+
+   The function then checks if the recursion limit is reached.  If this is the
    case, a :exc:`RecursionError` is set and a nonzero value is returned.
    Otherwise, zero is returned.
 

Include/cpython/object.h

@@ -487,19 +487,18 @@ PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(PyThreadState *tstate);
  * we have headroom above the trigger limit */
 #define Py_TRASHCAN_HEADROOM 50
 
-/* Helper function for Py_TRASHCAN_BEGIN */
-PyAPI_FUNC(int) _Py_ReachedRecursionLimitWithMargin(PyThreadState *tstate, int margin_count);
-
 #define Py_TRASHCAN_BEGIN(op, dealloc) \
 do { \
     PyThreadState *tstate = PyThreadState_Get(); \
-    if (_Py_ReachedRecursionLimitWithMargin(tstate, 1) && Py_TYPE(op)->tp_dealloc == (destructor)dealloc) { \
+    if (tstate->c_recursion_remaining <= Py_TRASHCAN_HEADROOM && 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 \
-    if (tstate->delete_later && !_Py_ReachedRecursionLimitWithMargin(tstate, 2)) { \
+    tstate->c_recursion_remaining++; \
+    if (tstate->delete_later && tstate->c_recursion_remaining > (Py_TRASHCAN_HEADROOM*2)) { \
         _PyTrash_thread_destroy_chain(tstate); \
     } \
 } while (0);

Include/cpython/pystate.h

@@ -112,7 +112,7 @@ struct _ts {
     int py_recursion_remaining;
     int py_recursion_limit;
 
-    int c_recursion_remaining; /* Retained for backwards compatibility. Do not use */
+    int c_recursion_remaining;
     int recursion_headroom; /* Allow 50 more calls to handle any errors. */
 
     /* 'tracing' keeps track of the execution depth when tracing/profiling.
@@ -202,7 +202,36 @@ struct _ts {
     PyObject *threading_local_sentinel;
 };
 
-# define Py_C_RECURSION_LIMIT 5000
+#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 */
 
@@ -217,6 +246,7 @@ _PyThreadState_UncheckedGet(void)
     return PyThreadState_GetUnchecked();
 }
 
+
 // Disable tracing and profiling.
 PyAPI_FUNC(void) PyThreadState_EnterTracing(PyThreadState *tstate);
 

Include/internal/pycore_ceval.h

@@ -193,12 +193,18 @@ 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)  {
-    char here;
-    uintptr_t here_addr = (uintptr_t)&here;
-    _PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
-    return here_addr < _tstate->c_stack_soft_limit;
+    return (tstate->c_recursion_remaining-- < 0
+            || (tstate->c_recursion_remaining & 63) == 0);
 }
+#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.
@@ -214,31 +220,23 @@ 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) {
-    (void)tstate;
-}
-
-PyAPI_FUNC(void) _Py_InitializeRecursionLimits(PyThreadState *tstate);
-
-static inline int _Py_ReachedRecursionLimit(PyThreadState *tstate)  {
-    char here;
-    uintptr_t here_addr = (uintptr_t)&here;
-    _PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
-    if (here_addr > _tstate->c_stack_soft_limit) {
-        return 0;
-    }
-    if (_tstate->c_stack_hard_limit == 0) {
-        _Py_InitializeRecursionLimits(tstate);
-    }
-    return here_addr <= _tstate->c_stack_soft_limit;
+static inline void _Py_LeaveRecursiveCallTstate(PyThreadState *tstate)  {
+    tstate->c_recursion_remaining++;
 }
 
 static inline void _Py_LeaveRecursiveCall(void)  {
+    PyThreadState *tstate = _PyThreadState_GET();
+    _Py_LeaveRecursiveCallTstate(tstate);
 }
 
 extern struct _PyInterpreterFrame* _PyEval_GetFrame(void);
@@ -329,6 +327,7 @@ 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

Include/internal/pycore_symtable.h

@@ -82,6 +82,8 @@ 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 {

Include/internal/pycore_tstate.h

@@ -21,11 +21,6 @@ typedef struct _PyThreadStateImpl {
     // semi-public fields are in PyThreadState.
     PyThreadState base;
 
-    // These are addresses, but we need to convert to ints to avoid UB.
-    uintptr_t c_stack_top;
-    uintptr_t c_stack_soft_limit;
-    uintptr_t c_stack_hard_limit;
-
     PyObject *asyncio_running_loop; // Strong reference
     PyObject *asyncio_running_task; // Strong reference
 

Include/pythonrun.h

@@ -21,18 +21,14 @@ PyAPI_FUNC(void) PyErr_DisplayException(PyObject *);
 /* Stuff with no proper home (yet) */
 PyAPI_DATA(int) (*PyOS_InputHook)(void);
 
-/* 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)
+/* 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
+
+#if defined(WIN32) && !defined(MS_WIN64) && !defined(_M_ARM) && defined(_MSC_VER) && _MSC_VER >= 1300
+/* Enable stack checking under Microsoft C */
+// When changing the platforms, ensure PyOS_CheckStack() docs are still correct
 #define USE_STACKCHECK
 #endif
 

Lib/test/list_tests.py

@@ -6,8 +6,7 @@
 from functools import cmp_to_key
 
 from test import seq_tests
-from test.support import ALWAYS_EQ, NEVER_EQ
-from test.support import skip_emscripten_stack_overflow, skip_wasi_stack_overflow
+from test.support import ALWAYS_EQ, NEVER_EQ, get_c_recursion_limit, skip_emscripten_stack_overflow
 
 
 class CommonTest(seq_tests.CommonTest):
@@ -60,11 +59,10 @@ 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(100_000):
+        for i in range(get_c_recursion_limit() + 1):
             a = self.type2test([a])
         self.assertRaises(RecursionError, repr, a)
 

Lib/test/mapping_tests.py

@@ -1,7 +1,7 @@
 # tests common to dict and UserDict
 import unittest
 import collections
-from test import support
+from test.support import get_c_recursion_limit, skip_emscripten_stack_overflow
 
 
 class BasicTestMappingProtocol(unittest.TestCase):
@@ -622,11 +622,10 @@ def __repr__(self):
         d = self._full_mapping({1: BadRepr()})
         self.assertRaises(Exc, repr, d)
 
-    @support.skip_wasi_stack_overflow()
-    @support.skip_emscripten_stack_overflow()
+    @skip_emscripten_stack_overflow()
     def test_repr_deep(self):
         d = self._empty_mapping()
-        for i in range(support.exceeds_recursion_limit()):
+        for i in range(get_c_recursion_limit() + 1):
             d0 = d
             d = self._empty_mapping()
             d[1] = d0

Lib/test/pythoninfo.py

@@ -684,6 +684,7 @@ 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
     ):

Lib/test/support/__init__.py

@@ -56,7 +56,8 @@
     "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", "skip_on_s390x",
+    "Py_DEBUG", "exceeds_recursion_limit", "get_c_recursion_limit",
+    "skip_on_s390x",
     "requires_jit_enabled",
     "requires_jit_disabled",
     "force_not_colorized",
@@ -557,9 +558,6 @@ 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"
 
@@ -2626,9 +2624,17 @@ def adjust_int_max_str_digits(max_digits):
         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 100_000
+    return get_c_recursion_limit() * 3
 
 
 # Windows doesn't have os.uname() but it doesn't support s390x.

Lib/test/test_ast/test_ast.py

@@ -18,8 +18,7 @@
     _testinternalcapi = None
 
 from test import support
-from test.support import os_helper, script_helper
-from test.support import skip_emscripten_stack_overflow, skip_wasi_stack_overflow
+from test.support import os_helper, script_helper, skip_emscripten_stack_overflow
 from test.support.ast_helper import ASTTestMixin
 from test.test_ast.utils import to_tuple
 from test.test_ast.snippets import (
@@ -751,25 +750,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):
-        crash_depth = 200_000
-        success_depth = 200
+        fail_depth = support.exceeds_recursion_limit()
+        crash_depth = 100_000
+        success_depth = int(support.get_c_recursion_limit() * 0.8)
         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)
-
-            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)
+            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)
 
         check_limit("a", "()")
         check_limit("a", ".b")

Lib/test/test_call.py

@@ -1,6 +1,6 @@
 import unittest
 from test.support import (cpython_only, is_wasi, requires_limited_api, Py_DEBUG,
-                          set_recursion_limit, skip_on_s390x, exceeds_recursion_limit, skip_emscripten_stack_overflow,
+                          set_recursion_limit, skip_on_s390x, skip_emscripten_stack_overflow,
                           skip_if_sanitizer, import_helper)
 try:
     import _testcapi
@@ -1064,10 +1064,10 @@ def c_py_recurse(m):
             recurse(90_000)
             with self.assertRaises(RecursionError):
                 recurse(101_000)
-            c_recurse(50)
+            c_recurse(100)
             with self.assertRaises(RecursionError):
                 c_recurse(90_000)
-            c_py_recurse(50)
+            c_py_recurse(90)
             with self.assertRaises(RecursionError):
                 c_py_recurse(100_000)
 

Lib/test/test_capi/test_misc.py

@@ -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(100):
+        for i in range(1000):
             L = MyList((L,))
 
     @support.requires_resource('cpu')