Skip to content

Commit 2e004e1

Browse files
committed
guaranteed tail calls: support indirect arguments
1 parent 9b81629 commit 2e004e1

5 files changed

Lines changed: 264 additions & 70 deletions

File tree

compiler/rustc_codegen_ssa/src/mir/block.rs

Lines changed: 103 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,19 +1140,50 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
11401140
(args, None)
11411141
};
11421142

1143+
// Special logic for tail calls with `PassMode::Indirect { on_stack: false, .. }` arguments.
1144+
//
1145+
// Normally an indirect argument with `on_stack: false` would be passed as a pointer into
1146+
// the caller's stack frame. For tail calls, that would be unsound, because the caller's
1147+
// stack frame is overwritten by the callee's stack frame.
1148+
//
1149+
// Therefore we store the argument for the callee in the corresponding caller's slot.
1150+
// Because guaranteed tail calls demand that the caller's signature matches the callee's,
1151+
// the corresponding slot has the correct type.
1152+
//
1153+
// To handle cases like the one below, the tail call arguments must first be copied to a
1154+
// temporary, and only then copied to the caller's argument slots.
1155+
//
1156+
// ```
1157+
// // A struct big enough that it is not passed via registers.
1158+
// pub struct Big([u64; 4]);
1159+
//
1160+
// fn swapper(a: Big, b: Big) -> (Big, Big) {
1161+
// become swapper_helper(b, a);
1162+
// }
1163+
// ```
1164+
let mut tail_call_temporaries = vec![];
1165+
if kind == CallKind::Tail {
1166+
tail_call_temporaries = vec![None; first_args.len()];
1167+
// First copy the arguments of this call to temporary stack allocations.
1168+
for (i, arg) in first_args.iter().enumerate() {
1169+
if !matches!(fn_abi.args[i].mode, PassMode::Indirect { on_stack: false, .. }) {
1170+
continue;
1171+
}
1172+
1173+
let op = self.codegen_operand(bx, &arg.node);
1174+
let tmp = PlaceRef::alloca(bx, op.layout);
1175+
bx.lifetime_start(tmp.val.llval, tmp.layout.size);
1176+
op.store_with_annotation(bx, tmp);
1177+
1178+
tail_call_temporaries[i] = Some(tmp);
1179+
}
1180+
}
1181+
11431182
// When generating arguments we sometimes introduce temporary allocations with lifetime
11441183
// that extend for the duration of a call. Keep track of those allocations and their sizes
11451184
// to generate `lifetime_end` when the call returns.
11461185
let mut lifetime_ends_after_call: Vec<(Bx::Value, Size)> = Vec::new();
11471186
'make_args: for (i, arg) in first_args.iter().enumerate() {
1148-
if kind == CallKind::Tail && matches!(fn_abi.args[i].mode, PassMode::Indirect { .. }) {
1149-
// FIXME: https://github.com/rust-lang/rust/pull/144232#discussion_r2218543841
1150-
span_bug!(
1151-
fn_span,
1152-
"arguments using PassMode::Indirect are currently not supported for tail calls"
1153-
);
1154-
}
1155-
11561187
let mut op = self.codegen_operand(bx, &arg.node);
11571188

11581189
if let (0, Some(ty::InstanceKind::Virtual(_, idx))) = (i, instance.map(|i| i.def)) {
@@ -1203,18 +1234,71 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
12031234
}
12041235
}
12051236

1206-
// The callee needs to own the argument memory if we pass it
1207-
// by-ref, so make a local copy of non-immediate constants.
1208-
match (&arg.node, op.val) {
1209-
(&mir::Operand::Copy(_), Ref(PlaceValue { llextra: None, .. }))
1210-
| (&mir::Operand::Constant(_), Ref(PlaceValue { llextra: None, .. })) => {
1211-
let tmp = PlaceRef::alloca(bx, op.layout);
1212-
bx.lifetime_start(tmp.val.llval, tmp.layout.size);
1213-
op.store_with_annotation(bx, tmp);
1214-
op.val = Ref(tmp.val);
1215-
lifetime_ends_after_call.push((tmp.val.llval, tmp.layout.size));
1237+
match kind {
1238+
CallKind::Tail => {
1239+
match fn_abi.args[i].mode {
1240+
PassMode::Indirect { on_stack: false, .. } => {
1241+
let Some(tmp) = tail_call_temporaries[i].take() else {
1242+
span_bug!(
1243+
fn_span,
1244+
"missing temporary for indirect tail call argument #{i}"
1245+
)
1246+
};
1247+
1248+
let local = self.mir.args_iter().nth(i).unwrap();
1249+
1250+
match &self.locals[local] {
1251+
LocalRef::Place(arg) => {
1252+
bx.typed_place_copy(arg.val, tmp.val, fn_abi.args[i].layout);
1253+
op.val = Ref(arg.val);
1254+
}
1255+
LocalRef::Operand(arg) => {
1256+
bx.typed_place_copy(
1257+
arg.val.deref(fn_abi.args[i].layout.align.abi),
1258+
tmp.val,
1259+
fn_abi.args[i].layout,
1260+
);
1261+
op.val = arg.val;
1262+
}
1263+
LocalRef::UnsizedPlace(_) => {
1264+
span_bug!(fn_span, "unsized types are not supported")
1265+
}
1266+
LocalRef::PendingOperand => {
1267+
span_bug!(fn_span, "argument local should not be pending")
1268+
}
1269+
};
1270+
1271+
bx.lifetime_end(tmp.val.llval, tmp.layout.size);
1272+
}
1273+
PassMode::Indirect { on_stack: true, .. } => {
1274+
// FIXME: some LLVM backends (notably x86) do not correctly pass byval
1275+
// arguments to tail calls (as of LLVM 21). See also:
1276+
//
1277+
// - https://github.com/rust-lang/rust/pull/144232#discussion_r2218543841
1278+
// - https://github.com/rust-lang/rust/issues/144855
1279+
span_bug!(
1280+
fn_span,
1281+
"arguments using PassMode::Indirect {{ on_stack: true, .. }} are currently not supported for tail calls"
1282+
)
1283+
}
1284+
_ => (),
1285+
}
1286+
}
1287+
CallKind::Normal => {
1288+
// The callee needs to own the argument memory if we pass it
1289+
// by-ref, so make a local copy of non-immediate constants.
1290+
match (&arg.node, op.val) {
1291+
(&mir::Operand::Copy(_), Ref(PlaceValue { llextra: None, .. }))
1292+
| (&mir::Operand::Constant(_), Ref(PlaceValue { llextra: None, .. })) => {
1293+
let tmp = PlaceRef::alloca(bx, op.layout);
1294+
bx.lifetime_start(tmp.val.llval, tmp.layout.size);
1295+
op.store_with_annotation(bx, tmp);
1296+
op.val = Ref(tmp.val);
1297+
lifetime_ends_after_call.push((tmp.val.llval, tmp.layout.size));
1298+
}
1299+
_ => {}
1300+
}
12161301
}
1217-
_ => {}
12181302
}
12191303

12201304
self.codegen_argument(

compiler/rustc_mir_transform/src/deduce_param_attrs.rs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -121,17 +121,29 @@ impl<'tcx> Visitor<'tcx> for DeduceParamAttrs {
121121
// `f` passes. Note that function arguments are the only situation in which this problem can
122122
// arise: every other use of `move` in MIR doesn't actually write to the value it moves
123123
// from.
124-
if let TerminatorKind::Call { ref args, .. } = terminator.kind {
125-
for arg in args {
126-
if let Operand::Move(place) = arg.node
127-
&& !place.is_indirect_first_projection()
128-
&& let Some(i) = self.as_param(place.local)
129-
{
130-
self.usage[i] |= UsageSummary::MUTATE;
131-
self.usage[i] |= UsageSummary::CAPTURE;
124+
match terminator.kind {
125+
TerminatorKind::Call { ref args, .. } => {
126+
for arg in args {
127+
if let Operand::Move(place) = arg.node
128+
&& !place.is_indirect_first_projection()
129+
&& let Some(i) = self.as_param(place.local)
130+
{
131+
self.usage[i] |= UsageSummary::MUTATE;
132+
self.usage[i] |= UsageSummary::CAPTURE;
133+
}
132134
}
133135
}
134-
};
136+
137+
// Like a call, but more conservative because the backend may introduce writes to an
138+
// argument if the argument is passed as `PassMode::Indirect { on_stack: false, ... }`.
139+
TerminatorKind::TailCall { .. } => {
140+
for usage in self.usage.iter_mut() {
141+
*usage |= UsageSummary::MUTATE;
142+
*usage |= UsageSummary::CAPTURE;
143+
}
144+
}
145+
_ => {}
146+
}
135147

136148
self.super_terminator(terminator, location);
137149
}
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
//@ add-minicore
2+
//@ assembly-output: emit-asm
3+
//@ needs-llvm-components: x86
4+
//@ compile-flags: --target=x86_64-unknown-linux-gnu
5+
//@ compile-flags: -Copt-level=3 -C llvm-args=-x86-asm-syntax=intel
6+
7+
#![feature(no_core, explicit_tail_calls)]
8+
#![expect(incomplete_features)]
9+
#![no_core]
10+
#![crate_type = "lib"]
11+
12+
// Test tail calls with `PassMode::Indirect { on_stack: false, .. }` arguments.
13+
//
14+
// Normally an indirect argument with `on_stack: false` would be passed as a pointer to the
15+
// caller's stack frame. For tail calls, that would be unsound, because the caller's stack
16+
// frame is overwritten by the callee's stack frame.
17+
//
18+
// The solution is to write the argument into the caller's argument place (stored somewhere further
19+
// up the stack), and forward that place.
20+
21+
extern crate minicore;
22+
use minicore::*;
23+
24+
#[repr(C)]
25+
struct S {
26+
x: u64,
27+
y: u64,
28+
z: u64,
29+
}
30+
31+
unsafe extern "C" {
32+
safe fn force_usage(_: u64, _: u64, _: u64) -> u64;
33+
}
34+
35+
// CHECK-LABEL: callee:
36+
// CHECK-NEXT: .cfi_startproc
37+
//
38+
// CHECK-NEXT: mov rax, qword ptr [rdi]
39+
// CHECK-NEXT: mov rsi, qword ptr [rdi + 8]
40+
// CHECK-NEXT: mov rdx, qword ptr [rdi + 16]
41+
// CHECK-NEXT: mov rdi, rax
42+
//
43+
// CHECK-NEXT: jmp qword ptr [rip + force_usage@GOTPCREL]
44+
#[inline(never)]
45+
#[unsafe(no_mangle)]
46+
fn callee(s: S) -> u64 {
47+
force_usage(s.x, s.y, s.z)
48+
}
49+
50+
// CHECK-LABEL: caller1:
51+
// CHECK-NEXT: .cfi_startproc
52+
//
53+
// Just forward the argument:
54+
//
55+
// CHECK-NEXT: jmp qword ptr [rip + callee@GOTPCREL]
56+
#[unsafe(no_mangle)]
57+
fn caller1(s: S) -> u64 {
58+
become callee(s);
59+
}
60+
61+
// CHECK-LABEL: caller2:
62+
// CHECK-NEXT: .cfi_startproc
63+
//
64+
// Construct the S value directly into the argument slot:
65+
//
66+
// CHECK-NEXT: mov qword ptr [rdi], 1
67+
// CHECK-NEXT: mov qword ptr [rdi + 8], 2
68+
// CHECK-NEXT: mov qword ptr [rdi + 16], 3
69+
//
70+
// CHECK-NEXT: jmp qword ptr [rip + callee@GOTPCREL]
71+
#[unsafe(no_mangle)]
72+
fn caller2(_: S) -> u64 {
73+
let s = S { x: 1, y: 2, z: 3 };
74+
become callee(s);
75+
}

tests/crashes/144293-indirect-ops-llvm.rs

Lines changed: 0 additions & 42 deletions
This file was deleted.
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
//@ run-pass
2+
//@ ignore-backends: gcc
3+
#![feature(explicit_tail_calls)]
4+
#![expect(incomplete_features)]
5+
6+
// Test tail calls with `PassMode::Indirect { on_stack: false, .. }` arguments.
7+
//
8+
// Normally an indirect argument with `on_stack: false` would be passed as a pointer to the
9+
// caller's stack frame. For tail calls, that would be unsound, because the caller's stack
10+
// frame is overwritten by the callee's stack frame.
11+
//
12+
// The solution is to write the argument into the caller's argument place (stored somewhere further
13+
// up the stack), and forward that place.
14+
15+
// A struct big enough that it is not passed via registers, so that the rust calling convention uses
16+
// `Indirect { on_stack: false, .. }`.
17+
#[repr(C)]
18+
#[derive(Default, Debug, Clone, Copy, PartialEq, Eq)]
19+
pub struct Big([u64; 4]);
20+
21+
#[inline(never)]
22+
fn update_in_caller(y: Big) -> u64 {
23+
#[inline(never)]
24+
fn helper(x: Big) -> u64 {
25+
x.0.iter().sum()
26+
}
27+
28+
let x = Big([y.0[0], 2, 3, 4]);
29+
30+
// `x` is actually stored in `y`'s space.
31+
become helper(x)
32+
}
33+
34+
#[inline(never)]
35+
fn swapper<T>(a: T, b: T) -> (T, T) {
36+
#[inline(never)]
37+
fn helper<T>(a: T, b: T) -> (T, T) {
38+
(a, b)
39+
}
40+
41+
become helper(b, a)
42+
}
43+
44+
#[inline(never)]
45+
fn swapper_derived(a: Big, _: (u64, u64), b: Big, _: (u64, u64)) -> ((u64, u64), (u64, u64)) {
46+
#[inline(never)]
47+
fn helper(_: Big, x: (u64, u64), _: Big, y: (u64, u64)) -> ((u64, u64), (u64, u64)) {
48+
(x, y)
49+
}
50+
51+
// Read the values at various points in the swapping process, testing that they have the correct
52+
// value at every point.
53+
become helper(b, (a.0[0], b.0[0]), a, (a.0[0], b.0[0]));
54+
}
55+
56+
fn main() {
57+
assert_eq!(update_in_caller(Big::default()), 0 + 2 + 3 + 4);
58+
59+
assert_eq!(swapper(u8::MIN, u8::MAX), (u8::MAX, u8::MIN));
60+
// i128 uses `PassMode::Indirect { on_stack: false, .. }` on x86_64 MSVC.
61+
assert_eq!(swapper(i128::MIN, i128::MAX), (i128::MAX, i128::MIN));
62+
assert_eq!(swapper(Big([1; 4]), Big([2; 4])), (Big([2; 4]), Big([1; 4])));
63+
64+
assert_eq!(swapper_derived(Big([1; 4]), (0, 0), Big([2; 4]), (0, 0)), ((1, 2), (1, 2)));
65+
}

0 commit comments

Comments
 (0)