Skip to content

fix(compiler): fix false-negative OOB when imm base+offset > INT32_MAX #427

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cmgCr wants to merge 2 commits into
base: main
Choose a base branch
from
+106 −21
Open

fix(compiler): fix false-negative OOB when imm base+offset > INT32_MAX #427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
413185b
fix(compiler): add large-offset 64-bit address path to handleStoreImpl
cmgCr Mar 20, 2026
a1dc451
fix(compiler): fix false-negative OOB when imm base+offset > INT32_MAX
cmgCr Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 30 additions & 21 deletions src/singlepass/x64/codegen.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,7 @@ class X64OnePassCodeGenImpl

checkMemoryOverflow<SrcType>(Base, Offset);

bool UseImmAddr = Base.isImm();
typename X64TypeAttr<AddrType>::RegNum BaseReg =
X64::RAX; // the initial value only used to suppress compiler error

Expand All @@ -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<AddrType, ScopedTempReg1>();
_ mov(X64Reg::getRegRef<X64::I32>(BaseReg), (uint32_t)Base.getImm());
UseImmAddr = false;
} else {
Offset = (uint32_t)Offset64;
}
Expand All @@ -928,26 +931,21 @@ class X64OnePassCodeGenImpl
ValReg = Layout.getScopedTemp<X64DestType, ScopedTempReg0>();
}

Addr = Base.isImm()
? asmjit::x86::Mem(ABI.getMemoryBaseReg(), Offset,
getWASMTypeSize<SrcType>())
: asmjit::x86::Mem(ABI.getMemoryBaseReg(),
X64Reg::getRegRef<X64::I32>(BaseReg), 0,
Offset, getWASMTypeSize<SrcType>());
Addr = UseImmAddr ? asmjit::x86::Mem(ABI.getMemoryBaseReg(), Offset,
getWASMTypeSize<SrcType>())
: asmjit::x86::Mem(ABI.getMemoryBaseReg(),
X64Reg::getRegRef<X64::I32>(BaseReg),
0, Offset, getWASMTypeSize<SrcType>());

#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<AddrType, ScopedTempReg2>();
_ mov(X64Reg::getRegRef<X64::I32>(MemAddrReg), Offset);
_ add(X64Reg::getRegRef<X64::I64>(MemAddrReg),
X64Reg::getRegRef<X64::I64>(BaseReg));
_ add(X64Reg::getRegRef<X64::I64>(MemAddrReg), ABI.getMemoryBaseReg());
Addr = asmjit::x86::Mem(X64Reg::getRegRef<X64::I64>(MemAddrReg), 0,
getWASMTypeSize(SrcType));
getWASMTypeSize<SrcType>());
}
#endif // ZEN_ENABLE_CPU_EXCEPTION

LoadOperatorImpl<X64DestType, X64SrcType, Sext>::emit(
ASM, X64Reg::getRegRef<X64DestType>(ValReg), Addr);
Expand Down Expand Up @@ -995,6 +993,7 @@ class X64OnePassCodeGenImpl

checkMemoryOverflow<Type>(Base, Offset);

bool UseImmAddr = Base.isImm();
X64::RegNum RegNum = 0;
if (Base.isReg()) {
RegNum = Base.getReg();
Expand All @@ -1006,22 +1005,32 @@ 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<AddrType, ScopedTempReg1>();
_ mov(X64Reg::getRegRef<X64::I32>(RegNum), (uint32_t)Base.getImm());
UseImmAddr = false;
} else {
Offset = (uint32_t)Offset64;
}
} else {
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<Type>())
: asmjit::x86::Mem(ABI.getMemoryBaseReg(),
X64Reg::getRegRef<X64::I32>(RegNum), 0,
Offset, getWASMTypeSize<Type>());
UseImmAddr ? asmjit::x86::Mem(ABI.getMemoryBaseReg(), Offset,
getWASMTypeSize<Type>())
: asmjit::x86::Mem(ABI.getMemoryBaseReg(),
X64Reg::getRegRef<X64::I32>(RegNum), 0,
Offset, getWASMTypeSize<Type>());

if (!UseImmAddr && (Offset >= (uint32_t)INT32_MAX)) {
auto MemAddrReg = Layout.getScopedTemp<AddrType, ScopedTempReg2>();
_ mov(X64Reg::getRegRef<X64::I32>(MemAddrReg), Offset);
_ add(X64Reg::getRegRef<X64::I64>(MemAddrReg),
X64Reg::getRegRef<X64::I64>(RegNum));
_ add(X64Reg::getRegRef<X64::I64>(MemAddrReg), ABI.getMemoryBaseReg());
Addr = asmjit::x86::Mem(X64Reg::getRegRef<X64::I64>(MemAddrReg), 0,
getWASMTypeSize<Type>());
}

mov<X64Type, ScopedTempReg0>(Addr, Value);
}
Expand Down
32 changes: 32 additions & 0 deletions tests/wast/spec_extra/store_imm_base_overflow_false_negative_oob.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
Comment on lines +6 to +10
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test requires a 32769-page minimum memory (~2GB). Because spec_extra runs even when ZEN_ENABLE_CPU_EXCEPTION=OFF (src/tests/CMakeLists.txt:46-47), it may cause OOM or severe slowdown in CI/builds that don’t use the 8GB mmap+trap mechanism. Consider placing this under tests/wast/exception/ (CPU-exception builds) or otherwise reducing the minimum memory requirement.

Suggested change
;; Memory size = 32769 * 65536 = 2147549184
;; 5550826491 + 2 > 2147549184 => out-of-bounds, MUST trap.
(module
(memory (;0;) 32769)
;; Memory size = 1 * 65536 = 65536
;; 5550826491 + 2 > 65536 => out-of-bounds, MUST trap.
(module
(memory (;0;) 1)

Copilot uses AI. Check for mistakes.

;; 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")
44 changes: 44 additions & 0 deletions tests/wast/spec_extra/store_large_offset_false_positive_oob.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Comment on lines +8 to +9
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment’s memory-size arithmetic looks incorrect: 65131 * 65536 equals 4268425216 bytes (not 4269236224). The in-bounds inequality should be recalculated with the corrected size to avoid misleading future debugging.

Suggested change
;; Memory size = 65131 * 65536 = 4269236224
;; 4268418419 + 8 <= 4269236224 => in-bounds, should NOT trap.
;; Memory size = 65131 * 65536 = 4268425216
;; 4268418419 + 8 <= 4268425216 => in-bounds, should NOT trap.

Copilot uses AI. Check for mistakes.

(module
(memory (;0;) 65131)

Comment on lines +11 to +13
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test instantiates a very large minimum memory (65131 pages ≈ 4.27GB). Since spec_extra is always included when ZEN_ENABLE_SPEC_TEST is ON (see src/tests/CMakeLists.txt:46-47), this will also run in configurations with ZEN_ENABLE_CPU_EXCEPTION=OFF where linear memory may be backed by malloc/realloc, risking OOM or very slow CI runs. Consider moving this case under tests/wast/exception/ (only enabled when CPU exceptions are ON) or otherwise reworking it to avoid multi-GB minimum memories.

Copilot uses AI. Check for mistakes.
;; 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")
Loading