wasm-micro-runtime
wasm-micro-runtime copied to clipboard
bytecodealliance
Missing bounds check in code generated by AOT
The following wasm module:
(module
(func (result i32)
(i32.load (i32.const 0))
)
(memory 0)
)
when compiled by wamrc (version f3f8d684b319de3f7cb0895f4b7be88e796ec4a5) with option --bounds-checks=1, produces the following machine code:
0000000000000000 <aot_func#0>:
0: 48 8b 47 10 mov 0x10(%rdi),%rax
4: 48 8b 80 58 01 00 00 mov 0x158(%rax),%rax
b: 8b 00 mov (%rax),%eax
d: c3 retq
Apparently, there lacks bounds check in the generated code. If iwasm is built with hardware bound check disabled, running this wasm module would probably crash.
Wasm memory index starts with 0, this should be valid code. Try adding i32 parameter to the function and using it instead of i32.const 0, this should generate a bounds check in case passed value is too big.
Wasm memory index starts with 0, this should be valid code. Try adding
i32parameter to the function and using it instead ofi32.const 0, this should generate a bounds check in case passed value is too big.
Yes, using a non-const address would make wamrc generate bounds check. But that doesn't mean bounds check is not needed for const addresses.
Check for 0 addresses can probably be elided as always within bounds, right @wenyongh?
Check for 0 addresses can probably be elided as always within bounds, right @wenyongh?
Yes, if the page count is not 0. In fact, if the const address and its byte count to access are in the range of the initial page, we can ignore the check, as the linear memory cannot shrink -- it can only keep unchanged or grow by opcode memory.grow. For example, if the initial page count is 1 and the bytes of each page is 65536, then the initial linear memory size must be at least 65536, the const address for i32.load/store can be 0 to 65532, no need to add check for them.
@lei-april Thanks for reporting the issue! It should only occur when the initial page count is 0 and there is no memory.grow opcode, we will fix it soon.
