Fix a few native stack address calculations (bytecodealliance#3351)

victoryang00 · May 1, 2024 · 9b8bcd8 · 9b8bcd8
1 parent b041d48
commit 9b8bcd8
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 11 deletions.
diff --git a/.github/workflows/spec_test_on_nuttx.yml b/.github/workflows/spec_test_on_nuttx.yml
@@ -22,11 +22,14 @@ on:
 
   workflow_dispatch:
 
+# Note on INTERPRETERS_WAMR_STACK_GUARD_SIZE:
+# https://github.com/apache/nuttx-apps/pull/2241 is not included in
+# releases/12.4 branch as of writing this.
 env:
   LLVM_CACHE_SUFFIX: "build-llvm_libraries_ex"
   WASI_SDK_PATH: "/opt/wasi-sdk"
   WAMR_COMMON_OPTION:
-    "CONFIG_INTERPRETERS_WAMR=y\\nCONFIG_INTERPRETERS_WAMR_STACKSIZE=327680\\nCONFIG_INTERPRETERS_WAMR_LOG=y\\nCONFIG_INTERPRETERS_WAMR_LIBC_BUILTIN=y\\nCONFIG_INTERPRETERS_WAMR_REF_TYPES=y\\nCONFIG_INTERPRETERS_WAMR_ENABLE_SPEC_TEST=y\\nCONFIG_INTERPRETERS_WAMR_SHARED_MEMORY=y\\nCONFIG_INTERPRETERS_WAMR_BULK_MEMORY=y\\nCONFIG_EOL_IS_LF=y\\nCONFIG_ARM_SEMIHOSTING_HOSTFS=y\\nCONFIG_ARM_SEMIHOSTING_HOSTFS_CACHE_COHERENCE=y\\nCONFIG_RISCV_SEMIHOSTING_HOSTFS=y\\nCONFIG_FS_HOSTFS=y\\nCONFIG_LIBC_FLOATINGPOINT=y\\n"
+    "CONFIG_INTERPRETERS_WAMR=y\\nCONFIG_INTERPRETERS_WAMR_STACKSIZE=327680\\nCONFIG_INTERPRETERS_WAMR_LOG=y\\nCONFIG_INTERPRETERS_WAMR_LIBC_BUILTIN=y\\nCONFIG_INTERPRETERS_WAMR_REF_TYPES=y\\nCONFIG_INTERPRETERS_WAMR_ENABLE_SPEC_TEST=y\\nCONFIG_INTERPRETERS_WAMR_SHARED_MEMORY=y\\nCONFIG_INTERPRETERS_WAMR_BULK_MEMORY=y\\nCONFIG_EOL_IS_LF=y\\nCONFIG_ARM_SEMIHOSTING_HOSTFS=y\\nCONFIG_ARM_SEMIHOSTING_HOSTFS_CACHE_COHERENCE=y\\nCONFIG_RISCV_SEMIHOSTING_HOSTFS=y\\nCONFIG_FS_HOSTFS=y\\nCONFIG_LIBC_FLOATINGPOINT=y\\nCONFIG_INTERPRETERS_WAMR_STACK_GUARD_SIZE=1024\\n"
 
 jobs:
   build_llvm_libraries:

diff --git a/core/iwasm/aot/aot_runtime.c b/core/iwasm/aot/aot_runtime.c
@@ -2003,8 +2003,8 @@ invoke_native_with_hw_bound_check(WASMExecEnv *exec_env, void *func_ptr,
        native stack to run the following codes before actually calling
        the aot function in invokeNative function. */
     RECORD_STACK_USAGE(exec_env, (uint8 *)&module_inst);
-    if ((uint8 *)&module_inst < exec_env->native_stack_boundary
-                                    + page_size * (guard_page_count + 1)) {
+    if ((uint8 *)&module_inst
+        < exec_env->native_stack_boundary + page_size * guard_page_count) {
         aot_set_exception_with_id(module_inst, EXCE_NATIVE_STACK_OVERFLOW);
         return false;
     }

diff --git a/core/iwasm/common/wasm_runtime_common.c b/core/iwasm/common/wasm_runtime_common.c
@@ -225,7 +225,7 @@ runtime_signal_handler(void *sig_addr)
             os_longjmp(jmpbuf_node->jmpbuf, 1);
         }
 #if WASM_DISABLE_STACK_HW_BOUND_CHECK == 0
-        else if (stack_min_addr - page_size <= (uint8 *)sig_addr
+        else if (stack_min_addr <= (uint8 *)sig_addr
                  && (uint8 *)sig_addr
                         < stack_min_addr + page_size * guard_page_count) {
             /* The address which causes segmentation fault is inside

diff --git a/core/iwasm/interpreter/wasm_runtime.c b/core/iwasm/interpreter/wasm_runtime.c
@@ -3154,8 +3154,8 @@ call_wasm_with_hw_bound_check(WASMModuleInstance *module_inst,
        native stack to run the following codes before actually calling
        the aot function in invokeNative function. */
     RECORD_STACK_USAGE(exec_env, (uint8 *)&exec_env_tls);
-    if ((uint8 *)&exec_env_tls < exec_env->native_stack_boundary
-                                     + page_size * (guard_page_count + 1)) {
+    if ((uint8 *)&exec_env_tls
+        < exec_env->native_stack_boundary + page_size * guard_page_count) {
         wasm_set_exception(module_inst, "native stack overflow");
         return;
     }

diff --git a/core/shared/platform/common/posix/posix_thread.c b/core/shared/platform/common/posix/posix_thread.c
@@ -445,9 +445,6 @@ os_thread_get_stack_boundary()
         pthread_attr_destroy(&attr);
         if (stack_size > max_stack_size)
             addr = addr + stack_size - max_stack_size;
-        if (guard_size < (size_t)page_size)
-            /* Reserved 1 guard page at least for safety */
-            guard_size = (size_t)page_size;
         addr += guard_size;
     }
     (void)stack_size;
@@ -466,8 +463,6 @@ os_thread_get_stack_boundary()
             stack_size = max_stack_size;
 
         addr -= stack_size;
-        /* Reserved 1 guard page at least for safety */
-        addr += page_size;
     }
 #endif