From 51e085a7bad41dc6e279d986f1d0da22e1655944 Mon Sep 17 00:00:00 2001 From: losfair Date: Tue, 12 May 2020 02:11:43 +0800 Subject: [PATCH 1/6] Garbage in upper 32 bits shouldn't propagate to I64ExtendI32U's result. --- lib/singlepass-backend/src/codegen_x64.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/singlepass-backend/src/codegen_x64.rs b/lib/singlepass-backend/src/codegen_x64.rs index 9dfc648cc..de7248db3 100644 --- a/lib/singlepass-backend/src/codegen_x64.rs +++ b/lib/singlepass-backend/src/codegen_x64.rs @@ -4043,6 +4043,7 @@ impl FunctionCodeGenerator for X64FunctionCode { false, )[0]; self.value_stack.push(ret); + Self::emit_relaxed_binop( a, &mut self.machine, @@ -4051,6 +4052,16 @@ impl FunctionCodeGenerator for X64FunctionCode { loc, ret, ); + 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 = From 6790702901fcd6ab829b245f0bb08826ebeb0c72 Mon Sep 17 00:00:00 2001 From: losfair Date: Tue, 12 May 2020 03:03:39 +0800 Subject: [PATCH 2/6] Add comment to explain the change to I64ExtendI32U. --- lib/singlepass-backend/src/codegen_x64.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/singlepass-backend/src/codegen_x64.rs b/lib/singlepass-backend/src/codegen_x64.rs index de7248db3..c40ada2f1 100644 --- a/lib/singlepass-backend/src/codegen_x64.rs +++ b/lib/singlepass-backend/src/codegen_x64.rs @@ -4052,6 +4052,9 @@ impl FunctionCodeGenerator 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, From eeb608ced7c8c9eb85249f36eab681f1f107d75f Mon Sep 17 00:00:00 2001 From: losfair Date: Tue, 12 May 2020 03:04:26 +0800 Subject: [PATCH 3/6] Add int-extend-garbage test --- tests/custom/int-extend-garbage.wast | 35 ++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 tests/custom/int-extend-garbage.wast diff --git a/tests/custom/int-extend-garbage.wast b/tests/custom/int-extend-garbage.wast new file mode 100644 index 000000000..9b290d26f --- /dev/null +++ b/tests/custom/int-extend-garbage.wast @@ -0,0 +1,35 @@ +;; https://github.com/wasmerio/wasmer/pull/1436 +;; +;; 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)) From fd63ee91f06b5b2d824c7f896a05ff8c57cc60e2 Mon Sep 17 00:00:00 2001 From: Heyang Zhou Date: Tue, 12 May 2020 03:23:50 +0800 Subject: [PATCH 4/6] Apply review suggestion Co-authored-by: nlewycky --- tests/custom/int-extend-garbage.wast | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/custom/int-extend-garbage.wast b/tests/custom/int-extend-garbage.wast index 9b290d26f..eb91279b5 100644 --- a/tests/custom/int-extend-garbage.wast +++ b/tests/custom/int-extend-garbage.wast @@ -1,4 +1,4 @@ -;; https://github.com/wasmerio/wasmer/pull/1436 +;; 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. From 685af7b9fe88a4e95756a42f53081305c77f19ac Mon Sep 17 00:00:00 2001 From: Syrus Date: Mon, 11 May 2020 12:35:13 -0700 Subject: [PATCH 5/6] Added custom tests to the infrastructure --- build.rs | 6 +++++- tests/custom/nan-canonicalization.wast | 10 +++++----- tests/custom/stack-overflow.wast | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/build.rs b/build.rs index e3d7ecace..c1b84739b 100644 --- a/build.rs +++ b/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(()) diff --git a/tests/custom/nan-canonicalization.wast b/tests/custom/nan-canonicalization.wast index 2e80ab39a..2df42add9 100644 --- a/tests/custom/nan-canonicalization.wast +++ b/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)) \ No newline at end of file diff --git a/tests/custom/stack-overflow.wast b/tests/custom/stack-overflow.wast index 5ac245b88..5b9c78f5f 100644 --- a/tests/custom/stack-overflow.wast +++ b/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") From 254f9bbddd647b3b44fddc79f14a96331e6fa100 Mon Sep 17 00:00:00 2001 From: Syrus Date: Mon, 11 May 2020 13:06:55 -0700 Subject: [PATCH 6/6] Fixed ignores --- tests/ignores.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/ignores.txt b/tests/ignores.txt index a2bb620bd..f10ac180d 100644 --- a/tests/ignores.txt +++ b/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