From 9570cac0199c4da6d4c493a4946d6b715eb437d9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 30 Aug 2023 09:57:48 +0200 Subject: [PATCH 01/12] rustc_abi: also support debugging function pointers --- compiler/rustc_passes/messages.ftl | 2 +- compiler/rustc_passes/src/abi_test.rs | 124 ++++++++++++++++++-------- tests/ui/abi/debug.rs | 2 + tests/ui/abi/debug.stderr | 106 ++++++++++++++++++++-- 4 files changed, 190 insertions(+), 44 deletions(-) diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index 57598cf8bcf01..aa7db3348d028 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -8,7 +8,7 @@ passes_abi = abi: {$abi} passes_abi_of = - fn_abi_of_instance({$fn_name}) = {$fn_abi} + fn_abi_of({$fn_name}) = {$fn_abi} passes_align = align: {$align} diff --git a/compiler/rustc_passes/src/abi_test.rs b/compiler/rustc_passes/src/abi_test.rs index 5c0438e78aebc..aeaa264216dac 100644 --- a/compiler/rustc_passes/src/abi_test.rs +++ b/compiler/rustc_passes/src/abi_test.rs @@ -2,9 +2,10 @@ use rustc_ast::Attribute; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_middle::ty::layout::{FnAbiError, LayoutError}; -use rustc_middle::ty::{self, GenericArgs, Instance, TyCtxt}; +use rustc_middle::ty::{self, GenericArgs, Instance, Ty, TyCtxt}; use rustc_span::source_map::Spanned; use rustc_span::symbol::sym; +use rustc_target::abi::call::FnAbi; use crate::errors::{AbiOf, UnrecognizedField}; @@ -17,7 +18,12 @@ pub fn test_abi(tcx: TyCtxt<'_>) { match tcx.def_kind(id.owner_id) { DefKind::Fn => { for attr in tcx.get_attrs(id.owner_id, sym::rustc_abi) { - dump_abi_of(tcx, id.owner_id.def_id.into(), attr); + dump_abi_of_fn_item(tcx, id.owner_id.def_id.into(), attr); + } + } + DefKind::TyAlias { .. } => { + for attr in tcx.get_attrs(id.owner_id, sym::rustc_abi) { + dump_abi_of_fn_type(tcx, id.owner_id.def_id.into(), attr); } } DefKind::Impl { .. } => { @@ -25,7 +31,7 @@ pub fn test_abi(tcx: TyCtxt<'_>) { for &id in tcx.associated_item_def_ids(id.owner_id) { if matches!(tcx.def_kind(id), DefKind::AssocFn) { for attr in tcx.get_attrs(id, sym::rustc_abi) { - dump_abi_of(tcx, id, attr); + dump_abi_of_fn_item(tcx, id, attr); } } } @@ -35,7 +41,32 @@ pub fn test_abi(tcx: TyCtxt<'_>) { } } -fn dump_abi_of(tcx: TyCtxt<'_>, item_def_id: DefId, attr: &Attribute) { +fn unwrap_fn_abi<'tcx>( + abi: Result<&'tcx FnAbi<'tcx, Ty<'tcx>>, &'tcx FnAbiError<'tcx>>, + tcx: TyCtxt<'tcx>, + item_def_id: DefId, +) -> &'tcx FnAbi<'tcx, Ty<'tcx>> { + match abi { + Ok(abi) => abi, + Err(FnAbiError::Layout(layout_error)) => { + tcx.sess.emit_fatal(Spanned { + node: layout_error.into_diagnostic(), + span: tcx.def_span(item_def_id), + }); + } + Err(FnAbiError::AdjustForForeignAbi(e)) => { + // Sadly there seems to be no `into_diagnostic` for this case... and I am not sure if + // this can even be reached. Anyway this is a perma-unstable debug attribute, an ICE + // isn't the worst thing. Also this matches what codegen does. + span_bug!( + tcx.def_span(item_def_id), + "error computing fn_abi_of_instance, cannot adjust for foreign ABI: {e:?}", + ) + } + } +} + +fn dump_abi_of_fn_item(tcx: TyCtxt<'_>, item_def_id: DefId, attr: &Attribute) { let param_env = tcx.param_env(item_def_id); let args = GenericArgs::identity_for_item(tcx, item_def_id); let instance = match Instance::resolve(tcx, param_env, item_def_id, args) { @@ -51,43 +82,62 @@ fn dump_abi_of(tcx: TyCtxt<'_>, item_def_id: DefId, attr: &Attribute) { } Err(_guaranteed) => return, }; - match tcx.fn_abi_of_instance(param_env.and((instance, /* extra_args */ ty::List::empty()))) { - Ok(abi) => { - // Check out the `#[rustc_abi(..)]` attribute to tell what to dump. - // The `..` are the names of fields to dump. - let meta_items = attr.meta_item_list().unwrap_or_default(); - for meta_item in meta_items { - match meta_item.name_or_empty() { - sym::debug => { - let fn_name = tcx.item_name(item_def_id); - tcx.sess.emit_err(AbiOf { - span: tcx.def_span(item_def_id), - fn_name, - fn_abi: format!("{:#?}", abi), - }); - } + let abi = unwrap_fn_abi( + tcx.fn_abi_of_instance(param_env.and((instance, /* extra_args */ ty::List::empty()))), + tcx, + item_def_id, + ); - name => { - tcx.sess.emit_err(UnrecognizedField { span: meta_item.span(), name }); - } - } + // Check out the `#[rustc_abi(..)]` attribute to tell what to dump. + // The `..` are the names of fields to dump. + let meta_items = attr.meta_item_list().unwrap_or_default(); + for meta_item in meta_items { + match meta_item.name_or_empty() { + sym::debug => { + let fn_name = tcx.item_name(item_def_id); + tcx.sess.emit_err(AbiOf { + span: tcx.def_span(item_def_id), + fn_name, + fn_abi: format!("{:#?}", abi), + }); } - } - Err(FnAbiError::Layout(layout_error)) => { - tcx.sess.emit_fatal(Spanned { - node: layout_error.into_diagnostic(), - span: tcx.def_span(item_def_id), - }); + name => { + tcx.sess.emit_err(UnrecognizedField { span: meta_item.span(), name }); + } } - Err(FnAbiError::AdjustForForeignAbi(e)) => { - // Sadly there seems to be no `into_diagnostic` for this case... and I am not sure if - // this can even be reached. Anyway this is a perma-unstable debug attribute, an ICE - // isn't the worst thing. Also this matches what codegen does. - span_bug!( - tcx.def_span(item_def_id), - "error computing fn_abi_of_instance, cannot adjust for foreign ABI: {e:?}", - ) + } +} + +fn dump_abi_of_fn_type(tcx: TyCtxt<'_>, item_def_id: DefId, attr: &Attribute) { + let param_env = tcx.param_env(item_def_id); + let ty = tcx.type_of(item_def_id).instantiate_identity(); + let meta_items = attr.meta_item_list().unwrap_or_default(); + for meta_item in meta_items { + match meta_item.name_or_empty() { + sym::debug => { + let ty::FnPtr(sig) = ty.kind() else { + span_bug!( + meta_item.span(), + "`#[rustc_abi(debug)]` on a type alias requires function pointer type" + ); + }; + let abi = unwrap_fn_abi( + tcx.fn_abi_of_fn_ptr(param_env.and((*sig, /* extra_args */ ty::List::empty()))), + tcx, + item_def_id, + ); + + let fn_name = tcx.item_name(item_def_id); + tcx.sess.emit_err(AbiOf { + span: tcx.def_span(item_def_id), + fn_name, + fn_abi: format!("{:#?}", abi), + }); + } + name => { + tcx.sess.emit_err(UnrecognizedField { span: meta_item.span(), name }); + } } } } diff --git a/tests/ui/abi/debug.rs b/tests/ui/abi/debug.rs index 13464be275e7c..84d08ead08df3 100644 --- a/tests/ui/abi/debug.rs +++ b/tests/ui/abi/debug.rs @@ -12,6 +12,8 @@ #[rustc_abi(debug)] fn test(_x: u8) -> bool { true } //~ ERROR: fn_abi +#[rustc_abi(debug)] +type TestFnPtr = fn(bool) -> u8; //~ ERROR: fn_abi #[rustc_abi(debug)] fn test_generic(_x: *const T) { } //~ ERROR: fn_abi diff --git a/tests/ui/abi/debug.stderr b/tests/ui/abi/debug.stderr index 4f4ee3de4b8a0..dd0cfea351faa 100644 --- a/tests/ui/abi/debug.stderr +++ b/tests/ui/abi/debug.stderr @@ -1,4 +1,4 @@ -error: fn_abi_of_instance(test) = FnAbi { +error: fn_abi_of(test) = FnAbi { args: [ ArgAbi { layout: TyAndLayout { @@ -92,7 +92,101 @@ error: fn_abi_of_instance(test) = FnAbi { LL | fn test(_x: u8) -> bool { true } | ^^^^^^^^^^^^^^^^^^^^^^^ -error: fn_abi_of_instance(test_generic) = FnAbi { +error: fn_abi_of(TestFnPtr) = FnAbi { + args: [ + ArgAbi { + layout: TyAndLayout { + ty: bool, + layout: Layout { + size: Size(1 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Scalar( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + ), + fields: Primitive, + largest_niche: Some( + Niche { + offset: Size(0 bytes), + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + ), + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Direct( + ArgAttributes { + regular: NoUndef, + arg_ext: Zext, + pointee_size: Size(0 bytes), + pointee_align: None, + }, + ), + }, + ], + ret: ArgAbi { + layout: TyAndLayout { + ty: u8, + layout: Layout { + size: Size(1 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Scalar( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=255, + }, + ), + fields: Primitive, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Direct( + ArgAttributes { + regular: NoUndef, + arg_ext: None, + pointee_size: Size(0 bytes), + pointee_align: None, + }, + ), + }, + c_variadic: false, + fixed_count: 1, + conv: Rust, + can_unwind: $SOME_BOOL, + } + --> $DIR/debug.rs:16:1 + | +LL | type TestFnPtr = fn(bool) -> u8; + | ^^^^^^^^^^^^^^ + +error: fn_abi_of(test_generic) = FnAbi { args: [ ArgAbi { layout: TyAndLayout { @@ -163,12 +257,12 @@ error: fn_abi_of_instance(test_generic) = FnAbi { conv: Rust, can_unwind: $SOME_BOOL, } - --> $DIR/debug.rs:17:1 + --> $DIR/debug.rs:19:1 | LL | fn test_generic(_x: *const T) { } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: fn_abi_of_instance(assoc_test) = FnAbi { +error: fn_abi_of(assoc_test) = FnAbi { args: [ ArgAbi { layout: TyAndLayout { @@ -251,10 +345,10 @@ error: fn_abi_of_instance(assoc_test) = FnAbi { conv: Rust, can_unwind: $SOME_BOOL, } - --> $DIR/debug.rs:22:5 + --> $DIR/debug.rs:24:5 | LL | fn assoc_test(&self) { } | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors From e66913f8fefd54a37c0a8b86b27b7b5117360be0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Sep 2023 14:03:25 +0200 Subject: [PATCH 02/12] rustc_layout/abi: error when attribute is applied to the wrong thing --- compiler/rustc_passes/messages.ftl | 25 ++++++++--------- compiler/rustc_passes/src/abi_test.rs | 31 ++++++++++++--------- compiler/rustc_passes/src/errors.rs | 30 ++++++++++++++------ compiler/rustc_passes/src/layout_test.rs | 35 ++++++++++++++++-------- tests/ui/abi/debug.rs | 11 +++++++- tests/ui/abi/debug.stderr | 22 +++++++++++---- tests/ui/layout/debug.rs | 11 ++++++-- tests/ui/layout/debug.stderr | 32 +++++++++++++++------- 8 files changed, 134 insertions(+), 63 deletions(-) diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index aa7db3348d028..602d614295c18 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -4,15 +4,11 @@ -passes_see_issue = see issue #{$issue} for more information -passes_abi = - abi: {$abi} - +passes_abi_invalid_attribute = + `#[rustc_abi]` can only be applied to function items, type aliases, and associated functions passes_abi_of = fn_abi_of({$fn_name}) = {$fn_abi} -passes_align = - align: {$align} - passes_allow_incoherent_impl = `rustc_allow_incoherent_impl` attribute should be applied to impl items. .label = the only currently supported targets are inherent methods @@ -318,9 +314,6 @@ passes_has_incoherent_inherent_impl = `rustc_has_incoherent_inherent_impls` attribute should be applied to types or traits. .label = only adts, extern types and traits are supported -passes_homogeneous_aggregate = - homogeneous_aggregate: {$homogeneous_aggregate} - passes_ignored_attr = `#[{$sym}]` is ignored on struct fields and match arms .warn = {-passes_previously_accepted} @@ -404,9 +397,18 @@ passes_lang_item_on_incorrect_target = passes_layout = layout error: {$layout_error} - +passes_layout_abi = + abi: {$abi} +passes_layout_align = + align: {$align} +passes_layout_homogeneous_aggregate = + homogeneous_aggregate: {$homogeneous_aggregate} +passes_layout_invalid_attribute = + `#[rustc_layout]` can only be applied to `struct`/`enum`/`union` declarations and type aliases passes_layout_of = layout_of({$normalized_ty}) = {$ty_layout} +passes_layout_size = + size: {$size} passes_link = attribute should be applied to an `extern` block with non-Rust ABI @@ -662,9 +664,6 @@ passes_should_be_applied_to_trait = attribute should be applied to a trait .label = not a trait -passes_size = - size: {$size} - passes_skipping_const_checks = skipping const checks passes_stability_promotable = diff --git a/compiler/rustc_passes/src/abi_test.rs b/compiler/rustc_passes/src/abi_test.rs index aeaa264216dac..4653a9c2b39da 100644 --- a/compiler/rustc_passes/src/abi_test.rs +++ b/compiler/rustc_passes/src/abi_test.rs @@ -7,7 +7,7 @@ use rustc_span::source_map::Spanned; use rustc_span::symbol::sym; use rustc_target::abi::call::FnAbi; -use crate::errors::{AbiOf, UnrecognizedField}; +use crate::errors::{AbiInvalidAttribute, AbiOf, UnrecognizedField}; pub fn test_abi(tcx: TyCtxt<'_>) { if !tcx.features().rustc_attrs { @@ -15,28 +15,33 @@ pub fn test_abi(tcx: TyCtxt<'_>) { return; } for id in tcx.hir().items() { - match tcx.def_kind(id.owner_id) { - DefKind::Fn => { - for attr in tcx.get_attrs(id.owner_id, sym::rustc_abi) { + for attr in tcx.get_attrs(id.owner_id, sym::rustc_abi) { + match tcx.def_kind(id.owner_id) { + DefKind::Fn => { dump_abi_of_fn_item(tcx, id.owner_id.def_id.into(), attr); } - } - DefKind::TyAlias { .. } => { - for attr in tcx.get_attrs(id.owner_id, sym::rustc_abi) { + DefKind::TyAlias { .. } => { dump_abi_of_fn_type(tcx, id.owner_id.def_id.into(), attr); } + _ => { + tcx.sess.emit_err(AbiInvalidAttribute { span: tcx.def_span(id.owner_id) }); + } } - DefKind::Impl { .. } => { - // To find associated functions we need to go into the child items here. - for &id in tcx.associated_item_def_ids(id.owner_id) { - if matches!(tcx.def_kind(id), DefKind::AssocFn) { - for attr in tcx.get_attrs(id, sym::rustc_abi) { + } + if matches!(tcx.def_kind(id.owner_id), DefKind::Impl { .. }) { + // To find associated functions we need to go into the child items here. + for &id in tcx.associated_item_def_ids(id.owner_id) { + for attr in tcx.get_attrs(id, sym::rustc_abi) { + match tcx.def_kind(id) { + DefKind::AssocFn => { dump_abi_of_fn_item(tcx, id, attr); } + _ => { + tcx.sess.emit_err(AbiInvalidAttribute { span: tcx.def_span(id) }); + } } } } - _ => {} } } } diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 32dd02a4aa946..de55b5ec2cbda 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -873,32 +873,32 @@ pub struct DuplicateDiagnosticItemInCrate { } #[derive(Diagnostic)] -#[diag(passes_abi)] -pub struct Abi { +#[diag(passes_layout_abi)] +pub struct LayoutAbi { #[primary_span] pub span: Span, pub abi: String, } #[derive(Diagnostic)] -#[diag(passes_align)] -pub struct Align { +#[diag(passes_layout_align)] +pub struct LayoutAlign { #[primary_span] pub span: Span, pub align: String, } #[derive(Diagnostic)] -#[diag(passes_size)] -pub struct Size { +#[diag(passes_layout_size)] +pub struct LayoutSize { #[primary_span] pub span: Span, pub size: String, } #[derive(Diagnostic)] -#[diag(passes_homogeneous_aggregate)] -pub struct HomogeneousAggregate { +#[diag(passes_layout_homogeneous_aggregate)] +pub struct LayoutHomogeneousAggregate { #[primary_span] pub span: Span, pub homogeneous_aggregate: String, @@ -913,6 +913,13 @@ pub struct LayoutOf { pub ty_layout: String, } +#[derive(Diagnostic)] +#[diag(passes_layout_invalid_attribute)] +pub struct LayoutInvalidAttribute { + #[primary_span] + pub span: Span, +} + #[derive(Diagnostic)] #[diag(passes_abi_of)] pub struct AbiOf { @@ -922,6 +929,13 @@ pub struct AbiOf { pub fn_abi: String, } +#[derive(Diagnostic)] +#[diag(passes_abi_invalid_attribute)] +pub struct AbiInvalidAttribute { + #[primary_span] + pub span: Span, +} + #[derive(Diagnostic)] #[diag(passes_unrecognized_field)] pub struct UnrecognizedField { diff --git a/compiler/rustc_passes/src/layout_test.rs b/compiler/rustc_passes/src/layout_test.rs index d839fee07a60d..f3c12e0746d97 100644 --- a/compiler/rustc_passes/src/layout_test.rs +++ b/compiler/rustc_passes/src/layout_test.rs @@ -8,7 +8,10 @@ use rustc_span::symbol::sym; use rustc_span::Span; use rustc_target::abi::{HasDataLayout, TargetDataLayout}; -use crate::errors::{Abi, Align, HomogeneousAggregate, LayoutOf, Size, UnrecognizedField}; +use crate::errors::{ + LayoutAbi, LayoutAlign, LayoutHomogeneousAggregate, LayoutInvalidAttribute, LayoutOf, + LayoutSize, UnrecognizedField, +}; pub fn test_layout(tcx: TyCtxt<'_>) { if !tcx.features().rustc_attrs { @@ -16,12 +19,22 @@ pub fn test_layout(tcx: TyCtxt<'_>) { return; } for id in tcx.hir().items() { - if matches!( - tcx.def_kind(id.owner_id), - DefKind::TyAlias { .. } | DefKind::Enum | DefKind::Struct | DefKind::Union - ) { - for attr in tcx.get_attrs(id.owner_id, sym::rustc_layout) { - dump_layout_of(tcx, id.owner_id.def_id, attr); + for attr in tcx.get_attrs(id.owner_id, sym::rustc_layout) { + match tcx.def_kind(id.owner_id) { + DefKind::TyAlias { .. } | DefKind::Enum | DefKind::Struct | DefKind::Union => { + dump_layout_of(tcx, id.owner_id.def_id, attr); + } + _ => { + tcx.sess.emit_err(LayoutInvalidAttribute { span: tcx.def_span(id.owner_id) }); + } + } + } + if matches!(tcx.def_kind(id.owner_id), DefKind::Impl { .. }) { + // To find associated functions we need to go into the child items here. + for &id in tcx.associated_item_def_ids(id.owner_id) { + for _attr in tcx.get_attrs(id, sym::rustc_layout) { + tcx.sess.emit_err(LayoutInvalidAttribute { span: tcx.def_span(id) }); + } } } } @@ -38,28 +51,28 @@ fn dump_layout_of(tcx: TyCtxt<'_>, item_def_id: LocalDefId, attr: &Attribute) { for meta_item in meta_items { match meta_item.name_or_empty() { sym::abi => { - tcx.sess.emit_err(Abi { + tcx.sess.emit_err(LayoutAbi { span: tcx.def_span(item_def_id.to_def_id()), abi: format!("{:?}", ty_layout.abi), }); } sym::align => { - tcx.sess.emit_err(Align { + tcx.sess.emit_err(LayoutAlign { span: tcx.def_span(item_def_id.to_def_id()), align: format!("{:?}", ty_layout.align), }); } sym::size => { - tcx.sess.emit_err(Size { + tcx.sess.emit_err(LayoutSize { span: tcx.def_span(item_def_id.to_def_id()), size: format!("{:?}", ty_layout.size), }); } sym::homogeneous_aggregate => { - tcx.sess.emit_err(HomogeneousAggregate { + tcx.sess.emit_err(LayoutHomogeneousAggregate { span: tcx.def_span(item_def_id.to_def_id()), homogeneous_aggregate: format!( "{:?}", diff --git a/tests/ui/abi/debug.rs b/tests/ui/abi/debug.rs index 84d08ead08df3..8575021494d61 100644 --- a/tests/ui/abi/debug.rs +++ b/tests/ui/abi/debug.rs @@ -9,6 +9,8 @@ #![feature(rustc_attrs)] #![crate_type = "lib"] +struct S(u16); + #[rustc_abi(debug)] fn test(_x: u8) -> bool { true } //~ ERROR: fn_abi @@ -18,7 +20,14 @@ type TestFnPtr = fn(bool) -> u8; //~ ERROR: fn_abi #[rustc_abi(debug)] fn test_generic(_x: *const T) { } //~ ERROR: fn_abi -struct S(u16); +#[rustc_abi(debug)] +const C: () = (); //~ ERROR: can only be applied to + +impl S { + #[rustc_abi(debug)] + const C: () = (); //~ ERROR: can only be applied to +} + impl S { #[rustc_abi(debug)] fn assoc_test(&self) { } //~ ERROR: fn_abi diff --git a/tests/ui/abi/debug.stderr b/tests/ui/abi/debug.stderr index dd0cfea351faa..2315288c3c750 100644 --- a/tests/ui/abi/debug.stderr +++ b/tests/ui/abi/debug.stderr @@ -87,7 +87,7 @@ error: fn_abi_of(test) = FnAbi { conv: Rust, can_unwind: $SOME_BOOL, } - --> $DIR/debug.rs:13:1 + --> $DIR/debug.rs:15:1 | LL | fn test(_x: u8) -> bool { true } | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -181,7 +181,7 @@ error: fn_abi_of(TestFnPtr) = FnAbi { conv: Rust, can_unwind: $SOME_BOOL, } - --> $DIR/debug.rs:16:1 + --> $DIR/debug.rs:18:1 | LL | type TestFnPtr = fn(bool) -> u8; | ^^^^^^^^^^^^^^ @@ -257,11 +257,23 @@ error: fn_abi_of(test_generic) = FnAbi { conv: Rust, can_unwind: $SOME_BOOL, } - --> $DIR/debug.rs:19:1 + --> $DIR/debug.rs:21:1 | LL | fn test_generic(_x: *const T) { } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +error: `#[rustc_abi]` can only be applied to function items, type aliases, and associated functions + --> $DIR/debug.rs:24:1 + | +LL | const C: () = (); + | ^^^^^^^^^^^ + +error: `#[rustc_abi]` can only be applied to function items, type aliases, and associated functions + --> $DIR/debug.rs:28:5 + | +LL | const C: () = (); + | ^^^^^^^^^^^ + error: fn_abi_of(assoc_test) = FnAbi { args: [ ArgAbi { @@ -345,10 +357,10 @@ error: fn_abi_of(assoc_test) = FnAbi { conv: Rust, can_unwind: $SOME_BOOL, } - --> $DIR/debug.rs:24:5 + --> $DIR/debug.rs:33:5 | LL | fn assoc_test(&self) { } | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 4 previous errors +error: aborting due to 6 previous errors diff --git a/tests/ui/layout/debug.rs b/tests/ui/layout/debug.rs index b74a8d3b91730..97dc73d3aa70c 100644 --- a/tests/ui/layout/debug.rs +++ b/tests/ui/layout/debug.rs @@ -17,6 +17,9 @@ type Test = Result; //~ ERROR: layout_of #[rustc_layout(debug)] type T = impl std::fmt::Debug; //~ ERROR: layout_of +fn f() -> T { + 0i32 +} #[rustc_layout(debug)] pub union V { //~ ERROR: layout_of @@ -63,6 +66,10 @@ union P5 { zst: [u16; 0], byte: u8 } //~ ERROR: layout_of #[rustc_layout(debug)] type X = std::mem::MaybeUninit; //~ ERROR: layout_of -fn f() -> T { - 0i32 +#[rustc_layout(debug)] +const C: () = (); //~ ERROR: can only be applied to + +impl S { + #[rustc_layout(debug)] + const C: () = (); //~ ERROR: can only be applied to } diff --git a/tests/ui/layout/debug.stderr b/tests/ui/layout/debug.stderr index c20a0198ccb8b..0973043c67814 100644 --- a/tests/ui/layout/debug.stderr +++ b/tests/ui/layout/debug.stderr @@ -344,7 +344,7 @@ error: layout_of(V) = Layout { max_repr_align: None, unadjusted_abi_align: Align(2 bytes), } - --> $DIR/debug.rs:22:1 + --> $DIR/debug.rs:25:1 | LL | pub union V { | ^^^^^^^^^^^ @@ -368,7 +368,7 @@ error: layout_of(W) = Layout { max_repr_align: None, unadjusted_abi_align: Align(2 bytes), } - --> $DIR/debug.rs:28:1 + --> $DIR/debug.rs:31:1 | LL | pub union W { | ^^^^^^^^^^^ @@ -392,7 +392,7 @@ error: layout_of(Y) = Layout { max_repr_align: None, unadjusted_abi_align: Align(2 bytes), } - --> $DIR/debug.rs:34:1 + --> $DIR/debug.rs:37:1 | LL | pub union Y { | ^^^^^^^^^^^ @@ -416,7 +416,7 @@ error: layout_of(P1) = Layout { max_repr_align: None, unadjusted_abi_align: Align(1 bytes), } - --> $DIR/debug.rs:41:1 + --> $DIR/debug.rs:44:1 | LL | union P1 { x: u32 } | ^^^^^^^^ @@ -440,7 +440,7 @@ error: layout_of(P2) = Layout { max_repr_align: None, unadjusted_abi_align: Align(1 bytes), } - --> $DIR/debug.rs:45:1 + --> $DIR/debug.rs:48:1 | LL | union P2 { x: (u32, u32) } | ^^^^^^^^ @@ -464,7 +464,7 @@ error: layout_of(P3) = Layout { max_repr_align: None, unadjusted_abi_align: Align(1 bytes), } - --> $DIR/debug.rs:53:1 + --> $DIR/debug.rs:56:1 | LL | union P3 { x: F32x4 } | ^^^^^^^^ @@ -488,7 +488,7 @@ error: layout_of(P4) = Layout { max_repr_align: None, unadjusted_abi_align: Align(1 bytes), } - --> $DIR/debug.rs:57:1 + --> $DIR/debug.rs:60:1 | LL | union P4 { x: E } | ^^^^^^^^ @@ -517,7 +517,7 @@ error: layout_of(P5) = Layout { max_repr_align: None, unadjusted_abi_align: Align(1 bytes), } - --> $DIR/debug.rs:61:1 + --> $DIR/debug.rs:64:1 | LL | union P5 { zst: [u16; 0], byte: u8 } | ^^^^^^^^ @@ -546,10 +546,22 @@ error: layout_of(std::mem::MaybeUninit) = Layout { max_repr_align: None, unadjusted_abi_align: Align(1 bytes), } - --> $DIR/debug.rs:64:1 + --> $DIR/debug.rs:67:1 | LL | type X = std::mem::MaybeUninit; | ^^^^^^ -error: aborting due to 14 previous errors +error: `#[rustc_layout]` can only be applied to `struct`/`enum`/`union` declarations and type aliases + --> $DIR/debug.rs:70:1 + | +LL | const C: () = (); + | ^^^^^^^^^^^ + +error: `#[rustc_layout]` can only be applied to `struct`/`enum`/`union` declarations and type aliases + --> $DIR/debug.rs:74:5 + | +LL | const C: () = (); + | ^^^^^^^^^^^ + +error: aborting due to 16 previous errors From c9810261956faa1152ade6a37efe6583b2caaa20 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 6 Sep 2023 11:12:23 +0200 Subject: [PATCH 03/12] extend comments around PassMode::Direct --- compiler/rustc_codegen_llvm/src/abi.rs | 42 ++++++++++++++++++++++- compiler/rustc_target/src/abi/call/mod.rs | 4 ++- compiler/rustc_ty_utils/src/abi.rs | 2 ++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index 863cb7068f865..1e72b7db1e4e7 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -340,15 +340,53 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { }; for arg in args { + // Note that the exact number of arguments pushed here is carefully synchronized with + // code all over the place, both in the codegen_llvm and codegen_ssa crates. That's how + // other code then knows which LLVM argument(s) correspond to the n-th Rust argument. let llarg_ty = match &arg.mode { PassMode::Ignore => continue, - PassMode::Direct(_) => arg.layout.immediate_llvm_type(cx), + 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 { .. }) { + // This is the most critical case for ABI compatibility, since + // `immediate_llvm_type` will use `layout.fields` to turn this Rust type + // into an LLVM type. ABI-compatible Rust types can have different `fields`, + // so we need to be very sure that LLVM wil treat those different types in + // an ABI-compatible way. Mostly we do this by disallowing + // `PassMode::Direct` for aggregates, but we actually do use that mode on + // wasm. wasm doesn't have aggregate types so we are fairly sure that LLVM + // will treat `{ i32, i32, i32 }` and `{ { i32, i32, i32 } }` the same way + // for ABI purposes. + assert!( + matches!(&*cx.tcx.sess.target.arch, "wasm32" | "wasm64"), + "`PassMode::Direct` for aggregates only allowed on wasm targets\nProblematic type: {:#?}", + arg.layout, + ); + } + 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: _, extra_attrs: Some(_), on_stack: _ } => { + assert!(arg.layout.is_unsized()); + // 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 + // alignment, so this respects ABI compatibility. let ptr_ty = Ty::new_mut_ptr(cx.tcx, arg.layout.ty); let ptr_layout = cx.layout_of(ptr_ty); llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 0, true)); @@ -360,6 +398,8 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { if *pad_i32 { llargument_tys.push(Reg::i32().llvm_type(cx)); } + // Compute the LLVM type we use for this function from the cast type. + // We assume here that ABI-compatible Rust types have the same cast type. cast.llvm_type(cx) } PassMode::Indirect { attrs: _, extra_attrs: None, on_stack: _ } => cx.type_ptr(), diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 8d573def9bb05..9a905dbc806d2 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -36,7 +36,7 @@ pub enum PassMode { Ignore, /// Pass the argument directly. /// - /// The argument has a layout abi of `Scalar`, `Vector` or in rare cases `Aggregate`. + /// The argument has a layout abi of `Scalar`, `Vector` or in rare cases (e.g. on wasm) `Aggregate`. Direct(ArgAttributes), /// Pass a pair's elements directly in two arguments. /// @@ -465,6 +465,7 @@ pub struct ArgAbi<'a, Ty> { } impl<'a, Ty> ArgAbi<'a, Ty> { + /// This defines the "default ABI" for that type, that is then later adjusted in `fn_abi_adjust_for_abi`. pub fn new( cx: &impl HasDataLayout, layout: TyAndLayout<'a, Ty>, @@ -478,6 +479,7 @@ impl<'a, Ty> ArgAbi<'a, Ty> { scalar_attrs(&layout, b, a.size(cx).align_to(b.align(cx).abi)), ), Abi::Vector { .. } => PassMode::Direct(ArgAttributes::new()), + // The `Aggregate` ABI is almost always adjusted later. Abi::Aggregate { .. } => PassMode::Direct(ArgAttributes::new()), }; ArgAbi { layout, mode } diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs index 802391f1aadea..3ffe670d87ac1 100644 --- a/compiler/rustc_ty_utils/src/abi.rs +++ b/compiler/rustc_ty_utils/src/abi.rs @@ -520,6 +520,8 @@ fn fn_abi_adjust_for_abi<'tcx>( _ => return, } + // `Aggregate` ABI must be adjusted to ensure that ABI-compatible Rust types are passed + // the same way. let size = arg.layout.size; if arg.layout.is_unsized() || size > Pointer(AddressSpace::DATA).size(cx) { From 8922c0c541c0fbfe6d0465eed6362a2bdbe6a0f0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 6 Sep 2023 11:13:20 +0200 Subject: [PATCH 04/12] add support for rustc_abi(assert_eq) and use it to test some repr(transparent) cases --- compiler/rustc_passes/messages.ftl | 4 + compiler/rustc_passes/src/abi_test.rs | 78 +++++++++++++- compiler/rustc_passes/src/errors.rs | 9 ++ compiler/rustc_span/src/symbol.rs | 1 + tests/codegen/repr/transparent.rs | 1 - tests/ui/abi/debug.rs | 6 ++ tests/ui/abi/debug.stderr | 148 +++++++++++++++++++++++++- tests/ui/abi/transparent.rs | 79 ++++++++++++++ 8 files changed, 322 insertions(+), 4 deletions(-) create mode 100644 tests/ui/abi/transparent.rs diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index 602d614295c18..9f3cd656bd100 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -6,6 +6,10 @@ passes_abi_invalid_attribute = `#[rustc_abi]` can only be applied to function items, type aliases, and associated functions +passes_abi_ne = + ABIs are not compatible + left ABI = {$left} + right ABI = {$right} passes_abi_of = fn_abi_of({$fn_name}) = {$fn_abi} diff --git a/compiler/rustc_passes/src/abi_test.rs b/compiler/rustc_passes/src/abi_test.rs index 4653a9c2b39da..48c18912703de 100644 --- a/compiler/rustc_passes/src/abi_test.rs +++ b/compiler/rustc_passes/src/abi_test.rs @@ -5,9 +5,9 @@ use rustc_middle::ty::layout::{FnAbiError, LayoutError}; use rustc_middle::ty::{self, GenericArgs, Instance, Ty, TyCtxt}; use rustc_span::source_map::Spanned; use rustc_span::symbol::sym; -use rustc_target::abi::call::FnAbi; +use rustc_target::abi::call::{ArgAbi, FnAbi}; -use crate::errors::{AbiInvalidAttribute, AbiOf, UnrecognizedField}; +use crate::errors::{AbiInvalidAttribute, AbiNe, AbiOf, UnrecognizedField}; pub fn test_abi(tcx: TyCtxt<'_>) { if !tcx.features().rustc_attrs { @@ -114,6 +114,32 @@ fn dump_abi_of_fn_item(tcx: TyCtxt<'_>, item_def_id: DefId, attr: &Attribute) { } } +fn test_arg_abi_eq<'tcx>( + abi1: &'tcx ArgAbi<'tcx, Ty<'tcx>>, + abi2: &'tcx ArgAbi<'tcx, Ty<'tcx>>, +) -> bool { + // Ideally we'd just compare the `mode`, but that is not enough -- for some modes LLVM will look + // at the type. Comparing the `mode` and `layout.abi` should catch basically everything though + // (except for tricky cases around unized types). + // This *is* overly strict (e.g. we compare the sign of integer `Primitive`s, or parts of `ArgAttributes` that do not affect ABI), + // but for the purpose of ensuring repr(transparent) ABI compatibility that is fine. + abi1.mode == abi2.mode && abi1.layout.abi == abi2.layout.abi +} + +fn test_abi_eq<'tcx>(abi1: &'tcx FnAbi<'tcx, Ty<'tcx>>, abi2: &'tcx FnAbi<'tcx, Ty<'tcx>>) -> bool { + if abi1.conv != abi2.conv + || abi1.args.len() != abi2.args.len() + || abi1.c_variadic != abi2.c_variadic + || abi1.fixed_count != abi2.fixed_count + || abi1.can_unwind != abi2.can_unwind + { + return false; + } + + test_arg_abi_eq(&abi1.ret, &abi2.ret) + && abi1.args.iter().zip(abi2.args.iter()).all(|(arg1, arg2)| test_arg_abi_eq(arg1, arg2)) +} + fn dump_abi_of_fn_type(tcx: TyCtxt<'_>, item_def_id: DefId, attr: &Attribute) { let param_env = tcx.param_env(item_def_id); let ty = tcx.type_of(item_def_id).instantiate_identity(); @@ -140,6 +166,54 @@ fn dump_abi_of_fn_type(tcx: TyCtxt<'_>, item_def_id: DefId, attr: &Attribute) { fn_abi: format!("{:#?}", abi), }); } + sym::assert_eq => { + let ty::Tuple(fields) = ty.kind() else { + span_bug!( + meta_item.span(), + "`#[rustc_abi(assert_eq)]` on a type alias requires pair type" + ); + }; + let [field1, field2] = ***fields else { + span_bug!( + meta_item.span(), + "`#[rustc_abi(assert_eq)]` on a type alias requires pair type" + ); + }; + let ty::FnPtr(sig1) = field1.kind() else { + span_bug!( + meta_item.span(), + "`#[rustc_abi(assert_eq)]` on a type alias requires pair of function pointer types" + ); + }; + let abi1 = unwrap_fn_abi( + tcx.fn_abi_of_fn_ptr( + param_env.and((*sig1, /* extra_args */ ty::List::empty())), + ), + tcx, + item_def_id, + ); + let ty::FnPtr(sig2) = field2.kind() else { + span_bug!( + meta_item.span(), + "`#[rustc_abi(assert_eq)]` on a type alias requires pair of function pointer types" + ); + }; + let abi2 = unwrap_fn_abi( + tcx.fn_abi_of_fn_ptr( + param_env.and((*sig2, /* extra_args */ ty::List::empty())), + ), + tcx, + item_def_id, + ); + + if !test_abi_eq(abi1, abi2) { + tcx.sess.emit_err(AbiNe { + span: tcx.def_span(item_def_id), + left: format!("{:#?}", abi1), + right: format!("{:#?}", abi2), + }); + } + } name => { tcx.sess.emit_err(UnrecognizedField { span: meta_item.span(), name }); } diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index de55b5ec2cbda..d7319ef91e03e 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -929,6 +929,15 @@ pub struct AbiOf { pub fn_abi: String, } +#[derive(Diagnostic)] +#[diag(passes_abi_ne)] +pub struct AbiNe { + #[primary_span] + pub span: Span, + pub left: String, + pub right: String, +} + #[derive(Diagnostic)] #[diag(passes_abi_invalid_attribute)] pub struct AbiInvalidAttribute { diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 656deebb5d06b..63070b67717de 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -387,6 +387,7 @@ symbols! { asm_sym, asm_unwind, assert, + assert_eq, assert_eq_macro, assert_inhabited, assert_macro, diff --git a/tests/codegen/repr/transparent.rs b/tests/codegen/repr/transparent.rs index b140fc719daf3..c5974248bb373 100644 --- a/tests/codegen/repr/transparent.rs +++ b/tests/codegen/repr/transparent.rs @@ -43,7 +43,6 @@ pub extern "C" fn test_WithZst(_: WithZst) -> WithZst { loop {} } #[repr(transparent)] pub struct WithZeroSizedArray(*const f32, [i8; 0]); -// Apparently we use i32* when newtype-unwrapping f32 pointers. Whatever. // CHECK: define{{.*}}ptr @test_WithZeroSizedArray(ptr noundef %_1) #[no_mangle] pub extern "C" fn test_WithZeroSizedArray(_: WithZeroSizedArray) -> WithZeroSizedArray { loop {} } diff --git a/tests/ui/abi/debug.rs b/tests/ui/abi/debug.rs index 8575021494d61..058c205abd8d6 100644 --- a/tests/ui/abi/debug.rs +++ b/tests/ui/abi/debug.rs @@ -32,3 +32,9 @@ impl S { #[rustc_abi(debug)] fn assoc_test(&self) { } //~ ERROR: fn_abi } + +#[rustc_abi(assert_eq)] +type TestAbiEq = (fn(bool), fn(bool)); + +#[rustc_abi(assert_eq)] +type TestAbiNe = (fn(u8), fn(u32)); //~ ERROR: ABIs are not compatible diff --git a/tests/ui/abi/debug.stderr b/tests/ui/abi/debug.stderr index 2315288c3c750..321ec8715c6bb 100644 --- a/tests/ui/abi/debug.stderr +++ b/tests/ui/abi/debug.stderr @@ -362,5 +362,151 @@ error: fn_abi_of(assoc_test) = FnAbi { LL | fn assoc_test(&self) { } | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 6 previous errors +error: ABIs are not compatible + left ABI = FnAbi { + args: [ + ArgAbi { + layout: TyAndLayout { + ty: u8, + layout: Layout { + size: Size(1 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Scalar( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=255, + }, + ), + fields: Primitive, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Direct( + ArgAttributes { + regular: NoUndef, + arg_ext: None, + pointee_size: Size(0 bytes), + pointee_align: None, + }, + ), + }, + ], + ret: ArgAbi { + layout: TyAndLayout { + ty: (), + layout: Layout { + size: Size(0 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Ignore, + }, + c_variadic: false, + fixed_count: 1, + conv: Rust, + can_unwind: $SOME_BOOL, + } + right ABI = FnAbi { + args: [ + ArgAbi { + layout: TyAndLayout { + ty: u32, + layout: Layout { + size: $SOME_SIZE, + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Scalar( + Initialized { + value: Int( + I32, + false, + ), + valid_range: $FULL, + }, + ), + fields: Primitive, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Direct( + ArgAttributes { + regular: NoUndef, + arg_ext: None, + pointee_size: Size(0 bytes), + pointee_align: None, + }, + ), + }, + ], + ret: ArgAbi { + layout: TyAndLayout { + ty: (), + layout: Layout { + size: Size(0 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Ignore, + }, + c_variadic: false, + fixed_count: 1, + conv: Rust, + can_unwind: $SOME_BOOL, + } + --> $DIR/debug.rs:40:1 + | +LL | type TestAbiNe = (fn(u8), fn(u32)); + | ^^^^^^^^^^^^^^ + +error: aborting due to 7 previous errors diff --git a/tests/ui/abi/transparent.rs b/tests/ui/abi/transparent.rs new file mode 100644 index 0000000000000..90bdc129a4aa0 --- /dev/null +++ b/tests/ui/abi/transparent.rs @@ -0,0 +1,79 @@ +// check-pass +#![feature(rustc_attrs)] +#![allow(unused, improper_ctypes_definitions)] + +use std::marker::PhantomData; + +macro_rules! assert_abi_compatible { + ($name:ident, $t1:ty, $t2:ty) => { + mod $name { + use super::*; + // Test argument and return value, `Rust` and `C` ABIs. + #[rustc_abi(assert_eq)] + type TestRust = (fn($t1) -> $t1, fn($t2) -> $t2); + #[rustc_abi(assert_eq)] + type TestC = (extern "C" fn($t1) -> $t1, extern "C" fn($t2) -> $t2); + } + } +} + +#[derive(Copy, Clone)] +struct Zst; + +// Check that various `transparent` wrappers result in equal ABIs. +#[repr(transparent)] +struct Wrapper1(T); +#[repr(transparent)] +struct Wrapper2((), Zst, T); +#[repr(transparent)] +struct Wrapper3(T, [u8; 0], PhantomData); + +#[repr(C)] +struct ReprCStruct(T, f32, i32, T); +#[repr(C)] +enum ReprCEnum { + Variant1, + Variant2(T), +} +#[repr(C)] +union ReprCUnion { + nothing: (), + something: T, +} + +macro_rules! test_transparent { + ($name:ident, $t:ty) => { + mod $name { + use super::*; + assert_abi_compatible!(wrap1, $t, Wrapper1<$t>); + assert_abi_compatible!(wrap2, $t, Wrapper2<$t>); + assert_abi_compatible!(wrap3, $t, Wrapper3<$t>); + // Also try adding some surrounding `repr(C)` types. + assert_abi_compatible!(repr_c_struct_wrap1, ReprCStruct<$t>, ReprCStruct>); + assert_abi_compatible!(repr_c_enum_wrap1, ReprCEnum<$t>, ReprCEnum>); + assert_abi_compatible!(repr_c_union_wrap1, ReprCUnion<$t>, ReprCUnion>); + assert_abi_compatible!(repr_c_struct_wrap2, ReprCStruct<$t>, ReprCStruct>); + assert_abi_compatible!(repr_c_enum_wrap2, ReprCEnum<$t>, ReprCEnum>); + assert_abi_compatible!(repr_c_union_wrap2, ReprCUnion<$t>, ReprCUnion>); + assert_abi_compatible!(repr_c_struct_wrap3, ReprCStruct<$t>, ReprCStruct>); + assert_abi_compatible!(repr_c_enum_wrap3, ReprCEnum<$t>, ReprCEnum>); + assert_abi_compatible!(repr_c_union_wrap3, ReprCUnion<$t>, ReprCUnion>); + } + } +} + +test_transparent!(simple, i32); +test_transparent!(reference, &'static i32); +test_transparent!(zst, Zst); +test_transparent!(unit, ()); +test_transparent!(pair, (i32, f32)); +test_transparent!(triple, (i8, i16, f32)); // chosen to fit into 64bit +test_transparent!(tuple, (i32, f32, i64, f64)); +test_transparent!(empty_array, [u32; 0]); +test_transparent!(empty_1zst_array, [u8; 0]); +test_transparent!(small_array, [i32; 2]); // chosen to fit into 64bit +test_transparent!(large_array, [i32; 16]); +test_transparent!(enum_, Option); +test_transparent!(enum_niched, Option<&'static i32>); + +fn main() {} From c3e14edd8b7cbb502002a2390bf0ca5c6d09a315 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Sep 2023 11:59:31 +0200 Subject: [PATCH 05/12] accept some differences for rustc_abi(assert_eq), so that we can test more things to be compatible --- compiler/rustc_abi/src/lib.rs | 17 + .../src/interpret/terminator.rs | 70 +---- compiler/rustc_passes/src/abi_test.rs | 4 +- compiler/rustc_target/src/abi/call/mod.rs | 48 +++ tests/ui/abi/compatibility.rs | 76 +++++ tests/ui/abi/debug.rs | 7 + tests/ui/abi/debug.stderr | 291 +++++++++++++++++- 7 files changed, 449 insertions(+), 64 deletions(-) create mode 100644 tests/ui/abi/compatibility.rs diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index 571aaf631bd02..f9393539ea457 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -1348,6 +1348,23 @@ impl Abi { Abi::Uninhabited | Abi::Aggregate { .. } => Abi::Aggregate { sized: true }, } } + + pub fn eq_up_to_validity(&self, other: &Self) -> bool { + match (self, other) { + // Scalar, Vector, ScalarPair have `Scalar` in them where we ignore validity ranges. + // We do *not* ignore the sign since it matters for some ABIs (e.g. s390x). + (Abi::Scalar(l), Abi::Scalar(r)) => l.primitive() == r.primitive(), + ( + Abi::Vector { element: element_l, count: count_l }, + Abi::Vector { element: element_r, count: count_r }, + ) => element_l.primitive() == element_r.primitive() && count_l == count_r, + (Abi::ScalarPair(l1, l2), Abi::ScalarPair(r1, r2)) => { + l1.primitive() == r1.primitive() && l2.primitive() == r2.primitive() + } + // Everything else must be strictly identical. + _ => self == other, + } + } } #[derive(PartialEq, Eq, Hash, Clone, Debug)] diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index bc4edf1c4b60b..6312dc7414da0 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -10,7 +10,7 @@ use rustc_middle::{ Instance, Ty, }, }; -use rustc_target::abi::call::{ArgAbi, ArgAttribute, ArgAttributes, FnAbi, PassMode}; +use rustc_target::abi::call::{ArgAbi, FnAbi, PassMode}; use rustc_target::abi::{self, FieldIdx}; use rustc_target::spec::abi::Abi; @@ -291,32 +291,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { return true; } - match (caller_layout.abi, callee_layout.abi) { - // If both sides have Scalar/Vector/ScalarPair ABI, we can easily directly compare them. - // Different valid ranges are okay (the validity check will complain if this leads to - // invalid transmutes). Different signs are *not* okay on some targets (e.g. `extern - // "C"` on `s390x` where small integers are passed zero/sign-extended in large - // registers), so we generally reject them to increase portability. + match caller_layout.abi { + // For Scalar/Vector/ScalarPair ABI, we directly compare them. // NOTE: this is *not* a stable guarantee! It just reflects a property of our current // ABIs. It's also fragile; the same pair of types might be considered ABI-compatible // when used directly by-value but not considered compatible as a struct field or array // element. - (abi::Abi::Scalar(caller), abi::Abi::Scalar(callee)) => { - caller.primitive() == callee.primitive() + abi::Abi::Scalar(..) | abi::Abi::ScalarPair(..) | abi::Abi::Vector { .. } => { + caller_layout.abi.eq_up_to_validity(&callee_layout.abi) } - ( - abi::Abi::Vector { element: caller_element, count: caller_count }, - abi::Abi::Vector { element: callee_element, count: callee_count }, - ) => { - caller_element.primitive() == callee_element.primitive() - && caller_count == callee_count - } - (abi::Abi::ScalarPair(caller1, caller2), abi::Abi::ScalarPair(callee1, callee2)) => { - caller1.primitive() == callee1.primitive() - && caller2.primitive() == callee2.primitive() - } - (abi::Abi::Aggregate { .. }, abi::Abi::Aggregate { .. }) => { - // Aggregates are compatible only if they newtype-wrap the same type, or if they are both 1-ZST. + _ => { + // Everything else is compatible only if they newtype-wrap the same type, or if they are both 1-ZST. // (The latter part is needed to ensure e.g. that `struct Zst` is compatible with `struct Wrap((), Zst)`.) // This is conservative, but also means that our check isn't quite so heavily dependent on the `PassMode`, // which means having ABI-compatibility on one target is much more likely to imply compatibility for other targets. @@ -329,9 +314,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { == self.unfold_transparent(callee_layout).ty } } - // What remains is `Abi::Uninhabited` (which can never be passed anyway) and - // mismatching ABIs, that should all be rejected. - _ => false, } } @@ -340,40 +322,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { caller_abi: &ArgAbi<'tcx, Ty<'tcx>>, callee_abi: &ArgAbi<'tcx, Ty<'tcx>>, ) -> bool { - // When comparing the PassMode, we have to be smart about comparing the attributes. - let arg_attr_compat = |a1: &ArgAttributes, a2: &ArgAttributes| { - // There's only one regular attribute that matters for the call ABI: InReg. - // Everything else is things like noalias, dereferenceable, nonnull, ... - // (This also applies to pointee_size, pointee_align.) - if a1.regular.contains(ArgAttribute::InReg) != a2.regular.contains(ArgAttribute::InReg) - { - return false; - } - // We also compare the sign extension mode -- this could let the callee make assumptions - // about bits that conceptually were not even passed. - if a1.arg_ext != a2.arg_ext { - return false; - } - return true; - }; - let mode_compat = || match (&caller_abi.mode, &callee_abi.mode) { - (PassMode::Ignore, PassMode::Ignore) => true, // can still be reached for the return type - (PassMode::Direct(a1), PassMode::Direct(a2)) => arg_attr_compat(a1, a2), - (PassMode::Pair(a1, b1), PassMode::Pair(a2, b2)) => { - arg_attr_compat(a1, a2) && arg_attr_compat(b1, b2) - } - (PassMode::Cast(c1, pad1), PassMode::Cast(c2, pad2)) => c1 == c2 && pad1 == pad2, - ( - PassMode::Indirect { attrs: a1, extra_attrs: None, on_stack: s1 }, - PassMode::Indirect { attrs: a2, extra_attrs: None, on_stack: s2 }, - ) => arg_attr_compat(a1, a2) && s1 == s2, - ( - PassMode::Indirect { attrs: a1, extra_attrs: Some(e1), on_stack: s1 }, - PassMode::Indirect { attrs: a2, extra_attrs: Some(e2), on_stack: s2 }, - ) => arg_attr_compat(a1, a2) && arg_attr_compat(e1, e2) && s1 == s2, - _ => false, - }; - // Ideally `PassMode` would capture everything there is about argument passing, but that is // not the case: in `FnAbi::llvm_type`, also parts of the layout and type information are // used. So we need to check that *both* sufficiently agree to ensures the arguments are @@ -381,7 +329,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // For instance, `layout_compat` is needed to reject `i32` vs `f32`, which is not reflected // in `PassMode`. `mode_compat` is needed to reject `u8` vs `bool`, which have the same // `abi::Primitive` but different `arg_ext`. - if self.layout_compat(caller_abi.layout, callee_abi.layout) && mode_compat() { + if self.layout_compat(caller_abi.layout, callee_abi.layout) + && caller_abi.mode.eq_abi(&callee_abi.mode) + { // Something went very wrong if our checks don't even imply that the layout is the same. assert!( caller_abi.layout.size == callee_abi.layout.size diff --git a/compiler/rustc_passes/src/abi_test.rs b/compiler/rustc_passes/src/abi_test.rs index 48c18912703de..0c707b4ef9cbd 100644 --- a/compiler/rustc_passes/src/abi_test.rs +++ b/compiler/rustc_passes/src/abi_test.rs @@ -121,9 +121,7 @@ fn test_arg_abi_eq<'tcx>( // Ideally we'd just compare the `mode`, but that is not enough -- for some modes LLVM will look // at the type. Comparing the `mode` and `layout.abi` should catch basically everything though // (except for tricky cases around unized types). - // This *is* overly strict (e.g. we compare the sign of integer `Primitive`s, or parts of `ArgAttributes` that do not affect ABI), - // but for the purpose of ensuring repr(transparent) ABI compatibility that is fine. - abi1.mode == abi2.mode && abi1.layout.abi == abi2.layout.abi + abi1.mode.eq_abi(&abi2.mode) && abi1.layout.abi.eq_up_to_validity(&abi2.layout.abi) } fn test_abi_eq<'tcx>(abi1: &'tcx FnAbi<'tcx, Ty<'tcx>>, abi2: &'tcx FnAbi<'tcx, Ty<'tcx>>) -> bool { diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 9a905dbc806d2..42483b7c6a524 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -55,6 +55,28 @@ pub enum PassMode { Indirect { attrs: ArgAttributes, extra_attrs: Option, on_stack: bool }, } +impl PassMode { + /// Checks if these two `PassMode` are equal enough to be considered "the same for all + /// function call ABIs". + pub fn eq_abi(&self, other: &Self) -> bool { + match (self, other) { + (PassMode::Ignore, PassMode::Ignore) => true, // can still be reached for the return type + (PassMode::Direct(a1), PassMode::Direct(a2)) => a1.eq_abi(a2), + (PassMode::Pair(a1, b1), PassMode::Pair(a2, b2)) => a1.eq_abi(a2) && b1.eq_abi(b2), + (PassMode::Cast(c1, pad1), PassMode::Cast(c2, pad2)) => c1.eq_abi(c2) && pad1 == pad2, + ( + PassMode::Indirect { attrs: a1, extra_attrs: None, on_stack: s1 }, + PassMode::Indirect { attrs: a2, extra_attrs: None, on_stack: s2 }, + ) => a1.eq_abi(a2) && s1 == s2, + ( + PassMode::Indirect { attrs: a1, extra_attrs: Some(e1), on_stack: s1 }, + PassMode::Indirect { attrs: a2, extra_attrs: Some(e2), on_stack: s2 }, + ) => a1.eq_abi(a2) && e1.eq_abi(e2) && s1 == s2, + _ => false, + } + } +} + // Hack to disable non_upper_case_globals only for the bitflags! and not for the rest // of this module pub use attr_impl::ArgAttribute; @@ -127,6 +149,24 @@ impl ArgAttributes { pub fn contains(&self, attr: ArgAttribute) -> bool { self.regular.contains(attr) } + + /// Checks if these two `ArgAttributes` are equal enough to be considered "the same for all + /// function call ABIs". + pub fn eq_abi(&self, other: &Self) -> bool { + // There's only one regular attribute that matters for the call ABI: InReg. + // Everything else is things like noalias, dereferenceable, nonnull, ... + // (This also applies to pointee_size, pointee_align.) + if self.regular.contains(ArgAttribute::InReg) != other.regular.contains(ArgAttribute::InReg) + { + return false; + } + // We also compare the sign extension mode -- this could let the callee make assumptions + // about bits that conceptually were not even passed. + if self.arg_ext != other.arg_ext { + return false; + } + return true; + } } #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, HashStable_Generic)] @@ -272,6 +312,14 @@ impl CastTarget { acc.max(align) }) } + + /// Checks if these two `CastTarget` are equal enough to be considered "the same for all + /// function call ABIs". + pub fn eq_abi(&self, other: &Self) -> bool { + let CastTarget { prefix: prefix_l, rest: rest_l, attrs: attrs_l } = self; + let CastTarget { prefix: prefix_r, rest: rest_r, attrs: attrs_r } = other; + prefix_l == prefix_r && rest_l == rest_r && attrs_l.eq_abi(attrs_r) + } } /// Return value from the `homogeneous_aggregate` test function. diff --git a/tests/ui/abi/compatibility.rs b/tests/ui/abi/compatibility.rs new file mode 100644 index 0000000000000..0bf218730ba0f --- /dev/null +++ b/tests/ui/abi/compatibility.rs @@ -0,0 +1,76 @@ +// check-pass +#![feature(rustc_attrs)] +#![allow(unused, improper_ctypes_definitions)] +use std::num::NonZeroI32; +use std::ptr::NonNull; + +macro_rules! assert_abi_compatible { + ($name:ident, $t1:ty, $t2:ty) => { + mod $name { + use super::*; + // Test argument and return value, `Rust` and `C` ABIs. + #[rustc_abi(assert_eq)] + type TestRust = (fn($t1) -> $t1, fn($t2) -> $t2); + #[rustc_abi(assert_eq)] + type TestC = (extern "C" fn($t1) -> $t1, extern "C" fn($t2) -> $t2); + } + }; +} + +#[derive(Copy, Clone)] +struct Zst; + +#[repr(C)] +struct ReprC1(T); +#[repr(C)] +struct ReprC2Int(i32, T); +#[repr(C)] +struct ReprC2Float(f32, T); +#[repr(C)] +struct ReprC4(T, T, T, T); +#[repr(C)] +struct ReprC4Mixed(T, f32, i32, T); +#[repr(C)] +enum ReprCEnum { + Variant1, + Variant2(T), +} +#[repr(C)] +union ReprCUnion { + nothing: (), + something: T, +} + +macro_rules! test_abi_compatible { + ($name:ident, $t1:ty, $t2:ty) => { + mod $name { + use super::*; + assert_abi_compatible!(plain, $t1, $t2); + // We also do some tests with differences in fields of `repr(C)` types. + assert_abi_compatible!(repr_c_1, ReprC1<$t1>, ReprC1<$t2>); + assert_abi_compatible!(repr_c_2_int, ReprC2Int<$t1>, ReprC2Int<$t2>); + assert_abi_compatible!(repr_c_2_float, ReprC2Float<$t1>, ReprC2Float<$t2>); + assert_abi_compatible!(repr_c_4, ReprC4<$t1>, ReprC4<$t2>); + assert_abi_compatible!(repr_c_4mixed, ReprC4Mixed<$t1>, ReprC4Mixed<$t2>); + assert_abi_compatible!(repr_c_enum, ReprCEnum<$t1>, ReprCEnum<$t2>); + assert_abi_compatible!(repr_c_union, ReprCUnion<$t1>, ReprCUnion<$t2>); + } + }; +} + +// Compatibility of pointers is probably de-facto guaranteed, +// but that does not seem to be documented. +test_abi_compatible!(ptr_mut, *const i32, *mut i32); +test_abi_compatible!(ptr_pointee, *const i32, *const Vec); +test_abi_compatible!(ref_mut, &i32, &mut i32); +test_abi_compatible!(ref_ptr, &i32, *const i32); +test_abi_compatible!(box_ptr, Box, *const i32); +test_abi_compatible!(nonnull_ptr, NonNull, *const i32); +test_abi_compatible!(fn_fn, fn(), fn(i32) -> i32); + +// Some further guarantees we will likely (have to) make. +test_abi_compatible!(zst_unit, Zst, ()); +test_abi_compatible!(zst_array, Zst, [u8; 0]); +test_abi_compatible!(nonzero_int, NonZeroI32, i32); + +fn main() {} diff --git a/tests/ui/abi/debug.rs b/tests/ui/abi/debug.rs index 058c205abd8d6..d08945ce3c2f1 100644 --- a/tests/ui/abi/debug.rs +++ b/tests/ui/abi/debug.rs @@ -38,3 +38,10 @@ type TestAbiEq = (fn(bool), fn(bool)); #[rustc_abi(assert_eq)] type TestAbiNe = (fn(u8), fn(u32)); //~ ERROR: ABIs are not compatible + +#[rustc_abi(assert_eq)] +type TestAbiNeFloat = (fn(f32), fn(u32)); //~ ERROR: ABIs are not compatible + +// Sign matters on some targets (such as s390x), so let's make sure we never accept this. +#[rustc_abi(assert_eq)] +type TestAbiNeSign = (fn(i32), fn(u32)); //~ ERROR: ABIs are not compatible diff --git a/tests/ui/abi/debug.stderr b/tests/ui/abi/debug.stderr index 321ec8715c6bb..dffeef444f524 100644 --- a/tests/ui/abi/debug.stderr +++ b/tests/ui/abi/debug.stderr @@ -508,5 +508,294 @@ error: ABIs are not compatible LL | type TestAbiNe = (fn(u8), fn(u32)); | ^^^^^^^^^^^^^^ -error: aborting due to 7 previous errors +error: ABIs are not compatible + left ABI = FnAbi { + args: [ + ArgAbi { + layout: TyAndLayout { + ty: f32, + layout: Layout { + size: $SOME_SIZE, + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Scalar( + Initialized { + value: F32, + valid_range: $FULL, + }, + ), + fields: Primitive, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Direct( + ArgAttributes { + regular: NoUndef, + arg_ext: None, + pointee_size: Size(0 bytes), + pointee_align: None, + }, + ), + }, + ], + ret: ArgAbi { + layout: TyAndLayout { + ty: (), + layout: Layout { + size: Size(0 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Ignore, + }, + c_variadic: false, + fixed_count: 1, + conv: Rust, + can_unwind: $SOME_BOOL, + } + right ABI = FnAbi { + args: [ + ArgAbi { + layout: TyAndLayout { + ty: u32, + layout: Layout { + size: $SOME_SIZE, + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Scalar( + Initialized { + value: Int( + I32, + false, + ), + valid_range: $FULL, + }, + ), + fields: Primitive, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Direct( + ArgAttributes { + regular: NoUndef, + arg_ext: None, + pointee_size: Size(0 bytes), + pointee_align: None, + }, + ), + }, + ], + ret: ArgAbi { + layout: TyAndLayout { + ty: (), + layout: Layout { + size: Size(0 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Ignore, + }, + c_variadic: false, + fixed_count: 1, + conv: Rust, + can_unwind: $SOME_BOOL, + } + --> $DIR/debug.rs:43:1 + | +LL | type TestAbiNeFloat = (fn(f32), fn(u32)); + | ^^^^^^^^^^^^^^^^^^^ + +error: ABIs are not compatible + left ABI = FnAbi { + args: [ + ArgAbi { + layout: TyAndLayout { + ty: i32, + layout: Layout { + size: $SOME_SIZE, + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Scalar( + Initialized { + value: Int( + I32, + true, + ), + valid_range: $FULL, + }, + ), + fields: Primitive, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Direct( + ArgAttributes { + regular: NoUndef, + arg_ext: None, + pointee_size: Size(0 bytes), + pointee_align: None, + }, + ), + }, + ], + ret: ArgAbi { + layout: TyAndLayout { + ty: (), + layout: Layout { + size: Size(0 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Ignore, + }, + c_variadic: false, + fixed_count: 1, + conv: Rust, + can_unwind: $SOME_BOOL, + } + right ABI = FnAbi { + args: [ + ArgAbi { + layout: TyAndLayout { + ty: u32, + layout: Layout { + size: $SOME_SIZE, + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Scalar( + Initialized { + value: Int( + I32, + false, + ), + valid_range: $FULL, + }, + ), + fields: Primitive, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Direct( + ArgAttributes { + regular: NoUndef, + arg_ext: None, + pointee_size: Size(0 bytes), + pointee_align: None, + }, + ), + }, + ], + ret: ArgAbi { + layout: TyAndLayout { + ty: (), + layout: Layout { + size: Size(0 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Ignore, + }, + c_variadic: false, + fixed_count: 1, + conv: Rust, + can_unwind: $SOME_BOOL, + } + --> $DIR/debug.rs:47:1 + | +LL | type TestAbiNeSign = (fn(i32), fn(u32)); + | ^^^^^^^^^^^^^^^^^^ + +error: aborting due to 9 previous errors From 02217d1a1638ffd836afd6736d6cd0e4d4e469aa Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Sep 2023 13:19:40 +0200 Subject: [PATCH 06/12] add tests for RFC 3391 --- tests/ui/abi/compatibility.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/ui/abi/compatibility.rs b/tests/ui/abi/compatibility.rs index 0bf218730ba0f..064d765368e84 100644 --- a/tests/ui/abi/compatibility.rs +++ b/tests/ui/abi/compatibility.rs @@ -73,4 +73,29 @@ test_abi_compatible!(zst_unit, Zst, ()); test_abi_compatible!(zst_array, Zst, [u8; 0]); test_abi_compatible!(nonzero_int, NonZeroI32, i32); +// RFC 3391 . +macro_rules! test_nonnull { + ($name:ident, $t:ty) => { + mod $name { + use super::*; + test_abi_compatible!(option, Option<$t>, $t); + test_abi_compatible!(result_err_unit, Result<$t, ()>, $t); + test_abi_compatible!(result_ok_unit, Result<(), $t>, $t); + test_abi_compatible!(result_err_zst, Result<$t, Zst>, $t); + test_abi_compatible!(result_ok_zst, Result, $t); + test_abi_compatible!(result_err_arr, Result<$t, [i8; 0]>, $t); + test_abi_compatible!(result_ok_arr, Result<[i8; 0], $t>, $t); + } + } +} + +test_nonnull!(ref_, &i32); +test_nonnull!(mut_, &mut i32); +test_nonnull!(ref_unsized, &[i32]); +test_nonnull!(mut_unsized, &mut [i32]); +test_nonnull!(fn_, fn()); +test_nonnull!(nonnull, NonNull); +test_nonnull!(nonnull_unsized, NonNull); +test_nonnull!(non_zero, NonZeroI32); + fn main() {} From f6ef555823542d5e4193f7adf7e2debb5220a008 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Sep 2023 18:58:48 +0200 Subject: [PATCH 07/12] merge transparent-abi test into general abi compatibility test, and test repr(transparent) unions --- tests/ui/abi/compatibility.rs | 44 ++++++++++++++++++- tests/ui/abi/transparent.rs | 79 ----------------------------------- 2 files changed, 42 insertions(+), 81 deletions(-) delete mode 100644 tests/ui/abi/transparent.rs diff --git a/tests/ui/abi/compatibility.rs b/tests/ui/abi/compatibility.rs index 064d765368e84..cb82e7b6ee252 100644 --- a/tests/ui/abi/compatibility.rs +++ b/tests/ui/abi/compatibility.rs @@ -1,6 +1,7 @@ // check-pass -#![feature(rustc_attrs)] +#![feature(rustc_attrs, transparent_unions)] #![allow(unused, improper_ctypes_definitions)] +use std::marker::PhantomData; use std::num::NonZeroI32; use std::ptr::NonNull; @@ -27,7 +28,7 @@ struct ReprC2Int(i32, T); #[repr(C)] struct ReprC2Float(f32, T); #[repr(C)] -struct ReprC4(T, T, T, T); +struct ReprC4(T, Vec, Zst, T); #[repr(C)] struct ReprC4Mixed(T, f32, i32, T); #[repr(C)] @@ -73,6 +74,45 @@ test_abi_compatible!(zst_unit, Zst, ()); test_abi_compatible!(zst_array, Zst, [u8; 0]); test_abi_compatible!(nonzero_int, NonZeroI32, i32); +// `repr(transparent)` compatibility. +#[repr(transparent)] +struct Wrapper1(T); +#[repr(transparent)] +struct Wrapper2((), Zst, T); +#[repr(transparent)] +struct Wrapper3(T, [u8; 0], PhantomData); +#[repr(transparent)] +union WrapperUnion { + nothing: (), + something: T, +} + +macro_rules! test_transparent { + ($name:ident, $t:ty) => { + mod $name { + use super::*; + test_abi_compatible!(wrap1, $t, Wrapper1<$t>); + test_abi_compatible!(wrap2, $t, Wrapper2<$t>); + test_abi_compatible!(wrap3, $t, Wrapper3<$t>); + test_abi_compatible!(wrap4, $t, WrapperUnion<$t>); + } + }; +} + +test_transparent!(simple, i32); +test_transparent!(reference, &'static i32); +test_transparent!(zst, Zst); +test_transparent!(unit, ()); +test_transparent!(pair, (i32, f32)); // mixing in some floats since they often get special treatment +test_transparent!(triple, (i8, i16, f32)); // chosen to fit into 64bit +test_transparent!(tuple, (i32, f32, i64, f64)); +test_transparent!(empty_array, [u32; 0]); +test_transparent!(empty_1zst_array, [u8; 0]); +test_transparent!(small_array, [i32; 2]); // chosen to fit into 64bit +test_transparent!(large_array, [i32; 16]); +test_transparent!(enum_, Option); +test_transparent!(enum_niched, Option<&'static i32>); + // RFC 3391 . macro_rules! test_nonnull { ($name:ident, $t:ty) => { diff --git a/tests/ui/abi/transparent.rs b/tests/ui/abi/transparent.rs deleted file mode 100644 index 90bdc129a4aa0..0000000000000 --- a/tests/ui/abi/transparent.rs +++ /dev/null @@ -1,79 +0,0 @@ -// check-pass -#![feature(rustc_attrs)] -#![allow(unused, improper_ctypes_definitions)] - -use std::marker::PhantomData; - -macro_rules! assert_abi_compatible { - ($name:ident, $t1:ty, $t2:ty) => { - mod $name { - use super::*; - // Test argument and return value, `Rust` and `C` ABIs. - #[rustc_abi(assert_eq)] - type TestRust = (fn($t1) -> $t1, fn($t2) -> $t2); - #[rustc_abi(assert_eq)] - type TestC = (extern "C" fn($t1) -> $t1, extern "C" fn($t2) -> $t2); - } - } -} - -#[derive(Copy, Clone)] -struct Zst; - -// Check that various `transparent` wrappers result in equal ABIs. -#[repr(transparent)] -struct Wrapper1(T); -#[repr(transparent)] -struct Wrapper2((), Zst, T); -#[repr(transparent)] -struct Wrapper3(T, [u8; 0], PhantomData); - -#[repr(C)] -struct ReprCStruct(T, f32, i32, T); -#[repr(C)] -enum ReprCEnum { - Variant1, - Variant2(T), -} -#[repr(C)] -union ReprCUnion { - nothing: (), - something: T, -} - -macro_rules! test_transparent { - ($name:ident, $t:ty) => { - mod $name { - use super::*; - assert_abi_compatible!(wrap1, $t, Wrapper1<$t>); - assert_abi_compatible!(wrap2, $t, Wrapper2<$t>); - assert_abi_compatible!(wrap3, $t, Wrapper3<$t>); - // Also try adding some surrounding `repr(C)` types. - assert_abi_compatible!(repr_c_struct_wrap1, ReprCStruct<$t>, ReprCStruct>); - assert_abi_compatible!(repr_c_enum_wrap1, ReprCEnum<$t>, ReprCEnum>); - assert_abi_compatible!(repr_c_union_wrap1, ReprCUnion<$t>, ReprCUnion>); - assert_abi_compatible!(repr_c_struct_wrap2, ReprCStruct<$t>, ReprCStruct>); - assert_abi_compatible!(repr_c_enum_wrap2, ReprCEnum<$t>, ReprCEnum>); - assert_abi_compatible!(repr_c_union_wrap2, ReprCUnion<$t>, ReprCUnion>); - assert_abi_compatible!(repr_c_struct_wrap3, ReprCStruct<$t>, ReprCStruct>); - assert_abi_compatible!(repr_c_enum_wrap3, ReprCEnum<$t>, ReprCEnum>); - assert_abi_compatible!(repr_c_union_wrap3, ReprCUnion<$t>, ReprCUnion>); - } - } -} - -test_transparent!(simple, i32); -test_transparent!(reference, &'static i32); -test_transparent!(zst, Zst); -test_transparent!(unit, ()); -test_transparent!(pair, (i32, f32)); -test_transparent!(triple, (i8, i16, f32)); // chosen to fit into 64bit -test_transparent!(tuple, (i32, f32, i64, f64)); -test_transparent!(empty_array, [u32; 0]); -test_transparent!(empty_1zst_array, [u8; 0]); -test_transparent!(small_array, [i32; 2]); // chosen to fit into 64bit -test_transparent!(large_array, [i32; 16]); -test_transparent!(enum_, Option); -test_transparent!(enum_niched, Option<&'static i32>); - -fn main() {} From a53c6ee0ba53ba7c4424e8ec1ca08561a880a226 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Sep 2023 09:20:40 +0200 Subject: [PATCH 08/12] also ensure that size and alignment are the same --- compiler/rustc_passes/src/abi_test.rs | 10 +- tests/ui/abi/debug.rs | 3 + tests/ui/abi/debug.stderr | 154 +++++++++++++++++++++++++- 3 files changed, 161 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_passes/src/abi_test.rs b/compiler/rustc_passes/src/abi_test.rs index 0c707b4ef9cbd..82392e7267780 100644 --- a/compiler/rustc_passes/src/abi_test.rs +++ b/compiler/rustc_passes/src/abi_test.rs @@ -119,9 +119,13 @@ fn test_arg_abi_eq<'tcx>( abi2: &'tcx ArgAbi<'tcx, Ty<'tcx>>, ) -> bool { // Ideally we'd just compare the `mode`, but that is not enough -- for some modes LLVM will look - // at the type. Comparing the `mode` and `layout.abi` should catch basically everything though - // (except for tricky cases around unized types). - abi1.mode.eq_abi(&abi2.mode) && abi1.layout.abi.eq_up_to_validity(&abi2.layout.abi) + // at the type. Comparing the `mode` and `layout.abi` as well as size and alignment should catch + // basically everything though (except for tricky cases around unized types). + abi1.mode.eq_abi(&abi2.mode) + && abi1.layout.abi.eq_up_to_validity(&abi2.layout.abi) + && abi1.layout.size == abi2.layout.size + && abi1.layout.align.abi == abi2.layout.align.abi + && abi1.layout.is_sized() == abi2.layout.is_sized() } fn test_abi_eq<'tcx>(abi1: &'tcx FnAbi<'tcx, Ty<'tcx>>, abi2: &'tcx FnAbi<'tcx, Ty<'tcx>>) -> bool { diff --git a/tests/ui/abi/debug.rs b/tests/ui/abi/debug.rs index d08945ce3c2f1..9decb41d56517 100644 --- a/tests/ui/abi/debug.rs +++ b/tests/ui/abi/debug.rs @@ -39,6 +39,9 @@ type TestAbiEq = (fn(bool), fn(bool)); #[rustc_abi(assert_eq)] type TestAbiNe = (fn(u8), fn(u32)); //~ ERROR: ABIs are not compatible +#[rustc_abi(assert_eq)] +type TestAbiNeLarger = (fn([u8; 32]), fn([u32; 32])); //~ ERROR: ABIs are not compatible + #[rustc_abi(assert_eq)] type TestAbiNeFloat = (fn(f32), fn(u32)); //~ ERROR: ABIs are not compatible diff --git a/tests/ui/abi/debug.stderr b/tests/ui/abi/debug.stderr index dffeef444f524..0feaf0971d8d2 100644 --- a/tests/ui/abi/debug.stderr +++ b/tests/ui/abi/debug.stderr @@ -508,6 +508,154 @@ error: ABIs are not compatible LL | type TestAbiNe = (fn(u8), fn(u32)); | ^^^^^^^^^^^^^^ +error: ABIs are not compatible + left ABI = FnAbi { + args: [ + ArgAbi { + layout: TyAndLayout { + ty: [u8; 32], + layout: Layout { + size: Size(32 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Array { + stride: Size(1 bytes), + count: 32, + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Indirect { + attrs: ArgAttributes { + regular: NoAlias | NoCapture | NonNull | NoUndef, + arg_ext: None, + pointee_size: Size(32 bytes), + pointee_align: Some( + Align(1 bytes), + ), + }, + extra_attrs: None, + on_stack: false, + }, + }, + ], + ret: ArgAbi { + layout: TyAndLayout { + ty: (), + layout: Layout { + size: Size(0 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Ignore, + }, + c_variadic: false, + fixed_count: 1, + conv: Rust, + can_unwind: $SOME_BOOL, + } + right ABI = FnAbi { + args: [ + ArgAbi { + layout: TyAndLayout { + ty: [u32; 32], + layout: Layout { + size: Size(128 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Array { + stride: Size(4 bytes), + count: 32, + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Indirect { + attrs: ArgAttributes { + regular: NoAlias | NoCapture | NonNull | NoUndef, + arg_ext: None, + pointee_size: Size(128 bytes), + pointee_align: Some( + Align(4 bytes), + ), + }, + extra_attrs: None, + on_stack: false, + }, + }, + ], + ret: ArgAbi { + layout: TyAndLayout { + ty: (), + layout: Layout { + size: Size(0 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Ignore, + }, + c_variadic: false, + fixed_count: 1, + conv: Rust, + can_unwind: $SOME_BOOL, + } + --> $DIR/debug.rs:43:1 + | +LL | type TestAbiNeLarger = (fn([u8; 32]), fn([u32; 32])); + | ^^^^^^^^^^^^^^^^^^^^ + error: ABIs are not compatible left ABI = FnAbi { args: [ @@ -646,7 +794,7 @@ error: ABIs are not compatible conv: Rust, can_unwind: $SOME_BOOL, } - --> $DIR/debug.rs:43:1 + --> $DIR/debug.rs:46:1 | LL | type TestAbiNeFloat = (fn(f32), fn(u32)); | ^^^^^^^^^^^^^^^^^^^ @@ -792,10 +940,10 @@ error: ABIs are not compatible conv: Rust, can_unwind: $SOME_BOOL, } - --> $DIR/debug.rs:47:1 + --> $DIR/debug.rs:50:1 | LL | type TestAbiNeSign = (fn(i32), fn(u32)); | ^^^^^^^^^^^^^^^^^^ -error: aborting due to 9 previous errors +error: aborting due to 10 previous errors From 243ef313a52835e5e980ffcc2a6d3cfce849e33d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 6 Sep 2023 17:48:25 +0200 Subject: [PATCH 09/12] add a testcase for another MIPS64 bug --- tests/ui/abi/compatibility.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ui/abi/compatibility.rs b/tests/ui/abi/compatibility.rs index cb82e7b6ee252..c5c476ac46d2c 100644 --- a/tests/ui/abi/compatibility.rs +++ b/tests/ui/abi/compatibility.rs @@ -105,6 +105,7 @@ test_transparent!(zst, Zst); test_transparent!(unit, ()); test_transparent!(pair, (i32, f32)); // mixing in some floats since they often get special treatment test_transparent!(triple, (i8, i16, f32)); // chosen to fit into 64bit +test_transparent!(float_triple, (f64, f64, f64)); // hits a bug in our MIPS64 adjustments test_transparent!(tuple, (i32, f32, i64, f64)); test_transparent!(empty_array, [u32; 0]); test_transparent!(empty_1zst_array, [u8; 0]); From 28d152935e5f62ecbb6d2c99865de39c1993189b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 7 Sep 2023 16:04:22 +0200 Subject: [PATCH 10/12] the wasm ABI behavior is a bug --- compiler/rustc_codegen_llvm/src/abi.rs | 15 ++++++--------- compiler/rustc_target/src/abi/call/mod.rs | 7 +++++-- compiler/rustc_target/src/abi/call/wasm.rs | 4 ++++ 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index 1e72b7db1e4e7..64587f98b8a10 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -351,15 +351,12 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { // guarnateeing that we generate ABI-compatible LLVM IR. Things get tricky for // aggregates... if matches!(arg.layout.abi, abi::Abi::Aggregate { .. }) { - // This is the most critical case for ABI compatibility, since - // `immediate_llvm_type` will use `layout.fields` to turn this Rust type - // into an LLVM type. ABI-compatible Rust types can have different `fields`, - // so we need to be very sure that LLVM wil treat those different types in - // an ABI-compatible way. Mostly we do this by disallowing - // `PassMode::Direct` for aggregates, but we actually do use that mode on - // wasm. wasm doesn't have aggregate types so we are fairly sure that LLVM - // will treat `{ i32, i32, i32 }` and `{ { i32, i32, i32 } }` the same way - // for ABI purposes. + // 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 .) assert!( matches!(&*cx.tcx.sess.target.arch, "wasm32" | "wasm64"), "`PassMode::Direct` for aggregates only allowed on wasm targets\nProblematic type: {:#?}", diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 42483b7c6a524..d4619bb675338 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -36,7 +36,10 @@ pub enum PassMode { Ignore, /// Pass the argument directly. /// - /// The argument has a layout abi of `Scalar`, `Vector` or in rare cases (e.g. on wasm) `Aggregate`. + /// The argument has a layout abi of `Scalar` or `Vector`. + /// Unfortunately due to past mistakes, in rare cases on wasm, it can also be `Aggregate`. + /// This is bad since it leaks LLVM implementation details into the ABI. + /// (Also see .) Direct(ArgAttributes), /// Pass a pair's elements directly in two arguments. /// @@ -527,7 +530,7 @@ impl<'a, Ty> ArgAbi<'a, Ty> { scalar_attrs(&layout, b, a.size(cx).align_to(b.align(cx).abi)), ), Abi::Vector { .. } => PassMode::Direct(ArgAttributes::new()), - // The `Aggregate` ABI is almost always adjusted later. + // The `Aggregate` ABI should always be adjusted later. Abi::Aggregate { .. } => PassMode::Direct(ArgAttributes::new()), }; ArgAbi { layout, mode } diff --git a/compiler/rustc_target/src/abi/call/wasm.rs b/compiler/rustc_target/src/abi/call/wasm.rs index 0eb2309ecb28b..796b752ff9d41 100644 --- a/compiler/rustc_target/src/abi/call/wasm.rs +++ b/compiler/rustc_target/src/abi/call/wasm.rs @@ -61,6 +61,10 @@ where /// The purpose of this ABI is for matching the WebAssembly standard. This /// intentionally diverges from the C ABI and is specifically crafted to take /// advantage of LLVM's support of multiple returns in WebAssembly. +/// +/// This ABI is *bad*! It uses `PassMode::Direct` for `abi::Aggregate` types, which leaks LLVM +/// implementation details into the ABI. It's just hard to fix because ABIs are hard to change. +/// Also see . pub fn compute_wasm_abi_info(fn_abi: &mut FnAbi<'_, Ty>) { if !fn_abi.ret.is_ignore() { classify_ret(&mut fn_abi.ret); From b0cf4c28ea0f2ac0a7c6fb5bc3ee4291b7595c24 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 7 Sep 2023 16:48:02 +0200 Subject: [PATCH 11/12] turns out Layout has some more things to worry about -- move ABI comparison into helper function like is_bool, and some special magic extra fields --- compiler/rustc_abi/src/lib.rs | 24 ++++++++++++++++++- .../src/interpret/terminator.rs | 8 ++----- compiler/rustc_passes/src/abi_test.rs | 20 +++------------- compiler/rustc_target/src/abi/call/mod.rs | 11 ++++++++- 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index f9393539ea457..b30ff058a3092 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -1300,12 +1300,18 @@ impl Abi { matches!(*self, Abi::Uninhabited) } - /// Returns `true` is this is a scalar type + /// Returns `true` if this is a scalar type #[inline] pub fn is_scalar(&self) -> bool { matches!(*self, Abi::Scalar(_)) } + /// Returns `true` if this is a bool + #[inline] + pub fn is_bool(&self) -> bool { + matches!(*self, Abi::Scalar(s) if s.is_bool()) + } + /// Returns the fixed alignment of this ABI, if any is mandated. pub fn inherent_align(&self, cx: &C) -> Option { Some(match *self { @@ -1703,6 +1709,22 @@ impl LayoutS { Abi::Aggregate { sized } => sized && self.size.bytes() == 0, } } + + /// Checks if these two `Layout` are equal enough to be considered "the same for all function + /// call ABIs". Note however that real ABIs depend on more details that are not reflected in the + /// `Layout`; the `PassMode` need to be compared as well. + pub fn eq_abi(&self, other: &Self) -> bool { + // The one thing that we are not capturing here is that for unsized types, the metadata must + // also have the same ABI, and moreover that the same metadata leads to the same size. The + // 2nd point is quite hard to check though. + self.size == other.size + && self.is_sized() == other.is_sized() + && self.abi.eq_up_to_validity(&other.abi) + && self.abi.is_bool() == other.abi.is_bool() + && self.align.abi == other.align.abi + && self.max_repr_align == other.max_repr_align + && self.unadjusted_abi_align == other.unadjusted_abi_align + } } #[derive(Copy, Clone, Debug)] diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index 6312dc7414da0..eb4673c0edcd5 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -332,12 +332,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { if self.layout_compat(caller_abi.layout, callee_abi.layout) && caller_abi.mode.eq_abi(&callee_abi.mode) { - // Something went very wrong if our checks don't even imply that the layout is the same. - assert!( - caller_abi.layout.size == callee_abi.layout.size - && caller_abi.layout.align.abi == callee_abi.layout.align.abi - && caller_abi.layout.is_sized() == callee_abi.layout.is_sized() - ); + // Something went very wrong if our checks don't imply layout ABI compatibility. + assert!(caller_abi.layout.eq_abi(&callee_abi.layout)); return true; } else { trace!( diff --git a/compiler/rustc_passes/src/abi_test.rs b/compiler/rustc_passes/src/abi_test.rs index 82392e7267780..7e781bdd6f4a8 100644 --- a/compiler/rustc_passes/src/abi_test.rs +++ b/compiler/rustc_passes/src/abi_test.rs @@ -5,7 +5,7 @@ use rustc_middle::ty::layout::{FnAbiError, LayoutError}; use rustc_middle::ty::{self, GenericArgs, Instance, Ty, TyCtxt}; use rustc_span::source_map::Spanned; use rustc_span::symbol::sym; -use rustc_target::abi::call::{ArgAbi, FnAbi}; +use rustc_target::abi::call::FnAbi; use crate::errors::{AbiInvalidAttribute, AbiNe, AbiOf, UnrecognizedField}; @@ -114,20 +114,6 @@ fn dump_abi_of_fn_item(tcx: TyCtxt<'_>, item_def_id: DefId, attr: &Attribute) { } } -fn test_arg_abi_eq<'tcx>( - abi1: &'tcx ArgAbi<'tcx, Ty<'tcx>>, - abi2: &'tcx ArgAbi<'tcx, Ty<'tcx>>, -) -> bool { - // Ideally we'd just compare the `mode`, but that is not enough -- for some modes LLVM will look - // at the type. Comparing the `mode` and `layout.abi` as well as size and alignment should catch - // basically everything though (except for tricky cases around unized types). - abi1.mode.eq_abi(&abi2.mode) - && abi1.layout.abi.eq_up_to_validity(&abi2.layout.abi) - && abi1.layout.size == abi2.layout.size - && abi1.layout.align.abi == abi2.layout.align.abi - && abi1.layout.is_sized() == abi2.layout.is_sized() -} - fn test_abi_eq<'tcx>(abi1: &'tcx FnAbi<'tcx, Ty<'tcx>>, abi2: &'tcx FnAbi<'tcx, Ty<'tcx>>) -> bool { if abi1.conv != abi2.conv || abi1.args.len() != abi2.args.len() @@ -138,8 +124,8 @@ fn test_abi_eq<'tcx>(abi1: &'tcx FnAbi<'tcx, Ty<'tcx>>, abi2: &'tcx FnAbi<'tcx, return false; } - test_arg_abi_eq(&abi1.ret, &abi2.ret) - && abi1.args.iter().zip(abi2.args.iter()).all(|(arg1, arg2)| test_arg_abi_eq(arg1, arg2)) + abi1.ret.eq_abi(&abi2.ret) + && abi1.args.iter().zip(abi2.args.iter()).all(|(arg1, arg2)| arg1.eq_abi(arg2)) } fn dump_abi_of_fn_type(tcx: TyCtxt<'_>, item_def_id: DefId, attr: &Attribute) { diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index d4619bb675338..5d75974279ed0 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -60,7 +60,8 @@ pub enum PassMode { impl PassMode { /// Checks if these two `PassMode` are equal enough to be considered "the same for all - /// function call ABIs". + /// function call ABIs". However, the `Layout` can also impact ABI decisions, + /// so that needs to be compared as well! pub fn eq_abi(&self, other: &Self) -> bool { match (self, other) { (PassMode::Ignore, PassMode::Ignore) => true, // can still be reached for the return type @@ -623,6 +624,14 @@ impl<'a, Ty> ArgAbi<'a, Ty> { pub fn is_ignore(&self) -> bool { matches!(self.mode, PassMode::Ignore) } + + /// Checks if these two `ArgAbi` are equal enough to be considered "the same for all + /// function call ABIs". + pub fn eq_abi(&self, other: &Self) -> bool { + // Ideally we'd just compare the `mode`, but that is not enough -- for some modes LLVM will look + // at the type. + self.layout.eq_abi(&other.layout) && self.mode.eq_abi(&other.mode) + } } #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, HashStable_Generic)] From bd13b261bc9730c7df326ce5a5f5f43bff66c01a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 8 Sep 2023 08:02:02 +0200 Subject: [PATCH 12/12] need to disable part of this test on aarch64 --- tests/ui/abi/compatibility.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/ui/abi/compatibility.rs b/tests/ui/abi/compatibility.rs index c5c476ac46d2c..2cc19d6325678 100644 --- a/tests/ui/abi/compatibility.rs +++ b/tests/ui/abi/compatibility.rs @@ -105,7 +105,6 @@ test_transparent!(zst, Zst); test_transparent!(unit, ()); test_transparent!(pair, (i32, f32)); // mixing in some floats since they often get special treatment test_transparent!(triple, (i8, i16, f32)); // chosen to fit into 64bit -test_transparent!(float_triple, (f64, f64, f64)); // hits a bug in our MIPS64 adjustments test_transparent!(tuple, (i32, f32, i64, f64)); test_transparent!(empty_array, [u32; 0]); test_transparent!(empty_1zst_array, [u8; 0]); @@ -113,6 +112,14 @@ test_transparent!(small_array, [i32; 2]); // chosen to fit into 64bit test_transparent!(large_array, [i32; 16]); test_transparent!(enum_, Option); test_transparent!(enum_niched, Option<&'static i32>); +// Pure-float types that are not ScalarPair seem to be tricky. +// FIXME: +#[cfg(not(target_arch = "aarch64"))] +mod tricky { + use super::*; + test_transparent!(triple_f32, (f32, f32, f32)); + test_transparent!(triple_f64, (f64, f64, f64)); +} // RFC 3391 . macro_rules! test_nonnull {