Skip to content

Commit

Permalink
move ABI sanity check from LLVM codegen backend to ABI computation logic
Browse files Browse the repository at this point in the history
  • Loading branch information
@RalfJung
RalfJung committed Oct 29, 2023
1 parent c6a7fb1 commit f0bc6cc
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 43 deletions.
43 changes: 3 additions & 40 deletions compiler/rustc_codegen_llvm/src/abi.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,50 +348,18 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
PassMode::Direct(_) => {
// ABI-compatible Rust types have the same `layout.abi` (up to validity ranges),
// and for Scalar ABIs the LLVM type is fully determined by `layout.abi`,
// guarnateeing that we generate ABI-compatible LLVM IR. Things get tricky for
// aggregates...
if matches!(arg.layout.abi, abi::Abi::Aggregate { .. }) {
assert!(
arg.layout.is_sized(),
"`PassMode::Direct` for unsized type: {}",
arg.layout.ty
);
// This really shouldn't happen, 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!(&*cx.tcx.sess.target.arch, "wasm32" | "wasm64")
|| self.conv == Conv::PtxKernel,
"`PassMode::Direct` for aggregates only allowed on wasm and `extern \"ptx-kernel\"` fns\nProblematic type: {:#?}",
arg.layout,
);
}
// guarnateeing that we generate ABI-compatible LLVM IR.
arg.layout.immediate_llvm_type(cx)
}
PassMode::Pair(..) => {
// ABI-compatible Rust types have the same `layout.abi` (up to validity ranges),
// so for ScalarPair we can easily be sure that we are generating ABI-compatible
// LLVM IR.
assert!(
matches!(arg.layout.abi, abi::Abi::ScalarPair(..)),
"PassMode::Pair for type {}",
arg.layout.ty
);
llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 0, true));
llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 1, true));
continue;
}
PassMode::Indirect { attrs: _, meta_attrs: Some(_), on_stack } => {
// `Indirect` with metadata is only for unsized types, and doesn't work with
// on-stack passing.
assert!(arg.layout.is_unsized() && !on_stack);
PassMode::Indirect { attrs: _, meta_attrs: Some(_), on_stack: _ } => {
// Construct the type of a (wide) pointer to `ty`, and pass its two fields.
// Any two ABI-compatible unsized types have the same metadata type and
// moreover the same metadata value leads to the same dynamic size and
Expand All @@ -402,13 +370,8 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 1, true));
continue;
}
PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: _ } => {
assert!(arg.layout.is_sized());
cx.type_ptr()
}
PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: _ } => cx.type_ptr(),
PassMode::Cast { cast, pad_i32 } => {
// `Cast` means "transmute to `CastType`"; that only makes sense for sized types.
assert!(arg.layout.is_sized());
// add padding
if *pad_i32 {
llargument_tys.push(Reg::i32().llvm_type(cx));
Expand Down
69 changes: 69 additions & 0 deletions compiler/rustc_ty_utils/src/abi.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,74 @@ fn adjust_for_rust_scalar<'tcx>(
}
}

/// Ensure that the ABI makes basic sense.
fn fn_abi_sanity_check<'tcx>(cx: &LayoutCx<'tcx, TyCtxt<'tcx>>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) {
fn fn_arg_sanity_check<'tcx>(
cx: &LayoutCx<'tcx, TyCtxt<'tcx>>,
fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
arg: &ArgAbi<'tcx, Ty<'tcx>>,
) {
match &arg.mode {
PassMode::Ignore => {}
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.abi, Abi::Aggregate { .. }) {
assert!(
arg.layout.is_sized(),
"`PassMode::Direct` for unsized type in ABI: {:#?}",
fn_abi
);
// This really shouldn't happen, 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!(&*cx.tcx.sess.target.arch, "wasm32" | "wasm64")
|| fn_abi.conv == Conv::PtxKernel,
"`PassMode::Direct` for aggregates only allowed on wasm and `extern \"ptx-kernel\"` fns\nProblematic type: {:#?}",
arg.layout,
);
}
}
PassMode::Pair(_, _) => {
// Similar to `Direct`, we need to make sure that backends use `layout.abi` and
// ignore the rest of the layout.
assert!(
matches!(arg.layout.abi, Abi::ScalarPair(..)),
"PassMode::Pair for type {}",
arg.layout.ty
);
}
PassMode::Cast { .. } => {
// `Cast` means "transmute to `CastType`"; that only makes sense for sized types.
assert!(arg.layout.is_sized());
}
PassMode::Indirect { meta_attrs: None, .. } => {
// No metadata, must be sized.
assert!(arg.layout.is_sized());
}
PassMode::Indirect { meta_attrs: Some(_), on_stack, .. } => {
// With metadata. Must be unsized and not on the stack.
assert!(arg.layout.is_unsized() && !on_stack);
}
}
}

for arg in fn_abi.args.iter() {
fn_arg_sanity_check(cx, fn_abi, arg);
}
fn_arg_sanity_check(cx, fn_abi, &fn_abi.ret);
}

// FIXME(eddyb) perhaps group the signature/type-containing (or all of them?)
// arguments of this method, into a separate `struct`.
#[tracing::instrument(level = "debug", skip(cx, caller_location, fn_def_id, force_thin_self_ptr))]
Expand Down Expand Up @@ -453,6 +521,7 @@ fn fn_abi_new_uncached<'tcx>(
};
fn_abi_adjust_for_abi(cx, &mut fn_abi, sig.abi, fn_def_id)?;
debug!("fn_abi_new_uncached = {:?}", fn_abi);
fn_abi_sanity_check(cx, &fn_abi);
Ok(cx.tcx.arena.alloc(fn_abi))
}

Expand Down
8 changes: 5 additions & 3 deletions tests/ui/abi/compatibility.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@
// revisions: wasi
//[wasi] compile-flags: --target wasm32-wasi
//[wasi] needs-llvm-components: webassembly
// revisions: nvptx64
//[nvptx64] compile-flags: --target nvptx64-nvidia-cuda
//[nvptx64] needs-llvm-components: nvptx
// FIXME: disabled on nvptx64 since the target ABI fails the sanity check
/* revisions: nvptx64
[nvptx64] compile-flags: --target nvptx64-nvidia-cuda
[nvptx64] needs-llvm-components: nvptx
*/
#![feature(rustc_attrs, unsized_fn_params, transparent_unions)]
#![cfg_attr(not(host), feature(no_core, lang_items), no_std, no_core)]
#![allow(unused, improper_ctypes_definitions, internal_features)]
Expand Down

0 comments on commit f0bc6cc

Please sign in to comment.