From 93c2bace58b36ba297f06505b55aef5b8eba954f Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Tue, 2 Apr 2024 18:23:28 +0000 Subject: [PATCH 1/5] CFI: Switch sense of type erasure flag Previously, we had `NO_SELF_TYPE_ERASURE`, a negative configuration. Now we have `ERASE_SELF_TYPE`, a positive configuration. --- compiler/rustc_codegen_llvm/src/declare.rs | 6 ++++-- compiler/rustc_symbol_mangling/src/typeid.rs | 6 +++--- .../src/typeid/typeid_itanium_cxx_abi.rs | 2 +- ...e-metadata-id-itanium-cxx-abi-method-secondary-typeid.rs | 4 ++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/declare.rs b/compiler/rustc_codegen_llvm/src/declare.rs index 3ef8538ced3a5..f58dd4066ad71 100644 --- a/compiler/rustc_codegen_llvm/src/declare.rs +++ b/compiler/rustc_codegen_llvm/src/declare.rs @@ -147,7 +147,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { for options in [ TypeIdOptions::GENERALIZE_POINTERS, TypeIdOptions::NORMALIZE_INTEGERS, - TypeIdOptions::NO_SELF_TYPE_ERASURE, + TypeIdOptions::ERASE_SELF_TYPE, ] .into_iter() .powerset() @@ -173,7 +173,9 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { if self.tcx.sess.is_sanitizer_kcfi_enabled() { // LLVM KCFI does not support multiple !kcfi_type attachments - let mut options = TypeIdOptions::empty(); + // Default to erasing the self type. If we need the concrete type, there will be a + // hint in the instance. + let mut options = TypeIdOptions::ERASE_SELF_TYPE; if self.tcx.sess.is_sanitizer_cfi_generalize_pointers_enabled() { options.insert(TypeIdOptions::GENERALIZE_POINTERS); } diff --git a/compiler/rustc_symbol_mangling/src/typeid.rs b/compiler/rustc_symbol_mangling/src/typeid.rs index fc1e8e46e8de5..11657b3b303af 100644 --- a/compiler/rustc_symbol_mangling/src/typeid.rs +++ b/compiler/rustc_symbol_mangling/src/typeid.rs @@ -24,9 +24,9 @@ bitflags! { /// `-fsanitize-cfi-icall-experimental-normalize-integers` option for cross-language LLVM /// CFI and KCFI support. const NORMALIZE_INTEGERS = 4; - /// Do not perform self type erasure for attaching a secondary type id to methods with their - /// concrete self so they can be used as function pointers. - const NO_SELF_TYPE_ERASURE = 8; + /// Generalize the instance by erasing the concrete `Self` type where possible. + /// Only has an effect on `{kcfi_,}typeid_for_instance`. + const ERASE_SELF_TYPE = 8; } } diff --git a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs index 5963bd7c5f16c..25194e2c87ed3 100644 --- a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs +++ b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs @@ -1172,7 +1172,7 @@ pub fn typeid_for_instance<'tcx>( instance.args = tcx.mk_args_trait(invoke_ty, trait_ref.args.into_iter().skip(1)); } - if !options.contains(EncodeTyOptions::NO_SELF_TYPE_ERASURE) { + if options.contains(EncodeTyOptions::ERASE_SELF_TYPE) { if let Some(impl_id) = tcx.impl_of_method(instance.def_id()) && let Some(trait_ref) = tcx.impl_trait_ref(impl_id) { diff --git a/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-method-secondary-typeid.rs b/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-method-secondary-typeid.rs index 671db563dde78..999fd292fe7e4 100644 --- a/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-method-secondary-typeid.rs +++ b/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-method-secondary-typeid.rs @@ -18,5 +18,5 @@ impl Trait1 for Type1 { } -// CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvu3refIu3dynIu{{[0-9]+}}NtC{{[[:print:]]+}}_{{[[:print:]]+}}6Trait1u6regionEEE"} -// CHECK: ![[TYPE2]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NtC{{[[:print:]]+}}_{{[[:print:]]+}}5Type1EE"} +// CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NtC{{[[:print:]]+}}_{{[[:print:]]+}}5Type1EE"} +// CHECK: ![[TYPE2]] = !{i64 0, !"_ZTSFvu3refIu3dynIu{{[0-9]+}}NtC{{[[:print:]]+}}_{{[[:print:]]+}}6Trait1u6regionEEE"} From 6aa89f684e4427a9d08e35b572f9071705105140 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Mon, 25 Mar 2024 19:27:43 +0000 Subject: [PATCH 2/5] Track reason for creating a `ReifyShim` KCFI needs to be able to tell which kind of `ReifyShim` it is examining in order to decide whether to use a concrete type (`FnPtr` case) or an abstract case (`Vtable` case). You can *almost* tell this from context, but there is one case where you can't - if a trait has a method which is *not* `#[track_caller]`, with an impl that *is* `#[track_caller]`, both the vtable and a function pointer created from that method will be `ReifyShim(def_id)`. Currently, the reason is optional to ensure no additional unique `ReifyShim`s are added without KCFI on. However, the case in which an extra `ReifyShim` is created is sufficiently rare that this may be worth revisiting to reduce complexity. --- compiler/rustc_middle/src/mir/visit.rs | 2 +- compiler/rustc_middle/src/ty/instance.rs | 48 +++++++++++++++---- compiler/rustc_middle/src/ty/mod.rs | 2 +- .../rustc_middle/src/ty/structural_impls.rs | 1 + compiler/rustc_mir_transform/src/inline.rs | 2 +- .../rustc_mir_transform/src/inline/cycle.rs | 2 +- compiler/rustc_mir_transform/src/shim.rs | 2 +- compiler/rustc_symbol_mangling/src/legacy.rs | 12 +++-- compiler/rustc_symbol_mangling/src/v0.rs | 8 ++-- 9 files changed, 60 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 3835bd371d996..4f7b2f7cbe48b 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -341,7 +341,7 @@ macro_rules! make_mir_visitor { ty::InstanceDef::Intrinsic(_def_id) | ty::InstanceDef::VTableShim(_def_id) | - ty::InstanceDef::ReifyShim(_def_id) | + ty::InstanceDef::ReifyShim(_def_id, _) | ty::InstanceDef::Virtual(_def_id, _) | ty::InstanceDef::ThreadLocalShim(_def_id) | ty::InstanceDef::ClosureOnceShim { call_once: _def_id, track_caller: _ } | diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index 4fec5653a798c..e5625c8a5c634 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -31,6 +31,28 @@ pub struct Instance<'tcx> { pub args: GenericArgsRef<'tcx>, } +/// Describes why a `ReifyShim` was created. This is needed to distingish a ReifyShim created to +/// adjust for things like `#[track_caller]` in a vtable from a `ReifyShim` created to produce a +/// function pointer from a vtable entry. +/// Currently, this is only used when KCFI is enabled, as only KCFI needs to treat those two +/// `ReifyShim`s differently. +#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] +#[derive(TyEncodable, TyDecodable, HashStable)] +pub enum ReifyReason { + /// The `ReifyShim` was created to produce a function pointer. This happens when: + /// * A vtable entry is directly converted to a function call (e.g. creating a fn ptr from a + /// method on a `dyn` object). + /// * A function with `#[track_caller]` is converted to a function pointer + /// * If KCFI is enabled, creating a function pointer from a method on an object-safe trait. + /// This includes the case of converting `::call`-like methods on closure-likes to function + /// pointers. + FnPtr, + /// This `ReifyShim` was created to populate a vtable. Currently, this happens when a + /// `#[track_caller]` mismatch occurs between the implementation of a method and the method. + /// This includes the case of `::call`-like methods in closure-likes' vtables. + Vtable, +} + #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] #[derive(TyEncodable, TyDecodable, HashStable, TypeFoldable, TypeVisitable, Lift)] pub enum InstanceDef<'tcx> { @@ -67,7 +89,13 @@ pub enum InstanceDef<'tcx> { /// Because this is a required part of the function's ABI but can't be tracked /// as a property of the function pointer, we use a single "caller location" /// (the definition of the function itself). - ReifyShim(DefId), + /// + /// The second field encodes *why* this shim was created. This allows distinguishing between + /// a `ReifyShim` that appears in a vtable vs one that appears as a function pointer. + /// + /// This field will only be populated if we are compiling in a mode that needs these shims + /// to be separable, currently only when KCFI is enabled. + ReifyShim(DefId, Option), /// `::call_*` (generated `FnTrait` implementation for `fn()` pointers). /// @@ -194,7 +222,7 @@ impl<'tcx> InstanceDef<'tcx> { match self { InstanceDef::Item(def_id) | InstanceDef::VTableShim(def_id) - | InstanceDef::ReifyShim(def_id) + | InstanceDef::ReifyShim(def_id, _) | InstanceDef::FnPtrShim(def_id, _) | InstanceDef::Virtual(def_id, _) | InstanceDef::Intrinsic(def_id) @@ -354,7 +382,9 @@ fn fmt_instance( match instance.def { InstanceDef::Item(_) => Ok(()), InstanceDef::VTableShim(_) => write!(f, " - shim(vtable)"), - InstanceDef::ReifyShim(_) => write!(f, " - shim(reify)"), + InstanceDef::ReifyShim(_, None) => write!(f, " - shim(reify)"), + InstanceDef::ReifyShim(_, Some(ReifyReason::FnPtr)) => write!(f, " - shim(reify-fnptr)"), + InstanceDef::ReifyShim(_, Some(ReifyReason::Vtable)) => write!(f, " - shim(reify-vtable)"), InstanceDef::ThreadLocalShim(_) => write!(f, " - shim(tls)"), InstanceDef::Intrinsic(_) => write!(f, " - intrinsic"), InstanceDef::Virtual(_, num) => write!(f, " - virtual#{num}"), @@ -476,15 +506,16 @@ impl<'tcx> Instance<'tcx> { debug!("resolve(def_id={:?}, args={:?})", def_id, args); // Use either `resolve_closure` or `resolve_for_vtable` assert!(!tcx.is_closure_like(def_id), "Called `resolve_for_fn_ptr` on closure: {def_id:?}"); + let reason = tcx.sess.is_sanitizer_kcfi_enabled().then_some(ReifyReason::FnPtr); Instance::resolve(tcx, param_env, def_id, args).ok().flatten().map(|mut resolved| { match resolved.def { InstanceDef::Item(def) if resolved.def.requires_caller_location(tcx) => { debug!(" => fn pointer created for function with #[track_caller]"); - resolved.def = InstanceDef::ReifyShim(def); + resolved.def = InstanceDef::ReifyShim(def, reason); } InstanceDef::Virtual(def_id, _) => { debug!(" => fn pointer created for virtual call"); - resolved.def = InstanceDef::ReifyShim(def_id); + resolved.def = InstanceDef::ReifyShim(def_id, reason); } _ => {} } @@ -508,6 +539,7 @@ impl<'tcx> Instance<'tcx> { debug!(" => associated item with unsizeable self: Self"); Some(Instance { def: InstanceDef::VTableShim(def_id), args }) } else { + let reason = tcx.sess.is_sanitizer_kcfi_enabled().then_some(ReifyReason::Vtable); Instance::resolve(tcx, param_env, def_id, args).ok().flatten().map(|mut resolved| { match resolved.def { InstanceDef::Item(def) => { @@ -544,18 +576,18 @@ impl<'tcx> Instance<'tcx> { // Create a shim for the `FnOnce/FnMut/Fn` method we are calling // - unlike functions, invoking a closure always goes through a // trait. - resolved = Instance { def: InstanceDef::ReifyShim(def_id), args }; + resolved = Instance { def: InstanceDef::ReifyShim(def_id, reason), args }; } else { debug!( " => vtable fn pointer created for function with #[track_caller]: {:?}", def ); - resolved.def = InstanceDef::ReifyShim(def); + resolved.def = InstanceDef::ReifyShim(def, reason); } } } InstanceDef::Virtual(def_id, _) => { debug!(" => vtable fn pointer created for virtual call"); - resolved.def = InstanceDef::ReifyShim(def_id); + resolved.def = InstanceDef::ReifyShim(def_id, reason) } _ => {} } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 4e1baaec39ea7..eef623d77b06c 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -88,7 +88,7 @@ pub use self::context::{ tls, CtxtInterners, CurrentGcx, DeducedParamAttrs, Feed, FreeRegionInfo, GlobalCtxt, Lift, TyCtxt, TyCtxtFeed, }; -pub use self::instance::{Instance, InstanceDef, ShortInstance, UnusedGenericParams}; +pub use self::instance::{Instance, InstanceDef, ReifyReason, ShortInstance, UnusedGenericParams}; pub use self::list::List; pub use self::parameterized::ParameterizedOverTcx; pub use self::predicate::{ diff --git a/compiler/rustc_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs index a62379def534e..0e7010e67d7dc 100644 --- a/compiler/rustc_middle/src/ty/structural_impls.rs +++ b/compiler/rustc_middle/src/ty/structural_impls.rs @@ -449,6 +449,7 @@ TrivialTypeTraversalAndLiftImpls! { crate::ty::ClosureKind, crate::ty::ParamConst, crate::ty::ParamTy, + crate::ty::instance::ReifyReason, interpret::AllocId, interpret::CtfeProvenance, interpret::Scalar, diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 5f74841151cda..ab9fa165a200a 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -324,7 +324,7 @@ impl<'tcx> Inliner<'tcx> { // do not need to catch this here, we can wait until the inliner decides to continue // inlining a second time. InstanceDef::VTableShim(_) - | InstanceDef::ReifyShim(_) + | InstanceDef::ReifyShim(..) | InstanceDef::FnPtrShim(..) | InstanceDef::ClosureOnceShim { .. } | InstanceDef::ConstructCoroutineInClosureShim { .. } diff --git a/compiler/rustc_mir_transform/src/inline/cycle.rs b/compiler/rustc_mir_transform/src/inline/cycle.rs index f2b6dcac58632..7a1340f3a5527 100644 --- a/compiler/rustc_mir_transform/src/inline/cycle.rs +++ b/compiler/rustc_mir_transform/src/inline/cycle.rs @@ -84,7 +84,7 @@ pub(crate) fn mir_callgraph_reachable<'tcx>( // again, a function item can end up getting inlined. Thus we'll be able to cause // a cycle that way InstanceDef::VTableShim(_) - | InstanceDef::ReifyShim(_) + | InstanceDef::ReifyShim(..) | InstanceDef::FnPtrShim(..) | InstanceDef::ClosureOnceShim { .. } | InstanceDef::ConstructCoroutineInClosureShim { .. } diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index b60ee7649b24a..eaef2b80c860c 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -55,7 +55,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<' // a virtual call, or a direct call to a function for which // indirect calls must be codegen'd differently than direct ones // (such as `#[track_caller]`). - ty::InstanceDef::ReifyShim(def_id) => { + ty::InstanceDef::ReifyShim(def_id, _) => { build_call_shim(tcx, instance, None, CallKind::Direct(def_id)) } ty::InstanceDef::ClosureOnceShim { call_once: _, track_caller: _ } => { diff --git a/compiler/rustc_symbol_mangling/src/legacy.rs b/compiler/rustc_symbol_mangling/src/legacy.rs index 1c62ce2d214b7..f68668a91e688 100644 --- a/compiler/rustc_symbol_mangling/src/legacy.rs +++ b/compiler/rustc_symbol_mangling/src/legacy.rs @@ -2,7 +2,7 @@ use rustc_data_structures::stable_hasher::{Hash64, HashStable, StableHasher}; use rustc_hir::def_id::CrateNum; use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData}; use rustc_middle::ty::print::{PrettyPrinter, Print, PrintError, Printer}; -use rustc_middle::ty::{self, Instance, Ty, TyCtxt, TypeVisitableExt}; +use rustc_middle::ty::{self, Instance, ReifyReason, Ty, TyCtxt, TypeVisitableExt}; use rustc_middle::ty::{GenericArg, GenericArgKind}; use std::fmt::{self, Write}; @@ -71,8 +71,14 @@ pub(super) fn mangle<'tcx>( ty::InstanceDef::VTableShim(..) => { printer.write_str("{{vtable-shim}}").unwrap(); } - ty::InstanceDef::ReifyShim(..) => { - printer.write_str("{{reify-shim}}").unwrap(); + ty::InstanceDef::ReifyShim(_, reason) => { + printer.write_str("{{reify-shim").unwrap(); + match reason { + Some(ReifyReason::FnPtr) => printer.write_str("-fnptr").unwrap(), + Some(ReifyReason::Vtable) => printer.write_str("-vtable").unwrap(), + None => (), + } + printer.write_str("}}").unwrap(); } // FIXME(async_closures): This shouldn't be needed when we fix // `Instance::ty`/`Instance::def_id`. diff --git a/compiler/rustc_symbol_mangling/src/v0.rs b/compiler/rustc_symbol_mangling/src/v0.rs index 4369f020d27a3..8cb5370bb4a87 100644 --- a/compiler/rustc_symbol_mangling/src/v0.rs +++ b/compiler/rustc_symbol_mangling/src/v0.rs @@ -8,8 +8,8 @@ use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData}; use rustc_middle::ty::layout::IntegerExt; use rustc_middle::ty::print::{Print, PrintError, Printer}; use rustc_middle::ty::{ - self, EarlyBinder, FloatTy, Instance, IntTy, Ty, TyCtxt, TypeVisitable, TypeVisitableExt, - UintTy, + self, EarlyBinder, FloatTy, Instance, IntTy, ReifyReason, Ty, TyCtxt, TypeVisitable, + TypeVisitableExt, UintTy, }; use rustc_middle::ty::{GenericArg, GenericArgKind}; use rustc_span::symbol::kw; @@ -44,7 +44,9 @@ pub(super) fn mangle<'tcx>( let shim_kind = match instance.def { ty::InstanceDef::ThreadLocalShim(_) => Some("tls"), ty::InstanceDef::VTableShim(_) => Some("vtable"), - ty::InstanceDef::ReifyShim(_) => Some("reify"), + ty::InstanceDef::ReifyShim(_, None) => Some("reify"), + ty::InstanceDef::ReifyShim(_, Some(ReifyReason::FnPtr)) => Some("reify-fnptr"), + ty::InstanceDef::ReifyShim(_, Some(ReifyReason::Vtable)) => Some("reify-vtable"), ty::InstanceDef::ConstructCoroutineInClosureShim { .. } | ty::InstanceDef::CoroutineKindShim { .. } => Some("fn_once"), From 473a70de8457645df7a49558d6ba27405f966ee0 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Mon, 25 Mar 2024 17:23:55 +0000 Subject: [PATCH 3/5] CFI: Support function pointers for trait methods Adds support for both CFI and KCFI for attaching concrete and abstract types to functions. KCFI does this through generation of `ReifyShim` on any function pointer that could go in a vtable, and checking the `ReifyReason` when emitting the instance. CFI does this by attaching both the concrete and abstract type to every instance. TypeID codegen tests are switched to be anchored on the left rather than the right in order to allow emission of additional type attachments. Fixes #115953 --- compiler/rustc_middle/src/ty/instance.rs | 18 +++++++++ compiler/rustc_symbol_mangling/src/typeid.rs | 9 ++++- tests/ui/sanitizer/cfi-closures.rs | 4 -- tests/ui/sanitizer/cfi-method-fn-ptr-cast.rs | 42 ++++++++++++++++++-- 4 files changed, 63 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index e5625c8a5c634..4a7720b38f850 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -517,6 +517,24 @@ impl<'tcx> Instance<'tcx> { debug!(" => fn pointer created for virtual call"); resolved.def = InstanceDef::ReifyShim(def_id, reason); } + // Reify `Trait::method` implementations if KCFI is enabled + // FIXME(maurer) only reify it if it is a vtable-safe function + _ if tcx.sess.is_sanitizer_kcfi_enabled() + && tcx.associated_item(def_id).trait_item_def_id.is_some() => + { + // If this function could also go in a vtable, we need to `ReifyShim` it with + // KCFI because it can only attach one type per function. + resolved.def = InstanceDef::ReifyShim(resolved.def_id(), reason) + } + // Reify `::call`-like method implementations if KCFI is enabled + _ if tcx.sess.is_sanitizer_kcfi_enabled() + && tcx.is_closure_like(resolved.def_id()) => + { + // Reroute through a reify via the *unresolved* instance. The resolved one can't + // be directly reified because it's closure-like. The reify can handle the + // unresolved instance. + resolved = Instance { def: InstanceDef::ReifyShim(def_id, reason), args } + } _ => {} } diff --git a/compiler/rustc_symbol_mangling/src/typeid.rs b/compiler/rustc_symbol_mangling/src/typeid.rs index 11657b3b303af..862ba285db807 100644 --- a/compiler/rustc_symbol_mangling/src/typeid.rs +++ b/compiler/rustc_symbol_mangling/src/typeid.rs @@ -4,7 +4,7 @@ /// For more information about LLVM CFI and cross-language LLVM CFI support for the Rust compiler, /// see design document in the tracking issue #89653. use bitflags::bitflags; -use rustc_middle::ty::{Instance, Ty, TyCtxt}; +use rustc_middle::ty::{Instance, InstanceDef, ReifyReason, Ty, TyCtxt}; use rustc_target::abi::call::FnAbi; use std::hash::Hasher; use twox_hash::XxHash64; @@ -67,8 +67,13 @@ pub fn kcfi_typeid_for_fnabi<'tcx>( pub fn kcfi_typeid_for_instance<'tcx>( tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, - options: TypeIdOptions, + mut options: TypeIdOptions, ) -> u32 { + // If we receive a `ReifyShim` intended to produce a function pointer, we need to remain + // concrete - abstraction is for vtables. + if matches!(instance.def, InstanceDef::ReifyShim(_, Some(ReifyReason::FnPtr))) { + options.remove(TypeIdOptions::ERASE_SELF_TYPE); + } // A KCFI type metadata identifier is a 32-bit constant produced by taking the lower half of the // xxHash64 of the type metadata identifier. (See llvm/llvm-project@cff5bef.) let mut hash: XxHash64 = Default::default(); diff --git a/tests/ui/sanitizer/cfi-closures.rs b/tests/ui/sanitizer/cfi-closures.rs index 54f1cc035bc5b..f3d9be3571664 100644 --- a/tests/ui/sanitizer/cfi-closures.rs +++ b/tests/ui/sanitizer/cfi-closures.rs @@ -15,7 +15,6 @@ #![feature(fn_traits)] #![feature(unboxed_closures)] -#![feature(cfg_sanitize)] fn foo<'a, T>() -> Box &'a T> { Box::new(|x| x) @@ -72,9 +71,6 @@ fn use_closure(call: extern "rust-call" fn(&C, ()) -> i32, f: &C) -> i32 { } #[test] -// FIXME after KCFI reify support is added, remove this -// It will appear to work if you test locally, set -C opt-level=0 to see it fail. -#[cfg_attr(sanitize = "kcfi", ignore)] fn closure_addr_taken() { let x = 3i32; let f = || x; diff --git a/tests/ui/sanitizer/cfi-method-fn-ptr-cast.rs b/tests/ui/sanitizer/cfi-method-fn-ptr-cast.rs index 273b8785faeed..8f79de1174883 100644 --- a/tests/ui/sanitizer/cfi-method-fn-ptr-cast.rs +++ b/tests/ui/sanitizer/cfi-method-fn-ptr-cast.rs @@ -1,11 +1,41 @@ // Verifies that casting a method to a function pointer works. -// -// FIXME(#122848): Remove only-linux when fixed. + +//@ revisions: cfi kcfi +// FIXME(#122848) Remove only-linux once OSX CFI binaries work //@ only-linux -//@ needs-sanitizer-cfi -//@ compile-flags: -Clto -Copt-level=0 -Cprefer-dynamic=off -Ctarget-feature=-crt-static -Zsanitizer=cfi +//@ [cfi] needs-sanitizer-cfi +//@ [kcfi] needs-sanitizer-kcfi +//@ compile-flags: -C target-feature=-crt-static +//@ [cfi] compile-flags: -C opt-level=0 -C codegen-units=1 -C lto +//@ [cfi] compile-flags: -C prefer-dynamic=off +//@ [cfi] compile-flags: -Z sanitizer=cfi +//@ [kcfi] compile-flags: -Z sanitizer=kcfi +//@ [kcfi] compile-flags: -C panic=abort -C prefer-dynamic=off //@ run-pass +trait Foo { + fn foo(&self); + fn bar(&self); +} + +struct S; + +impl Foo for S { + fn foo(&self) {} + #[track_caller] + fn bar(&self) {} +} + +struct S2 { + f: fn(&S) +} + +impl S2 { + fn foo(&self, s: &S) { + (self.f)(s) + } +} + trait Trait1 { fn foo(&self); } @@ -20,4 +50,8 @@ fn main() { let type1 = Type1 {}; let f = ::foo; f(&type1); + // Check again with different optimization barriers + S2 { f: ::foo }.foo(&S); + // Check mismatched #[track_caller] + S2 { f: ::bar }.foo(&S) } From 3baf8be4088492f744110a9cf098a63a4bf34508 Mon Sep 17 00:00:00 2001 From: Ramon de C Valle Date: Fri, 22 Mar 2024 14:03:23 -0700 Subject: [PATCH 4/5] Revert "CFI: Skip non-passed arguments" Since we're now using an approach which is a variant of #123082 (that transforms closures into dynamic Fn traits but isolating it to the Fn call methods only) instead of #121962 or #122573, skipping non-passed arguments shouldn't be necessary for KCFI anymore and we can claim back the reduced granularity. This reverts commit f2f0d255df8d8345a481b949b63a35bcc5db7f2f. --- .../src/typeid/typeid_itanium_cxx_abi.rs | 22 +++----- ...-type-metadata-id-itanium-cxx-abi-paths.rs | 50 +++++++++---------- ...data-id-itanium-cxx-abi-primitive-types.rs | 6 ++- ...-itanium-cxx-abi-repr-transparent-types.rs | 2 +- .../sanitizer/cfi/normalize-integers.rs | 6 +-- 5 files changed, 39 insertions(+), 47 deletions(-) diff --git a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs index 25194e2c87ed3..331a4371fe3d5 100644 --- a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs +++ b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs @@ -20,7 +20,7 @@ use rustc_middle::ty::{ use rustc_middle::ty::{GenericArg, GenericArgKind, GenericArgsRef}; use rustc_span::def_id::DefId; use rustc_span::sym; -use rustc_target::abi::call::{Conv, FnAbi, PassMode}; +use rustc_target::abi::call::{Conv, FnAbi}; use rustc_target::abi::Integer; use rustc_target::spec::abi::Abi; use rustc_trait_selection::traits; @@ -1072,27 +1072,19 @@ pub fn typeid_for_fnabi<'tcx>( typeid.push_str(&encode_ty(tcx, ty, &mut dict, encode_ty_options)); // Encode the parameter types - - // We erase ZSTs as we go if the argument is skipped. This is an implementation detail of how - // MIR is currently treated by rustc, and subject to change in the future. Specifically, MIR - // interpretation today will allow skipped arguments to simply not be passed at a call-site. if !fn_abi.c_variadic { - let mut pushed_arg = false; - for arg in fn_abi.args.iter().filter(|arg| arg.mode != PassMode::Ignore) { - pushed_arg = true; - let ty = transform_ty(tcx, arg.layout.ty, &mut Vec::new(), transform_ty_options); - typeid.push_str(&encode_ty(tcx, ty, &mut dict, encode_ty_options)); - } - if !pushed_arg { + if !fn_abi.args.is_empty() { + for arg in fn_abi.args.iter() { + let ty = transform_ty(tcx, arg.layout.ty, &mut Vec::new(), transform_ty_options); + typeid.push_str(&encode_ty(tcx, ty, &mut dict, encode_ty_options)); + } + } else { // Empty parameter lists, whether declared as () or conventionally as (void), are // encoded with a void parameter specifier "v". typeid.push('v'); } } else { for n in 0..fn_abi.fixed_count as usize { - if fn_abi.args[n].mode == PassMode::Ignore { - continue; - } let ty = transform_ty(tcx, fn_abi.args[n].layout.ty, &mut Vec::new(), transform_ty_options); typeid.push_str(&encode_ty(tcx, ty, &mut dict, encode_ty_options)); diff --git a/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-paths.rs b/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-paths.rs index ca781a99296be..c5d8e0f22a2a0 100644 --- a/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-paths.rs +++ b/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-paths.rs @@ -47,42 +47,40 @@ pub fn foo() where let _: Type4 = ::bar; } -// Force arguments to be passed by using a reference. Otherwise, they may end up PassMode::Ignore - -pub fn foo1(_: &Type1) { } +pub fn foo1(_: Type1) { } // CHECK: define{{.*}}4foo1{{.*}}!type ![[TYPE1:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo2(_: &Type1, _: &Type1) { } +pub fn foo2(_: Type1, _: Type1) { } // CHECK: define{{.*}}4foo2{{.*}}!type ![[TYPE2:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo3(_: &Type1, _: &Type1, _: &Type1) { } +pub fn foo3(_: Type1, _: Type1, _: Type1) { } // CHECK: define{{.*}}4foo3{{.*}}!type ![[TYPE3:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo4(_: &Type2) { } +pub fn foo4(_: Type2) { } // CHECK: define{{.*}}4foo4{{.*}}!type ![[TYPE4:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo5(_: &Type2, _: &Type2) { } +pub fn foo5(_: Type2, _: Type2) { } // CHECK: define{{.*}}4foo5{{.*}}!type ![[TYPE5:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo6(_: &Type2, _: &Type2, _: &Type2) { } +pub fn foo6(_: Type2, _: Type2, _: Type2) { } // CHECK: define{{.*}}4foo6{{.*}}!type ![[TYPE6:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo7(_: &Type3) { } +pub fn foo7(_: Type3) { } // CHECK: define{{.*}}4foo7{{.*}}!type ![[TYPE7:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo8(_: &Type3, _: &Type3) { } +pub fn foo8(_: Type3, _: Type3) { } // CHECK: define{{.*}}4foo8{{.*}}!type ![[TYPE8:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo9(_: &Type3, _: &Type3, _: &Type3) { } +pub fn foo9(_: Type3, _: Type3, _: Type3) { } // CHECK: define{{.*}}4foo9{{.*}}!type ![[TYPE9:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo10(_: &Type4) { } +pub fn foo10(_: Type4) { } // CHECK: define{{.*}}5foo10{{.*}}!type ![[TYPE10:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo11(_: &Type4, _: &Type4) { } +pub fn foo11(_: Type4, _: Type4) { } // CHECK: define{{.*}}5foo11{{.*}}!type ![[TYPE11:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo12(_: &Type4, _: &Type4, _: &Type4) { } +pub fn foo12(_: Type4, _: Type4, _: Type4) { } // CHECK: define{{.*}}5foo12{{.*}}!type ![[TYPE12:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -// CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NvNFNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo10{{[{}][{}]}}extern{{[}][}]}}3barEE"} -// CHECK: ![[TYPE2]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NvNFNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo10{{[{}][{}]}}extern{{[}][}]}}3barES0_E"} -// CHECK: ![[TYPE3]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NvNFNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo10{{[{}][{}]}}extern{{[}][}]}}3barES0_S0_E"} -// CHECK: ![[TYPE4]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NtNCNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo11{{[{}][{}]}}closure{{[}][}]}}3FooEE"} -// CHECK: ![[TYPE5]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NtNCNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo11{{[{}][{}]}}closure{{[}][}]}}3FooES0_E"} -// CHECK: ![[TYPE6]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NtNCNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo11{{[{}][{}]}}closure{{[}][}]}}3FooES0_S0_E"} -// CHECK: ![[TYPE7]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NtNkNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo12{{[{}][{}]}}constant{{[}][}]}}3FooEE"} -// CHECK: ![[TYPE8]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NtNkNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo12{{[{}][{}]}}constant{{[}][}]}}3FooES0_E"} -// CHECK: ![[TYPE9]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NtNkNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo12{{[{}][{}]}}constant{{[}][}]}}3FooES0_S0_E"} -// CHECK: ![[TYPE10]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NvNINvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo8{{[{}][{}]}}impl{{[}][}]}}3barEE"} -// CHECK: ![[TYPE11]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NvNINvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo8{{[{}][{}]}}impl{{[}][}]}}3barES0_E"} -// CHECK: ![[TYPE12]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NvNINvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo8{{[{}][{}]}}impl{{[}][}]}}3barES0_S0_E"} +// CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNFNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo10{{[{}][{}]}}extern{{[}][}]}}3barE"} +// CHECK: ![[TYPE2]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNFNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo10{{[{}][{}]}}extern{{[}][}]}}3barS_E"} +// CHECK: ![[TYPE3]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNFNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo10{{[{}][{}]}}extern{{[}][}]}}3barS_S_E"} +// CHECK: ![[TYPE4]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNCNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo11{{[{}][{}]}}closure{{[}][}]}}3FooE"} +// CHECK: ![[TYPE5]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNCNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo11{{[{}][{}]}}closure{{[}][}]}}3FooS_E"} +// CHECK: ![[TYPE6]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNCNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo11{{[{}][{}]}}closure{{[}][}]}}3FooS_S_E"} +// CHECK: ![[TYPE7]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNkNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo12{{[{}][{}]}}constant{{[}][}]}}3FooE"} +// CHECK: ![[TYPE8]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNkNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo12{{[{}][{}]}}constant{{[}][}]}}3FooS_E"} +// CHECK: ![[TYPE9]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNkNvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo12{{[{}][{}]}}constant{{[}][}]}}3FooS_S_E"} +// CHECK: ![[TYPE10]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNINvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo8{{[{}][{}]}}impl{{[}][}]}}3barE"} +// CHECK: ![[TYPE11]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNINvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo8{{[{}][{}]}}impl{{[}][}]}}3barS_E"} +// CHECK: ![[TYPE12]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNINvC{{[[:print:]]+}}_{{[[:print:]]+}}3foo8{{[{}][{}]}}impl{{[}][}]}}3barS_S_E"} diff --git a/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-primitive-types.rs b/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-primitive-types.rs index 38f507856bdee..3a1a09150eae9 100644 --- a/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-primitive-types.rs +++ b/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-primitive-types.rs @@ -12,9 +12,9 @@ use core::ffi::*; pub fn foo1(_: ()) { } // CHECK: define{{.*}}4foo1{{.*}}!type ![[TYPE1:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo2(_: (), _: c_void) { } -// CHECK: define{{.*}}4foo2{{.*}}!type ![[TYPE1:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} +// CHECK: define{{.*}}4foo2{{.*}}!type ![[TYPE2:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo3(_: (), _: c_void, _: c_void) { } -// CHECK: define{{.*}}4foo3{{.*}}!type ![[TYPE2:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} +// CHECK: define{{.*}}4foo3{{.*}}!type ![[TYPE3:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo4(_: *mut ()) { } // CHECK: define{{.*}}4foo4{{.*}}!type ![[TYPE4:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo5(_: *mut (), _: *mut c_void) { } @@ -131,6 +131,8 @@ pub fn foo60(_: &str, _: &str, _: &str) { } // CHECK: define{{.*}}5foo60{{.*}}!type ![[TYPE60:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} // CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvvE"} +// CHECK: ![[TYPE2]] = !{i64 0, !"_ZTSFvvvE"} +// CHECK: ![[TYPE3]] = !{i64 0, !"_ZTSFvvvvE"} // CHECK: ![[TYPE4]] = !{i64 0, !"_ZTSFvPvE"} // CHECK: ![[TYPE5]] = !{i64 0, !"_ZTSFvPvS_E"} // CHECK: ![[TYPE6]] = !{i64 0, !"_ZTSFvPvS_S_E"} diff --git a/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-repr-transparent-types.rs b/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-repr-transparent-types.rs index 1332338b26ab9..6bad4a7820628 100644 --- a/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-repr-transparent-types.rs +++ b/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-repr-transparent-types.rs @@ -28,7 +28,7 @@ pub struct Type2<'a> { member3: &'a Type2<'a>, } -pub struct Bar(i32); +pub struct Bar; // repr(transparent) user-defined generic type #[repr(transparent)] diff --git a/tests/codegen/sanitizer/cfi/normalize-integers.rs b/tests/codegen/sanitizer/cfi/normalize-integers.rs index 801ed312be5b1..210814eb9ae1f 100644 --- a/tests/codegen/sanitizer/cfi/normalize-integers.rs +++ b/tests/codegen/sanitizer/cfi/normalize-integers.rs @@ -41,6 +41,6 @@ pub fn foo11(_: (), _: usize, _: usize, _: usize) { } // CHECK: ![[TYPE6]] = !{i64 0, !"_ZTSFv{{u3i16|u3i32|u3i64|u4i128}}E.normalized"} // CHECK: ![[TYPE7]] = !{i64 0, !"_ZTSFv{{u3i16|u3i32|u3i64|u4i128}}S_E.normalized"} // CHECK: ![[TYPE8]] = !{i64 0, !"_ZTSFv{{u3i16|u3i32|u3i64|u4i128}}S_S_E.normalized"} -// CHECK: ![[TYPE9]] = !{i64 0, !"_ZTSFv{{u3u16|u3u32|u3u64|u4u128}}E.normalized"} -// CHECK: ![[TYPE10]] = !{i64 0, !"_ZTSFv{{u3u16|u3u32|u3u64|u4u128}}S_E.normalized"} -// CHECK: ![[TYPE11]] = !{i64 0, !"_ZTSFv{{u3u16|u3u32|u3u64|u4u128}}S_S_E.normalized"} +// CHECK: ![[TYPE9]] = !{i64 0, !"_ZTSFvv{{u3u16|u3u32|u3u64|u4u128}}E.normalized"} +// CHECK: ![[TYPE10]] = !{i64 0, !"_ZTSFvv{{u3u16|u3u32|u3u64|u4u128}}S_E.normalized"} +// CHECK: ![[TYPE11]] = !{i64 0, !"_ZTSFvv{{u3u16|u3u32|u3u64|u4u128}}S_S_E.normalized"} From d73c594a9d10eb31527b476b8d291961e8433ca8 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Wed, 3 Apr 2024 23:45:11 +0000 Subject: [PATCH 5/5] CFI: Only erase leading non-passed arguments The purpose of erasing non-passed arguments was to support the compiler's implicit cast between `call(ZSTClosure, (T, U, ..))` and `fn(T, U, ..)`. We can narrow this erasure by only performing it if the call Abi was "rust-call". --- compiler/rustc_symbol_mangling/src/typeid.rs | 6 +++++ .../src/typeid/typeid_itanium_cxx_abi.rs | 27 ++++++++++++++----- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_symbol_mangling/src/typeid.rs b/compiler/rustc_symbol_mangling/src/typeid.rs index 862ba285db807..2182dd12916ad 100644 --- a/compiler/rustc_symbol_mangling/src/typeid.rs +++ b/compiler/rustc_symbol_mangling/src/typeid.rs @@ -27,6 +27,12 @@ bitflags! { /// Generalize the instance by erasing the concrete `Self` type where possible. /// Only has an effect on `{kcfi_,}typeid_for_instance`. const ERASE_SELF_TYPE = 8; + /// If the leading argument is non-passed, skip it. This is used to support the cast of + /// non-capturing closures to function pointers. This is normally set automatically by + /// `typeid_for_instance` upon encountering a closure-like. It may be set for calls to + /// `typeid_for_fnabi` if the caller knows they're calling a `call` method to potentially + /// non-capturing closure. + const ERASE_LEADING_NONPASSED = 16; } } diff --git a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs index 331a4371fe3d5..734d3d0bcf13a 100644 --- a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs +++ b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs @@ -20,7 +20,7 @@ use rustc_middle::ty::{ use rustc_middle::ty::{GenericArg, GenericArgKind, GenericArgsRef}; use rustc_span::def_id::DefId; use rustc_span::sym; -use rustc_target::abi::call::{Conv, FnAbi}; +use rustc_target::abi::call::{Conv, FnAbi, PassMode}; use rustc_target::abi::Integer; use rustc_target::spec::abi::Abi; use rustc_trait_selection::traits; @@ -1073,12 +1073,19 @@ pub fn typeid_for_fnabi<'tcx>( // Encode the parameter types if !fn_abi.c_variadic { - if !fn_abi.args.is_empty() { - for arg in fn_abi.args.iter() { - let ty = transform_ty(tcx, arg.layout.ty, &mut Vec::new(), transform_ty_options); - typeid.push_str(&encode_ty(tcx, ty, &mut dict, encode_ty_options)); + let mut pushed_arg = false; + for (idx, arg) in fn_abi.args.iter().enumerate() { + if idx == 0 + && options.contains(TypeIdOptions::ERASE_LEADING_NONPASSED) + && arg.mode == PassMode::Ignore + { + continue; } - } else { + pushed_arg = true; + let ty = transform_ty(tcx, arg.layout.ty, &mut Vec::new(), transform_ty_options); + typeid.push_str(&encode_ty(tcx, ty, &mut dict, encode_ty_options)); + } + if !pushed_arg { // Empty parameter lists, whether declared as () or conventionally as (void), are // encoded with a void parameter specifier "v". typeid.push('v'); @@ -1113,7 +1120,7 @@ pub fn typeid_for_fnabi<'tcx>( pub fn typeid_for_instance<'tcx>( tcx: TyCtxt<'tcx>, mut instance: Instance<'tcx>, - options: TypeIdOptions, + mut options: TypeIdOptions, ) -> String { if (matches!(instance.def, ty::InstanceDef::Virtual(..)) && Some(instance.def_id()) == tcx.lang_items().drop_in_place_fn()) @@ -1242,6 +1249,12 @@ pub fn typeid_for_instance<'tcx>( } } + if tcx.is_closure_like(instance.def_id()) + || tcx.fn_sig(instance.def_id()).skip_binder().abi() == Abi::RustCall + { + options.insert(TypeIdOptions::ERASE_LEADING_NONPASSED) + } + let fn_abi = tcx .fn_abi_of_instance(tcx.param_env(instance.def_id()).and((instance, ty::List::empty()))) .unwrap_or_else(|error| {