Skip to content

Commit dca2d1f

Browse files
committed
Auto merge of #115374 - RalfJung:miri-fn-abi, r=oli-obk
miri function ABI check: accept repr(transparent) wrappers as compatible
2 parents 8cbd2c8 + c37bd09 commit dca2d1f

18 files changed

+219
-63
lines changed

compiler/rustc_const_eval/src/interpret/terminator.rs

+103-52
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@ use std::borrow::Cow;
22

33
use either::Either;
44
use rustc_ast::ast::InlineAsmOptions;
5-
use rustc_middle::mir::ProjectionElem;
6-
use rustc_middle::ty::layout::{FnAbiOf, LayoutOf, TyAndLayout};
7-
use rustc_middle::ty::Instance;
85
use rustc_middle::{
96
mir,
10-
ty::{self, Ty},
7+
ty::{
8+
self,
9+
layout::{FnAbiOf, LayoutOf, TyAndLayout},
10+
Instance, Ty,
11+
},
1112
};
1213
use rustc_target::abi::call::{ArgAbi, ArgAttribute, ArgAttributes, FnAbi, PassMode};
1314
use rustc_target::abi::{self, FieldIdx};
@@ -252,11 +253,43 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
252253
.collect()
253254
}
254255

255-
fn check_argument_compat(
256-
caller_abi: &ArgAbi<'tcx, Ty<'tcx>>,
257-
callee_abi: &ArgAbi<'tcx, Ty<'tcx>>,
256+
/// Find the wrapped inner type of a transparent wrapper.
257+
fn unfold_transparent(&self, layout: TyAndLayout<'tcx>) -> TyAndLayout<'tcx> {
258+
match layout.ty.kind() {
259+
ty::Adt(adt_def, _) if adt_def.repr().transparent() => {
260+
assert!(!adt_def.is_enum());
261+
// Find the non-1-ZST field.
262+
let mut non_1zst_fields = (0..layout.fields.count()).filter_map(|idx| {
263+
let field = layout.field(self, idx);
264+
if field.is_1zst() { None } else { Some(field) }
265+
});
266+
let Some(first) = non_1zst_fields.next() else {
267+
// All fields are 1-ZST, so this is basically the same as `()`.
268+
// (We still also compare the `PassMode`, so if this target does something strange with 1-ZST there, we'll know.)
269+
return self.layout_of(self.tcx.types.unit).unwrap();
270+
};
271+
assert!(
272+
non_1zst_fields.next().is_none(),
273+
"more than one non-1-ZST field in a transparent type"
274+
);
275+
276+
// Found it!
277+
self.unfold_transparent(first)
278+
}
279+
// Not a transparent type, no further unfolding.
280+
_ => layout,
281+
}
282+
}
283+
284+
/// Check if these two layouts look like they are fn-ABI-compatible.
285+
/// (We also compare the `PassMode`, so this doesn't have to check everything. But it turns out
286+
/// that only checking the `PassMode` is insufficient.)
287+
fn layout_compat(
288+
&self,
289+
caller_layout: TyAndLayout<'tcx>,
290+
callee_layout: TyAndLayout<'tcx>,
258291
) -> bool {
259-
let primitive_abi_compat = |a1: abi::Primitive, a2: abi::Primitive| -> bool {
292+
fn primitive_abi_compat(a1: abi::Primitive, a2: abi::Primitive) -> bool {
260293
match (a1, a2) {
261294
// For integers, ignore the sign.
262295
(abi::Primitive::Int(int_ty1, _sign1), abi::Primitive::Int(int_ty2, _sign2)) => {
@@ -265,40 +298,49 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
265298
// For everything else we require full equality.
266299
_ => a1 == a2,
267300
}
268-
};
269-
// Heuristic for type comparison.
270-
let layout_compat = || {
271-
if caller_abi.layout.ty == callee_abi.layout.ty {
272-
// No question
273-
return true;
301+
}
302+
303+
if caller_layout.ty == callee_layout.ty {
304+
// Fast path: equal types are definitely compatible.
305+
return true;
306+
}
307+
308+
match (caller_layout.abi, callee_layout.abi) {
309+
// If both sides have Scalar/Vector/ScalarPair ABI, we can easily directly compare them.
310+
// Different valid ranges are okay (the validity check will complain if this leads to
311+
// invalid transmutes).
312+
(abi::Abi::Scalar(caller), abi::Abi::Scalar(callee)) => {
313+
primitive_abi_compat(caller.primitive(), callee.primitive())
274314
}
275-
if caller_abi.layout.is_unsized() || callee_abi.layout.is_unsized() {
276-
// No, no, no. We require the types to *exactly* match for unsized arguments. If
277-
// these are somehow unsized "in a different way" (say, `dyn Trait` vs `[i32]`),
278-
// then who knows what happens.
279-
return false;
315+
(
316+
abi::Abi::Vector { element: caller_element, count: caller_count },
317+
abi::Abi::Vector { element: callee_element, count: callee_count },
318+
) => {
319+
primitive_abi_compat(caller_element.primitive(), callee_element.primitive())
320+
&& caller_count == callee_count
280321
}
281-
// This is tricky. Some ABIs split aggregates up into multiple registers etc, so we have
282-
// to be super careful here. For the scalar ABIs we conveniently already have all the
283-
// newtypes unwrapped etc, so in those cases we can just compare the scalar components.
284-
// Everything else we just reject for now.
285-
match (caller_abi.layout.abi, callee_abi.layout.abi) {
286-
// Different valid ranges are okay (the validity check will complain if this leads
287-
// to invalid transmutes).
288-
(abi::Abi::Scalar(caller), abi::Abi::Scalar(callee)) => {
289-
primitive_abi_compat(caller.primitive(), callee.primitive())
290-
}
291-
(
292-
abi::Abi::ScalarPair(caller1, caller2),
293-
abi::Abi::ScalarPair(callee1, callee2),
294-
) => {
295-
primitive_abi_compat(caller1.primitive(), callee1.primitive())
296-
&& primitive_abi_compat(caller2.primitive(), callee2.primitive())
297-
}
298-
// Be conservative.
299-
_ => false,
322+
(abi::Abi::ScalarPair(caller1, caller2), abi::Abi::ScalarPair(callee1, callee2)) => {
323+
primitive_abi_compat(caller1.primitive(), callee1.primitive())
324+
&& primitive_abi_compat(caller2.primitive(), callee2.primitive())
300325
}
301-
};
326+
(abi::Abi::Aggregate { .. }, abi::Abi::Aggregate { .. }) => {
327+
// Aggregates are compatible only if they newtype-wrap the same type.
328+
// This is conservative, but also means that our check isn't quite so heavily dependent on the `PassMode`,
329+
// which means having ABI-compatibility on one target is much more likely to imply compatibility for other targets.
330+
self.unfold_transparent(caller_layout).ty
331+
== self.unfold_transparent(callee_layout).ty
332+
}
333+
// What remains is `Abi::Uninhabited` (which can never be passed anyway) and
334+
// mismatching ABIs, that should all be rejected.
335+
_ => false,
336+
}
337+
}
338+
339+
fn check_argument_compat(
340+
&self,
341+
caller_abi: &ArgAbi<'tcx, Ty<'tcx>>,
342+
callee_abi: &ArgAbi<'tcx, Ty<'tcx>>,
343+
) -> bool {
302344
// When comparing the PassMode, we have to be smart about comparing the attributes.
303345
let arg_attr_compat = |a1: &ArgAttributes, a2: &ArgAttributes| {
304346
// There's only one regular attribute that matters for the call ABI: InReg.
@@ -333,23 +375,29 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
333375
_ => false,
334376
};
335377

336-
// We have to check both. `layout_compat` is needed to reject e.g. `i32` vs `f32`,
337-
// which is not reflected in `PassMode`. `mode_compat` is needed to reject `u8` vs `bool`,
338-
// which have the same `abi::Primitive` but different `arg_ext`.
339-
if layout_compat() && mode_compat() {
378+
// Ideally `PassMode` would capture everything there is about argument passing, but that is
379+
// not the case: in `FnAbi::llvm_type`, also parts of the layout and type information are
380+
// used. So we need to check that *both* sufficiently agree to ensures the arguments are
381+
// compatible.
382+
// For instance, `layout_compat` is needed to reject `i32` vs `f32`, which is not reflected
383+
// in `PassMode`. `mode_compat` is needed to reject `u8` vs `bool`, which have the same
384+
// `abi::Primitive` but different `arg_ext`.
385+
if self.layout_compat(caller_abi.layout, callee_abi.layout) && mode_compat() {
340386
// Something went very wrong if our checks don't even imply that the layout is the same.
341387
assert!(
342388
caller_abi.layout.size == callee_abi.layout.size
343389
&& caller_abi.layout.align.abi == callee_abi.layout.align.abi
390+
&& caller_abi.layout.is_sized() == callee_abi.layout.is_sized()
344391
);
345392
return true;
393+
} else {
394+
trace!(
395+
"check_argument_compat: incompatible ABIs:\ncaller: {:?}\ncallee: {:?}",
396+
caller_abi,
397+
callee_abi
398+
);
399+
return false;
346400
}
347-
trace!(
348-
"check_argument_compat: incompatible ABIs:\ncaller: {:?}\ncallee: {:?}",
349-
caller_abi,
350-
callee_abi
351-
);
352-
return false;
353401
}
354402

355403
/// Initialize a single callee argument, checking the types for compatibility.
@@ -379,7 +427,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
379427
throw_ub_custom!(fluent::const_eval_not_enough_caller_args);
380428
};
381429
// Check compatibility
382-
if !Self::check_argument_compat(caller_abi, callee_abi) {
430+
if !self.check_argument_compat(caller_abi, callee_abi) {
383431
let callee_ty = format!("{}", callee_ty);
384432
let caller_ty = format!("{}", caller_arg.layout().ty);
385433
throw_ub_custom!(
@@ -612,7 +660,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
612660
};
613661
for (i, field_ty) in fields.iter().enumerate() {
614662
let dest = dest.project_deeper(
615-
&[ProjectionElem::Field(FieldIdx::from_usize(i), field_ty)],
663+
&[mir::ProjectionElem::Field(
664+
FieldIdx::from_usize(i),
665+
field_ty,
666+
)],
616667
*self.tcx,
617668
);
618669
let callee_abi = callee_args_abis.next().unwrap();
@@ -649,7 +700,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
649700
throw_ub_custom!(fluent::const_eval_too_many_caller_args);
650701
}
651702
// Don't forget to check the return type!
652-
if !Self::check_argument_compat(&caller_fn_abi.ret, &callee_fn_abi.ret) {
703+
if !self.check_argument_compat(&caller_fn_abi.ret, &callee_fn_abi.ret) {
653704
let callee_ty = format!("{}", callee_fn_abi.ret.layout.ty);
654705
let caller_ty = format!("{}", caller_fn_abi.ret.layout.ty);
655706
throw_ub_custom!(
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#![feature(portable_simd)]
2+
3+
// Some targets treat arrays and structs very differently. We would probably catch that on those
4+
// targets since we check the `PassMode`; here we ensure that we catch it on *all* targets
5+
// (in particular, on x86-64 the pass mode is `Indirect` for both of these).
6+
struct S(i32, i32, i32, i32);
7+
type A = [i32; 4];
8+
9+
fn main() {
10+
fn f(_: S) {}
11+
12+
// These two types have the same size but are still not compatible.
13+
let g = unsafe { std::mem::transmute::<fn(S), fn(A)>(f) };
14+
15+
g(Default::default()) //~ ERROR: calling a function with argument of type S passing data of type [i32; 4]
16+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: Undefined Behavior: calling a function with argument of type S passing data of type [i32; 4]
2+
--> $DIR/abi_mismatch_array_vs_struct.rs:LL:CC
3+
|
4+
LL | g(Default::default())
5+
| ^^^^^^^^^^^^^^^^^^^^^ calling a function with argument of type S passing data of type [i32; 4]
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `main` at $DIR/abi_mismatch_array_vs_struct.rs:LL:CC
11+
12+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
13+
14+
error: aborting due to previous error
15+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
fn main() {
2+
fn f(_: f32) {}
3+
4+
let g = unsafe { std::mem::transmute::<fn(f32), fn(i32)>(f) };
5+
6+
g(42) //~ ERROR: calling a function with argument of type f32 passing data of type i32
7+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: Undefined Behavior: calling a function with argument of type f32 passing data of type i32
2+
--> $DIR/abi_mismatch_int_vs_float.rs:LL:CC
3+
|
4+
LL | g(42)
5+
| ^^^^^ calling a function with argument of type f32 passing data of type i32
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `main` at $DIR/abi_mismatch_int_vs_float.rs:LL:CC
11+
12+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
13+
14+
error: aborting due to previous error
15+

src/tools/miri/tests/fail/function_pointers/cast_fn_ptr4.stderr renamed to src/tools/miri/tests/fail/function_pointers/abi_mismatch_raw_pointer.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
error: Undefined Behavior: calling a function with argument of type *const [i32] passing data of type *const i32
2-
--> $DIR/cast_fn_ptr4.rs:LL:CC
2+
--> $DIR/abi_mismatch_raw_pointer.rs:LL:CC
33
|
44
LL | g(&42 as *const i32)
55
| ^^^^^^^^^^^^^^^^^^^^ calling a function with argument of type *const [i32] passing data of type *const i32
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
99
= note: BACKTRACE:
10-
= note: inside `main` at $DIR/cast_fn_ptr4.rs:LL:CC
10+
= note: inside `main` at $DIR/abi_mismatch_raw_pointer.rs:LL:CC
1111

1212
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
1313

src/tools/miri/tests/fail/function_pointers/cast_fn_ptr5.stderr renamed to src/tools/miri/tests/fail/function_pointers/abi_mismatch_return_type.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
error: Undefined Behavior: calling a function with return type u32 passing return place of type ()
2-
--> $DIR/cast_fn_ptr5.rs:LL:CC
2+
--> $DIR/abi_mismatch_return_type.rs:LL:CC
33
|
44
LL | g()
55
| ^^^ calling a function with return type u32 passing return place of type ()
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
99
= note: BACKTRACE:
10-
= note: inside `main` at $DIR/cast_fn_ptr5.rs:LL:CC
10+
= note: inside `main` at $DIR/abi_mismatch_return_type.rs:LL:CC
1111

1212
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
1313

src/tools/miri/tests/fail/function_pointers/cast_fn_ptr2.stderr renamed to src/tools/miri/tests/fail/function_pointers/abi_mismatch_simple.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
error: Undefined Behavior: calling a function with argument of type (i32, i32) passing data of type i32
2-
--> $DIR/cast_fn_ptr2.rs:LL:CC
2+
--> $DIR/abi_mismatch_simple.rs:LL:CC
33
|
44
LL | g(42)
55
| ^^^^^ calling a function with argument of type (i32, i32) passing data of type i32
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
99
= note: BACKTRACE:
10-
= note: inside `main` at $DIR/cast_fn_ptr2.rs:LL:CC
10+
= note: inside `main` at $DIR/abi_mismatch_simple.rs:LL:CC
1111

1212
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
1313

src/tools/miri/tests/fail/function_pointers/cast_fn_ptr3.stderr renamed to src/tools/miri/tests/fail/function_pointers/abi_mismatch_too_few_args.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
error: Undefined Behavior: calling a function with fewer arguments than it requires
2-
--> $DIR/cast_fn_ptr3.rs:LL:CC
2+
--> $DIR/abi_mismatch_too_few_args.rs:LL:CC
33
|
44
LL | g()
55
| ^^^ calling a function with fewer arguments than it requires
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
99
= note: BACKTRACE:
10-
= note: inside `main` at $DIR/cast_fn_ptr3.rs:LL:CC
10+
= note: inside `main` at $DIR/abi_mismatch_too_few_args.rs:LL:CC
1111

1212
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
1313

src/tools/miri/tests/fail/function_pointers/cast_fn_ptr1.stderr renamed to src/tools/miri/tests/fail/function_pointers/abi_mismatch_too_many_args.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
error: Undefined Behavior: calling a function with more arguments than it expected
2-
--> $DIR/cast_fn_ptr1.rs:LL:CC
2+
--> $DIR/abi_mismatch_too_many_args.rs:LL:CC
33
|
44
LL | g(42)
55
| ^^^^^ calling a function with more arguments than it expected
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
99
= note: BACKTRACE:
10-
= note: inside `main` at $DIR/cast_fn_ptr1.rs:LL:CC
10+
= note: inside `main` at $DIR/abi_mismatch_too_many_args.rs:LL:CC
1111

1212
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
1313

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#![feature(portable_simd)]
2+
use std::simd;
3+
4+
fn main() {
5+
fn f(_: simd::u32x8) {}
6+
7+
// These two vector types have the same size but are still not compatible.
8+
let g = unsafe { std::mem::transmute::<fn(simd::u32x8), fn(simd::u64x4)>(f) };
9+
10+
g(Default::default()) //~ ERROR: calling a function with argument of type std::simd::Simd<u32, 8> passing data of type std::simd::Simd<u64, 4>
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: Undefined Behavior: calling a function with argument of type std::simd::Simd<u32, 8> passing data of type std::simd::Simd<u64, 4>
2+
--> $DIR/abi_mismatch_vector.rs:LL:CC
3+
|
4+
LL | g(Default::default())
5+
| ^^^^^^^^^^^^^^^^^^^^^ calling a function with argument of type std::simd::Simd<u32, 8> passing data of type std::simd::Simd<u64, 4>
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `main` at $DIR/abi_mismatch_vector.rs:LL:CC
11+
12+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
13+
14+
error: aborting due to previous error
15+

0 commit comments

Comments
 (0)