Cranelift: Add fixed register constraints to indirect calls with `tail` calling convention (#6632)

This is a workaround to fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=60035 in the meantime, until https://github.com/bytecodealliance/regalloc2/issues/145 is fixed. Co-authored-by: Jamey Sharp <jsharp@fastly.com> Co-authored-by: Trevor Elliott <telliott@fastly.com>
1 year ago · f08faf5999
11 changed files with 353 additions and 23 deletions
--- 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);
            }
--- 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);
            }
--- 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
    }
--- 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,
        )
    }

--- a/cranelift/codegen/src/isa/x64/inst/mod.rs
+++ b/cranelift/codegen/src/isa/x64/inst/mod.rs
@ -48,6 +48,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]
@ -522,6 +524,7 @@ impl Inst {
        clobbers: PRegSet,
        opcode: Opcode,
        callee_pop_size: u32,
+        callee_conv: CallConv,
    ) -> Inst {
        Inst::CallKnown {
            dest,
@ -531,6 +534,7 @@ impl Inst {
                clobbers,
                opcode,
                callee_pop_size,
+                callee_conv,
            }),
        }
    }
@ -542,6 +546,7 @@ impl Inst {
        clobbers: PRegSet,
        opcode: Opcode,
        callee_pop_size: u32,
+        callee_conv: CallConv,
    ) -> Inst {
        dest.assert_regclass_is(RegClass::Int);
        Inst::CallUnknown {
@ -552,6 +557,7 @@ impl Inst {
                clobbers,
                opcode,
                callee_pop_size,
+                callee_conv,
            }),
        }
    }
@ -2189,7 +2195,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);
            }
--- 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
+
--- 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
--- a/cranelift/filetests/filetests/isa/riscv64/fuzzbug-60035.clif
+++ b/cranelift/filetests/filetests/isa/riscv64/fuzzbug-60035.clif
@ -0,0 +1,141 @@
+test compile precise-output
+target riscv64
+
+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:
+;   add sp,-16
+;   sd ra,8(sp)
+;   sd fp,0(sp)
+;   mv fp,sp
+;   sd s1,-8(sp)
+;   sd s2,-16(sp)
+;   sd s3,-24(sp)
+;   sd s4,-32(sp)
+;   sd s5,-40(sp)
+;   sd s6,-48(sp)
+;   sd s7,-56(sp)
+;   sd s8,-64(sp)
+;   sd s9,-72(sp)
+;   sd s10,-80(sp)
+;   sd s11,-88(sp)
+;   fsd fs2,-96(sp)
+;   fsd fs3,-104(sp)
+;   fsd fs4,-112(sp)
+;   fsd fs5,-120(sp)
+;   fsd fs6,-128(sp)
+;   fsd fs7,-136(sp)
+;   fsd fs8,-144(sp)
+;   fsd fs9,-152(sp)
+;   fsd fs10,-160(sp)
+;   fsd fs11,-168(sp)
+;   add sp,-192
+; block0:
+;   load_sym t0,userextname0+0
+;   sd t0,0(nominal_sp)
+;   ld t0,0(nominal_sp)
+;   callind t0
+;   ld t0,0(nominal_sp)
+;   callind t0
+;   add sp,+192
+;   ld s1,-8(sp)
+;   ld s2,-16(sp)
+;   ld s3,-24(sp)
+;   ld s4,-32(sp)
+;   ld s5,-40(sp)
+;   ld s6,-48(sp)
+;   ld s7,-56(sp)
+;   ld s8,-64(sp)
+;   ld s9,-72(sp)
+;   ld s10,-80(sp)
+;   ld s11,-88(sp)
+;   fld fs2,-96(sp)
+;   fld fs3,-104(sp)
+;   fld fs4,-112(sp)
+;   fld fs5,-120(sp)
+;   fld fs6,-128(sp)
+;   fld fs7,-136(sp)
+;   fld fs8,-144(sp)
+;   fld fs9,-152(sp)
+;   fld fs10,-160(sp)
+;   fld fs11,-168(sp)
+;   ld ra,8(sp)
+;   ld fp,0(sp)
+;   add sp,+16
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   addi sp, sp, -0x10
+;   sd ra, 8(sp)
+;   sd s0, 0(sp)
+;   ori s0, sp, 0
+;   sd s1, -8(sp)
+;   sd s2, -0x10(sp)
+;   sd s3, -0x18(sp)
+;   sd s4, -0x20(sp)
+;   sd s5, -0x28(sp)
+;   sd s6, -0x30(sp)
+;   sd s7, -0x38(sp)
+;   sd s8, -0x40(sp)
+;   sd s9, -0x48(sp)
+;   sd s10, -0x50(sp)
+;   sd s11, -0x58(sp)
+;   fsd fs2, -0x60(sp)
+;   fsd fs3, -0x68(sp)
+;   fsd fs4, -0x70(sp)
+;   fsd fs5, -0x78(sp)
+;   fsd fs6, -0x80(sp)
+;   fsd fs7, -0x88(sp)
+;   fsd fs8, -0x90(sp)
+;   fsd fs9, -0x98(sp)
+;   fsd fs10, -0xa0(sp)
+;   fsd fs11, -0xa8(sp)
+;   addi sp, sp, -0xc0
+; block1: ; offset 0x68
+;   auipc t0, 0
+;   ld t0, 0xc(t0)
+;   j 0xc
+;   .byte 0x00, 0x00, 0x00, 0x00 ; reloc_external Abs8 u1:7 0
+;   .byte 0x00, 0x00, 0x00, 0x00
+;   sd t0, 0(sp)
+;   ld t0, 0(sp)
+;   jalr t0
+;   ld t0, 0(sp)
+;   jalr t0
+;   addi sp, sp, 0xc0
+;   ld s1, -8(sp)
+;   ld s2, -0x10(sp)
+;   ld s3, -0x18(sp)
+;   ld s4, -0x20(sp)
+;   ld s5, -0x28(sp)
+;   ld s6, -0x30(sp)
+;   ld s7, -0x38(sp)
+;   ld s8, -0x40(sp)
+;   ld s9, -0x48(sp)
+;   ld s10, -0x50(sp)
+;   ld s11, -0x58(sp)
+;   fld fs2, -0x60(sp)
+;   fld fs3, -0x68(sp)
+;   fld fs4, -0x70(sp)
+;   fld fs5, -0x78(sp)
+;   fld fs6, -0x80(sp)
+;   fld fs7, -0x88(sp)
+;   fld fs8, -0x90(sp)
+;   fld fs9, -0x98(sp)
+;   fld fs10, -0xa0(sp)
+;   fld fs11, -0xa8(sp)
+;   ld ra, 8(sp)
+;   ld s0, 0(sp)
+;   addi sp, sp, 0x10
+;   ret
+
--- a/cranelift/filetests/filetests/isa/riscv64/tail-call-conv.clif
+++ b/cranelift/filetests/filetests/isa/riscv64/tail-call-conv.clif
@ -335,8 +335,8 @@ block0:
 ;   add sp,-48
 ;   virtual_sp_offset_adj +48
 ;   load_addr s1,0(sp)
-;   load_sym a1,%tail_callee_stack_rets+0
-;   callind a1
+;   load_sym t0,%tail_callee_stack_rets+0
+;   callind t0
 ;   ld t0,0(sp)
 ;   ld t2,8(sp)
 ;   ld a1,16(sp)
@ -358,12 +358,12 @@ block0:
 ; block1: ; offset 0x10
 ;   addi sp, sp, -0x30
 ;   mv s1, sp
-;   auipc a1, 0
-;   ld a1, 0xc(a1)
+;   auipc t0, 0
+;   ld t0, 0xc(t0)
 ;   j 0xc
 ;   .byte 0x00, 0x00, 0x00, 0x00 ; reloc_external Abs8 %tail_callee_stack_rets 0
 ;   .byte 0x00, 0x00, 0x00, 0x00
-;   jalr a1
+;   jalr t0
 ;   ld t0, 0(sp)
 ;   ld t2, 8(sp)
 ;   ld a1, 0x10(sp)
--- a/cranelift/filetests/filetests/isa/x64/fuzzbug-60035.clif
+++ b/cranelift/filetests/filetests/isa/x64/fuzzbug-60035.clif
@ -0,0 +1,67 @@
+test compile precise-output
+target x86_64
+
+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:
+;   pushq   %rbp
+;   movq    %rsp, %rbp
+;   subq    %rsp, $64, %rsp
+;   movq    %rbx, 16(%rsp)
+;   movq    %r12, 24(%rsp)
+;   movq    %r13, 32(%rsp)
+;   movq    %r14, 40(%rsp)
+;   movq    %r15, 48(%rsp)
+; block0:
+;   load_ext_name userextname0+0, %r15
+;   movq    %r15, rsp(0 + virtual offset)
+;   movq    rsp(0 + virtual offset), %r15
+;   call    *%r15
+;   movq    rsp(0 + virtual offset), %r15
+;   call    *%r15
+;   movq    16(%rsp), %rbx
+;   movq    24(%rsp), %r12
+;   movq    32(%rsp), %r13
+;   movq    40(%rsp), %r14
+;   movq    48(%rsp), %r15
+;   addq    %rsp, $64, %rsp
+;   movq    %rbp, %rsp
+;   popq    %rbp
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   pushq %rbp
+;   movq %rsp, %rbp
+;   subq $0x40, %rsp
+;   movq %rbx, 0x10(%rsp)
+;   movq %r12, 0x18(%rsp)
+;   movq %r13, 0x20(%rsp)
+;   movq %r14, 0x28(%rsp)
+;   movq %r15, 0x30(%rsp)
+; block1: ; offset 0x21
+;   movabsq $0, %r15 ; reloc_external Abs8 u1:7 0
+;   movq %r15, (%rsp)
+;   movq (%rsp), %r15
+;   callq *%r15
+;   movq (%rsp), %r15
+;   callq *%r15
+;   movq 0x10(%rsp), %rbx
+;   movq 0x18(%rsp), %r12
+;   movq 0x20(%rsp), %r13
+;   movq 0x28(%rsp), %r14
+;   movq 0x30(%rsp), %r15
+;   addq $0x40, %rsp
+;   movq %rbp, %rsp
+;   popq %rbp
+;   retq
+
--- a/winch/codegen/src/isa/x64/asm.rs
+++ b/winch/codegen/src/isa/x64/asm.rs
@ -8,13 +8,16 @@ use cranelift_codegen::{
    entity::EntityRef,
    ir::TrapCode,
    ir::{ExternalName, Opcode, UserExternalNameRef},
-    isa::x64::{
-        args::{
-            self, AluRmiROpcode, Amode, CmpOpcode, DivSignedness, ExtMode, FromWritableReg, Gpr,
-            GprMem, GprMemImm, Imm8Gpr, Imm8Reg, RegMem, RegMemImm,
-            ShiftKind as CraneliftShiftKind, SyntheticAmode, WritableGpr, CC,
+    isa::{
+        x64::{
+            args::{
+                self, AluRmiROpcode, Amode, CmpOpcode, DivSignedness, ExtMode, FromWritableReg,
+                Gpr, GprMem, GprMemImm, Imm8Gpr, Imm8Reg, RegMem, RegMemImm,
+                ShiftKind as CraneliftShiftKind, SyntheticAmode, WritableGpr, CC,
+            },
+            settings as x64_settings, CallInfo, EmitInfo, EmitState, Inst,
        },
-        settings as x64_settings, CallInfo, EmitInfo, EmitState, Inst,
+        CallConv,
    },
    settings, Final, MachBuffer, MachBufferFinalized, MachInstEmit, MachInstEmitState, MachLabel,
    Writable,
@ -802,6 +805,7 @@ impl Assembler {
                        clobbers: Default::default(),
                        opcode: Opcode::Call,
                        callee_pop_size: 0,
+                        callee_conv: CallConv::SystemV,
                    }),
                });
            }
@ -815,6 +819,7 @@ impl Assembler {
                        clobbers: Default::default(),
                        opcode: Opcode::Call,
                        callee_pop_size: 0,
+                        callee_conv: CallConv::SystemV,
                    }),
                });
            }