Merge #1436

1436: fix(singlepass-backend): I64ExtendI32U: Garbage in upper 32 bits shouldn't propagate to the result. r=syrusakbary a=losfair

Fixes https://github.com/wasmerio/wasmer/issues/1429 .

Co-authored-by: losfair <zhy20000919@hotmail.com>
Co-authored-by: Heyang Zhou <zhy20000919@hotmail.com>
Co-authored-by: Syrus <me@syrusakbary.com>

build.rs

@@ -64,8 +64,12 @@ fn main() -> anyhow::Result<()> {
     };
     let backends = vec!["singlepass", "cranelift", "llvm"];
     with_backends(&mut spectests, &backends, |mut spectests| {
-        with_test_module(&mut spectests, "spec", |spectests| {
+        with_test_module(&mut spectests, "spec", |mut spectests| {
             let _spec_tests = test_directory(spectests, "tests/spectests", wast_processor)?;
+            with_test_module(&mut spectests, "custom", |spectests| {
+                let _spec_tests = test_directory(spectests, "tests/custom", wast_processor)?;
+                Ok(())
+            })?;
             Ok(())
         })?;
         Ok(())

lib/singlepass-backend/src/codegen_x64.rs

@@ -4043,6 +4043,7 @@ impl FunctionCodeGenerator<CodegenError> for X64FunctionCode {
                     false,
                 )[0];
                 self.value_stack.push(ret);
+
                 Self::emit_relaxed_binop(
                     a,
                     &mut self.machine,
@@ -4051,6 +4052,19 @@ impl FunctionCodeGenerator<CodegenError> for X64FunctionCode {
                     loc,
                     ret,
                 );
+
+                // A 32-bit memory write does not automatically clear the upper 32 bits of a 64-bit word.
+                // So, we need to explicitly write zero to the upper half here.
+                if let Location::Memory(base, off) = ret {
+                    Self::emit_relaxed_binop(
+                        a,
+                        &mut self.machine,
+                        Assembler::emit_mov,
+                        Size::S32,
+                        Location::Imm32(0),
+                        Location::Memory(base, off + 4),
+                    );
+                }
             }
             Operator::I64ExtendI32S => {
                 let loc =

tests/custom/int-extend-garbage.wast

@@ -0,0 +1,35 @@
+;; https://github.com/wasmerio/wasmer/issues/1429
+;;
+;; When doing an I64ExtendI32U or other integer extension operations, the
+;; upper bits in the underlying storage must be cleared.
+;;
+;; On x86 sign extension is done with its own instruction, `movsx`, so here we only
+;; test the unsigned extension case.
+
+(module
+
+  (func (export "i64-extend-i32-u") (result i64)
+    ;; fill in stack slots allocated to registers
+    (i64.add (i64.const 0) (i64.const 0))
+    (i64.add (i64.const 0) (i64.const 0))
+    (i64.add (i64.const 0) (i64.const 0))
+    (i64.add (i64.const 0) (i64.const 0))
+    (i64.add (i64.const 0) (i64.const 0))
+    (i64.add (i64.const 0) (i64.const 0))
+
+    ;; push an i64 to produce garbage on the higher 32 bits
+    (i64.add (i64.const -1) (i64.const 0))
+
+    ;; pop it
+    (drop)
+
+    ;; push an i32
+    (i32.add (i32.const 0) (i32.const 0))
+
+    ;; extend
+    (i64.extend_i32_u)
+    (return)
+  )
+)
+
+(assert_return (invoke "i64-extend-i32-u") (i64.const 0))

tests/custom/nan-canonicalization.wast

@@ -79,10 +79,10 @@
     (i32.reinterpret_f32 (call $nan-canonicalization-f32-func-call-target (f32.add (f32.reinterpret_i32 (get_local 0)) (f32.const 0))))
   )
   (func (export "nan-canonicalization-f32-func-call-indirect") (param i32) (result i32)
-    (i32.reinterpret_f32 (call_indirect (type $f32-id) (f32.reinterpret_i32 (get_local 0)) (i32.const 1)))
+    (i32.reinterpret_f32 (call_indirect (type $f32-id) (f32.reinterpret_i32 (get_local 0)) (i32.const 0)))
   )
   (func (export "nan-canonicalization-f32-func-call-indirect-cncl") (param i32) (result i32)
-    (i32.reinterpret_f32 (call_indirect (type $f32-id) (f32.add (f32.reinterpret_i32 (get_local 0)) (f32.const 0)) (i32.const 1)))
+    (i32.reinterpret_f32 (call_indirect (type $f32-id) (f32.add (f32.reinterpret_i32 (get_local 0)) (f32.const 0)) (i32.const 0)))
   )
 
   (func (export "nan-canonicalization-f64-add") (param i64) (result i64)
@@ -146,10 +146,10 @@
     (i64.reinterpret_f64 (call $nan-canonicalization-f64-func-call-target (f64.add (f64.reinterpret_i64 (get_local 0)) (f64.const 0))))
   )
   (func (export "nan-canonicalization-f64-func-call-indirect") (param i64) (result i64)
-    (i64.reinterpret_f64 (call_indirect (type $f64-id) (f64.reinterpret_i64 (get_local 0)) (i32.const 2)))
+    (i64.reinterpret_f64 (call_indirect (type $f64-id) (f64.reinterpret_i64 (get_local 0)) (i32.const 1)))
   )
   (func (export "nan-canonicalization-f64-func-call-indirect-cncl") (param i64) (result i64)
-    (i64.reinterpret_f64 (call_indirect (type $f64-id) (f64.add (f64.reinterpret_i64 (get_local 0)) (f64.const 0)) (i32.const 2)))
+    (i64.reinterpret_f64 (call_indirect (type $f64-id) (f64.add (f64.reinterpret_i64 (get_local 0)) (f64.const 0)) (i32.const 1)))
   )
 )
 
@@ -191,4 +191,4 @@
 (assert_return (invoke "nan-canonicalization-f64-func-call" (i64.const 0x7ff8000000000001)) (i64.const 0x7ff8000000000001))
 (assert_return (invoke "nan-canonicalization-f64-func-call-cncl" (i64.const 0x7ff8000000000001)) (i64.const 0x7ff8000000000000))
 (assert_return (invoke "nan-canonicalization-f64-func-call-indirect" (i64.const 0x7ff8000000000001)) (i64.const 0x7ff8000000000001))
-(assert_return (invoke "nan-canonicalization-f64-func-call-indirect-cncl" (i64.const 0x7ff8000000000001)) (i64.const 0x7ff8000000000000))
+(assert_return (invoke "nan-canonicalization-f64-func-call-indirect-cncl" (i64.const 0x7ff8000000000001)) (i64.const 0x7ff8000000000000))

tests/custom/stack-overflow.wast

@@ -7,4 +7,4 @@
       (export "stack-overflow" (func 0))
       (elem (;0;) (i32.const 0) 0))
 
-(assert_exhaustion (invoke "stack-overflow"))
+(assert_exhaustion (invoke "stack-overflow") "stack overflow")

tests/ignores.txt

@@ -100,8 +100,7 @@ singlepass::spec::int_exprs on aarch64
 singlepass::spec::traps on aarch64
 
 # NaN canonicalization is not yet implemented for aarch64.
-singlepass::spec::wasmer on aarch64
+singlepass::spec::custom::nan_canonicalization on aarch64
-
 
 # Emscripten