From 996758cd4a32f5c5c04af4f6640ae1c510f0916d Mon Sep 17 00:00:00 2001 From: "liang.he" Date: Thu, 17 Apr 2025 15:21:02 +0800 Subject: [PATCH] Remove the dlen to optimize it. (#4193) There are two reasons for this optimization: - The value of dlen can equal 0x1_0000_0000, even in wasm32 mode, because it is derived from (4G-0). This results in a truncation when it is passed to b_memmove_s(). Consequently, s1max becomes 0 and n is greater than s1max. To correct this, a longer type is required. - The dlen is only used to check if there is enough space in b_memmove_s(). However, from a different angle, after confirming that both src+len and dst+len are within the memory range, we can be assured and there is no need for this explicit check. --- core/iwasm/interpreter/wasm_interp_classic.c | 35 ++++++++++---------- core/iwasm/interpreter/wasm_interp_fast.c | 27 +++++++++------ 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/core/iwasm/interpreter/wasm_interp_classic.c b/core/iwasm/interpreter/wasm_interp_classic.c index d504e984c..41ac4c724 100644 --- a/core/iwasm/interpreter/wasm_interp_classic.c +++ b/core/iwasm/interpreter/wasm_interp_classic.c @@ -5784,7 +5784,6 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, { mem_offset_t dst, src, len; uint8 *mdst, *msrc; - uint64 dlen; len = POP_MEM_OFFSET(); src = POP_MEM_OFFSET(); @@ -5797,24 +5796,17 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, /* skip dst memidx */ frame_ip += 1; #endif + // TODO: apply memidx #if WASM_ENABLE_THREAD_MGR != 0 linear_mem_size = get_linear_mem_size(); #endif - - dlen = linear_mem_size - dst; - /* dst boundary check */ #ifndef OS_ENABLE_HW_BOUND_CHECK CHECK_BULK_MEMORY_OVERFLOW(dst, len, mdst); -#if WASM_ENABLE_SHARED_HEAP != 0 - if (app_addr_in_shared_heap((uint64)dst, len)) - dlen = shared_heap_end_off - dst + 1; -#endif #else /* else of OS_ENABLE_HW_BOUND_CHECK */ #if WASM_ENABLE_SHARED_HEAP != 0 if (app_addr_in_shared_heap((uint64)dst, len)) { shared_heap_addr_app_to_native((uint64)dst, mdst); - dlen = shared_heap_end_off - dst + 1; } else #endif @@ -5832,6 +5824,7 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, /* skip src memidx */ frame_ip += 1; #endif + // TODO: apply memidx #if WASM_ENABLE_THREAD_MGR != 0 linear_mem_size = get_linear_mem_size(); #endif @@ -5851,15 +5844,21 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, } #endif -#if WASM_ENABLE_MEMORY64 == 0 - /* allowing the destination and source to overlap */ - bh_memmove_s(mdst, (uint32)dlen, msrc, (uint32)len); -#else - /* use memmove when memory64 is enabled since len - may be larger than UINT32_MAX */ - memmove(mdst, msrc, len); - (void)dlen; -#endif + /* + * avoid unnecessary operations + * + * since dst and src both are valid indexes in the + * linear memory, mdst and msrc can't be NULL + * + * The spec. converts memory.copy into i32.load8 and + * i32.store8; the following are runtime-specific + * optimizations. + * + */ + if (len && mdst != msrc) { + /* allowing the destination and source to overlap */ + memmove(mdst, msrc, len); + } break; } case WASM_OP_MEMORY_FILL: diff --git a/core/iwasm/interpreter/wasm_interp_fast.c b/core/iwasm/interpreter/wasm_interp_fast.c index aa0330568..f02f3f2f0 100644 --- a/core/iwasm/interpreter/wasm_interp_fast.c +++ b/core/iwasm/interpreter/wasm_interp_fast.c @@ -5163,7 +5163,6 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, { uint32 dst, src, len; uint8 *mdst, *msrc; - uint64 dlen; len = POP_I32(); src = POP_I32(); @@ -5173,15 +5172,9 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, linear_mem_size = get_linear_mem_size(); #endif - dlen = linear_mem_size - dst; - #ifndef OS_ENABLE_HW_BOUND_CHECK CHECK_BULK_MEMORY_OVERFLOW(src, len, msrc); CHECK_BULK_MEMORY_OVERFLOW(dst, len, mdst); -#if WASM_ENABLE_SHARED_HEAP != 0 - if (app_addr_in_shared_heap((uint64)dst, len)) - dlen = shared_heap_end_off - dst + 1; -#endif #else /* else of OS_ENABLE_HW_BOUND_CHECK */ #if WASM_ENABLE_SHARED_HEAP != 0 if (app_addr_in_shared_heap((uint64)src, len)) @@ -5197,7 +5190,6 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, #if WASM_ENABLE_SHARED_HEAP != 0 if (app_addr_in_shared_heap((uint64)dst, len)) { shared_heap_addr_app_to_native((uint64)dst, mdst); - dlen = shared_heap_end_off - dst + 1; } else #endif @@ -5208,8 +5200,21 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, } #endif /* end of OS_ENABLE_HW_BOUND_CHECK */ - /* allowing the destination and source to overlap */ - bh_memmove_s(mdst, (uint32)dlen, msrc, len); + /* + * avoid unnecessary operations + * + * since dst and src both are valid indexes in the + * linear memory, mdst and msrc can't be NULL + * + * The spec. converts memory.copy into i32.load8 and + * i32.store8; the following are runtime-specific + * optimizations. + * + */ + if (len && mdst != msrc) { + /* allowing the destination and source to overlap */ + memmove(mdst, msrc, len); + } break; } case WASM_OP_MEMORY_FILL: @@ -7488,7 +7493,7 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module, #if WASM_ENABLE_LABELS_AS_VALUES == 0 continue; #else - FETCH_OPCODE_AND_DISPATCH(); + FETCH_OPCODE_AND_DISPATCH(); #endif #if WASM_ENABLE_TAIL_CALL != 0 || WASM_ENABLE_GC != 0