diff --git a/cranelift/codegen/src/legalizer/heap.rs b/cranelift/codegen/src/legalizer/heap.rs index 5239c67daf..f423632a6a 100644 --- a/cranelift/codegen/src/legalizer/heap.rs +++ b/cranelift/codegen/src/legalizer/heap.rs @@ -157,10 +157,20 @@ fn static_addr( let mut spectre_oob_comparison = None; offset = cast_offset_to_pointer_ty(offset, offset_ty, addr_ty, &mut pos); if offset_ty != ir::types::I32 || limit < 0xffff_ffff { + // Here we want to test the condition `offset > limit` and if that's + // true then this is an out-of-bounds access and needs to trap. For ARM + // and other RISC architectures it's easier to test against an immediate + // that's even instead of odd, so if `limit` is odd then we instead test + // for `offset >= limit + 1`. + // + // The thinking behind this is that: + // + // A >= B + 1 => A - 1 >= B => A > B + // + // where the last step here is true because A/B are integers, which + // should mean that `A >= B + 1` is an equivalent check for `A > B` let (cc, lhs, limit_imm) = if limit & 1 == 1 { - // Prefer testing `offset >= limit - 1` when limit is odd because an even number is - // likely to be a convenient constant on ARM and other RISC architectures. - let limit = limit as i64 - 1; + let limit = limit as i64 + 1; (IntCC::UnsignedGreaterThanOrEqual, offset, limit) } else { let limit = limit as i64; diff --git a/tests/all/memory.rs b/tests/all/memory.rs index bf8f564aa3..e2d823508b 100644 --- a/tests/all/memory.rs +++ b/tests/all/memory.rs @@ -321,3 +321,40 @@ fn massive_64_bit_still_limited() -> Result<()> { } } } + +#[test] +fn tiny_static_heap() -> Result<()> { + // The size of the memory in the module below is the exact same size as + // the static memory size limit in the configuration. This is intended to + // specifically test that a load of all the valid addresses of the memory + // all pass bounds-checks in cranelift to help weed out any off-by-one bugs. + let mut config = Config::new(); + config.static_memory_maximum_size(65536); + let engine = Engine::new(&config)?; + let mut store = Store::new(&engine, ()); + + let module = Module::new( + &engine, + r#" + (module + (memory 1 1) + (func (export "run") + (local $i i32) + + (loop + (if (i32.eq (local.get $i) (i32.const 65536)) + (return)) + (drop (i32.load8_u (local.get $i))) + (local.set $i (i32.add (local.get $i) (i32.const 1))) + br 0 + ) + ) + ) + "#, + )?; + + let i = Instance::new(&mut store, &module, &[])?; + let f = i.get_typed_func::<(), (), _>(&mut store, "run")?; + f.call(&mut store, ())?; + Ok(()) +}