From 1f32f7bd78e26d673162a168cac01943b544090d Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 4 Mar 2025 16:38:07 -0800 Subject: [PATCH 1/5] compiler: add `ExternAbi::is_rustic_abi` --- compiler/rustc_abi/src/extern_abi.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/compiler/rustc_abi/src/extern_abi.rs b/compiler/rustc_abi/src/extern_abi.rs index 543c2f8ab12cf..4d70afd4e0bca 100644 --- a/compiler/rustc_abi/src/extern_abi.rs +++ b/compiler/rustc_abi/src/extern_abi.rs @@ -191,6 +191,17 @@ impl StableOrd for ExternAbi { } impl ExternAbi { + /// An ABI "like Rust" + /// + /// These ABIs are fully controlled by the Rust compiler, which means they + /// - support unwinding with `-Cpanic=unwind`, unlike `extern "C"` + /// - often diverge from the C ABI + /// - are subject to change between compiler versions + pub fn is_rustic_abi(self) -> bool { + use ExternAbi::*; + matches!(self, Rust | RustCall | RustIntrinsic | RustCold) + } + pub fn supports_varargs(self) -> bool { // * C and Cdecl obviously support varargs. // * C can be based on Aapcs, SysV64 or Win64, so they must support varargs. From 08b578330e4cad3319f2c0f9530591c3610120c2 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 4 Mar 2025 16:38:19 -0800 Subject: [PATCH 2/5] compiler: use `is_rustic_abi` in mir_transform no functional changes. --- compiler/rustc_mir_transform/src/ffi_unwind_calls.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs index 7b3553e7afd06..abbff1c48dd9a 100644 --- a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs +++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs @@ -53,11 +53,7 @@ fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool { // Rust calls cannot themselves create foreign unwinds. // We assume this is true for intrinsics as well. - if let ExternAbi::RustIntrinsic - | ExternAbi::Rust - | ExternAbi::RustCall - | ExternAbi::RustCold = sig.abi() - { + if sig.abi().is_rustic_abi() { continue; }; From 5abf36b486a426c20c71dccc9252aab6c724a0cd Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 4 Mar 2025 16:38:34 -0800 Subject: [PATCH 3/5] compiler: use `is_rustic_abi` in ImproperCTypesVisitor no functional changes --- compiler/rustc_lint/src/types.rs | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index cb83d405cc3ed..d2b08eab47916 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -1,7 +1,7 @@ use std::iter; use std::ops::ControlFlow; -use rustc_abi::{BackendRepr, ExternAbi, TagEncoding, VariantIdx, Variants, WrappingRange}; +use rustc_abi::{BackendRepr, TagEncoding, VariantIdx, Variants, WrappingRange}; use rustc_data_structures::fx::FxHashSet; use rustc_errors::DiagMessage; use rustc_hir::intravisit::VisitorExt; @@ -1349,7 +1349,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::FnPtr(sig_tys, hdr) => { let sig = sig_tys.with(hdr); - if self.is_internal_abi(sig.abi()) { + if sig.abi().is_rustic_abi() { return FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_fnptr_reason, @@ -1552,13 +1552,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { self.check_type_for_ffi_and_report_errors(span, ty, true, false); } - fn is_internal_abi(&self, abi: ExternAbi) -> bool { - matches!( - abi, - ExternAbi::Rust | ExternAbi::RustCall | ExternAbi::RustCold | ExternAbi::RustIntrinsic - ) - } - /// Find any fn-ptr types with external ABIs in `ty`. /// /// For example, `Option` returns `extern "C" fn()` @@ -1567,17 +1560,16 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { hir_ty: &hir::Ty<'tcx>, ty: Ty<'tcx>, ) -> Vec<(Ty<'tcx>, Span)> { - struct FnPtrFinder<'a, 'b, 'tcx> { - visitor: &'a ImproperCTypesVisitor<'b, 'tcx>, + struct FnPtrFinder<'tcx> { spans: Vec, tys: Vec>, } - impl<'a, 'b, 'tcx> hir::intravisit::Visitor<'_> for FnPtrFinder<'a, 'b, 'tcx> { + impl<'tcx> hir::intravisit::Visitor<'_> for FnPtrFinder<'tcx> { fn visit_ty(&mut self, ty: &'_ hir::Ty<'_, AmbigArg>) { debug!(?ty); if let hir::TyKind::BareFn(hir::BareFnTy { abi, .. }) = ty.kind - && !self.visitor.is_internal_abi(*abi) + && !abi.is_rustic_abi() { self.spans.push(ty.span); } @@ -1586,12 +1578,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } } - impl<'a, 'b, 'tcx> ty::visit::TypeVisitor> for FnPtrFinder<'a, 'b, 'tcx> { + impl<'tcx> ty::visit::TypeVisitor> for FnPtrFinder<'tcx> { type Result = (); fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result { if let ty::FnPtr(_, hdr) = ty.kind() - && !self.visitor.is_internal_abi(hdr.abi) + && !hdr.abi.is_rustic_abi() { self.tys.push(ty); } @@ -1600,7 +1592,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } } - let mut visitor = FnPtrFinder { visitor: self, spans: Vec::new(), tys: Vec::new() }; + let mut visitor = FnPtrFinder { spans: Vec::new(), tys: Vec::new() }; ty.visit_with(&mut visitor); visitor.visit_ty_unambig(hir_ty); @@ -1615,13 +1607,13 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDeclarations { match it.kind { hir::ForeignItemKind::Fn(sig, _, _) => { - if vis.is_internal_abi(abi) { + if abi.is_rustic_abi() { vis.check_fn(it.owner_id.def_id, sig.decl) } else { vis.check_foreign_fn(it.owner_id.def_id, sig.decl); } } - hir::ForeignItemKind::Static(ty, _, _) if !vis.is_internal_abi(abi) => { + hir::ForeignItemKind::Static(ty, _, _) if !abi.is_rustic_abi() => { vis.check_foreign_static(it.owner_id, ty.span); } hir::ForeignItemKind::Static(..) | hir::ForeignItemKind::Type => (), @@ -1775,7 +1767,7 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDefinitions { }; let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Definition }; - if vis.is_internal_abi(abi) { + if abi.is_rustic_abi() { vis.check_fn(id, decl); } else { vis.check_foreign_fn(id, decl); From e81fbe30e6bfbfb62f599b03baf0f34bab447bef Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 4 Mar 2025 16:48:13 -0800 Subject: [PATCH 4/5] compiler: use `is_rustic_abi` in abi_check warns on fewer ABIs now --- compiler/rustc_monomorphize/src/mono_checks/abi_check.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_monomorphize/src/mono_checks/abi_check.rs b/compiler/rustc_monomorphize/src/mono_checks/abi_check.rs index 9f26da7d5c64b..06d6c3ab8050e 100644 --- a/compiler/rustc_monomorphize/src/mono_checks/abi_check.rs +++ b/compiler/rustc_monomorphize/src/mono_checks/abi_check.rs @@ -1,6 +1,6 @@ //! This module ensures that if a function's ABI requires a particular target feature, //! that target feature is enabled both on the callee and all callers. -use rustc_abi::{BackendRepr, ExternAbi, RegKind}; +use rustc_abi::{BackendRepr, RegKind}; use rustc_hir::CRATE_HIR_ID; use rustc_middle::mir::{self, traversal}; use rustc_middle::ty::{self, Instance, InstanceKind, Ty, TyCtxt}; @@ -115,8 +115,8 @@ fn check_call_site_abi<'tcx>( span: Span, caller: InstanceKind<'tcx>, ) { - if callee.fn_sig(tcx).abi() == ExternAbi::Rust { - // "Rust" ABI never passes arguments in vector registers. + if callee.fn_sig(tcx).abi().is_rustic_abi() { + // we directly handle the soundness of Rust ABIs return; } let typing_env = ty::TypingEnv::fully_monomorphized(); From 8a689878ced1b3c0834ea32265c7d1cd43750db4 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 4 Mar 2025 16:50:16 -0800 Subject: [PATCH 5/5] compiler: use `is_rustic_abi` in ty_utils expands some conditionals to include different "rustic" ABIs, so that we actually handle passing args through all "rustic" ABIs --- compiler/rustc_ty_utils/src/abi.rs | 7 ++----- tests/crashes/132981.rs | 11 ----------- tests/ui/abi/rust-cold-works-with-rustic-args.rs | 6 ++++++ 3 files changed, 8 insertions(+), 16 deletions(-) delete mode 100644 tests/crashes/132981.rs create mode 100644 tests/ui/abi/rust-cold-works-with-rustic-args.rs diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs index e317768ff602c..a726ebae6fe1b 100644 --- a/compiler/rustc_ty_utils/src/abi.rs +++ b/compiler/rustc_ty_utils/src/abi.rs @@ -436,10 +436,7 @@ fn fn_abi_sanity_check<'tcx>( ) { let tcx = cx.tcx(); - if spec_abi == ExternAbi::Rust - || spec_abi == ExternAbi::RustCall - || spec_abi == ExternAbi::RustCold - { + if spec_abi.is_rustic_abi() { if arg.layout.is_zst() { // Casting closures to function pointers depends on ZST closure types being // omitted entirely in the calling convention. @@ -687,7 +684,7 @@ fn fn_abi_adjust_for_abi<'tcx>( let tcx = cx.tcx(); - if abi == ExternAbi::Rust || abi == ExternAbi::RustCall || abi == ExternAbi::RustIntrinsic { + if abi.is_rustic_abi() { fn_abi.adjust_for_rust_abi(cx, abi); // Look up the deduced parameter attributes for this function, if we have its def ID and diff --git a/tests/crashes/132981.rs b/tests/crashes/132981.rs deleted file mode 100644 index 916c15592404c..0000000000000 --- a/tests/crashes/132981.rs +++ /dev/null @@ -1,11 +0,0 @@ -//@ known-bug: #132981 -//@compile-flags: -Clink-dead-code=true --crate-type lib -//@ only-x86_64 -//@ ignore-windows -// The set of targets this crashes on is really fiddly, because it is deep in our ABI logic. It -// crashes on x86_64-unknown-linux-gnu, and i686-pc-windows-msvc, but not on -// x86_64-pc-windows-msvc. If you are trying to fix this crash, don't pay too much attention to the -// directives. - -#![feature(rust_cold_cc)] -pub extern "rust-cold" fn foo(_: [usize; 3]) {} diff --git a/tests/ui/abi/rust-cold-works-with-rustic-args.rs b/tests/ui/abi/rust-cold-works-with-rustic-args.rs new file mode 100644 index 0000000000000..5702736469965 --- /dev/null +++ b/tests/ui/abi/rust-cold-works-with-rustic-args.rs @@ -0,0 +1,6 @@ +//@build-pass +//@compile-flags: -Clink-dead-code=true --crate-type lib +// We used to not handle all "rustic" ABIs in a (relatively) uniform way, +// so we failed to fix up arguments for actually passing through the ABI... +#![feature(rust_cold_cc)] +pub extern "rust-cold" fn foo(_: [usize; 3]) {}