py: Add new cstack API for stack checking, with limit margin macro.

Currently the stack limit margin is hard-coded in each port's call to `mp_stack_set_limit()`, but on threaded ports it's fiddlier and can lead to bugs (such as incorrect thread stack margin on esp32). This commit provides a new API to initialise the C Stack in one function call, with a config macro to set the margin. Where possible the new call is inlined to reduce code size in thread-free ports. Intended replacement for `MP_TASK_STACK_LIMIT_MARGIN` on esp32. The previous `stackctrl.h` API is still present and unmodified apart from a deprecation comment. However it's not available when the `MICROPY_PREVIEW_VERSION_2` macro is set. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <[email protected]>
picoscratch · Aug 14, 2024 · 86f2c28 · 86f2c28
1 parent 6c870dc
commit 86f2c28
Show file tree

Hide file tree

Showing 18 changed files with 167 additions and 27 deletions.
diff --git a/examples/natmod/re/re.c b/examples/natmod/re/re.c
@@ -11,7 +11,7 @@
 
 const char *stack_top;
 
-void mp_stack_check(void) {
+void mp_cstack_check(void) {
     // Assumes descending stack on target
     volatile char dummy;
     if (stack_top - &dummy >= STACK_LIMIT) {

diff --git a/extmod/modre.c b/extmod/modre.c
@@ -31,15 +31,15 @@
 #include "py/runtime.h"
 #include "py/binary.h"
 #include "py/objstr.h"
-#include "py/stackctrl.h"
+#include "py/cstack.h"
 
 #if MICROPY_PY_BUILTINS_STR_UNICODE
 #include "py/unicode.h"
 #endif
 
 #if MICROPY_PY_RE
 
-#define re1_5_stack_chk() MP_STACK_CHECK()
+#define re1_5_stack_chk() mp_cstack_check()
 
 #include "lib/re1.5/re1.5.h"
 

diff --git a/py/cstack.c b/py/cstack.c
@@ -0,0 +1,57 @@
+/*
+ * This file is part of the MicroPython project, http://micropython.org/
+ *
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2014 Paul Sokolovsky
+ * Copryight (c) 2024 Angus Gratton
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "py/runtime.h"
+#include "py/cstack.h"
+
+void mp_cstack_init_with_sp_here(size_t stack_size) {
+    #if __GNUC__ >= 13
+    #pragma GCC diagnostic push
+    #pragma GCC diagnostic ignored "-Wdangling-pointer"
+    #endif
+    volatile int stack_dummy;
+    mp_cstack_init_with_top((void *)&stack_dummy, stack_size);
+    #if __GNUC__ >= 13
+    #pragma GCC diagnostic pop
+    #endif
+}
+
+mp_uint_t mp_cstack_usage(void) {
+    // Assumes descending stack
+    volatile int stack_dummy;
+    return MP_STATE_THREAD(stack_top) - (char *)&stack_dummy;
+}
+
+#if MICROPY_STACK_CHECK
+
+void mp_cstack_check(void) {
+    if (mp_cstack_usage() >= MP_STATE_THREAD(stack_limit)) {
+        mp_raise_recursion_depth();
+    }
+}
+
+#endif // MICROPY_STACK_CHECK
diff --git a/py/cstack.h b/py/cstack.h
@@ -0,0 +1,63 @@
+/*
+ * This file is part of the MicroPython project, http://micropython.org/
+ *
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2014 Paul Sokolovsky
+ * Copyright (c) 2024 Angus Gratton
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef MICROPY_INCLUDED_PY_CSTACK_H
+#define MICROPY_INCLUDED_PY_CSTACK_H
+
+#include "py/mpstate.h"
+
+// Both init functions below accept the full stack size. Set the
+// MICROPY_STACK_CHECK_MARGIN to the number of bytes subtracted to account
+// for stack usage between checks.
+
+void mp_cstack_init_with_sp_here(size_t stack_size);
+
+inline static void mp_cstack_init_with_top(void *top, size_t stack_size) {
+    MP_STATE_THREAD(stack_top) = (char *)top;
+
+    #if MICROPY_STACK_CHECK
+    assert(stack_size > MICROPY_STACK_CHECK_MARGIN); // Should be enforced by port
+    MP_STATE_THREAD(stack_limit) = stack_size - MICROPY_STACK_CHECK_MARGIN;
+    #else
+    (void)stack_size;
+    #endif
+}
+
+mp_uint_t mp_cstack_usage(void);
+
+#if MICROPY_STACK_CHECK
+
+void mp_cstack_check(void);
+
+#else
+
+inline static void mp_cstack_check(void) {
+    // No-op when stack checking is disabled
+}
+
+#endif
+
+#endif // MICROPY_INCLUDED_PY_CSTACK_H
diff --git a/py/modmicropython.c b/py/modmicropython.c
@@ -27,7 +27,7 @@
 #include <stdio.h>
 
 #include "py/builtin.h"
-#include "py/stackctrl.h"
+#include "py/cstack.h"
 #include "py/runtime.h"
 #include "py/gc.h"
 #include "py/mphal.h"
@@ -76,9 +76,9 @@ mp_obj_t mp_micropython_mem_info(size_t n_args, const mp_obj_t *args) {
     #endif
     #if MICROPY_STACK_CHECK
     mp_printf(&mp_plat_print, "stack: " UINT_FMT " out of " UINT_FMT "\n",
-        mp_stack_usage(), (mp_uint_t)MP_STATE_THREAD(stack_limit));
+        mp_cstack_usage(), (mp_uint_t)MP_STATE_THREAD(stack_limit));
     #else
-    mp_printf(&mp_plat_print, "stack: " UINT_FMT "\n", mp_stack_usage());
+    mp_printf(&mp_plat_print, "stack: " UINT_FMT "\n", mp_cstack_usage());
     #endif
     #if MICROPY_ENABLE_GC
     gc_dump_info(&mp_plat_print);
@@ -111,7 +111,7 @@ static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_micropython_qstr_info_obj, 0, 1, m
 
 #if MICROPY_PY_MICROPYTHON_STACK_USE
 static mp_obj_t mp_micropython_stack_use(void) {
-    return MP_OBJ_NEW_SMALL_INT(mp_stack_usage());
+    return MP_OBJ_NEW_SMALL_INT(mp_cstack_usage());
 }
 static MP_DEFINE_CONST_FUN_OBJ_0(mp_micropython_stack_use_obj, mp_micropython_stack_use);
 #endif

diff --git a/py/modthread.c b/py/modthread.c
@@ -28,7 +28,6 @@
 #include <string.h>
 
 #include "py/runtime.h"
-#include "py/stackctrl.h"
 
 #if MICROPY_PY_THREAD
 

diff --git a/py/mpconfig.h b/py/mpconfig.h
@@ -690,6 +690,13 @@
 #define MICROPY_STACK_CHECK (MICROPY_CONFIG_ROM_LEVEL_AT_LEAST_EXTRA_FEATURES)
 #endif
 
+// Additional margin between the places in the runtime where Python stack is
+// checked and the actual end of the C stack. Needs to be large enough to avoid
+// overflows from function calls made between checks.
+#ifndef MICROPY_STACK_CHECK_MARGIN
+#define MICROPY_STACK_CHECK_MARGIN (0)
+#endif
+
 // Whether to have an emergency exception buffer
 #ifndef MICROPY_ENABLE_EMERGENCY_EXCEPTION_BUF
 #define MICROPY_ENABLE_EMERGENCY_EXCEPTION_BUF (0)

diff --git a/py/obj.c b/py/obj.c
@@ -34,7 +34,7 @@
 #include "py/objint.h"
 #include "py/objstr.h"
 #include "py/runtime.h"
-#include "py/stackctrl.h"
+#include "py/cstack.h"
 #include "py/stream.h" // for mp_obj_print
 
 // Allocates an object and also sets type, for mp_obj_malloc{,_var} macros.
@@ -117,7 +117,7 @@ const char *mp_obj_get_type_str(mp_const_obj_t o_in) {
 
 void mp_obj_print_helper(const mp_print_t *print, mp_obj_t o_in, mp_print_kind_t kind) {
     // There can be data structures nested too deep, or just recursive
-    MP_STACK_CHECK();
+    mp_cstack_check();
     #ifndef NDEBUG
     if (o_in == MP_OBJ_NULL) {
         mp_print_str(print, "(nil)");

diff --git a/py/objfun.c b/py/objfun.c
@@ -32,7 +32,7 @@
 #include "py/objfun.h"
 #include "py/runtime.h"
 #include "py/bc.h"
-#include "py/stackctrl.h"
+#include "py/cstack.h"
 
 #if MICROPY_DEBUG_VERBOSE // print debugging info
 #define DEBUG_PRINT (1)
@@ -194,7 +194,7 @@ static void dump_args(const mp_obj_t *a, size_t sz) {
 
 #if MICROPY_STACKLESS
 mp_code_state_t *mp_obj_fun_bc_prepare_codestate(mp_obj_t self_in, size_t n_args, size_t n_kw, const mp_obj_t *args) {
-    MP_STACK_CHECK();
+    mp_cstack_check();
     mp_obj_fun_bc_t *self = MP_OBJ_TO_PTR(self_in);
 
     size_t n_state, state_size;
@@ -225,7 +225,7 @@ mp_code_state_t *mp_obj_fun_bc_prepare_codestate(mp_obj_t self_in, size_t n_args
 #endif
 
 static mp_obj_t fun_bc_call(mp_obj_t self_in, size_t n_args, size_t n_kw, const mp_obj_t *args) {
-    MP_STACK_CHECK();
+    mp_cstack_check();
 
     DEBUG_printf("Input n_args: " UINT_FMT ", n_kw: " UINT_FMT "\n", n_args, n_kw);
     DEBUG_printf("Input pos args: ");
@@ -397,7 +397,7 @@ mp_obj_t mp_obj_new_fun_bc(const mp_obj_t *def_args, const byte *code, const mp_
 #if MICROPY_EMIT_NATIVE
 
 static mp_obj_t fun_native_call(mp_obj_t self_in, size_t n_args, size_t n_kw, const mp_obj_t *args) {
-    MP_STACK_CHECK();
+    mp_cstack_check();
     mp_obj_fun_bc_t *self = MP_OBJ_TO_PTR(self_in);
     mp_call_fun_t fun = mp_obj_fun_native_get_function_start(self);
     return fun(self_in, n_args, n_kw, args);
@@ -431,7 +431,7 @@ MP_DEFINE_CONST_OBJ_TYPE(
 #if MICROPY_EMIT_NATIVE
 
 static mp_obj_t fun_viper_call(mp_obj_t self_in, size_t n_args, size_t n_kw, const mp_obj_t *args) {
-    MP_STACK_CHECK();
+    mp_cstack_check();
     mp_obj_fun_bc_t *self = MP_OBJ_TO_PTR(self_in);
     mp_call_fun_t fun = MICROPY_MAKE_POINTER_CALLABLE((void *)self->bytecode);
     return fun(self_in, n_args, n_kw, args);

diff --git a/py/objgenerator.c b/py/objgenerator.c
@@ -33,7 +33,7 @@
 #include "py/objstr.h"
 #include "py/objgenerator.h"
 #include "py/objfun.h"
-#include "py/stackctrl.h"
+#include "py/cstack.h"
 
 // Instance of GeneratorExit exception - needed by generator.close()
 const mp_obj_exception_t mp_const_GeneratorExit_obj = {{&mp_type_GeneratorExit}, 0, 0, NULL, (mp_obj_tuple_t *)&mp_const_empty_tuple_obj};
@@ -151,7 +151,7 @@ static void gen_instance_print(const mp_print_t *print, mp_obj_t self_in, mp_pri
 }
 
 mp_vm_return_kind_t mp_obj_gen_resume(mp_obj_t self_in, mp_obj_t send_value, mp_obj_t throw_value, mp_obj_t *ret_val) {
-    MP_STACK_CHECK();
+    mp_cstack_check();
     mp_check_self(mp_obj_is_type(self_in, &mp_type_gen_instance));
     mp_obj_gen_instance_t *self = MP_OBJ_TO_PTR(self_in);
     if (self->code_state.ip == 0) {

diff --git a/py/objlist.c b/py/objlist.c
@@ -29,7 +29,7 @@
 
 #include "py/objlist.h"
 #include "py/runtime.h"
-#include "py/stackctrl.h"
+#include "py/cstack.h"
 
 static mp_obj_t mp_obj_new_list_iterator(mp_obj_t list, size_t cur, mp_obj_iter_buf_t *iter_buf);
 static mp_obj_list_t *list_new(size_t n);
@@ -291,7 +291,7 @@ static mp_obj_t list_pop(size_t n_args, const mp_obj_t *args) {
 }
 
 static void mp_quicksort(mp_obj_t *head, mp_obj_t *tail, mp_obj_t key_fn, mp_obj_t binop_less_result) {
-    MP_STACK_CHECK();
+    mp_cstack_check();
     while (head < tail) {
         mp_obj_t *h = head - 1;
         mp_obj_t *t = tail;

diff --git a/py/objstr.c b/py/objstr.c
@@ -32,7 +32,7 @@
 #include "py/objstr.h"
 #include "py/objlist.h"
 #include "py/runtime.h"
-#include "py/stackctrl.h"
+#include "py/cstack.h"
 
 #if MICROPY_PY_BUILTINS_STR_OP_MODULO
 static mp_obj_t str_modulo_format(mp_obj_t pattern, size_t n_args, const mp_obj_t *args, mp_obj_t dict);
@@ -1181,7 +1181,7 @@ static vstr_t mp_obj_str_format_helper(const char *str, const char *top, int *ar
             // type        ::=  "b" | "c" | "d" | "e" | "E" | "f" | "F" | "g" | "G" | "n" | "o" | "s" | "x" | "X" | "%"
 
             // recursively call the formatter to format any nested specifiers
-            MP_STACK_CHECK();
+            mp_cstack_check();
             vstr_t format_spec_vstr = mp_obj_str_format_helper(format_spec, str, arg_i, n_args, args, kwargs);
             const char *s = vstr_null_terminated_str(&format_spec_vstr);
             const char *stop = s + format_spec_vstr.len;

diff --git a/py/py.cmake b/py/py.cmake
@@ -20,6 +20,7 @@ set(MICROPY_SOURCE_PY
     ${MICROPY_PY_DIR}/builtinhelp.c
     ${MICROPY_PY_DIR}/builtinimport.c
     ${MICROPY_PY_DIR}/compile.c
+    ${MICROPY_PY_DIR}/cstack.c
     ${MICROPY_PY_DIR}/emitbc.c
     ${MICROPY_PY_DIR}/emitcommon.c
     ${MICROPY_PY_DIR}/emitglue.c

diff --git a/py/py.mk b/py/py.mk
@@ -131,6 +131,7 @@ PY_CORE_O_BASENAME = $(addprefix py/,\
 	nativeglue.o \
 	pairheap.o \
 	ringbuf.o \
+	cstack.o \
 	stackctrl.o \
 	argcheck.o \
 	warning.o \

diff --git a/py/runtime.c b/py/runtime.c
@@ -43,7 +43,7 @@
 #include "py/stream.h"
 #include "py/runtime.h"
 #include "py/builtin.h"
-#include "py/stackctrl.h"
+#include "py/cstack.h"
 #include "py/gc.h"
 
 #if MICROPY_DEBUG_VERBOSE // print debugging info
@@ -1374,7 +1374,7 @@ mp_obj_t mp_iternext_allow_raise(mp_obj_t o_in) {
 // will always return MP_OBJ_STOP_ITERATION instead of raising StopIteration() (or any subclass thereof)
 // may raise other exceptions
 mp_obj_t mp_iternext(mp_obj_t o_in) {
-    MP_STACK_CHECK(); // enumerate, filter, map and zip can recursively call mp_iternext
+    mp_cstack_check(); // enumerate, filter, map and zip can recursively call mp_iternext
     const mp_obj_type_t *type = mp_obj_get_type(o_in);
     if (TYPE_HAS_ITERNEXT(type)) {
         MP_STATE_THREAD(stop_iteration_arg) = MP_OBJ_NULL;

diff --git a/py/runtime.h b/py/runtime.h
@@ -28,7 +28,7 @@
 
 #include "py/mpstate.h"
 #include "py/pystack.h"
-#include "py/stackctrl.h"
+#include "py/cstack.h"
 
 // For use with mp_call_function_1_from_nlr_jump_callback.
 #define MP_DEFINE_NLR_JUMP_CALLBACK_FUNCTION_1(ctx, f, a) \
@@ -159,8 +159,7 @@ void mp_call_function_1_from_nlr_jump_callback(void *ctx_in);
 static inline void mp_thread_init_state(mp_state_thread_t *ts, size_t stack_size, mp_obj_dict_t *locals, mp_obj_dict_t *globals) {
     mp_thread_set_state(ts);
 
-    mp_stack_set_top(ts + 1); // need to include ts in root-pointer scan
-    mp_stack_set_limit(stack_size);
+    mp_cstack_init_with_top(ts + 1, stack_size); // need to include ts in root-pointer scan
 
     // GC starts off unlocked
     ts->gc_lock_depth = 0;

diff --git a/py/stackctrl.c b/py/stackctrl.c
@@ -24,7 +24,12 @@
  * THE SOFTWARE.
  */
 
+// This API is deprecated, please use py/cstack.h instead
+
 #include "py/runtime.h"
+
+#if !MICROPY_PREVIEW_VERSION_2
+
 #include "py/stackctrl.h"
 
 void mp_stack_ctrl_init(void) {
@@ -62,3 +67,5 @@ void mp_stack_check(void) {
 }
 
 #endif // MICROPY_STACK_CHECK
+
+#endif // !MICROPY_PREVIEW_VERSION_2