Merge wasmerio#1412

1412: fix(singlepass-backend) Fix argument overwriting in a corner case when calling functions. r=nlewycky a=losfair

Fixes wasmerio#1409.

Co-authored-by: losfair <[email protected]>
Co-authored-by: Heyang Zhou <[email protected]>

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<I: Iterator<Item = Location>, 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`.

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))