From 826cf4f8e1f0f540ce5a2efa6203a72e9c5c54be Mon Sep 17 00:00:00 2001 From: Xu Jun <693788454@qq.com> Date: Thu, 13 Oct 2022 13:53:09 +0800 Subject: [PATCH] Fix threads spec test issues (#1586) --- core/iwasm/common/wasm_shared_memory.c | 41 +++++++++++++++- core/iwasm/compilation/aot_compiler.c | 4 ++ core/iwasm/fast-jit/jit_frontend.c | 4 ++ core/iwasm/interpreter/wasm_interp_classic.c | 15 ++++-- core/iwasm/interpreter/wasm_interp_fast.c | 6 ++- .../thread_proposal_fix_atomic_case.patch | 49 +++++++++++++++++++ tests/wamr-test-suites/test_wamr.sh | 5 ++ 7 files changed, 117 insertions(+), 7 deletions(-) create mode 100644 tests/wamr-test-suites/spec-test-script/thread_proposal_fix_atomic_case.patch diff --git a/core/iwasm/common/wasm_shared_memory.c b/core/iwasm/common/wasm_shared_memory.c index a0b4001e..395c677f 100644 --- a/core/iwasm/common/wasm_shared_memory.c +++ b/core/iwasm/common/wasm_shared_memory.c @@ -323,7 +323,13 @@ wasm_runtime_atomic_wait(WASMModuleInstanceCommon *module, void *address, WASMModuleInstance *module_inst = (WASMModuleInstance *)module; /* Currently we have only one memory instance */ if (!module_inst->memories[0]->is_shared) { - wasm_runtime_set_exception(module, "wait on unshared memory"); + wasm_runtime_set_exception(module, "expected shared memory"); + return -1; + } + if ((uint8 *)address < module_inst->memories[0]->memory_data + || (uint8 *)address + (wait64 ? 8 : 4) + > module_inst->memories[0]->memory_data_end) { + wasm_runtime_set_exception(module, "out of bounds memory access"); return -1; } } @@ -335,7 +341,13 @@ wasm_runtime_atomic_wait(WASMModuleInstanceCommon *module, void *address, ((AOTMemoryInstance **)aot_inst->memories.ptr)[0]; /* Currently we have only one memory instance */ if (!aot_memory->is_shared) { - wasm_runtime_set_exception(module, "wait on unshared memory"); + wasm_runtime_set_exception(module, "expected shared memory"); + return -1; + } + if ((uint8 *)address < (uint8 *)aot_memory->memory_data.ptr + || (uint8 *)address + (wait64 ? 8 : 4) + > (uint8 *)aot_memory->memory_data_end.ptr) { + wasm_runtime_set_exception(module, "out of bounds memory access"); return -1; } } @@ -424,6 +436,31 @@ wasm_runtime_atomic_notify(WASMModuleInstanceCommon *module, void *address, uint32 notify_result; AtomicWaitInfo *wait_info; +#if WASM_ENABLE_INTERP != 0 + if (module->module_type == Wasm_Module_Bytecode) { + WASMModuleInstance *module_inst = (WASMModuleInstance *)module; + if ((uint8 *)address < module_inst->memories[0]->memory_data + || (uint8 *)address + 4 + > module_inst->memories[0]->memory_data_end) { + wasm_runtime_set_exception(module, "out of bounds memory access"); + return -1; + } + } +#endif +#if WASM_ENABLE_AOT != 0 + if (module->module_type == Wasm_Module_AoT) { + AOTModuleInstance *aot_inst = (AOTModuleInstance *)module; + AOTMemoryInstance *aot_memory = + ((AOTMemoryInstance **)aot_inst->memories.ptr)[0]; + if ((uint8 *)address < (uint8 *)aot_memory->memory_data.ptr + || (uint8 *)address + 4 + > (uint8 *)aot_memory->memory_data_end.ptr) { + wasm_runtime_set_exception(module, "out of bounds memory access"); + return -1; + } + } +#endif + wait_info = acquire_wait_info(address, false); /* Nobody wait on this address */ diff --git a/core/iwasm/compilation/aot_compiler.c b/core/iwasm/compilation/aot_compiler.c index 064f5831..04425154 100644 --- a/core/iwasm/compilation/aot_compiler.c +++ b/core/iwasm/compilation/aot_compiler.c @@ -1217,6 +1217,10 @@ aot_compile_func(AOTCompContext *comp_ctx, uint32 func_index) comp_ctx, func_ctx, align, offset, bytes)) return false; break; + case WASM_OP_ATOMIC_FENCE: + /* Skip memory index */ + frame_ip++; + break; case WASM_OP_ATOMIC_I32_LOAD: bytes = 4; goto op_atomic_i32_load; diff --git a/core/iwasm/fast-jit/jit_frontend.c b/core/iwasm/fast-jit/jit_frontend.c index 44c06a7d..90e20da3 100644 --- a/core/iwasm/fast-jit/jit_frontend.c +++ b/core/iwasm/fast-jit/jit_frontend.c @@ -2052,6 +2052,10 @@ jit_compile_func(JitCompContext *cc) bytes)) return false; break; + case WASM_OP_ATOMIC_FENCE: + /* Skip memory index */ + frame_ip++; + break; case WASM_OP_ATOMIC_I32_LOAD: bytes = 4; goto op_atomic_i32_load; diff --git a/core/iwasm/interpreter/wasm_interp_classic.c b/core/iwasm/interpreter/wasm_interp_classic.c index 494b6825..b7edcf7c 100644 --- a/core/iwasm/interpreter/wasm_interp_classic.c +++ b/core/iwasm/interpreter/wasm_interp_classic.c @@ -3335,12 +3335,15 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, #if WASM_ENABLE_SHARED_MEMORY != 0 HANDLE_OP(WASM_OP_ATOMIC_PREFIX) { - uint32 offset, align, addr; + uint32 offset = 0, align, addr; opcode = *frame_ip++; - read_leb_uint32(frame_ip, frame_ip_end, align); - read_leb_uint32(frame_ip, frame_ip_end, offset); + if (opcode != WASM_OP_ATOMIC_FENCE) { + read_leb_uint32(frame_ip, frame_ip_end, align); + read_leb_uint32(frame_ip, frame_ip_end, offset); + } + switch (opcode) { case WASM_OP_ATOMIC_NOTIFY: { @@ -3399,6 +3402,12 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, PUSH_I32(ret); break; } + case WASM_OP_ATOMIC_FENCE: + { + /* Skip the memory index */ + frame_ip++; + break; + } case WASM_OP_ATOMIC_I32_LOAD: case WASM_OP_ATOMIC_I32_LOAD8_U: diff --git a/core/iwasm/interpreter/wasm_interp_fast.c b/core/iwasm/interpreter/wasm_interp_fast.c index e27919dd..2f9c654a 100644 --- a/core/iwasm/interpreter/wasm_interp_fast.c +++ b/core/iwasm/interpreter/wasm_interp_fast.c @@ -3227,11 +3227,13 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, #if WASM_ENABLE_SHARED_MEMORY != 0 HANDLE_OP(WASM_OP_ATOMIC_PREFIX) { - uint32 offset, addr; + uint32 offset = 0, addr; GET_OPCODE(); - offset = read_uint32(frame_ip); + if (opcode != WASM_OP_ATOMIC_FENCE) { + offset = read_uint32(frame_ip); + } switch (opcode) { case WASM_OP_ATOMIC_NOTIFY: diff --git a/tests/wamr-test-suites/spec-test-script/thread_proposal_fix_atomic_case.patch b/tests/wamr-test-suites/spec-test-script/thread_proposal_fix_atomic_case.patch new file mode 100644 index 00000000..a64dd178 --- /dev/null +++ b/tests/wamr-test-suites/spec-test-script/thread_proposal_fix_atomic_case.patch @@ -0,0 +1,49 @@ +diff --git a/test/core/atomic.wast b/test/core/atomic.wast +index 66ad0eb..40259a9 100644 +--- a/test/core/atomic.wast ++++ b/test/core/atomic.wast +@@ -324,7 +324,7 @@ + + (invoke "init" (i64.const 0x1111111111111111)) + (assert_return (invoke "i32.atomic.rmw8.cmpxchg_u" (i32.const 0) (i32.const 0x11111111) (i32.const 0xcdcdcdcd)) (i32.const 0x11)) +-(assert_return (invoke "i64.atomic.load" (i32.const 0)) (i64.const 0x1111111111111111)) ++(assert_return (invoke "i64.atomic.load" (i32.const 0)) (i64.const 0x11111111111111cd)) + + (invoke "init" (i64.const 0x1111111111111111)) + (assert_return (invoke "i32.atomic.rmw16.cmpxchg_u" (i32.const 0) (i32.const 0) (i32.const 0xcafecafe)) (i32.const 0x1111)) +@@ -332,7 +332,7 @@ + + (invoke "init" (i64.const 0x1111111111111111)) + (assert_return (invoke "i32.atomic.rmw16.cmpxchg_u" (i32.const 0) (i32.const 0x11111111) (i32.const 0xcafecafe)) (i32.const 0x1111)) +-(assert_return (invoke "i64.atomic.load" (i32.const 0)) (i64.const 0x1111111111111111)) ++(assert_return (invoke "i64.atomic.load" (i32.const 0)) (i64.const 0x111111111111cafe)) + + (invoke "init" (i64.const 0x1111111111111111)) + (assert_return (invoke "i64.atomic.rmw8.cmpxchg_u" (i32.const 0) (i64.const 0) (i64.const 0x4242424242424242)) (i64.const 0x11)) +@@ -340,7 +340,7 @@ + + (invoke "init" (i64.const 0x1111111111111111)) + (assert_return (invoke "i64.atomic.rmw8.cmpxchg_u" (i32.const 0) (i64.const 0x1111111111111111) (i64.const 0x4242424242424242)) (i64.const 0x11)) +-(assert_return (invoke "i64.atomic.load" (i32.const 0)) (i64.const 0x1111111111111111)) ++(assert_return (invoke "i64.atomic.load" (i32.const 0)) (i64.const 0x1111111111111142)) + + (invoke "init" (i64.const 0x1111111111111111)) + (assert_return (invoke "i64.atomic.rmw16.cmpxchg_u" (i32.const 0) (i64.const 0) (i64.const 0xbeefbeefbeefbeef)) (i64.const 0x1111)) +@@ -348,7 +348,7 @@ + + (invoke "init" (i64.const 0x1111111111111111)) + (assert_return (invoke "i64.atomic.rmw16.cmpxchg_u" (i32.const 0) (i64.const 0x1111111111111111) (i64.const 0xbeefbeefbeefbeef)) (i64.const 0x1111)) +-(assert_return (invoke "i64.atomic.load" (i32.const 0)) (i64.const 0x1111111111111111)) ++(assert_return (invoke "i64.atomic.load" (i32.const 0)) (i64.const 0x111111111111beef)) + + (invoke "init" (i64.const 0x1111111111111111)) + (assert_return (invoke "i64.atomic.rmw32.cmpxchg_u" (i32.const 0) (i64.const 0) (i64.const 0xcabba6e5cabba6e5)) (i64.const 0x11111111)) +@@ -356,7 +356,7 @@ + + (invoke "init" (i64.const 0x1111111111111111)) + (assert_return (invoke "i64.atomic.rmw32.cmpxchg_u" (i32.const 0) (i64.const 0x1111111111111111) (i64.const 0xcabba6e5cabba6e5)) (i64.const 0x11111111)) +-(assert_return (invoke "i64.atomic.load" (i32.const 0)) (i64.const 0x1111111111111111)) ++(assert_return (invoke "i64.atomic.load" (i32.const 0)) (i64.const 0x11111111cabba6e5)) + + ;; *.atomic.rmw*.cmpxchg (compare true) + diff --git a/tests/wamr-test-suites/test_wamr.sh b/tests/wamr-test-suites/test_wamr.sh index 3434e202..dfcde06d 100755 --- a/tests/wamr-test-suites/test_wamr.sh +++ b/tests/wamr-test-suites/test_wamr.sh @@ -308,6 +308,7 @@ function spec_test() git checkout threads/main git apply ../../spec-test-script/thread_proposal_ignore_cases.patch + git apply ../../spec-test-script/thread_proposal_fix_atomic_case.patch fi popd @@ -383,6 +384,10 @@ function spec_test() if [[ ${ENABLE_MULTI_THREAD} == 1 ]]; then ARGS_FOR_SPEC_TEST+="-p " + if [[ $1 == 'fast-jit' ]]; then + echo "fast-jit doesn't support multi-thread feature yet, skip it" + return + fi fi if [[ ${ENABLE_XIP} == 1 ]]; then