diff --git a/src/singlepass/x64/codegen.h b/src/singlepass/x64/codegen.h index 89e415d0b..4b08e3db4 100644 --- a/src/singlepass/x64/codegen.h +++ b/src/singlepass/x64/codegen.h @@ -900,6 +900,7 @@ class X64OnePassCodeGenImpl checkMemoryOverflow(Base, Offset); + bool UseImmAddr = Base.isImm(); typename X64TypeAttr::RegNum BaseReg = X64::RAX; // the initial value only used to suppress compiler error @@ -914,7 +915,9 @@ class X64OnePassCodeGenImpl uint64_t Offset64 = (uint64_t)Offset; Offset64 += (uint32_t)Base.getImm(); if (Offset64 > INT32_MAX) { - Offset = INT32_MAX; // invalid addr + BaseReg = Layout.getScopedTemp(); + _ mov(X64Reg::getRegRef(BaseReg), (uint32_t)Base.getImm()); + UseImmAddr = false; } else { Offset = (uint32_t)Offset64; } @@ -928,26 +931,21 @@ class X64OnePassCodeGenImpl ValReg = Layout.getScopedTemp(); } - Addr = Base.isImm() - ? asmjit::x86::Mem(ABI.getMemoryBaseReg(), Offset, - getWASMTypeSize()) - : asmjit::x86::Mem(ABI.getMemoryBaseReg(), - X64Reg::getRegRef(BaseReg), 0, - Offset, getWASMTypeSize()); + Addr = UseImmAddr ? asmjit::x86::Mem(ABI.getMemoryBaseReg(), Offset, + getWASMTypeSize()) + : asmjit::x86::Mem(ABI.getMemoryBaseReg(), + X64Reg::getRegRef(BaseReg), + 0, Offset, getWASMTypeSize()); -#ifdef ZEN_ENABLE_CPU_EXCEPTION - if (!Base.isImm() && (Offset >= INT32_MAX)) { - // when offset >= INT32_MAX, then will cause inst like mov edi, dword - // ptr[r13+edi-1]. + if (!UseImmAddr && (Offset >= (uint32_t)INT32_MAX)) { auto MemAddrReg = Layout.getScopedTemp(); _ mov(X64Reg::getRegRef(MemAddrReg), Offset); _ add(X64Reg::getRegRef(MemAddrReg), X64Reg::getRegRef(BaseReg)); _ add(X64Reg::getRegRef(MemAddrReg), ABI.getMemoryBaseReg()); Addr = asmjit::x86::Mem(X64Reg::getRegRef(MemAddrReg), 0, - getWASMTypeSize(SrcType)); + getWASMTypeSize()); } -#endif // ZEN_ENABLE_CPU_EXCEPTION LoadOperatorImpl::emit( ASM, X64Reg::getRegRef(ValReg), Addr); @@ -995,6 +993,7 @@ class X64OnePassCodeGenImpl checkMemoryOverflow(Base, Offset); + bool UseImmAddr = Base.isImm(); X64::RegNum RegNum = 0; if (Base.isReg()) { RegNum = Base.getReg(); @@ -1006,7 +1005,9 @@ class X64OnePassCodeGenImpl uint64_t Offset64 = (uint64_t)Offset; Offset64 += (uint32_t)Base.getImm(); if (Offset64 > INT32_MAX) { - Offset = INT32_MAX; // invalid addr + RegNum = Layout.getScopedTemp(); + _ mov(X64Reg::getRegRef(RegNum), (uint32_t)Base.getImm()); + UseImmAddr = false; } else { Offset = (uint32_t)Offset64; } @@ -1014,14 +1015,22 @@ class X64OnePassCodeGenImpl ZEN_ABORT(); } - // Addr = memoryBase + (in64) offset, so when offset < 0, - // the result i32 Addr works like add (2**32 + offset) asmjit::x86::Mem Addr = - Base.isImm() ? asmjit::x86::Mem(ABI.getMemoryBaseReg(), Offset, - getWASMTypeSize()) - : asmjit::x86::Mem(ABI.getMemoryBaseReg(), - X64Reg::getRegRef(RegNum), 0, - Offset, getWASMTypeSize()); + UseImmAddr ? asmjit::x86::Mem(ABI.getMemoryBaseReg(), Offset, + getWASMTypeSize()) + : asmjit::x86::Mem(ABI.getMemoryBaseReg(), + X64Reg::getRegRef(RegNum), 0, + Offset, getWASMTypeSize()); + + if (!UseImmAddr && (Offset >= (uint32_t)INT32_MAX)) { + auto MemAddrReg = Layout.getScopedTemp(); + _ mov(X64Reg::getRegRef(MemAddrReg), Offset); + _ add(X64Reg::getRegRef(MemAddrReg), + X64Reg::getRegRef(RegNum)); + _ add(X64Reg::getRegRef(MemAddrReg), ABI.getMemoryBaseReg()); + Addr = asmjit::x86::Mem(X64Reg::getRegRef(MemAddrReg), 0, + getWASMTypeSize()); + } mov(Addr, Value); } diff --git a/tests/wast/spec_extra/store_imm_base_overflow_false_negative_oob.wast b/tests/wast/spec_extra/store_imm_base_overflow_false_negative_oob.wast new file mode 100644 index 000000000..0763d6919 --- /dev/null +++ b/tests/wast/spec_extra/store_imm_base_overflow_false_negative_oob.wast @@ -0,0 +1,32 @@ +;; Test: store with immediate base where (uint64_t)base + offset overflows +;; 32-bit arithmetic. The effective address exceeds memory size and MUST trap. +;; +;; base(unsigned) = 0xFFDFFFFF = 4292870143, offset = 1257956348 +;; Effective address = 4292870143 + 1257956348 = 5550826491 +;; Memory size = 32769 * 65536 = 2147549184 +;; 5550826491 + 2 > 2147549184 => out-of-bounds, MUST trap. + +(module + (memory (;0;) 32769) + + ;; const base + offset overflows 32-bit => OOB + (func (export "i64_store16_const_base_overflow_oob") + i32.const -2097153 + i64.const 0 + i64.store16 offset=1257956348 align=1) + + ;; dynamic base (same value via param) + offset overflows 32-bit => OOB + (func (export "i64_store16_param_base_overflow_oob") (param i32) + local.get 0 + i64.const 0 + i64.store16 offset=1257956348 align=1) + + ;; const base + offset overflows 32-bit for load => OOB + (func (export "i64_load16_u_const_base_overflow_oob") (result i64) + i32.const -2097153 + i64.load16_u offset=1257956348 align=1) +) + +(assert_trap (invoke "i64_store16_const_base_overflow_oob") "out of bounds memory access") +(assert_trap (invoke "i64_store16_param_base_overflow_oob" (i32.const -2097153)) "out of bounds memory access") +(assert_trap (invoke "i64_load16_u_const_base_overflow_oob") "out of bounds memory access") diff --git a/tests/wast/spec_extra/store_large_offset_false_positive_oob.wast b/tests/wast/spec_extra/store_large_offset_false_positive_oob.wast new file mode 100644 index 000000000..e185ff258 --- /dev/null +++ b/tests/wast/spec_extra/store_large_offset_false_positive_oob.wast @@ -0,0 +1,44 @@ +;; Test: f64.store with offset >= INT32_MAX on large memory should NOT trap. +;; +;; When offset >= 0x80000000, x86-64 disp32 sign-extends to a negative 64-bit +;; value, causing the effective address to go before MemBase. The JIT must +;; compute the full 64-bit address explicitly to avoid this. +;; +;; Effective address = memory.size(65131) + offset(4268353288) = 4268418419 +;; Memory size = 65131 * 65536 = 4269236224 +;; 4268418419 + 8 <= 4269236224 => in-bounds, should NOT trap. + +(module + (memory (;0;) 65131) + + ;; dynamic base (memory.size result) + large offset: in-bounds store + (func (export "f64_store_dynamic_base_large_offset") + memory.size + f64.const -5.44203 + f64.store offset=4268353288) + + ;; dynamic base via parameter + large offset: in-bounds store + (func (export "f64_store_param_base_large_offset") (param i32) + local.get 0 + f64.const 1.0 + f64.store offset=4268353288) + + ;; dynamic base (memory.size result) + large offset: in-bounds load + (func (export "f64_load_dynamic_base_large_offset") (result f64) + memory.size + f64.load offset=4268353288) + + ;; large offset store that IS out-of-bounds must still trap + (func (export "f64_store_large_offset_oob") (param i32) + local.get 0 + f64.const 0.0 + f64.store offset=4294967295) +) + +;; These should all succeed without trapping +(assert_return (invoke "f64_store_dynamic_base_large_offset")) +(assert_return (invoke "f64_store_param_base_large_offset" (i32.const 65131))) +(assert_return (invoke "f64_load_dynamic_base_large_offset") (f64.const 1.0)) + +;; Large offset that actually exceeds memory should still trap +(assert_trap (invoke "f64_store_large_offset_oob" (i32.const 65131)) "out of bounds memory access")