From b2ecf8a06c452f8b9a11cee7155d3880a949e683 Mon Sep 17 00:00:00 2001 From: losfair Date: Thu, 30 Apr 2020 00:04:44 +0800 Subject: [PATCH 1/3] Use RCX instead of R10 as temp register in sysv call location. --- lib/singlepass-backend/src/codegen_x64.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/singlepass-backend/src/codegen_x64.rs b/lib/singlepass-backend/src/codegen_x64.rs index bc666d1ff..1eb549b4c 100644 --- a/lib/singlepass-backend/src/codegen_x64.rs +++ b/lib/singlepass-backend/src/codegen_x64.rs @@ -1942,7 +1942,10 @@ impl X64FunctionCode { /// Emits a System V call sequence. /// - /// This function must not use RAX before `cb` is called. + /// This function will not use RAX before `cb` is called. + /// + /// The caller MUST NOT hold any temporary registers allocated by `acquire_temp_gpr` when calling + /// this function. fn emit_call_sysv, F: FnOnce(&mut Assembler)>( a: &mut Assembler, m: &mut Machine, @@ -2063,14 +2066,17 @@ impl X64FunctionCode { // Dummy value slot to be filled with `mov`. a.emit_push(Size::S64, Location::GPR(GPR::RAX)); - // Use R10 as the temporary register here, since it is callee-saved and not - // used by the callback `cb`. - a.emit_mov(Size::S64, *param, Location::GPR(GPR::R10)); + // Use RCX as the temporary register here, since: + // - It is a temporary register that is not used for any persistent value. + // - This register as an argument location is only written to after `sort_call_movs`.' + m.reserve_unused_temp_gpr(GPR::RCX); + a.emit_mov(Size::S64, *param, Location::GPR(GPR::RCX)); a.emit_mov( Size::S64, - Location::GPR(GPR::R10), + Location::GPR(GPR::RCX), Location::Memory(GPR::RSP, 0), ); + m.release_temp_gpr(GPR::RCX); } Location::XMM(_) => { // Dummy value slot to be filled with `mov`. From e234096e6acb3662fdedcdf6651fd5e9e7609ab2 Mon Sep 17 00:00:00 2001 From: losfair Date: Thu, 30 Apr 2020 00:05:45 +0800 Subject: [PATCH 2/3] Add test for issue #1409. --- tests/custom/singlepass-r10-overwrite.wast | 53 ++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 tests/custom/singlepass-r10-overwrite.wast diff --git a/tests/custom/singlepass-r10-overwrite.wast b/tests/custom/singlepass-r10-overwrite.wast new file mode 100644 index 000000000..7e1f2a1ca --- /dev/null +++ b/tests/custom/singlepass-r10-overwrite.wast @@ -0,0 +1,53 @@ +;; A bug was introduced in the commit below, where the `R10` register is incorrectly overwritten. +;; This test case covers this specific case. +;; +;; https://github.com/wasmerio/wasmer/commit/ed826cb389b17273002e729160bf076213b7e2f2#diff-8c30560d501545a19acafa7ebb21ebfeR1784 +;; + +(module + (func $call_target (param i64) (param i64) (param i64) (param i64) (param i64) (param i64) (result i64) + (local.get 0) + ) + + (func (export "test") (result i64) + ;; Use `i64.add`s to actually push values onto the runtime stack. + + ;; rsi + (i64.const 1) + (i64.const 1) + (i64.add) + + ;; rdi + (i64.const 1) + (i64.const 1) + (i64.add) + + ;; r8 + (i64.const 1) + (i64.const 1) + (i64.add) + + ;; r9 + (i64.const 1) + (i64.const 1) + (i64.add) + + ;; r10 (!) + (i64.const 1) + (i64.const 1) + (i64.add) + + ;; Imm64's as arguments + (i64.const 1) + (i64.const 1) + (i64.const 1) + (i64.const 1) + (i64.const 1) ;; allocated to the first memory slot + + ;; call + (call $call_target) + (return) + ) +) + +(assert_return (invoke "test") (i64.const 2)) From 7bfa7723a68211c73e7a70def44050f31e95e8d1 Mon Sep 17 00:00:00 2001 From: Heyang Zhou Date: Thu, 30 Apr 2020 00:26:30 +0800 Subject: [PATCH 3/3] Fix typo in comment. Co-Authored-By: nlewycky --- lib/singlepass-backend/src/codegen_x64.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/singlepass-backend/src/codegen_x64.rs b/lib/singlepass-backend/src/codegen_x64.rs index 1eb549b4c..9dfc648cc 100644 --- a/lib/singlepass-backend/src/codegen_x64.rs +++ b/lib/singlepass-backend/src/codegen_x64.rs @@ -2068,7 +2068,7 @@ impl X64FunctionCode { // Use RCX as the temporary register here, since: // - It is a temporary register that is not used for any persistent value. - // - This register as an argument location is only written to after `sort_call_movs`.' + // - This register as an argument location is only written to after `sort_call_movs`. m.reserve_unused_temp_gpr(GPR::RCX); a.emit_mov(Size::S64, *param, Location::GPR(GPR::RCX)); a.emit_mov(