From b11dbcba0a7d1b1ed5ea8f99b84f97732fb2c1c4 Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Mon, 8 Apr 2024 09:06:49 +0800 Subject: [PATCH] thread mgr: Free aux stack only when it was allocated (#3282) When thread manager is enabled, the aux stack of exec_env may be allocated by wasm_cluster_allocate_aux_stack or disabled by setting aux_stack_bottom as UINTPTR_MAX directly. For the latter, no need to free it. And fix an issue when paring `--gc-heap-size=n` argument for iwasm, and fix a variable shadowed warning in fast-jit. --- core/iwasm/common/wasm_exec_env.h | 3 +++ .../fast-jit/cg/x86-64/jit_codegen_x86_64.cpp | 6 +++--- core/iwasm/libraries/thread-mgr/thread_manager.c | 15 ++++++++++++--- product-mini/platforms/posix/main.c | 2 +- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/core/iwasm/common/wasm_exec_env.h b/core/iwasm/common/wasm_exec_env.h index f9624233..53d24875 100644 --- a/core/iwasm/common/wasm_exec_env.h +++ b/core/iwasm/common/wasm_exec_env.h @@ -117,6 +117,9 @@ typedef struct WASMExecEnv { /* whether current thread is detached */ bool thread_is_detached; + + /* whether the aux stack is allocated */ + bool is_aux_stack_allocated; #endif #if WASM_ENABLE_GC != 0 diff --git a/core/iwasm/fast-jit/cg/x86-64/jit_codegen_x86_64.cpp b/core/iwasm/fast-jit/cg/x86-64/jit_codegen_x86_64.cpp index 53761e70..79c72503 100644 --- a/core/iwasm/fast-jit/cg/x86-64/jit_codegen_x86_64.cpp +++ b/core/iwasm/fast-jit/cg/x86-64/jit_codegen_x86_64.cpp @@ -9293,8 +9293,8 @@ jit_codegen_init() imm.setValue(INT32_MAX); a.jne(imm); - char *stream = (char *)a.code()->sectionById(0)->buffer().data() - + a.code()->sectionById(0)->buffer().size(); + char *stream_old = (char *)a.code()->sectionById(0)->buffer().data() + + a.code()->sectionById(0)->buffer().size(); /* If yes, call jit_set_exception_with_id to throw exception, and then set eax to JIT_INTERP_ACTION_THROWN, and jump to @@ -9319,7 +9319,7 @@ jit_codegen_init() /* Patch the offset of jne instruction */ char *stream_new = (char *)a.code()->sectionById(0)->buffer().data() + a.code()->sectionById(0)->buffer().size(); - *(int32 *)(stream - 4) = (int32)(stream_new - stream); + *(int32 *)(stream_old - 4) = (int32)(stream_new - stream_old); } /* Load compiled func ptr and call it */ diff --git a/core/iwasm/libraries/thread-mgr/thread_manager.c b/core/iwasm/libraries/thread-mgr/thread_manager.c index ac895750..ac4deb92 100644 --- a/core/iwasm/libraries/thread-mgr/thread_manager.c +++ b/core/iwasm/libraries/thread-mgr/thread_manager.c @@ -558,6 +558,7 @@ wasm_cluster_spawn_exec_env(WASMExecEnv *exec_env) aux_stack_size)) { goto fail3; } + new_exec_env->is_aux_stack_allocated = true; /* Inherit suspend_flags of parent thread */ new_exec_env->suspend_flags.flags = @@ -603,7 +604,9 @@ wasm_cluster_destroy_spawned_exec_env(WASMExecEnv *exec_env) exec_env_tls = exec_env; } - /* Free aux stack space */ + /* Free aux stack space which was allocated in + wasm_cluster_spawn_exec_env */ + bh_assert(exec_env_tls->is_aux_stack_allocated); wasm_cluster_free_aux_stack(exec_env_tls, (uint64)exec_env->aux_stack_bottom); @@ -655,7 +658,9 @@ thread_manager_start_routine(void *arg) #endif /* Free aux stack space */ - wasm_cluster_free_aux_stack(exec_env, (uint64)exec_env->aux_stack_bottom); + if (exec_env->is_aux_stack_allocated) + wasm_cluster_free_aux_stack(exec_env, + (uint64)exec_env->aux_stack_bottom); os_mutex_lock(&cluster_list_lock); @@ -723,11 +728,13 @@ wasm_cluster_create_thread(WASMExecEnv *exec_env, aux_stack_size)) { goto fail2; } + new_exec_env->is_aux_stack_allocated = true; } else { /* Disable aux stack */ new_exec_env->aux_stack_boundary = 0; new_exec_env->aux_stack_bottom = UINTPTR_MAX; + new_exec_env->is_aux_stack_allocated = false; } /* Inherit suspend_flags of parent thread */ @@ -1049,7 +1056,9 @@ wasm_cluster_exit_thread(WASMExecEnv *exec_env, void *retval) #endif /* Free aux stack space */ - wasm_cluster_free_aux_stack(exec_env, (uint64)exec_env->aux_stack_bottom); + if (exec_env->is_aux_stack_allocated) + wasm_cluster_free_aux_stack(exec_env, + (uint64)exec_env->aux_stack_bottom); /* App exit the thread, free the resources before exit native thread */ diff --git a/product-mini/platforms/posix/main.c b/product-mini/platforms/posix/main.c index 217eb20c..2acd2190 100644 --- a/product-mini/platforms/posix/main.c +++ b/product-mini/platforms/posix/main.c @@ -675,7 +675,7 @@ main(int argc, char *argv[]) #endif #if WASM_ENABLE_GC != 0 else if (!strncmp(argv[0], "--gc-heap-size=", 15)) { - if (argv[0][21] == '\0') + if (argv[0][15] == '\0') return print_help(); gc_heap_size = atoi(argv[0] + 15); }