Cranelift: Add fixed register constraints to indirect calls with `tai…

…l` calling convention This is a workaround to fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=60035 in the meantime, until bytecodealliance/regalloc2#145 is fixed. Co-Authored-By: Jamey Sharp <[email protected]> Co-Authored-By: Trevor Elliott <[email protected]>
fitzgen · Jun 22, 2023 · a742cd4 · a742cd4
1 parent 0c98078
commit a742cd4
Show file tree

Hide file tree

Showing 11 changed files with 353 additions and 23 deletions.
diff --git a/cranelift/codegen/src/isa/aarch64/inst/mod.rs b/cranelift/codegen/src/isa/aarch64/inst/mod.rs
@@ -858,7 +858,13 @@ fn aarch64_get_operands<F: Fn(VReg) -> VReg>(inst: &Inst, collector: &mut Operan
             collector.reg_clobbers(info.clobbers);
         }
         &Inst::CallInd { ref info, .. } => {
-            collector.reg_use(info.rn);
+            if info.callee_callconv == CallConv::Tail {
+                // TODO(https://github.com/bytecodealliance/regalloc2/issues/145):
+                // This shouldn't be a fixed register constraint.
+                collector.reg_fixed_use(info.rn, xreg(1));
+            } else {
+                collector.reg_use(info.rn);
+            }
             for u in &info.uses {
                 collector.reg_fixed_use(u.vreg, u.preg);
             }

diff --git a/cranelift/codegen/src/isa/riscv64/inst/mod.rs b/cranelift/codegen/src/isa/riscv64/inst/mod.rs
@@ -432,7 +432,14 @@ fn riscv64_get_operands<F: Fn(VReg) -> VReg>(inst: &Inst, collector: &mut Operan
             collector.reg_clobbers(info.clobbers);
         }
         &Inst::CallInd { ref info } => {
-            collector.reg_use(info.rn);
+            if info.callee_callconv == CallConv::Tail {
+                // TODO(https://github.com/bytecodealliance/regalloc2/issues/145):
+                // This shouldn't be a fixed register constraint.
+                collector.reg_fixed_use(info.rn, x_reg(5));
+            } else {
+                collector.reg_use(info.rn);
+            }
+
             for u in &info.uses {
                 collector.reg_fixed_use(u.vreg, u.preg);
             }

diff --git a/cranelift/codegen/src/isa/x64/abi.rs b/cranelift/codegen/src/isa/x64/abi.rs
@@ -489,6 +489,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
                 clobbers: PRegSet::empty(),
                 opcode: Opcode::Call,
                 callee_pop_size: 0,
+                callee_conv: CallConv::Probestack,
             }),
         });
     }
@@ -650,7 +651,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
         clobbers: PRegSet,
         opcode: ir::Opcode,
         tmp: Writable<Reg>,
-        _callee_conv: isa::CallConv,
+        callee_conv: isa::CallConv,
         _caller_conv: isa::CallConv,
         callee_pop_size: u32,
     ) -> SmallVec<[Self::I; 2]> {
@@ -664,6 +665,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
                     clobbers,
                     opcode,
                     callee_pop_size,
+                    callee_conv,
                 ));
             }
             &CallDest::ExtName(ref name, RelocDistance::Far) => {
@@ -680,6 +682,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
                     clobbers,
                     opcode,
                     callee_pop_size,
+                    callee_conv,
                 ));
             }
             &CallDest::Reg(reg) => {
@@ -690,6 +693,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
                     clobbers,
                     opcode,
                     callee_pop_size,
+                    callee_conv,
                 ));
             }
         }
@@ -741,6 +745,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
             /* clobbers = */ Self::get_regs_clobbered_by_call(call_conv),
             Opcode::Call,
             callee_pop_size,
+            call_conv,
         ));
         insts
     }

diff --git a/cranelift/codegen/src/isa/x64/inst/emit_tests.rs b/cranelift/codegen/src/isa/x64/inst/emit_tests.rs
@@ -4336,6 +4336,7 @@ fn test_x64_emit() {
             PRegSet::default(),
             Opcode::Call,
             0,
+            CallConv::SystemV,
         ),
         "E800000000",
         "call    User(userextname0)",
@@ -4351,6 +4352,7 @@ fn test_x64_emit() {
             PRegSet::default(),
             Opcode::CallIndirect,
             0,
+            CallConv::SystemV,
         )
     }
 

diff --git a/cranelift/codegen/src/isa/x64/inst/mod.rs b/cranelift/codegen/src/isa/x64/inst/mod.rs
@@ -46,6 +46,8 @@ pub struct CallInfo {
     /// caller, if any. (Used for popping stack arguments with the `tail`
     /// calling convention.)
     pub callee_pop_size: u32,
+    /// The calling convention of the callee.
+    pub callee_conv: CallConv,
 }
 
 #[test]
@@ -520,6 +522,7 @@ impl Inst {
         clobbers: PRegSet,
         opcode: Opcode,
         callee_pop_size: u32,
+        callee_conv: CallConv,
     ) -> Inst {
         Inst::CallKnown {
             dest,
@@ -529,6 +532,7 @@ impl Inst {
                 clobbers,
                 opcode,
                 callee_pop_size,
+                callee_conv,
             }),
         }
     }
@@ -540,6 +544,7 @@ impl Inst {
         clobbers: PRegSet,
         opcode: Opcode,
         callee_pop_size: u32,
+        callee_conv: CallConv,
     ) -> Inst {
         dest.assert_regclass_is(RegClass::Int);
         Inst::CallUnknown {
@@ -550,6 +555,7 @@ impl Inst {
                 clobbers,
                 opcode,
                 callee_pop_size,
+                callee_conv,
             }),
         }
     }
@@ -2187,7 +2193,14 @@ fn x64_get_operands<F: Fn(VReg) -> VReg>(inst: &Inst, collector: &mut OperandCol
         }
 
         Inst::CallUnknown { ref info, dest, .. } => {
-            dest.get_operands(collector);
+            match dest {
+                RegMem::Reg { reg } if info.callee_conv == CallConv::Tail => {
+                    // TODO(https://github.com/bytecodealliance/regalloc2/issues/145):
+                    // This shouldn't be a fixed register constraint.
+                    collector.reg_fixed_use(*reg, regs::r15())
+                }
+                _ => dest.get_operands(collector),
+            }
             for u in &info.uses {
                 collector.reg_fixed_use(u.vreg, u.preg);
             }

diff --git a/cranelift/filetests/filetests/isa/aarch64/fuzzbug-60035.clif b/cranelift/filetests/filetests/isa/aarch64/fuzzbug-60035.clif
@@ -0,0 +1,84 @@
+test compile precise-output
+target aarch64
+
+function u1:6() system_v {
+    sig0 = () tail
+    fn0 = u1:7 sig0
+
+block0:
+    v5 = func_addr.i64 fn0
+    call_indirect sig0, v5()
+    call_indirect sig0, v5()
+    return
+}
+
+; VCode:
+;   stp fp, lr, [sp, #-16]!
+;   mov fp, sp
+;   stp x27, x28, [sp, #-16]!
+;   stp x25, x26, [sp, #-16]!
+;   stp x23, x24, [sp, #-16]!
+;   stp x21, x22, [sp, #-16]!
+;   stp x19, x20, [sp, #-16]!
+;   stp d14, d15, [sp, #-16]!
+;   stp d12, d13, [sp, #-16]!
+;   stp d10, d11, [sp, #-16]!
+;   stp d8, d9, [sp, #-16]!
+;   sub sp, sp, #16
+; block0:
+;   load_ext_name x1, User(userextname0)+0
+;   str x1, [sp]
+;   ldr x1, [sp]
+;   blr x1
+;   ldr x1, [sp]
+;   blr x1
+;   add sp, sp, #16
+;   ldp d8, d9, [sp], #16
+;   ldp d10, d11, [sp], #16
+;   ldp d12, d13, [sp], #16
+;   ldp d14, d15, [sp], #16
+;   ldp x19, x20, [sp], #16
+;   ldp x21, x22, [sp], #16
+;   ldp x23, x24, [sp], #16
+;   ldp x25, x26, [sp], #16
+;   ldp x27, x28, [sp], #16
+;   ldp fp, lr, [sp], #16
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   stp x29, x30, [sp, #-0x10]!
+;   mov x29, sp
+;   stp x27, x28, [sp, #-0x10]!
+;   stp x25, x26, [sp, #-0x10]!
+;   stp x23, x24, [sp, #-0x10]!
+;   stp x21, x22, [sp, #-0x10]!
+;   stp x19, x20, [sp, #-0x10]!
+;   stp d14, d15, [sp, #-0x10]!
+;   stp d12, d13, [sp, #-0x10]!
+;   stp d10, d11, [sp, #-0x10]!
+;   stp d8, d9, [sp, #-0x10]!
+;   sub sp, sp, #0x10
+; block1: ; offset 0x30
+;   ldr x1, #0x38
+;   b #0x40
+;   .byte 0x00, 0x00, 0x00, 0x00 ; reloc_external Abs8 u1:7 0
+;   .byte 0x00, 0x00, 0x00, 0x00
+;   stur x1, [sp]
+;   ldur x1, [sp]
+;   blr x1
+;   ldur x1, [sp]
+;   blr x1
+;   add sp, sp, #0x10
+;   ldp d8, d9, [sp], #0x10
+;   ldp d10, d11, [sp], #0x10
+;   ldp d12, d13, [sp], #0x10
+;   ldp d14, d15, [sp], #0x10
+;   ldp x19, x20, [sp], #0x10
+;   ldp x21, x22, [sp], #0x10
+;   ldp x23, x24, [sp], #0x10
+;   ldp x25, x26, [sp], #0x10
+;   ldp x27, x28, [sp], #0x10
+;   ldp x29, x30, [sp], #0x10
+;   ret
+
diff --git a/cranelift/filetests/filetests/isa/aarch64/tail-call-conv.clif b/cranelift/filetests/filetests/isa/aarch64/tail-call-conv.clif
@@ -96,8 +96,8 @@ block0:
 ;   virtual_sp_offset_adjust 16
 ;   str x0, [sp]
 ;   str x1, [sp, #8]
-;   load_ext_name x0, TestCase(%tail_callee_stack_args)+0
-;   blr x0
+;   load_ext_name x1, TestCase(%tail_callee_stack_args)+0
+;   blr x1
 ;   ldp fp, lr, [sp], #16
 ;   ret
 ;
@@ -135,11 +135,11 @@ block0:
 ;   sub sp, sp, #0x10
 ;   stur x0, [sp]
 ;   stur x1, [sp, #8]
-;   ldr x0, #0x84
+;   ldr x1, #0x84
 ;   b #0x8c
 ;   .byte 0x00, 0x00, 0x00, 0x00 ; reloc_external Abs8 %tail_callee_stack_args 0
 ;   .byte 0x00, 0x00, 0x00, 0x00
-;   blr x0
+;   blr x1
 ;   ldp x29, x30, [sp], #0x10
 ;   ret
 
@@ -270,8 +270,8 @@ block0:
 ;   sub sp, sp, #16
 ;   virtual_sp_offset_adjust 16
 ;   mov x0, sp
-;   load_ext_name x14, TestCase(%tail_callee_stack_rets)+0
-;   blr x14
+;   load_ext_name x1, TestCase(%tail_callee_stack_rets)+0
+;   blr x1
 ;   ldr x13, [sp]
 ;   ldr x2, [sp, #8]
 ;   add sp, sp, #16
@@ -286,11 +286,11 @@ block0:
 ; block1: ; offset 0x8
 ;   sub sp, sp, #0x10
 ;   mov x0, sp
-;   ldr x14, #0x18
+;   ldr x1, #0x18
 ;   b #0x20
 ;   .byte 0x00, 0x00, 0x00, 0x00 ; reloc_external Abs8 %tail_callee_stack_rets 0
 ;   .byte 0x00, 0x00, 0x00, 0x00
-;   blr x14
+;   blr x1
 ;   ldur x13, [sp]
 ;   ldur x2, [sp, #8]
 ;   add sp, sp, #0x10