Cranelift: Adjust virtual SP after tail call-conv callees return (b…

…ytecodealliance#6586) * Cranelift: Adjust virtual SP after `tail` call-conv callees return Callees that use the `tail` calling convention will pop stack arguments from the stack for their callers. They do not, however, adjust the caller's virtual SP, so that still needs to happen in our ABI and `CallSite` code. This is, however, slightly trickier than just emitting a nominal SP adjustment pseudo-instruction because we cannot let regalloc attempt to spill or reload values between the call and the SP adjustment because the stack offsets will be off by the size of the stack arguments to the call. Therefore, we add the number of bytes that the callee pops to the `CallInfo` structures and have emission update the virtual SP atomically with regards to the call itself. Fixes bytecodealliance#6581 Fixes bytecodealliance#6582 Co-Authored-By: Jamey Sharp <[email protected]> * Cranelift: Have `fuzzgen` generate functions with the `tail` calling convention --------- Co-authored-by: Jamey Sharp <[email protected]>
gurry · Jun 16, 2023 · 9a67597 · 9a67597
1 parent 8f61ec2
commit 9a67597
Show file tree

Hide file tree

Showing 18 changed files with 174 additions and 8 deletions.
diff --git a/cranelift/Cargo.toml b/cranelift/Cargo.toml
@@ -20,7 +20,7 @@ harness = false
 
 [dependencies]
 cfg-if = { workspace = true }
-cranelift-codegen = { workspace = true, features = ["disas"] }
+cranelift-codegen = { workspace = true, features = ["disas", "trace-log"] }
 cranelift-entity = { workspace = true }
 cranelift-interpreter = { workspace = true }
 cranelift-reader = { workspace = true }

diff --git a/cranelift/codegen/src/isa/aarch64/abi.rs b/cranelift/codegen/src/isa/aarch64/abi.rs
@@ -990,6 +990,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
         tmp: Writable<Reg>,
         callee_conv: isa::CallConv,
         caller_conv: isa::CallConv,
+        callee_pop_size: u32,
     ) -> SmallVec<[Inst; 2]> {
         let mut insts = SmallVec::new();
         match &dest {
@@ -1002,6 +1003,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
                     opcode,
                     caller_callconv: caller_conv,
                     callee_callconv: callee_conv,
+                    callee_pop_size,
                 }),
             }),
             &CallDest::ExtName(ref name, RelocDistance::Far) => {
@@ -1019,6 +1021,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
                         opcode,
                         caller_callconv: caller_conv,
                         callee_callconv: callee_conv,
+                        callee_pop_size,
                     }),
                 });
             }
@@ -1031,6 +1034,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
                     opcode,
                     caller_callconv: caller_conv,
                     callee_callconv: callee_conv,
+                    callee_pop_size,
                 }),
             }),
         }
@@ -1073,6 +1077,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
                 opcode: Opcode::Call,
                 caller_callconv: call_conv,
                 callee_callconv: call_conv,
+                callee_pop_size: 0,
             }),
         });
         insts

diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit.rs b/cranelift/codegen/src/isa/aarch64/inst/emit.rs
@@ -3165,6 +3165,13 @@ impl MachInstEmit for Inst {
                 if info.opcode.is_call() {
                     sink.add_call_site(info.opcode);
                 }
+
+                let callee_pop_size = i64::from(info.callee_pop_size);
+                state.virtual_sp_offset -= callee_pop_size;
+                trace!(
+                    "call adjusts virtual sp offset by {callee_pop_size} -> {}",
+                    state.virtual_sp_offset
+                );
             }
             &Inst::CallInd { ref info } => {
                 if let Some(s) = state.take_stack_map() {
@@ -3175,6 +3182,13 @@ impl MachInstEmit for Inst {
                 if info.opcode.is_call() {
                     sink.add_call_site(info.opcode);
                 }
+
+                let callee_pop_size = i64::from(info.callee_pop_size);
+                state.virtual_sp_offset -= callee_pop_size;
+                trace!(
+                    "call adjusts virtual sp offset by {callee_pop_size} -> {}",
+                    state.virtual_sp_offset
+                );
             }
             &Inst::CondBr {
                 taken,
@@ -3587,6 +3601,7 @@ impl MachInstEmit for Inst {
                         opcode: Opcode::CallIndirect,
                         caller_callconv: CallConv::AppleAarch64,
                         callee_callconv: CallConv::AppleAarch64,
+                        callee_pop_size: 0,
                     }),
                 }
                 .emit(&[], sink, emit_info, state);

diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs b/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs
@@ -6084,6 +6084,7 @@ fn test_aarch64_binemit() {
                 opcode: Opcode::Call,
                 caller_callconv: CallConv::SystemV,
                 callee_callconv: CallConv::SystemV,
+                callee_pop_size: 0,
             }),
         },
         "00000094",
@@ -6100,6 +6101,7 @@ fn test_aarch64_binemit() {
                 opcode: Opcode::CallIndirect,
                 caller_callconv: CallConv::SystemV,
                 callee_callconv: CallConv::SystemV,
+                callee_pop_size: 0,
             }),
         },
         "40013FD6",

diff --git a/cranelift/codegen/src/isa/aarch64/inst/mod.rs b/cranelift/codegen/src/isa/aarch64/inst/mod.rs
@@ -92,6 +92,10 @@ pub struct CallInfo {
     pub caller_callconv: CallConv,
     /// Callee calling convention.
     pub callee_callconv: CallConv,
+    /// The number of bytes that the callee will pop from the stack for the
+    /// caller, if any. (Used for popping stack arguments with the `tail`
+    /// calling convention.)
+    pub callee_pop_size: u32,
 }
 
 /// Additional information for CallInd instructions, left out of line to lower the size of the Inst
@@ -112,6 +116,10 @@ pub struct CallIndInfo {
     pub caller_callconv: CallConv,
     /// Callee calling convention.
     pub callee_callconv: CallConv,
+    /// The number of bytes that the callee will pop from the stack for the
+    /// caller, if any. (Used for popping stack arguments with the `tail`
+    /// calling convention.)
+    pub callee_pop_size: u32,
 }
 
 /// Additional information for JTSequence instructions, left out of line to lower the size of the Inst

diff --git a/cranelift/codegen/src/isa/riscv64/abi.rs b/cranelift/codegen/src/isa/riscv64/abi.rs
@@ -421,6 +421,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
                 opcode: Opcode::Call,
                 callee_callconv: CallConv::SystemV,
                 caller_callconv: CallConv::SystemV,
+                callee_pop_size: 0,
             }),
         });
     }
@@ -527,6 +528,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
         tmp: Writable<Reg>,
         callee_conv: isa::CallConv,
         caller_conv: isa::CallConv,
+        callee_pop_size: u32,
     ) -> SmallVec<[Self::I; 2]> {
         let mut insts = SmallVec::new();
         match &dest {
@@ -539,6 +541,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
                     opcode,
                     caller_callconv: caller_conv,
                     callee_callconv: callee_conv,
+                    callee_pop_size,
                 }),
             }),
             &CallDest::ExtName(ref name, RelocDistance::Far) => {
@@ -556,6 +559,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
                         opcode,
                         caller_callconv: caller_conv,
                         callee_callconv: callee_conv,
+                        callee_pop_size,
                     }),
                 });
             }
@@ -568,6 +572,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
                     opcode,
                     caller_callconv: caller_conv,
                     callee_callconv: callee_conv,
+                    callee_pop_size,
                 }),
             }),
         }
@@ -609,6 +614,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
                 opcode: Opcode::Call,
                 caller_callconv: call_conv,
                 callee_callconv: call_conv,
+                callee_pop_size: 0,
             }),
         });
         insts

diff --git a/cranelift/codegen/src/isa/riscv64/inst/emit.rs b/cranelift/codegen/src/isa/riscv64/inst/emit.rs
@@ -6,6 +6,7 @@ use crate::ir::TrapCode;
 use crate::isa::riscv64::inst::*;
 use crate::isa::riscv64::inst::{zero_reg, AluOPRRR};
 use crate::machinst::{AllocationConsumer, Reg, Writable};
+use crate::trace;
 use cranelift_control::ControlPlane;
 use regalloc2::Allocation;
 
@@ -847,6 +848,13 @@ impl MachInstEmit for Inst {
                         .emit(&[], sink, emit_info, state);
                     }
                 }
+
+                let callee_pop_size = i64::from(info.callee_pop_size);
+                state.virtual_sp_offset -= callee_pop_size;
+                trace!(
+                    "call adjusts virtual sp offset by {callee_pop_size} -> {}",
+                    state.virtual_sp_offset
+                );
             }
             &Inst::CallInd { ref info } => {
                 let rn = allocs.next(info.rn);
@@ -863,6 +871,13 @@ impl MachInstEmit for Inst {
                     offset: Imm12::zero(),
                 }
                 .emit(&[], sink, emit_info, state);
+
+                let callee_pop_size = i64::from(info.callee_pop_size);
+                state.virtual_sp_offset -= callee_pop_size;
+                trace!(
+                    "call adjusts virtual sp offset by {callee_pop_size} -> {}",
+                    state.virtual_sp_offset
+                );
             }
 
             &Inst::Jal { dest } => {

diff --git a/cranelift/codegen/src/isa/riscv64/inst/mod.rs b/cranelift/codegen/src/isa/riscv64/inst/mod.rs
@@ -74,6 +74,7 @@ pub struct CallInfo {
     pub caller_callconv: CallConv,
     pub callee_callconv: CallConv,
     pub clobbers: PRegSet,
+    pub callee_pop_size: u32,
 }
 
 /// Additional information for CallInd instructions, left out of line to lower the size of the Inst
@@ -87,6 +88,7 @@ pub struct CallIndInfo {
     pub caller_callconv: CallConv,
     pub callee_callconv: CallConv,
     pub clobbers: PRegSet,
+    pub callee_pop_size: u32,
 }
 
 /// A branch target. Either unresolved (basic-block index) or resolved (offset

diff --git a/cranelift/codegen/src/isa/s390x/abi.rs b/cranelift/codegen/src/isa/s390x/abi.rs
@@ -759,6 +759,7 @@ impl ABIMachineSpec for S390xMachineDeps {
         _tmp: Writable<Reg>,
         _callee_conv: isa::CallConv,
         _caller_conv: isa::CallConv,
+        _callee_pop_size: u32,
     ) -> SmallVec<[Inst; 2]> {
         unreachable!();
     }

diff --git a/cranelift/codegen/src/isa/x64/abi.rs b/cranelift/codegen/src/isa/x64/abi.rs
@@ -435,6 +435,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
                 defs: smallvec![],
                 clobbers: PRegSet::empty(),
                 opcode: Opcode::Call,
+                callee_pop_size: 0,
             }),
         });
     }
@@ -594,11 +595,19 @@ impl ABIMachineSpec for X64ABIMachineSpec {
         tmp: Writable<Reg>,
         _callee_conv: isa::CallConv,
         _caller_conv: isa::CallConv,
+        callee_pop_size: u32,
     ) -> SmallVec<[Self::I; 2]> {
         let mut insts = SmallVec::new();
         match dest {
             &CallDest::ExtName(ref name, RelocDistance::Near) => {
-                insts.push(Inst::call_known(name.clone(), uses, defs, clobbers, opcode));
+                insts.push(Inst::call_known(
+                    name.clone(),
+                    uses,
+                    defs,
+                    clobbers,
+                    opcode,
+                    callee_pop_size,
+                ));
             }
             &CallDest::ExtName(ref name, RelocDistance::Far) => {
                 insts.push(Inst::LoadExtName {
@@ -613,6 +622,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
                     defs,
                     clobbers,
                     opcode,
+                    callee_pop_size,
                 ));
             }
             &CallDest::Reg(reg) => {
@@ -622,6 +632,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
                     defs,
                     clobbers,
                     opcode,
+                    callee_pop_size,
                 ));
             }
         }
@@ -651,6 +662,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
             offset: 0,
             distance: RelocDistance::Far,
         });
+        let callee_pop_size = 0;
         insts.push(Inst::call_unknown(
             RegMem::reg(temp2.to_reg()),
             /* uses = */
@@ -671,6 +683,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
             /* defs = */ smallvec![],
             /* clobbers = */ Self::get_regs_clobbered_by_call(call_conv),
             Opcode::Call,
+            callee_pop_size,
         ));
         insts
     }

diff --git a/cranelift/codegen/src/isa/x64/inst/emit.rs b/cranelift/codegen/src/isa/x64/inst/emit.rs
@@ -1579,6 +1579,13 @@ pub(crate) fn emit(
             if call_info.opcode.is_call() {
                 sink.add_call_site(call_info.opcode);
             }
+
+            let callee_pop_size = i64::from(call_info.callee_pop_size);
+            state.virtual_sp_offset -= callee_pop_size;
+            trace!(
+                "call adjusts virtual sp offset by {callee_pop_size} -> {}",
+                state.virtual_sp_offset
+            );
         }
 
         Inst::CallUnknown {
@@ -1623,6 +1630,13 @@ pub(crate) fn emit(
             if call_info.opcode.is_call() {
                 sink.add_call_site(call_info.opcode);
             }
+
+            let callee_pop_size = i64::from(call_info.callee_pop_size);
+            state.virtual_sp_offset -= callee_pop_size;
+            trace!(
+                "call adjusts virtual sp offset by {callee_pop_size} -> {}",
+                state.virtual_sp_offset
+            );
         }
 
         Inst::Args { .. } => {}

diff --git a/cranelift/codegen/src/isa/x64/inst/emit_tests.rs b/cranelift/codegen/src/isa/x64/inst/emit_tests.rs
@@ -4335,6 +4335,7 @@ fn test_x64_emit() {
             smallvec![],
             PRegSet::default(),
             Opcode::Call,
+            0,
         ),
         "E800000000",
         "call    User(userextname0)",
@@ -4349,6 +4350,7 @@ fn test_x64_emit() {
             smallvec![],
             PRegSet::default(),
             Opcode::CallIndirect,
+            0,
         )
     }
 

diff --git a/cranelift/codegen/src/isa/x64/inst/mod.rs b/cranelift/codegen/src/isa/x64/inst/mod.rs
@@ -42,6 +42,10 @@ pub struct CallInfo {
     pub clobbers: PRegSet,
     /// The opcode of this call.
     pub opcode: Opcode,
+    /// The number of bytes that the callee will pop from the stack for the
+    /// caller, if any. (Used for popping stack arguments with the `tail`
+    /// calling convention.)
+    pub callee_pop_size: u32,
 }
 
 #[test]
@@ -515,6 +519,7 @@ impl Inst {
         defs: CallRetList,
         clobbers: PRegSet,
         opcode: Opcode,
+        callee_pop_size: u32,
     ) -> Inst {
         Inst::CallKnown {
             dest,
@@ -523,6 +528,7 @@ impl Inst {
                 defs,
                 clobbers,
                 opcode,
+                callee_pop_size,
             }),
         }
     }
@@ -533,6 +539,7 @@ impl Inst {
         defs: CallRetList,
         clobbers: PRegSet,
         opcode: Opcode,
+        callee_pop_size: u32,
     ) -> Inst {
         dest.assert_regclass_is(RegClass::Int);
         Inst::CallUnknown {
@@ -542,6 +549,7 @@ impl Inst {
                 defs,
                 clobbers,
                 opcode,
+                callee_pop_size,
             }),
         }
     }