Skip to content

Make fn_abi_sanity_check a bit stricter #132729

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 7, 2024
Merged

Make fn_abi_sanity_check a bit stricter #132729

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 51 additions & 30 deletions compiler/rustc_ty_utils/src/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,43 +463,64 @@ fn fn_abi_sanity_check<'tcx>(
arg: &ArgAbi<'tcx, Ty<'tcx>>,
) {
let tcx = cx.tcx();

if spec_abi == ExternAbi::Rust
|| spec_abi == ExternAbi::RustCall
|| spec_abi == ExternAbi::RustCold
{
if arg.layout.is_zst() {
// Casting closures to function pointers depends on ZST closure types being
// omitted entirely in the calling convention.
assert!(arg.is_ignore());
}
if let PassMode::Indirect { on_stack, .. } = arg.mode {
assert!(!on_stack, "rust abi shouldn't use on_stack");
}
}

match &arg.mode {
PassMode::Ignore => {}
PassMode::Ignore => {
assert!(arg.layout.is_zst() || arg.layout.is_uninhabited());
}
PassMode::Direct(_) => {
// Here the Rust type is used to determine the actual ABI, so we have to be very
// careful. Scalar/ScalarPair is fine, since backends will generally use
// `layout.abi` and ignore everything else. We should just reject `Aggregate`
// entirely here, but some targets need to be fixed first.
if matches!(arg.layout.backend_repr, BackendRepr::Memory { .. }) {
// For an unsized type we'd only pass the sized prefix, so there is no universe
// in which we ever want to allow this.
assert!(
arg.layout.is_sized(),
"`PassMode::Direct` for unsized type in ABI: {:#?}",
fn_abi
);
// This really shouldn't happen even for sized aggregates, since
// `immediate_llvm_type` will use `layout.fields` to turn this Rust type into an
// LLVM type. This means all sorts of Rust type details leak into the ABI.
// However wasm sadly *does* currently use this mode so we have to allow it --
// but we absolutely shouldn't let any more targets do that.
// (Also see <https://github.com/rust-lang/rust/issues/115666>.)
//
// The unstable abi `PtxKernel` also uses Direct for now.
// It needs to switch to something else before stabilization can happen.
// (See issue: https://github.com/rust-lang/rust/issues/117271)
assert!(
matches!(&*tcx.sess.target.arch, "wasm32" | "wasm64")
|| matches!(spec_abi, ExternAbi::PtxKernel | ExternAbi::Unadjusted),
"`PassMode::Direct` for aggregates only allowed for \"unadjusted\" and \"ptx-kernel\" functions and on wasm\n\
// careful. Scalar/Vector is fine, since backends will generally use
// `layout.backend_repr` and ignore everything else. We should just reject
//`Aggregate` entirely here, but some targets need to be fixed first.
match arg.layout.backend_repr {
BackendRepr::Uninhabited
| BackendRepr::Scalar(_)
| BackendRepr::Vector { .. } => {}
BackendRepr::ScalarPair(..) => {
panic!("`PassMode::Direct` used for ScalarPair type {}", arg.layout.ty)
}
BackendRepr::Memory { sized } => {
// For an unsized type we'd only pass the sized prefix, so there is no universe
// in which we ever want to allow this.
assert!(sized, "`PassMode::Direct` for unsized type in ABI: {:#?}", fn_abi);
// This really shouldn't happen even for sized aggregates, since
// `immediate_llvm_type` will use `layout.fields` to turn this Rust type into an
// LLVM type. This means all sorts of Rust type details leak into the ABI.
// However wasm sadly *does* currently use this mode so we have to allow it --
// but we absolutely shouldn't let any more targets do that.
// (Also see <https://github.com/rust-lang/rust/issues/115666>.)
//
// The unstable abi `PtxKernel` also uses Direct for now.
// It needs to switch to something else before stabilization can happen.
// (See issue: https://github.com/rust-lang/rust/issues/117271)
assert!(
matches!(&*tcx.sess.target.arch, "wasm32" | "wasm64")
|| matches!(spec_abi, ExternAbi::PtxKernel | ExternAbi::Unadjusted),
"`PassMode::Direct` for aggregates only allowed for \"unadjusted\" and \"ptx-kernel\" functions and on wasm\n\
Problematic type: {:#?}",
arg.layout,
);
arg.layout,
);
}
}
}
PassMode::Pair(_, _) => {
// Similar to `Direct`, we need to make sure that backends use `layout.abi` and
// ignore the rest of the layout.
// Similar to `Direct`, we need to make sure that backends use `layout.backend_repr`
// and ignore the rest of the layout.
assert!(
matches!(arg.layout.backend_repr, BackendRepr::ScalarPair(..)),
"PassMode::Pair for type {}",
Expand Down
Loading