From add69585b9a267de20bf9fd2cdf1b06133ab23f9 Mon Sep 17 00:00:00 2001 From: TL Date: Mon, 27 Jan 2025 14:12:22 +0800 Subject: [PATCH] fix shared heap enhancement bugs and format code --- core/iwasm/common/wasm_memory.c | 33 ++++++------- core/iwasm/interpreter/wasm_interp_classic.c | 14 ++++-- core/iwasm/interpreter/wasm_interp_fast.c | 6 +-- samples/shared-heap/src/main.c | 8 ++-- tests/unit/shared-heap/shared_heap_test.cc | 50 ++++++++++---------- 5 files changed, 60 insertions(+), 51 deletions(-) diff --git a/core/iwasm/common/wasm_memory.c b/core/iwasm/common/wasm_memory.c index d3d7e2a51..ec57e9e67 100644 --- a/core/iwasm/common/wasm_memory.c +++ b/core/iwasm/common/wasm_memory.c @@ -183,6 +183,11 @@ wasm_runtime_create_shared_heap(SharedHeapInitArgs *init_args) heap->start_off_mem32 = UINT32_MAX - heap->size + 1; heap->attached_count = 0; + if (size > APP_HEAP_SIZE_MAX || size < APP_HEAP_SIZE_MIN) { + LOG_WARNING("Invalid size of shared heap"); + goto fail1; + } + if (init_args->pre_allocated_addr != NULL) { /* Create shared heap from a pre allocated buffer, its size need to * align with system page */ @@ -193,6 +198,7 @@ wasm_runtime_create_shared_heap(SharedHeapInitArgs *init_args) } heap->heap_handle = NULL; + heap->base_addr = init_args->pre_allocated_addr; } else { if (!(heap->heap_handle = @@ -200,11 +206,6 @@ wasm_runtime_create_shared_heap(SharedHeapInitArgs *init_args) goto fail2; } - if (size > APP_HEAP_SIZE_MAX || size < APP_HEAP_SIZE_MIN) { - LOG_WARNING("Invalid size of shared heap"); - goto fail3; - } - #ifndef OS_ENABLE_HW_BOUND_CHECK map_size = size; #else @@ -286,8 +287,8 @@ wasm_runtime_chain_shared_heaps(WASMSharedHeap *head, WASMSharedHeap *body) heap_handle_exist = true; } - head->start_off_mem64 = body->start_off_mem64 - head->size + 1; - head->start_off_mem32 = body->start_off_mem32 - head->size + 1; + head->start_off_mem64 = body->start_off_mem64 - head->size; + head->start_off_mem32 = body->start_off_mem32 - head->size; head->chain_next = body; os_mutex_unlock(&shared_heap_list_lock); return head; @@ -505,8 +506,9 @@ is_app_addr_in_shared_heap(WASMModuleInstanceCommon *module_inst, for (cur = heap; cur; cur = cur->chain_next) { shared_heap_start = is_memory64 ? cur->start_off_mem64 : cur->start_off_mem32; - shared_heap_end = shared_heap_start + cur->size; - if (app_offset >= shared_heap_start && app_offset <= shared_heap_end) { + shared_heap_end = shared_heap_start - 1 + cur->size; + if (app_offset >= shared_heap_start + && app_offset <= shared_heap_end - bytes + 1) { if (target_heap) *target_heap = cur; return true; @@ -535,19 +537,16 @@ is_native_addr_in_shared_heap(WASMModuleInstanceCommon *module_inst, for (cur = heap_head; cur != NULL; cur = cur->chain_next) { base_addr = (uintptr_t)cur->base_addr; addr_int = (uintptr_t)addr; - if (addr_int < base_addr) { + if (addr_int < base_addr) continue; - } end_addr = addr_int + bytes; /* Check for overflow */ - if (end_addr <= addr_int) { + if (end_addr <= addr_int) continue; - } - if (end_addr > base_addr + cur->size) { + if (end_addr > base_addr + cur->size) continue; - } if (target_heap) *target_heap = cur; @@ -1041,9 +1040,11 @@ wasm_runtime_addr_app_to_native(WASMModuleInstanceCommon *module_inst_comm, { WASMModuleInstance *module_inst = (WASMModuleInstance *)module_inst_comm; WASMMemoryInstance *memory_inst; - WASMSharedHeap *shared_heap; uint8 *addr; bool bounds_checks; +#if WASM_ENABLE_SHARED_HEAP != 0 + WASMSharedHeap *shared_heap; +#endif bh_assert(module_inst_comm->module_type == Wasm_Module_Bytecode || module_inst_comm->module_type == Wasm_Module_AoT); diff --git a/core/iwasm/interpreter/wasm_interp_classic.c b/core/iwasm/interpreter/wasm_interp_classic.c index d75fc55e7..76803cca7 100644 --- a/core/iwasm/interpreter/wasm_interp_classic.c +++ b/core/iwasm/interpreter/wasm_interp_classic.c @@ -72,7 +72,7 @@ typedef float64 CellType_F64; for (cur = shared_heap; cur; cur = cur->chain_next) { \ cur_shared_heap_start_off = get_shared_heap_start_off(cur); \ cur_shared_heap_end_off = \ - cur_shared_heap_start_off + cur->size - 1; \ + cur_shared_heap_start_off - 1 + cur->size; \ if ((app_addr) >= cur_shared_heap_start_off \ && (app_addr) <= cur_shared_heap_end_off - bytes + 1) { \ shared_heap_start_off = cur_shared_heap_start_off; \ @@ -1679,7 +1679,7 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, shared_heap ? get_shared_heap_start_off(shared_heap) : 0; uint64 shared_heap_end_off = shared_heap - ? (get_shared_heap_start_off(shared_heap) + shared_heap->size - 1) + ? (get_shared_heap_start_off(shared_heap) - 1 + shared_heap->size) : 0; #endif /* end of WASM_ENABLE_SHARED_HEAP != 0 */ #if WASM_ENABLE_MULTI_MEMORY != 0 @@ -1719,7 +1719,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, goto got_exception; } - HANDLE_OP(WASM_OP_NOP) { HANDLE_OP_END(); } + HANDLE_OP(WASM_OP_NOP) + { + HANDLE_OP_END(); + } #if WASM_ENABLE_EXCE_HANDLING != 0 HANDLE_OP(WASM_OP_RETHROW) @@ -5656,7 +5659,10 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, HANDLE_OP(WASM_OP_I32_REINTERPRET_F32) HANDLE_OP(WASM_OP_I64_REINTERPRET_F64) HANDLE_OP(WASM_OP_F32_REINTERPRET_I32) - HANDLE_OP(WASM_OP_F64_REINTERPRET_I64) { HANDLE_OP_END(); } + HANDLE_OP(WASM_OP_F64_REINTERPRET_I64) + { + HANDLE_OP_END(); + } HANDLE_OP(WASM_OP_I32_EXTEND8_S) { diff --git a/core/iwasm/interpreter/wasm_interp_fast.c b/core/iwasm/interpreter/wasm_interp_fast.c index 3a9715a66..6ab5d236e 100644 --- a/core/iwasm/interpreter/wasm_interp_fast.c +++ b/core/iwasm/interpreter/wasm_interp_fast.c @@ -53,7 +53,7 @@ typedef float64 CellType_F64; for (cur = shared_heap; cur; cur = cur->chain_next) { \ cur_shared_heap_start_off = get_shared_heap_start_off(cur); \ cur_shared_heap_end_off = \ - cur_shared_heap_start_off + cur->size - 1; \ + cur_shared_heap_start_off - 1 + cur->size; \ if ((app_addr) >= cur_shared_heap_start_off \ && (app_addr) <= cur_shared_heap_end_off - bytes + 1) { \ shared_heap_start_off = cur_shared_heap_start_off; \ @@ -67,7 +67,7 @@ typedef float64 CellType_F64; }) #define shared_heap_addr_app_to_native(app_addr, native_addr) \ - native_addr = shared_heap_base_addr + ((app_addr)-shared_heap_start_off) + native_addr = shared_heap_base_addr + ((app_addr) - shared_heap_start_off) #define CHECK_SHARED_HEAP_OVERFLOW(app_addr, bytes, native_addr) \ if (app_addr_in_shared_heap(app_addr, bytes)) \ @@ -1568,7 +1568,7 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, shared_heap ? get_shared_heap_start_off(shared_heap) : 0; uint64 shared_heap_end_off = shared_heap - ? (get_shared_heap_start_off(shared_heap) + shared_heap->size - 1) + ? (get_shared_heap_start_off(shared_heap) - 1 + shared_heap->size) : 0; /* #endif */ #endif /* end of WASM_ENABLE_SHARED_HEAP != 0 */ diff --git a/samples/shared-heap/src/main.c b/samples/shared-heap/src/main.c index f4024f08c..ed64cc5da 100644 --- a/samples/shared-heap/src/main.c +++ b/samples/shared-heap/src/main.c @@ -55,7 +55,7 @@ thread1_callback(void *arg) i + 1); printf("wasm app1 send buf: %s\n\n", buf); - if (!bh_post_msg(queue, 1, buf, 1024 * i)) { + if (!bh_post_msg(queue, 1, buf, 1024 * (i + 1))) { printf("Failed to post message to queue\n"); wasm_runtime_shared_heap_free(module_inst, offset); break; @@ -84,7 +84,7 @@ thread1_callback(void *arg) buf = wasm_runtime_addr_app_to_native(module_inst, argv[0]); printf("wasm app1 send buf: %s\n\n", buf); - if (!bh_post_msg(queue, 1, buf, 1024 * i)) { + if (!bh_post_msg(queue, 1, buf, 1024 * (i+1))) { printf("Failed to post message to queue\n"); wasm_runtime_shared_heap_free(module_inst, argv[0]); break; @@ -268,7 +268,7 @@ main(int argc, char **argv) } /* create thread 1 */ - struct thread_arg targ1 = { 0 }; + thread_arg targ1 = { 0 }; korp_tid tid1; targ1.queue = queue; targ1.module_inst = module_inst1; @@ -279,7 +279,7 @@ main(int argc, char **argv) } /* create thread 2 */ - struct thread_arg targ2 = { 0 }; + thread_arg targ2 = { 0 }; korp_tid tid2; targ2.queue = queue; targ2.module_inst = module_inst2; diff --git a/tests/unit/shared-heap/shared_heap_test.cc b/tests/unit/shared-heap/shared_heap_test.cc index deb4bbb38..76357711d 100644 --- a/tests/unit/shared-heap/shared_heap_test.cc +++ b/tests/unit/shared-heap/shared_heap_test.cc @@ -92,7 +92,9 @@ destroy_module_env(struct ret_env module_env) } } -static void test_shared_heap(WASMSharedHeap *shared_heap, const char *file, const char *func_name, uint32 argc, uint32 argv[]) +static void +test_shared_heap(WASMSharedHeap *shared_heap, const char *file, + const char *func_name, uint32 argc, uint32 argv[]) { struct ret_env tmp_module_env; WASMFunctionInstanceCommon *func_test = nullptr; @@ -101,7 +103,8 @@ static void test_shared_heap(WASMSharedHeap *shared_heap, const char *file, cons tmp_module_env = load_wasm((char *)file, 0); - if (!wasm_runtime_attach_shared_heap(tmp_module_env.wasm_module_inst, shared_heap)) { + if (!wasm_runtime_attach_shared_heap(tmp_module_env.wasm_module_inst, + shared_heap)) { printf("Failed to attach shared heap\n"); goto test_failed; } @@ -116,7 +119,8 @@ static void test_shared_heap(WASMSharedHeap *shared_heap, const char *file, cons wasm_runtime_call_wasm(tmp_module_env.exec_env, func_test, argc, argv); if (!ret) { printf("\nFailed to wasm_runtime_call_wasm!\n"); - const char *s = wasm_runtime_get_exception(tmp_module_env.wasm_module_inst); + const char *s = + wasm_runtime_get_exception(tmp_module_env.wasm_module_inst); printf("exception: %s\n", s); goto test_failed; } @@ -131,7 +135,7 @@ test_failed: TEST_F(shared_heap_test, test_shared_heap_basic) { - SharedHeapInitArgs args; + SharedHeapInitArgs args = { 0 }; WASMSharedHeap *shared_heap = nullptr; uint32 argv[1] = { 0 }; @@ -150,12 +154,11 @@ TEST_F(shared_heap_test, test_shared_heap_basic) // test aot test_shared_heap(shared_heap, "test.aot", "test", 1, argv); EXPECT_EQ(10, argv[0]); - } TEST_F(shared_heap_test, test_shared_heap_malloc_fail) { - SharedHeapInitArgs args; + SharedHeapInitArgs args = { 0 }; WASMSharedHeap *shared_heap = nullptr; uint32 argv[1] = { 0 }; @@ -177,36 +180,36 @@ TEST_F(shared_heap_test, test_shared_heap_malloc_fail) } #ifndef native_function +/* clang-format off */ #define native_function(func_name, signature) \ { #func_name, (void *)glue_##func_name, signature, NULL } - +/* clang-format on */ #endif #ifndef nitems #define nitems(_a) (sizeof(_a) / sizeof(0 [(_a)])) #endif /* nitems */ -uintptr_t glue_test_addr_conv(wasm_exec_env_t env, uintptr_t addr) +uintptr_t +glue_test_addr_conv(wasm_exec_env_t env, uintptr_t addr) { - wasm_module_inst_t module_inst = get_module_inst(env); - uintptr_t ret; - void *native_addr = (void *)addr; - uintptr_t app_addr = addr_native_to_app(native_addr); + wasm_module_inst_t module_inst = get_module_inst(env); + uintptr_t ret; + void *native_addr = (void *)addr; + uintptr_t app_addr = addr_native_to_app(native_addr); - native_addr = addr_app_to_native(app_addr); - if (native_addr != (void *)addr) - { - EXPECT_EQ(1, 0); - } - return app_addr; + native_addr = addr_app_to_native(app_addr); + if (native_addr != (void *)addr) { + EXPECT_EQ(1, 0); + } + return app_addr; } -static NativeSymbol g_test_native_symbols[] = -{ - native_function(test_addr_conv,"(*)i"), +static NativeSymbol g_test_native_symbols[] = { + native_function(test_addr_conv, "(*)i"), }; TEST_F(shared_heap_test, test_addr_conv) { - SharedHeapInitArgs args; + SharedHeapInitArgs args = { 0 }; WASMSharedHeap *shared_heap = nullptr; uint32 argv[1] = { 0 }; struct ret_env tmp_module_env; @@ -217,8 +220,7 @@ TEST_F(shared_heap_test, test_addr_conv) ret = wasm_native_register_natives("env", g_test_native_symbols, nitems(g_test_native_symbols)); - if (!ret) - { + if (!ret) { EXPECT_EQ(1, 0); return; }