Fix an off-by-two condition in heap legalization

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;

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(())
+}