From de2edc3b231a0ddc12483fbca110f1660aa2a414 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Sat, 2 Mar 2024 19:56:16 +0000 Subject: [PATCH 01/13] Introduce trait_obj_ty query This query computes the trait object, complete with associated type projections for its supertraits, from a trait ref. This is intended for use by CFI shimming. --- compiler/rustc_middle/src/query/mod.rs | 9 ++++++ .../src/traits/vtable.rs | 29 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 8357c21a3c2be..b52b681ae6b11 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -2247,6 +2247,15 @@ rustc_queries! { query find_field((def_id, ident): (DefId, rustc_span::symbol::Ident)) -> Option { desc { |tcx| "find the index of maybe nested field `{ident}` in `{}`", tcx.def_path_str(def_id) } } + + /// Construct a type for a trait object corresponding to `trait_ref`. This type will have all + /// associated types for it and its supertraits expanded and resolved as additional predicates. + /// + /// The provided `trait_ref` must be sufficiently instantiated that all associated types can be + /// successfully resolved. + query trait_object_ty(trait_ref: ty::PolyTraitRef<'tcx>) -> Ty<'tcx> { + desc { "Compute the trait object type for calling a method on a trait" } + } } rustc_query_append! { define_callbacks! } diff --git a/compiler/rustc_trait_selection/src/traits/vtable.rs b/compiler/rustc_trait_selection/src/traits/vtable.rs index 3c0316fce171d..1c2bd227ac000 100644 --- a/compiler/rustc_trait_selection/src/traits/vtable.rs +++ b/compiler/rustc_trait_selection/src/traits/vtable.rs @@ -1,4 +1,5 @@ use crate::errors::DumpVTableEntries; +use crate::traits; use crate::traits::{impossible_predicates, is_vtable_safe_method}; use rustc_hir::def_id::DefId; use rustc_hir::lang_items::LangItem; @@ -13,6 +14,7 @@ use rustc_span::{sym, Span}; use smallvec::SmallVec; use std::fmt::Debug; +use std::iter; use std::ops::ControlFlow; #[derive(Clone, Debug)] @@ -232,6 +234,32 @@ fn own_existential_vtable_entries_iter( own_entries } +fn trait_object_ty<'tcx>(tcx: TyCtxt<'tcx>, poly_trait_ref: ty::PolyTraitRef<'tcx>) -> Ty<'tcx> { + let principal_pred = poly_trait_ref.map_bound(|trait_ref| { + ty::ExistentialPredicate::Trait(ty::ExistentialTraitRef::erase_self_ty(tcx, trait_ref)) + }); + let assoc_preds = traits::supertraits(tcx, poly_trait_ref).flat_map(|super_poly_trait_ref| { + tcx.associated_items(super_poly_trait_ref.def_id()) + .in_definition_order() + .filter(|item| item.kind == ty::AssocKind::Type) + .map(move |assoc_ty| { + super_poly_trait_ref.map_bound(|super_trait_ref| { + let alias_ty = ty::AliasTy::new(tcx, assoc_ty.def_id, super_trait_ref.args); + let resolved = tcx + .normalize_erasing_regions(ty::ParamEnv::reveal_all(), alias_ty.to_ty(tcx)); + ty::ExistentialPredicate::Projection(ty::ExistentialProjection { + def_id: assoc_ty.def_id, + args: ty::ExistentialTraitRef::erase_self_ty(tcx, super_trait_ref).args, + term: resolved.into(), + }) + }) + }) + }); + let preds = + tcx.mk_poly_existential_predicates_from_iter(iter::once(principal_pred).chain(assoc_preds)); + Ty::new_dynamic(tcx, preds, tcx.lifetimes.re_erased, ty::Dyn) +} + /// Given a trait `trait_ref`, iterates the vtable entries /// that come from `trait_ref`, including its supertraits. fn vtable_entries<'tcx>( @@ -403,6 +431,7 @@ pub(super) fn provide(providers: &mut Providers) { own_existential_vtable_entries, vtable_entries, vtable_trait_upcasting_coercion_new_vptr_slot, + trait_object_ty, ..*providers }; } From 754c789460811b2723adcfaf8a5d4083b0c84938 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Fri, 16 Feb 2024 15:34:43 +0000 Subject: [PATCH 02/13] Refactor visiting instance_def In preparation to add recursive instance_defs, move this logic to its own convenience method. --- compiler/rustc_middle/src/mir/visit.rs | 54 +++++++++++++------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 2c5ca82a4cd39..5d90ac2f51f3e 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -331,33 +331,10 @@ macro_rules! make_mir_visitor { self.visit_source_scope($(& $mutability)? *parent_scope); } if let Some((callee, callsite_span)) = inlined { - let location = Location::START; - self.visit_span($(& $mutability)? *callsite_span); - - let ty::Instance { def: callee_def, args: callee_args } = callee; - match callee_def { - ty::InstanceDef::Item(_def_id) => {} - - ty::InstanceDef::Intrinsic(_def_id) | - ty::InstanceDef::VTableShim(_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: _ } | - ty::InstanceDef::ConstructCoroutineInClosureShim { coroutine_closure_def_id: _def_id, target_kind: _ } | - ty::InstanceDef::CoroutineKindShim { coroutine_def_id: _def_id, target_kind: _ } | - ty::InstanceDef::DropGlue(_def_id, None) => {} - - ty::InstanceDef::FnPtrShim(_def_id, ty) | - ty::InstanceDef::DropGlue(_def_id, Some(ty)) | - ty::InstanceDef::CloneShim(_def_id, ty) | - ty::InstanceDef::FnPtrAddrShim(_def_id, ty) => { - // FIXME(eddyb) use a better `TyContext` here. - self.visit_ty($(& $mutability)? *ty, TyContext::Location(location)); - } - } - self.visit_args(callee_args, location); + let ty::Instance { def, args } = callee; + self.visit_instance_def($(& $mutability)? *def); + self.visit_args(& $($mutability)? *args, Location::START); } if let Some(inlined_parent_scope) = inlined_parent_scope { self.visit_source_scope($(& $mutability)? *inlined_parent_scope); @@ -940,6 +917,31 @@ macro_rules! make_mir_visitor { // Convenience methods + fn visit_instance_def(&mut self, def: $(& $mutability)? ty::InstanceDef<'tcx>) { + let location = Location::START; + match def { + ty::InstanceDef::Item(_def_id) => {} + + ty::InstanceDef::Intrinsic(_def_id) | + ty::InstanceDef::VTableShim(_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: _ } | + ty::InstanceDef::ConstructCoroutineInClosureShim { coroutine_closure_def_id: _def_id, target_kind: _ } | + ty::InstanceDef::CoroutineKindShim { coroutine_def_id: _def_id, target_kind: _ } | + ty::InstanceDef::DropGlue(_def_id, None) => {} + + ty::InstanceDef::FnPtrShim(_def_id, ty) | + ty::InstanceDef::DropGlue(_def_id, Some(ty)) | + ty::InstanceDef::CloneShim(_def_id, ty) | + ty::InstanceDef::FnPtrAddrShim(_def_id, ty) => { + // FIXME(eddyb) use a better `TyContext` here. + self.visit_ty($(& $mutability *)? ty, TyContext::Location(location)); + } + } + } + fn visit_location( &mut self, body: &$($mutability)? Body<'tcx>, From 28bdb59a911323bba4e03832f2bed3faa089496a Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Thu, 22 Feb 2024 23:15:00 +0000 Subject: [PATCH 03/13] Refactor fmt_instance Factored out to minimize the amount of noise in the main CfiShim defining patch. --- compiler/rustc_middle/src/ty/instance.rs | 36 +++++++++++++----------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index 260d0885089ef..67cf450d2f321 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -323,6 +323,25 @@ impl<'tcx> InstanceDef<'tcx> { } } +fn fmt_instance_def(f: &mut fmt::Formatter<'_>, instance_def: &InstanceDef<'_>) -> fmt::Result { + match instance_def { + InstanceDef::Item(_) => Ok(()), + InstanceDef::VTableShim(_) => write!(f, " - shim(vtable)"), + InstanceDef::ReifyShim(_) => write!(f, " - shim(reify)"), + InstanceDef::ThreadLocalShim(_) => write!(f, " - shim(tls)"), + InstanceDef::Intrinsic(_) => write!(f, " - intrinsic"), + InstanceDef::Virtual(_, num) => write!(f, " - virtual#{num}"), + InstanceDef::FnPtrShim(_, ty) => write!(f, " - shim({ty})"), + InstanceDef::ClosureOnceShim { .. } => write!(f, " - shim"), + InstanceDef::ConstructCoroutineInClosureShim { .. } => write!(f, " - shim"), + InstanceDef::CoroutineKindShim { .. } => write!(f, " - shim"), + InstanceDef::DropGlue(_, None) => write!(f, " - shim(None)"), + InstanceDef::DropGlue(_, Some(ty)) => write!(f, " - shim(Some({ty}))"), + InstanceDef::CloneShim(_, ty) => write!(f, " - shim({ty})"), + InstanceDef::FnPtrAddrShim(_, ty) => write!(f, " - shim({ty})"), + } +} + fn fmt_instance( f: &mut fmt::Formatter<'_>, instance: &Instance<'_>, @@ -341,22 +360,7 @@ fn fmt_instance( f.write_str(&s) })?; - match instance.def { - InstanceDef::Item(_) => Ok(()), - InstanceDef::VTableShim(_) => write!(f, " - shim(vtable)"), - InstanceDef::ReifyShim(_) => write!(f, " - shim(reify)"), - InstanceDef::ThreadLocalShim(_) => write!(f, " - shim(tls)"), - InstanceDef::Intrinsic(_) => write!(f, " - intrinsic"), - InstanceDef::Virtual(_, num) => write!(f, " - virtual#{num}"), - InstanceDef::FnPtrShim(_, ty) => write!(f, " - shim({ty})"), - InstanceDef::ClosureOnceShim { .. } => write!(f, " - shim"), - InstanceDef::ConstructCoroutineInClosureShim { .. } => write!(f, " - shim"), - InstanceDef::CoroutineKindShim { .. } => write!(f, " - shim"), - InstanceDef::DropGlue(_, None) => write!(f, " - shim(None)"), - InstanceDef::DropGlue(_, Some(ty)) => write!(f, " - shim(Some({ty}))"), - InstanceDef::CloneShim(_, ty) => write!(f, " - shim({ty})"), - InstanceDef::FnPtrAddrShim(_, ty) => write!(f, " - shim({ty})"), - } + fmt_instance_def(f, &instance.def) } pub struct ShortInstance<'a, 'tcx>(pub &'a Instance<'tcx>, pub usize); From ff7fb597b9f53216bec91fdbd8ace6401d87f288 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Tue, 27 Feb 2024 23:35:29 +0000 Subject: [PATCH 04/13] Refactor to create InstanceDef::fn_sig --- compiler/rustc_middle/src/ty/instance.rs | 21 ++++++++++++++++++ compiler/rustc_mir_transform/src/shim.rs | 13 +----------- compiler/rustc_ty_utils/src/abi.rs | 27 ++++++------------------ 3 files changed, 28 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index 67cf450d2f321..386f33c00ee47 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -321,6 +321,27 @@ impl<'tcx> InstanceDef<'tcx> { | InstanceDef::VTableShim(..) => true, } } + + /// Computes the signature of the InstanceDef, possibly with adjustments based on the kind of + /// shim. + pub fn fn_sig(&self, tcx: TyCtxt<'tcx>) -> EarlyBinder> { + tcx.fn_sig(self.def_id()).map_bound(|sig| { + sig.map_bound(|mut sig| { + if let InstanceDef::VTableShim(..) = self { + // Modify fn(self, ...) to fn(self: *mut Self, ...) + let mut inputs_and_output = sig.inputs_and_output.to_vec(); + let self_arg = &mut inputs_and_output[0]; + debug_assert!( + tcx.generics_of(self.def_id()).has_self + && *self_arg == tcx.types.self_param + ); + *self_arg = Ty::new_mut_ptr(tcx, *self_arg); + sig.inputs_and_output = tcx.mk_type_list(&inputs_and_output); + } + sig + }) + }) + } } fn fmt_instance_def(f: &mut fmt::Formatter<'_>, instance_def: &InstanceDef<'_>) -> fmt::Result { diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index 733e2f93b2535..967158685ea4f 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -736,7 +736,7 @@ fn build_call_shim<'tcx>( }; let def_id = instance.def_id(); - let sig = tcx.fn_sig(def_id); + let sig = instance.fn_sig(tcx); let sig = sig.map_bound(|sig| tcx.instantiate_bound_regions_with_erased(sig)); assert_eq!(sig_args.is_some(), !instance.has_polymorphic_mir_body()); @@ -773,17 +773,6 @@ fn build_call_shim<'tcx>( sig.inputs_and_output = tcx.mk_type_list(&inputs_and_output); } - // FIXME(eddyb) avoid having this snippet both here and in - // `Instance::fn_sig` (introduce `InstanceDef::fn_sig`?). - if let ty::InstanceDef::VTableShim(..) = instance { - // Modify fn(self, ...) to fn(self: *mut Self, ...) - let mut inputs_and_output = sig.inputs_and_output.to_vec(); - let self_arg = &mut inputs_and_output[0]; - debug_assert!(tcx.generics_of(def_id).has_self && *self_arg == tcx.types.self_param); - *self_arg = Ty::new_mut_ptr(tcx, *self_arg); - sig.inputs_and_output = tcx.mk_type_list(&inputs_and_output); - } - let span = tcx.def_span(def_id); debug!(?sig); diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs index 43042dbd36641..8bbc9fddf6d21 100644 --- a/compiler/rustc_ty_utils/src/abi.rs +++ b/compiler/rustc_ty_utils/src/abi.rs @@ -43,7 +43,7 @@ fn fn_sig_for_fn_abi<'tcx>( let ty = instance.ty(tcx, param_env); match *ty.kind() { - ty::FnDef(..) => { + ty::FnDef(def_id, args) => { // HACK(davidtwco,eddyb): This is a workaround for polymorphization considering // parameters unused if they show up in the signature, but not in the `mir::Body` // (i.e. due to being inside a projection that got normalized, see @@ -51,26 +51,11 @@ fn fn_sig_for_fn_abi<'tcx>( // track of a polymorphization `ParamEnv` to allow normalizing later. // // We normalize the `fn_sig` again after instantiating at a later point. - let mut sig = match *ty.kind() { - ty::FnDef(def_id, args) => tcx - .fn_sig(def_id) - .map_bound(|fn_sig| { - tcx.normalize_erasing_regions(tcx.param_env(def_id), fn_sig) - }) - .instantiate(tcx, args), - _ => unreachable!(), - }; - - if let ty::InstanceDef::VTableShim(..) = instance.def { - // Modify `fn(self, ...)` to `fn(self: *mut Self, ...)`. - sig = sig.map_bound(|mut sig| { - let mut inputs_and_output = sig.inputs_and_output.to_vec(); - inputs_and_output[0] = Ty::new_mut_ptr(tcx, inputs_and_output[0]); - sig.inputs_and_output = tcx.mk_type_list(&inputs_and_output); - sig - }); - } - sig + instance + .def + .fn_sig(tcx) + .map_bound(|fn_sig| tcx.normalize_erasing_regions(tcx.param_env(def_id), fn_sig)) + .instantiate(tcx, args) } ty::Closure(def_id, args) => { let sig = args.as_closure().sig(); From 21025515d25126cb1fedc3f45aa2ab958b9e8c0d Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Mon, 29 Jan 2024 21:12:04 +0000 Subject: [PATCH 05/13] CFI: Introduce CFI shims Indirect calls through vtables (trait objects or drop_in_place) expect to have a type based on `dyn Trait` at the call-site. The actual implementations have types based on `MyImplType`. These shims allow the insertion of an explicit cast at the beginning of any instance, allowing a different type to be assigned. These shims function for both CFI and KCFI, as they have a single principal type. --- .../src/interpret/terminator.rs | 1 + compiler/rustc_middle/src/mir/mono.rs | 3 +- compiler/rustc_middle/src/mir/visit.rs | 15 +++ compiler/rustc_middle/src/ty/codec.rs | 1 + compiler/rustc_middle/src/ty/instance.rs | 96 ++++++++++++++++- compiler/rustc_middle/src/ty/mod.rs | 3 +- .../rustc_middle/src/ty/structural_impls.rs | 24 +++++ compiler/rustc_mir_transform/src/inline.rs | 3 +- .../rustc_mir_transform/src/inline/cycle.rs | 3 +- compiler/rustc_mir_transform/src/lib.rs | 1 + .../src/rewrite_receiver.rs | 100 ++++++++++++++++++ compiler/rustc_mir_transform/src/shim.rs | 17 ++- compiler/rustc_monomorphize/src/collector.rs | 3 +- .../rustc_monomorphize/src/partitioning.rs | 13 ++- compiler/rustc_session/src/session.rs | 9 ++ .../rustc_smir/src/rustc_smir/convert/ty.rs | 3 +- compiler/rustc_symbol_mangling/src/legacy.rs | 5 + compiler/rustc_symbol_mangling/src/v0.rs | 21 ++-- compiler/rustc_ty_utils/src/abi.rs | 2 +- 19 files changed, 301 insertions(+), 22 deletions(-) create mode 100644 compiler/rustc_mir_transform/src/rewrite_receiver.rs diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index e72ace8be3559..06aa22351566f 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -547,6 +547,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { | ty::InstanceDef::CloneShim(..) | ty::InstanceDef::FnPtrAddrShim(..) | ty::InstanceDef::ThreadLocalShim(..) + | ty::InstanceDef::CfiShim { .. } | ty::InstanceDef::Item(_) => { // We need MIR for this fn let Some((body, instance)) = M::find_mir_or_eval_fn( diff --git a/compiler/rustc_middle/src/mir/mono.rs b/compiler/rustc_middle/src/mir/mono.rs index 43e1318a75ab1..13b43593f413d 100644 --- a/compiler/rustc_middle/src/mir/mono.rs +++ b/compiler/rustc_middle/src/mir/mono.rs @@ -406,7 +406,8 @@ impl<'tcx> CodegenUnit<'tcx> { | InstanceDef::DropGlue(..) | InstanceDef::CloneShim(..) | InstanceDef::ThreadLocalShim(..) - | InstanceDef::FnPtrAddrShim(..) => None, + | InstanceDef::FnPtrAddrShim(..) + | InstanceDef::CfiShim { .. } => None, } } MonoItem::Static(def_id) => def_id.as_local().map(Idx::index), diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 5d90ac2f51f3e..18d2c7b442135 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -939,6 +939,21 @@ macro_rules! make_mir_visitor { // FIXME(eddyb) use a better `TyContext` here. self.visit_ty($(& $mutability *)? ty, TyContext::Location(location)); } + ty::InstanceDef::CfiShim { target_instance, invoke_ty } => { + self.visit_ty($(& $mutability *)? invoke_ty, TyContext::Location(location)); + let $($mutability)? local_target_instance = { + $( + let $mutability _unused = (); + * + )? + *target_instance + }; + self.visit_instance_def($(& $mutability)? local_target_instance); + $( + *target_instance = self.tcx().arena.alloc(local_target_instance); + let $mutability _unused = (); + )? + } } } diff --git a/compiler/rustc_middle/src/ty/codec.rs b/compiler/rustc_middle/src/ty/codec.rs index 69ae05ca820a3..475c125bf10ac 100644 --- a/compiler/rustc_middle/src/ty/codec.rs +++ b/compiler/rustc_middle/src/ty/codec.rs @@ -526,6 +526,7 @@ impl_arena_copy_decoder! {<'tcx> rustc_span::def_id::LocalDefId, (rustc_middle::middle::exported_symbols::ExportedSymbol<'tcx>, rustc_middle::middle::exported_symbols::SymbolExportInfo), ty::DeducedParamAttrs, + ty::InstanceDef<'tcx>, } #[macro_export] diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index 386f33c00ee47..f74bb4a7cf6e1 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -1,7 +1,7 @@ use crate::middle::codegen_fn_attrs::CodegenFnAttrFlags; use crate::ty::print::{FmtPrinter, Printer}; use crate::ty::{self, Ty, TyCtxt, TypeFoldable, TypeSuperFoldable}; -use crate::ty::{EarlyBinder, GenericArgs, GenericArgsRef, TypeVisitableExt}; +use crate::ty::{EarlyBinder, GenericArgs, GenericArgsRef, Lift, TypeVisitableExt}; use rustc_errors::ErrorGuaranteed; use rustc_hir as hir; use rustc_hir::def::Namespace; @@ -135,14 +135,26 @@ pub enum InstanceDef<'tcx> { /// /// The `DefId` is for `FnPtr::addr`, the `Ty` is the type `T`. FnPtrAddrShim(DefId, Ty<'tcx>), + + /// Typecast shim which replaces the `Self` type with the provided type. + /// This is used in vtable calls, where the type of `Self` is abstract as of the time of + /// the call. + /// + /// `target_instance` should be a generatable shim. This shim will have a transmute prefixed to + /// it to cast the receiver from abstract to concrete. + CfiShim { target_instance: &'tcx InstanceDef<'tcx>, invoke_ty: Ty<'tcx> }, } impl<'tcx> Instance<'tcx> { /// Returns the `Ty` corresponding to this `Instance`, with generic instantiations applied and /// lifetimes erased, allowing a `ParamEnv` to be specified for use during normalization. pub fn ty(&self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> Ty<'tcx> { - let ty = tcx.type_of(self.def.def_id()); - tcx.instantiate_and_normalize_erasing_regions(self.args, param_env, ty) + let args = if let InstanceDef::CfiShim { invoke_ty, .. } = self.def { + tcx.mk_args_trait(invoke_ty, (*self.args).into_iter().skip(1)) + } else { + self.args + }; + tcx.instantiate_and_normalize_erasing_regions(args, param_env, tcx.type_of(self.def_id())) } /// Finds a crate that contains a monomorphization of this instance that @@ -198,6 +210,7 @@ impl<'tcx> InstanceDef<'tcx> { | InstanceDef::DropGlue(def_id, _) | InstanceDef::CloneShim(def_id, _) | InstanceDef::FnPtrAddrShim(def_id, _) => def_id, + InstanceDef::CfiShim { target_instance, .. } => target_instance.def_id(), } } @@ -209,6 +222,7 @@ impl<'tcx> InstanceDef<'tcx> { Some(def_id) } InstanceDef::VTableShim(..) + | InstanceDef::CfiShim { .. } | InstanceDef::ReifyShim(..) | InstanceDef::FnPtrShim(..) | InstanceDef::Virtual(..) @@ -319,6 +333,9 @@ impl<'tcx> InstanceDef<'tcx> { | InstanceDef::ReifyShim(..) | InstanceDef::Virtual(..) | InstanceDef::VTableShim(..) => true, + InstanceDef::CfiShim { target_instance, .. } => { + target_instance.has_polymorphic_mir_body() + } } } @@ -360,6 +377,10 @@ fn fmt_instance_def(f: &mut fmt::Formatter<'_>, instance_def: &InstanceDef<'_>) InstanceDef::DropGlue(_, Some(ty)) => write!(f, " - shim(Some({ty}))"), InstanceDef::CloneShim(_, ty) => write!(f, " - shim({ty})"), InstanceDef::FnPtrAddrShim(_, ty) => write!(f, " - shim({ty})"), + InstanceDef::CfiShim { invoke_ty, target_instance } => { + fmt_instance_def(f, target_instance)?; + write!(f, " - cfi-shim({invoke_ty})") + } } } @@ -600,6 +621,75 @@ impl<'tcx> Instance<'tcx> { Instance::expect_resolve(tcx, ty::ParamEnv::reveal_all(), def_id, args) } + pub fn cfi_shim( + mut self, + tcx: TyCtxt<'tcx>, + invoke_trait: Option>, + ) -> ty::Instance<'tcx> { + if tcx.sess.cfi_shims() { + let invoke_ty = if let Some(poly_trait_ref) = invoke_trait { + tcx.trait_object_ty(poly_trait_ref) + } else { + Ty::new_dynamic(tcx, ty::List::empty(), tcx.lifetimes.re_erased, ty::Dyn) + }; + if tcx.is_closure_like(self.def.def_id()) { + // Closures don't have a fn_sig and can't be called directly. + // Adjust it into a call through + // `Fn`/`FnMut`/`FnOnce`/`AsyncFn`/`AsyncFnMut`/`AsyncFnOnce`/`Coroutine` + // based on its receiver. + let ty::TyKind::Dynamic(pep, _, _) = invoke_ty.kind() else { + bug!("Closure-like with non-dynamic invoke_ty {invoke_ty}") + }; + let Some(fn_trait) = pep.principal_def_id() else { + bug!("Closure-like with no principal trait on invoke_ty {invoke_ty}") + }; + let call = tcx + .associated_items(fn_trait) + .in_definition_order() + .find(|it| it.kind == ty::AssocKind::Fn) + .expect("No call-family function on closure-like principal trait?") + .def_id; + + let self_ty = self.ty(tcx, ty::ParamEnv::reveal_all()); + let tupled_inputs_ty = + self.args.as_closure().sig().map_bound(|sig| sig.inputs()[0]); + let tupled_inputs_ty = tcx.instantiate_bound_regions_with_erased(tupled_inputs_ty); + self.args = tcx.mk_args_trait(self_ty, [tupled_inputs_ty.into()]); + self.def = InstanceDef::ReifyShim(call); + } else if let Some(impl_id) = tcx.impl_of_method(self.def.def_id()) { + // Trait methods will have a Self polymorphic parameter, where the concreteized + // implementatation will not. We need to walk back to the more general trait method + // so that we can swap out Self when generating a type signature. + let Some(trait_ref) = tcx.impl_trait_ref(impl_id) else { + bug!("When trying to rewrite the type on {self}, found inherent impl method") + }; + let trait_ref = trait_ref.instantiate(tcx, self.args); + + let method_id = tcx + .impl_item_implementor_ids(impl_id) + .items() + .filter_map(|(trait_method, impl_method)| { + (*impl_method == self.def.def_id()).then_some(*trait_method) + }) + .min() + .unwrap(); + self.def = InstanceDef::ReifyShim(method_id); + self.args = trait_ref.args; + } + // At this point, we should have gauranteed that the first item in the args list is + // the dispatch type. We can't check for Self, because `drop_in_place` takes `T`. + self.def = InstanceDef::CfiShim { + target_instance: (&self.def).lift_to_tcx(tcx).expect("Could not lift for shimming"), + invoke_ty, + }; + } + self + } + + pub fn force_thin_self(&self) -> bool { + matches!(self.def, InstanceDef::Virtual(..) | InstanceDef::CfiShim { .. }) + } + #[instrument(level = "debug", skip(tcx), ret)] pub fn fn_once_adapter_instance( tcx: TyCtxt<'tcx>, diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 0a38d379a5216..5767eb454b2de 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -1723,7 +1723,8 @@ impl<'tcx> TyCtxt<'tcx> { | ty::InstanceDef::DropGlue(..) | ty::InstanceDef::CloneShim(..) | ty::InstanceDef::ThreadLocalShim(..) - | ty::InstanceDef::FnPtrAddrShim(..) => self.mir_shims(instance), + | ty::InstanceDef::FnPtrAddrShim(..) + | ty::InstanceDef::CfiShim { .. } => self.mir_shims(instance), } } diff --git a/compiler/rustc_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs index c8fb11673cf89..f2e447562d164 100644 --- a/compiler/rustc_middle/src/ty/structural_impls.rs +++ b/compiler/rustc_middle/src/ty/structural_impls.rs @@ -479,6 +479,14 @@ impl<'a, 'tcx> Lift<'tcx> for Term<'a> { } } +impl<'a, 'b, 'tcx> Lift<'tcx> for &'b ty::InstanceDef<'a> { + type Lifted = &'tcx ty::InstanceDef<'tcx>; + fn lift_to_tcx(self, tcx: TyCtxt<'tcx>) -> Option { + let lifted: ty::InstanceDef<'tcx> = (*self).lift_to_tcx(tcx)?; + Some(tcx.arena.alloc(lifted)) + } +} + /////////////////////////////////////////////////////////////////////////// // Traversal implementations. @@ -488,6 +496,22 @@ impl<'tcx> TypeVisitable> for ty::AdtDef<'tcx> { } } +impl<'tcx> TypeVisitable> for &ty::InstanceDef<'tcx> { + fn visit_with>>(&self, visitor: &mut V) -> V::Result { + (*self).visit_with(visitor) + } +} + +impl<'tcx> TypeFoldable> for &'tcx ty::InstanceDef<'tcx> { + fn try_fold_with>>( + self, + folder: &mut F, + ) -> Result { + let folded: ty::InstanceDef<'tcx> = (*self).try_fold_with(folder)?; + Ok(folder.interner().arena.alloc(folded)) + } +} + impl<'tcx, T: TypeFoldable>> TypeFoldable> for ty::Binder<'tcx, T> { fn try_fold_with>>( self, diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 36546a03cdfc5..b068939a4c948 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -331,7 +331,8 @@ impl<'tcx> Inliner<'tcx> { | InstanceDef::DropGlue(..) | InstanceDef::CloneShim(..) | InstanceDef::ThreadLocalShim(..) - | InstanceDef::FnPtrAddrShim(..) => return Ok(()), + | InstanceDef::FnPtrAddrShim(..) + | InstanceDef::CfiShim { .. } => return Ok(()), } if self.tcx.is_constructor(callee_def_id) { diff --git a/compiler/rustc_mir_transform/src/inline/cycle.rs b/compiler/rustc_mir_transform/src/inline/cycle.rs index f2b6dcac58632..c37309f085d52 100644 --- a/compiler/rustc_mir_transform/src/inline/cycle.rs +++ b/compiler/rustc_mir_transform/src/inline/cycle.rs @@ -90,7 +90,8 @@ pub(crate) fn mir_callgraph_reachable<'tcx>( | InstanceDef::ConstructCoroutineInClosureShim { .. } | InstanceDef::CoroutineKindShim { .. } | InstanceDef::ThreadLocalShim { .. } - | InstanceDef::CloneShim(..) => {} + | InstanceDef::CloneShim(..) + | InstanceDef::CfiShim { .. } => {} // This shim does not call any other functions, thus there can be no recursion. InstanceDef::FnPtrAddrShim(..) => continue, diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index cd9b98e4f32cd..e01b6dab9e4dc 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -101,6 +101,7 @@ mod remove_unneeded_drops; mod remove_zsts; mod required_consts; mod reveal_all; +mod rewrite_receiver; mod shim; mod ssa; // This pass is public to allow external drivers to perform MIR cleanup diff --git a/compiler/rustc_mir_transform/src/rewrite_receiver.rs b/compiler/rustc_mir_transform/src/rewrite_receiver.rs new file mode 100644 index 0000000000000..bb40894ca8e59 --- /dev/null +++ b/compiler/rustc_mir_transform/src/rewrite_receiver.rs @@ -0,0 +1,100 @@ +//! This pass rewrites the receiver of the function to a thin-pointer compatible representation of +//! the provided type. + +use rustc_middle::mir::*; +use rustc_middle::ty::{self, Ty, TyCtxt}; + +// This is a layering violation - this is replicating work that occurs when computing an ABI. +// +// We need to have a `dyn` receiver type in order to allow a type match at the vtable call. However, +// we want to match the existing ABI for vtable methods, which passes a *thin* pointer +// Existing shims make this cast implicit at the callsite, so they don't need to get this type +// correct. Without this, we will get a local type mismatch when we actually try to use nontrivial +// receiver (e.g. `Arc`). This means that we must use a thin self, because otherwise codegen +// will assume an argument is present for the vtable. Unfortunately, unwrapping the receiver type +// currently involves replicating ABI / layout work. +// +// Perhaps in the future we could avoid the thin-self hack with an explicit existentials, e.g. +// * `∃T: Foo. *const T` +// * `∃T: Foo. Arc` +// but until then, we need to unwrap receivers down to `* dyn Foo` of some variant to use the +// existing codegen paths. +fn unwrap_receiver<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Ty<'tcx> { + use ty::layout::{LayoutCx, LayoutOf, MaybeResult, TyAndLayout}; + let cx = LayoutCx { tcx, param_env: ty::ParamEnv::reveal_all() }; + let mut receiver_layout: TyAndLayout<'_> = + cx.layout_of(ty).to_result().expect("unable to compute layout of receiver type"); + // The VTableShim should have already done any `dyn Foo` -> `*const dyn Foo` coercions + assert!(!receiver_layout.is_unsized()); + // If we aren't a pointer or a ref already, we better be a no-padding wrapper around one + while !receiver_layout.ty.is_unsafe_ptr() && !receiver_layout.ty.is_ref() { + receiver_layout = receiver_layout + .non_1zst_field(&cx) + .expect("not exactly one non-1-ZST field in a CFI shim receiver") + .1 + } + receiver_layout.ty +} + +// Visitor to rewrite all uses of a given local to another +struct RewriteLocal<'tcx> { + tcx: TyCtxt<'tcx>, + source: Local, + target: Local, +} + +impl<'tcx> visit::MutVisitor<'tcx> for RewriteLocal<'tcx> { + fn tcx(&self) -> TyCtxt<'tcx> { + self.tcx + } + fn visit_local( + &mut self, + local: &mut Local, + _context: visit::PlaceContext, + _location: Location, + ) { + if self.source == *local { + *local = self.target; + } + } +} + +pub struct RewriteReceiver<'tcx> { + receiver: Ty<'tcx>, +} + +impl<'tcx> RewriteReceiver<'tcx> { + pub fn new(receiver: Ty<'tcx>) -> Self { + Self { receiver } + } +} + +impl<'tcx> MirPass<'tcx> for RewriteReceiver<'tcx> { + fn is_enabled(&self, sess: &rustc_session::Session) -> bool { + sess.cfi_shims() + } + fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { + use visit::MutVisitor; + let source_info = SourceInfo::outermost(body.span); + let receiver = + body.args_iter().next().expect("RewriteReceiver pass on function with no arguments?"); + let cast_receiver = body.local_decls.push(body.local_decls[receiver].clone()); + body.local_decls[receiver].ty = unwrap_receiver(tcx, self.receiver); + body.local_decls[receiver].mutability = Mutability::Not; + RewriteLocal { tcx, source: receiver, target: cast_receiver }.visit_body(body); + body.basic_blocks.as_mut_preserves_cfg()[START_BLOCK].statements.insert( + 0, + Statement { + source_info, + kind: StatementKind::Assign(Box::new(( + Place::from(cast_receiver), + Rvalue::Cast( + CastKind::Transmute, + Operand::Move(Place::from(receiver)), + body.local_decls[cast_receiver].ty, + ), + ))), + }, + ); + } +} diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index 967158685ea4f..a5a72e84607b0 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -17,7 +17,7 @@ use std::iter; use crate::{ abort_unwinding_calls, add_call_guards, add_moves_for_packed_drops, deref_separator, - pass_manager as pm, remove_noop_landing_pads, simplify, + pass_manager as pm, remove_noop_landing_pads, rewrite_receiver, simplify, }; use rustc_middle::mir::patch::MirPatch; use rustc_mir_dataflow::elaborate_drops::{self, DropElaborator, DropFlagMode, DropStyle}; @@ -50,6 +50,21 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<' build_call_shim(tcx, instance, Some(adjustment), CallKind::Indirect(ty)) } + ty::InstanceDef::CfiShim { target_instance, invoke_ty } => { + let mut body = make_shim(tcx, *target_instance); + body.source.instance = instance; + let receiver = target_instance + .fn_sig(tcx) + .map_bound(|sig| tcx.instantiate_bound_regions_with_erased(sig).inputs()[0]) + .instantiate(tcx, tcx.mk_args(&[invoke_ty.into()])); + pm::run_passes( + tcx, + &mut body, + &[&rewrite_receiver::RewriteReceiver::new(receiver)], + None, + ); + return body; + } // We are generating a call back to our def-id, which the // codegen backend knows to turn to an actual call, be it // a virtual call, or a direct call to a function for which diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index b0cb9fa517fd0..595b6c514f460 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -1001,7 +1001,8 @@ fn visit_instance_use<'tcx>( | ty::InstanceDef::Item(..) | ty::InstanceDef::FnPtrShim(..) | ty::InstanceDef::CloneShim(..) - | ty::InstanceDef::FnPtrAddrShim(..) => { + | ty::InstanceDef::FnPtrAddrShim(..) + | ty::InstanceDef::CfiShim { .. } => { output.push(create_fn_mono_item(tcx, instance, source)); } } diff --git a/compiler/rustc_monomorphize/src/partitioning.rs b/compiler/rustc_monomorphize/src/partitioning.rs index 8bebc30e4356a..9243b9a73aecb 100644 --- a/compiler/rustc_monomorphize/src/partitioning.rs +++ b/compiler/rustc_monomorphize/src/partitioning.rs @@ -627,7 +627,8 @@ fn characteristic_def_id_of_mono_item<'tcx>( | ty::InstanceDef::Virtual(..) | ty::InstanceDef::CloneShim(..) | ty::InstanceDef::ThreadLocalShim(..) - | ty::InstanceDef::FnPtrAddrShim(..) => return None, + | ty::InstanceDef::FnPtrAddrShim(..) + | ty::InstanceDef::CfiShim { .. } => return None, }; // If this is a method, we want to put it into the same module as @@ -789,7 +790,8 @@ fn mono_item_visibility<'tcx>( | InstanceDef::CoroutineKindShim { .. } | InstanceDef::DropGlue(..) | InstanceDef::CloneShim(..) - | InstanceDef::FnPtrAddrShim(..) => return Visibility::Hidden, + | InstanceDef::FnPtrAddrShim(..) + | InstanceDef::CfiShim { .. } => return Visibility::Hidden, }; // The `start_fn` lang item is actually a monomorphized instance of a @@ -1141,7 +1143,12 @@ fn collect_and_partition_mono_items(tcx: TyCtxt<'_>, (): ()) -> (&DefIdSet, &[Co let mono_items: DefIdSet = items .iter() .filter_map(|mono_item| match *mono_item { - MonoItem::Fn(ref instance) => Some(instance.def_id()), + // Don't count CfiShim's def_id, that resolves to a child instance + MonoItem::Fn(ref instance) + if !matches!(instance.def, ty::InstanceDef::CfiShim { .. }) => + { + Some(instance.def_id()) + } MonoItem::Static(def_id) => Some(def_id), _ => None, }) diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 4f4d8fabb7261..5681fa57a5fe7 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -391,6 +391,15 @@ impl Session { self.opts.unstable_opts.sanitizer.contains(SanitizerSet::KCFI) } + /// Should CFI shims be generated for vtable calls + pub fn cfi_shims(&self) -> bool { + // Keeping this predicate in one place will allow us to: + // 1. Migrate LLVM-CFI off this in the future via multiple type tags + // (KCFI will require it forever) + // 2. Test shim generation without type enforcement + self.is_sanitizer_cfi_enabled() || self.is_sanitizer_kcfi_enabled() + } + pub fn is_split_lto_unit_enabled(&self) -> bool { self.opts.unstable_opts.split_lto_unit == Some(true) } diff --git a/compiler/rustc_smir/src/rustc_smir/convert/ty.rs b/compiler/rustc_smir/src/rustc_smir/convert/ty.rs index 84c6cbf178bcb..d88a35e0cec9b 100644 --- a/compiler/rustc_smir/src/rustc_smir/convert/ty.rs +++ b/compiler/rustc_smir/src/rustc_smir/convert/ty.rs @@ -778,7 +778,8 @@ impl<'tcx> Stable<'tcx> for ty::Instance<'tcx> { | ty::InstanceDef::ThreadLocalShim(..) | ty::InstanceDef::DropGlue(..) | ty::InstanceDef::CloneShim(..) - | ty::InstanceDef::FnPtrShim(..) => stable_mir::mir::mono::InstanceKind::Shim, + | ty::InstanceDef::FnPtrShim(..) + | ty::InstanceDef::CfiShim { .. } => stable_mir::mir::mono::InstanceKind::Shim, }; stable_mir::mir::mono::Instance { def, kind } } diff --git a/compiler/rustc_symbol_mangling/src/legacy.rs b/compiler/rustc_symbol_mangling/src/legacy.rs index 83e920e2f8ee4..eda6ebcecfa41 100644 --- a/compiler/rustc_symbol_mangling/src/legacy.rs +++ b/compiler/rustc_symbol_mangling/src/legacy.rs @@ -74,6 +74,11 @@ pub(super) fn mangle<'tcx>( ty::InstanceDef::ReifyShim(..) => { printer.write_str("{{reify-shim}}").unwrap(); } + ty::InstanceDef::CfiShim { invoke_ty, .. } => { + printer.write_str("{{cfi-shim::<").unwrap(); + printer.print_type(invoke_ty).unwrap(); + printer.write_str(">}}").unwrap(); + } // FIXME(async_closures): This shouldn't be needed when we fix // `Instance::ty`/`Instance::def_id`. ty::InstanceDef::ConstructCoroutineInClosureShim { target_kind, .. } diff --git a/compiler/rustc_symbol_mangling/src/v0.rs b/compiler/rustc_symbol_mangling/src/v0.rs index f1b1b4ed2bb84..5bebb0887e3f2 100644 --- a/compiler/rustc_symbol_mangling/src/v0.rs +++ b/compiler/rustc_symbol_mangling/src/v0.rs @@ -40,24 +40,29 @@ pub(super) fn mangle<'tcx>( out: String::from(prefix), }; - // Append `::{shim:...#0}` to shims that can coexist with a non-shim instance. + // Append `::{shim:...#0}::` to shims that can coexist with a non-shim instance. let shim_kind = match instance.def { - ty::InstanceDef::ThreadLocalShim(_) => Some("tls"), - ty::InstanceDef::VTableShim(_) => Some("vtable"), - ty::InstanceDef::ReifyShim(_) => Some("reify"), + ty::InstanceDef::ThreadLocalShim(_) => Some(("tls", vec![])), + ty::InstanceDef::VTableShim(_) => Some(("vtable", vec![])), + ty::InstanceDef::ReifyShim(_) => Some(("reify", vec![])), + ty::InstanceDef::CfiShim { invoke_ty, .. } => Some(("cfi", vec![invoke_ty.into()])), ty::InstanceDef::ConstructCoroutineInClosureShim { target_kind, .. } | ty::InstanceDef::CoroutineKindShim { target_kind, .. } => match target_kind { ty::ClosureKind::Fn => unreachable!(), - ty::ClosureKind::FnMut => Some("fn_mut"), - ty::ClosureKind::FnOnce => Some("fn_once"), + ty::ClosureKind::FnMut => Some(("fn_mut", vec![])), + ty::ClosureKind::FnOnce => Some(("fn_once", vec![])), }, _ => None, }; - if let Some(shim_kind) = shim_kind { - cx.path_append_ns(|cx| cx.print_def_path(def_id, args), 'S', 0, shim_kind).unwrap() + if let Some((shim_kind, shim_args)) = shim_kind { + cx.path_generic_args( + |cx| cx.path_append_ns(|cx| cx.print_def_path(def_id, args), 'S', 0, shim_kind), + &shim_args, + ) + .unwrap(); } else { cx.print_def_path(def_id, args).unwrap() }; diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs index 8bbc9fddf6d21..b2a86d1c59cee 100644 --- a/compiler/rustc_ty_utils/src/abi.rs +++ b/compiler/rustc_ty_utils/src/abi.rs @@ -358,7 +358,7 @@ fn fn_abi_of_instance<'tcx>( extra_args, caller_location, Some(instance.def_id()), - matches!(instance.def, ty::InstanceDef::Virtual(..)), + instance.force_thin_self(), ) } From 5cca17407c89c0c14ea8741ccbdda3f764de4db9 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Fri, 1 Mar 2024 05:01:52 +0000 Subject: [PATCH 06/13] CFI: Apply CFI shims to drops Fixes: 118761 --- compiler/rustc_middle/src/ty/vtable.rs | 11 +++++---- compiler/rustc_monomorphize/src/collector.rs | 24 ++++++++++++------- .../kcfi-emit-type-metadata-trait-objects.rs | 6 +++-- tests/ui/sanitizer/cfi-drop-issue-118761.rs | 14 +++++++++++ .../ui/sanitizer/cfi-nontrivial-drop-glue.rs | 10 ++++++++ tests/ui/sanitizer/cfi-trait-object.rs | 22 +++++++++++++++++ 6 files changed, 72 insertions(+), 15 deletions(-) create mode 100644 tests/ui/sanitizer/cfi-drop-issue-118761.rs create mode 100644 tests/ui/sanitizer/cfi-nontrivial-drop-glue.rs create mode 100644 tests/ui/sanitizer/cfi-trait-object.rs diff --git a/compiler/rustc_middle/src/ty/vtable.rs b/compiler/rustc_middle/src/ty/vtable.rs index 62f41921d888a..796982d0b2600 100644 --- a/compiler/rustc_middle/src/ty/vtable.rs +++ b/compiler/rustc_middle/src/ty/vtable.rs @@ -53,11 +53,11 @@ pub(super) fn vtable_allocation_provider<'tcx>( ) -> AllocId { let (ty, poly_trait_ref) = key; - let vtable_entries = if let Some(poly_trait_ref) = poly_trait_ref { - let trait_ref = poly_trait_ref.with_self_ty(tcx, ty); - let trait_ref = tcx.erase_regions(trait_ref); + let invoke_trait = poly_trait_ref + .map(|poly_trait_ref| tcx.erase_regions(poly_trait_ref.with_self_ty(tcx, ty))); - tcx.vtable_entries(trait_ref) + let vtable_entries = if let Some(invoke_trait) = invoke_trait { + tcx.vtable_entries(invoke_trait) } else { TyCtxt::COMMON_VTABLE_ENTRIES }; @@ -83,7 +83,8 @@ pub(super) fn vtable_allocation_provider<'tcx>( let idx: u64 = u64::try_from(idx).unwrap(); let scalar = match entry { VtblEntry::MetadataDropInPlace => { - let instance = ty::Instance::resolve_drop_in_place(tcx, ty); + let instance = + ty::Instance::resolve_drop_in_place(tcx, ty).cfi_shim(tcx, invoke_trait); let fn_alloc_id = tcx.reserve_and_set_fn_alloc(instance); let fn_ptr = Pointer::from(fn_alloc_id); Scalar::from_pointer(fn_ptr, &tcx) diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 595b6c514f460..7e01008334fb7 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -381,7 +381,7 @@ fn collect_items_rec<'tcx>( debug_assert!(should_codegen_locally(tcx, &instance)); let ty = instance.ty(tcx, ty::ParamEnv::reveal_all()); - visit_drop_use(tcx, ty, true, starting_item.span, &mut used_items); + visit_drop_use(tcx, ty, None, true, starting_item.span, &mut used_items); recursion_depth_reset = None; @@ -855,7 +855,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { mir::TerminatorKind::Drop { ref place, .. } => { let ty = place.ty(self.body, self.tcx).ty; let ty = self.monomorphize(ty); - visit_drop_use(self.tcx, ty, true, source, self.output); + visit_drop_use(self.tcx, ty, None, true, source, self.output); } mir::TerminatorKind::InlineAsm { ref operands, .. } => { for op in operands { @@ -917,11 +917,17 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { fn visit_drop_use<'tcx>( tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, + invoke_trait: Option>, is_direct_call: bool, source: Span, output: &mut MonoItems<'tcx>, ) { - let instance = Instance::resolve_drop_in_place(tcx, ty); + let mut instance = Instance::resolve_drop_in_place(tcx, ty); + if tcx.sess.cfi_shims() && !is_direct_call { + // The CFI shim may generate a direct call to the unshimmed drop + visit_drop_use(tcx, ty, None, true, source, output); + instance = instance.cfi_shim(tcx, invoke_trait); + } visit_instance_use(tcx, instance, is_direct_call, source, output); } @@ -1193,8 +1199,10 @@ fn create_mono_items_for_vtable_methods<'tcx>( assert!(!trait_ty.has_escaping_bound_vars() && !impl_ty.has_escaping_bound_vars()); if let ty::Dynamic(trait_ty, ..) = trait_ty.kind() { - if let Some(principal) = trait_ty.principal() { - let poly_trait_ref = principal.with_self_ty(tcx, impl_ty); + let invoke_trait = + trait_ty.principal().map(|principal| principal.with_self_ty(tcx, impl_ty)); + + if let Some(poly_trait_ref) = invoke_trait { assert!(!poly_trait_ref.has_escaping_bound_vars()); // Walk all methods of the trait, including those of its supertraits @@ -1216,10 +1224,10 @@ fn create_mono_items_for_vtable_methods<'tcx>( }) .map(|item| create_fn_mono_item(tcx, item, source)); output.extend(methods); - } + }; // Also add the destructor. - visit_drop_use(tcx, impl_ty, false, source, output); + visit_drop_use(tcx, impl_ty, invoke_trait, false, source, output); } } @@ -1244,7 +1252,7 @@ impl<'v> RootCollector<'_, 'v> { debug!("RootCollector: ADT drop-glue for `{id:?}`",); let ty = self.tcx.type_of(id.owner_id.to_def_id()).no_bound_vars().unwrap(); - visit_drop_use(self.tcx, ty, true, DUMMY_SP, self.output); + visit_drop_use(self.tcx, ty, None, true, DUMMY_SP, self.output); } } DefKind::GlobalAsm => { diff --git a/tests/codegen/sanitizer/kcfi-emit-type-metadata-trait-objects.rs b/tests/codegen/sanitizer/kcfi-emit-type-metadata-trait-objects.rs index f08c9e6702ed9..3b6642e09ac21 100644 --- a/tests/codegen/sanitizer/kcfi-emit-type-metadata-trait-objects.rs +++ b/tests/codegen/sanitizer/kcfi-emit-type-metadata-trait-objects.rs @@ -12,7 +12,7 @@ #![no_core] #[lang="sized"] -trait Sized { } +pub trait Sized { } #[lang="copy"] trait Copy { } #[lang="receiver"] @@ -28,7 +28,9 @@ impl<'a, 'b: 'a, T: ?Sized + Unsize, U: ?Sized> CoerceUnsized<&'a U> for &'b #[lang="freeze"] trait Freeze { } #[lang="drop_in_place"] -fn drop_in_place_fn() { } +fn drop_in_place_fn(_to_drop: *mut T) { } +#[lang="unpin"] +pub trait Unpin { } pub trait Trait1 { fn foo(&self); diff --git a/tests/ui/sanitizer/cfi-drop-issue-118761.rs b/tests/ui/sanitizer/cfi-drop-issue-118761.rs new file mode 100644 index 0000000000000..166a5668b6302 --- /dev/null +++ b/tests/ui/sanitizer/cfi-drop-issue-118761.rs @@ -0,0 +1,14 @@ +// Validate that objects that might have custom drop can be dropped with CFI on. See #118761 + +//@ needs-sanitizer-cfi +//@ compile-flags: --crate-type=bin -Cprefer-dynamic=off -Clto -Zsanitizer=cfi -C codegen-units=1 +//@ compile-flags: -C opt-level=0 +//@ run-pass + +struct Bar; +trait Fooable {} +impl Fooable for Bar {} + +fn main() { + let _: Box = Box::new(Bar); +} diff --git a/tests/ui/sanitizer/cfi-nontrivial-drop-glue.rs b/tests/ui/sanitizer/cfi-nontrivial-drop-glue.rs new file mode 100644 index 0000000000000..b042d677bf036 --- /dev/null +++ b/tests/ui/sanitizer/cfi-nontrivial-drop-glue.rs @@ -0,0 +1,10 @@ +//@ needs-sanitizer-cfi +//@ compile-flags: --crate-type=bin -Cprefer-dynamic=off -Clto -Zsanitizer=cfi +//@ compile-flags: -C codegen-units=1 -C opt-level=0 +//@ run-pass + +use std::env; + +fn main() { + env::current_exe().unwrap(); +} diff --git a/tests/ui/sanitizer/cfi-trait-object.rs b/tests/ui/sanitizer/cfi-trait-object.rs new file mode 100644 index 0000000000000..e3734d9a3b368 --- /dev/null +++ b/tests/ui/sanitizer/cfi-trait-object.rs @@ -0,0 +1,22 @@ +// Check trait objects run correctly + +//@ needs-sanitizer-cfi +//@ compile-flags: --crate-type=bin -Cprefer-dynamic=off -Clto -Zsanitizer=cfi +//@ compile-flags: -C codegen-units=1 -C opt-level=0 +//@ run-pass + +struct Bar; +trait Fooable { + fn foo(&self) -> i32; +} + +impl Fooable for Bar { + fn foo(&self) -> i32 { + 3 + } +} + +fn main() { + let bar: Box = Box::new(Bar); + bar.foo(); +} From 646befed56151b8425c45d20d6ed2fe67e046a61 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Fri, 1 Mar 2024 05:29:01 +0000 Subject: [PATCH 07/13] CFI: Enable vtable shimming --- compiler/rustc_middle/src/ty/vtable.rs | 2 +- compiler/rustc_monomorphize/src/collector.rs | 5 ++--- tests/ui/sanitizer/cfi-alt-receiver.rs | 23 ++++++++++++++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 tests/ui/sanitizer/cfi-alt-receiver.rs diff --git a/compiler/rustc_middle/src/ty/vtable.rs b/compiler/rustc_middle/src/ty/vtable.rs index 796982d0b2600..39dc782779fd9 100644 --- a/compiler/rustc_middle/src/ty/vtable.rs +++ b/compiler/rustc_middle/src/ty/vtable.rs @@ -94,7 +94,7 @@ pub(super) fn vtable_allocation_provider<'tcx>( VtblEntry::Vacant => continue, VtblEntry::Method(instance) => { // Prepare the fn ptr we write into the vtable. - let instance = instance.polymorphize(tcx); + let instance = instance.cfi_shim(tcx, invoke_trait).polymorphize(tcx); let fn_alloc_id = tcx.reserve_and_set_fn_alloc(instance); let fn_ptr = Pointer::from(fn_alloc_id); Scalar::from_pointer(fn_ptr, &tcx) diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 7e01008334fb7..9950b6a960b66 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -1218,9 +1218,8 @@ fn create_mono_items_for_vtable_methods<'tcx>( // all super trait items already covered, so skip them. None } - VtblEntry::Method(instance) => { - Some(*instance).filter(|instance| should_codegen_locally(tcx, instance)) - } + VtblEntry::Method(instance) => Some(instance.cfi_shim(tcx, invoke_trait)) + .filter(|instance| should_codegen_locally(tcx, instance)), }) .map(|item| create_fn_mono_item(tcx, item, source)); output.extend(methods); diff --git a/tests/ui/sanitizer/cfi-alt-receiver.rs b/tests/ui/sanitizer/cfi-alt-receiver.rs new file mode 100644 index 0000000000000..e87fbc8849b30 --- /dev/null +++ b/tests/ui/sanitizer/cfi-alt-receiver.rs @@ -0,0 +1,23 @@ +// Check alternate receivers work + +//@ needs-sanitizer-cfi +//@ compile-flags: --crate-type=bin -Cprefer-dynamic=off -Clto -Zsanitizer=cfi +//@ compile-flags: -C codegen-units=1 -C opt-level=0 +//@ run-pass + +use std::sync::Arc; + +trait Fooable { + fn foo(self: Arc); +} + +struct Bar; + +impl Fooable for Bar { + fn foo(self: Arc) {} +} + +fn main() { + let bar: Arc = Arc::new(Bar); + bar.foo(); +} From ada412a5877437dae9808052a12f12eb21cf7e30 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Fri, 1 Mar 2024 00:37:46 +0000 Subject: [PATCH 08/13] Revert "CFI: Fix SIGILL reached via trait objects" We no longer need the special instance resolution this added, and it can be broken in edge cases (specifically with a FnPtr shim, which will cause the calculation of fn_abi to fail). * We keep the Clone impls it added, because they have since become used by other portions of the compiler. * Add a test for the address-taken calls that this previously broke. This reverts commit 7c7b22e62cd3aa34ef60ec98b145258caa55261f. --- compiler/rustc_codegen_llvm/src/callee.rs | 3 +- compiler/rustc_codegen_llvm/src/declare.rs | 64 +++++-------------- compiler/rustc_codegen_llvm/src/intrinsic.rs | 2 +- compiler/rustc_codegen_llvm/src/mono_item.rs | 2 +- compiler/rustc_symbol_mangling/src/typeid.rs | 24 +------ .../src/typeid/typeid_itanium_cxx_abi.rs | 59 +---------------- compiler/rustc_target/src/abi/call/mod.rs | 2 +- tests/ui/sanitizer/cfi-address-taken.rs | 30 +++++++++ 8 files changed, 54 insertions(+), 132 deletions(-) create mode 100644 tests/ui/sanitizer/cfi-address-taken.rs diff --git a/compiler/rustc_codegen_llvm/src/callee.rs b/compiler/rustc_codegen_llvm/src/callee.rs index e675362ac338c..a7e5aca4cecec 100644 --- a/compiler/rustc_codegen_llvm/src/callee.rs +++ b/compiler/rustc_codegen_llvm/src/callee.rs @@ -67,14 +67,13 @@ pub fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>) -> true, ), fn_abi, - Some(instance), ); unsafe { llvm::LLVMSetDLLStorageClass(llfn, llvm::DLLStorageClass::DllImport); } llfn } else { - cx.declare_fn(sym, fn_abi, Some(instance)) + cx.declare_fn(sym, fn_abi) }; debug!("get_fn: not casting pointer!"); diff --git a/compiler/rustc_codegen_llvm/src/declare.rs b/compiler/rustc_codegen_llvm/src/declare.rs index 78c0725a63784..1039783f70bec 100644 --- a/compiler/rustc_codegen_llvm/src/declare.rs +++ b/compiler/rustc_codegen_llvm/src/declare.rs @@ -19,11 +19,8 @@ use crate::llvm::AttributePlace::Function; use crate::type_::Type; use crate::value::Value; use rustc_codegen_ssa::traits::TypeMembershipMethods; -use rustc_middle::ty::{Instance, Ty}; -use rustc_symbol_mangling::typeid::{ - kcfi_typeid_for_fnabi, kcfi_typeid_for_instance, typeid_for_fnabi, typeid_for_instance, - TypeIdOptions, -}; +use rustc_middle::ty::Ty; +use rustc_symbol_mangling::typeid::{kcfi_typeid_for_fnabi, typeid_for_fnabi, TypeIdOptions}; use smallvec::SmallVec; /// Declare a function. @@ -119,12 +116,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { /// /// If there’s a value with the same name already declared, the function will /// update the declaration and return existing Value instead. - pub fn declare_fn( - &self, - name: &str, - fn_abi: &FnAbi<'tcx, Ty<'tcx>>, - instance: Option>, - ) -> &'ll Value { + pub fn declare_fn(&self, name: &str, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> &'ll Value { debug!("declare_rust_fn(name={:?}, fn_abi={:?})", name, fn_abi); // Function addresses in Rust are never significant, allowing functions to @@ -140,35 +132,18 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { fn_abi.apply_attrs_llfn(self, llfn); if self.tcx.sess.is_sanitizer_cfi_enabled() { - if let Some(instance) = instance { - let typeid = typeid_for_instance(self.tcx, &instance, TypeIdOptions::empty()); - self.set_type_metadata(llfn, typeid); - let typeid = - typeid_for_instance(self.tcx, &instance, TypeIdOptions::GENERALIZE_POINTERS); - self.add_type_metadata(llfn, typeid); - let typeid = - typeid_for_instance(self.tcx, &instance, TypeIdOptions::NORMALIZE_INTEGERS); - self.add_type_metadata(llfn, typeid); - let typeid = typeid_for_instance( - self.tcx, - &instance, - TypeIdOptions::GENERALIZE_POINTERS | TypeIdOptions::NORMALIZE_INTEGERS, - ); - self.add_type_metadata(llfn, typeid); - } else { - let typeid = typeid_for_fnabi(self.tcx, fn_abi, TypeIdOptions::empty()); - self.set_type_metadata(llfn, typeid); - let typeid = typeid_for_fnabi(self.tcx, fn_abi, TypeIdOptions::GENERALIZE_POINTERS); - self.add_type_metadata(llfn, typeid); - let typeid = typeid_for_fnabi(self.tcx, fn_abi, TypeIdOptions::NORMALIZE_INTEGERS); - self.add_type_metadata(llfn, typeid); - let typeid = typeid_for_fnabi( - self.tcx, - fn_abi, - TypeIdOptions::GENERALIZE_POINTERS | TypeIdOptions::NORMALIZE_INTEGERS, - ); - self.add_type_metadata(llfn, typeid); - } + let typeid = typeid_for_fnabi(self.tcx, fn_abi, TypeIdOptions::empty()); + self.set_type_metadata(llfn, typeid); + let typeid = typeid_for_fnabi(self.tcx, fn_abi, TypeIdOptions::GENERALIZE_POINTERS); + self.add_type_metadata(llfn, typeid); + let typeid = typeid_for_fnabi(self.tcx, fn_abi, TypeIdOptions::NORMALIZE_INTEGERS); + self.add_type_metadata(llfn, typeid); + let typeid = typeid_for_fnabi( + self.tcx, + fn_abi, + TypeIdOptions::GENERALIZE_POINTERS | TypeIdOptions::NORMALIZE_INTEGERS, + ); + self.add_type_metadata(llfn, typeid); } if self.tcx.sess.is_sanitizer_kcfi_enabled() { @@ -181,13 +156,8 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { options.insert(TypeIdOptions::NORMALIZE_INTEGERS); } - if let Some(instance) = instance { - let kcfi_typeid = kcfi_typeid_for_instance(self.tcx, &instance, options); - self.set_kcfi_type_metadata(llfn, kcfi_typeid); - } else { - let kcfi_typeid = kcfi_typeid_for_fnabi(self.tcx, fn_abi, options); - self.set_kcfi_type_metadata(llfn, kcfi_typeid); - } + let kcfi_typeid = kcfi_typeid_for_fnabi(self.tcx, fn_abi, options); + self.set_kcfi_type_metadata(llfn, kcfi_typeid); } llfn diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index f33a672aff0d7..e0a46af63139e 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -939,7 +939,7 @@ fn gen_fn<'ll, 'tcx>( ) -> (&'ll Type, &'ll Value) { let fn_abi = cx.fn_abi_of_fn_ptr(rust_fn_sig, ty::List::empty()); let llty = fn_abi.llvm_type(cx); - let llfn = cx.declare_fn(name, fn_abi, None); + let llfn = cx.declare_fn(name, fn_abi); cx.set_frame_pointer_type(llfn); cx.apply_target_cpu_attr(llfn); // FIXME(eddyb) find a nicer way to do this. diff --git a/compiler/rustc_codegen_llvm/src/mono_item.rs b/compiler/rustc_codegen_llvm/src/mono_item.rs index f763071936837..934a0db1d7941 100644 --- a/compiler/rustc_codegen_llvm/src/mono_item.rs +++ b/compiler/rustc_codegen_llvm/src/mono_item.rs @@ -51,7 +51,7 @@ impl<'tcx> PreDefineMethods<'tcx> for CodegenCx<'_, 'tcx> { assert!(!instance.args.has_infer()); let fn_abi = self.fn_abi_of_instance(instance, ty::List::empty()); - let lldecl = self.declare_fn(symbol_name, fn_abi, Some(instance)); + let lldecl = self.declare_fn(symbol_name, fn_abi); unsafe { llvm::LLVMRustSetLinkage(lldecl, base::linkage_to_llvm(linkage)) }; let attrs = self.tcx.codegen_fn_attrs(instance.def_id()); base::set_link_section(lldecl, attrs); diff --git a/compiler/rustc_symbol_mangling/src/typeid.rs b/compiler/rustc_symbol_mangling/src/typeid.rs index e8763e49e624b..2cabe05d848ab 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::{Ty, TyCtxt}; use rustc_target::abi::call::FnAbi; use std::hash::Hasher; use twox_hash::XxHash64; @@ -30,15 +30,6 @@ pub fn typeid_for_fnabi<'tcx>( typeid_itanium_cxx_abi::typeid_for_fnabi(tcx, fn_abi, options) } -/// Returns a type metadata identifier for the specified Instance. -pub fn typeid_for_instance<'tcx>( - tcx: TyCtxt<'tcx>, - instance: &Instance<'tcx>, - options: TypeIdOptions, -) -> String { - typeid_itanium_cxx_abi::typeid_for_instance(tcx, instance, options) -} - /// Returns a KCFI type metadata identifier for the specified FnAbi. pub fn kcfi_typeid_for_fnabi<'tcx>( tcx: TyCtxt<'tcx>, @@ -51,16 +42,3 @@ pub fn kcfi_typeid_for_fnabi<'tcx>( hash.write(typeid_itanium_cxx_abi::typeid_for_fnabi(tcx, fn_abi, options).as_bytes()); hash.finish() as u32 } - -/// Returns a KCFI type metadata identifier for the specified Instance. -pub fn kcfi_typeid_for_instance<'tcx>( - tcx: TyCtxt<'tcx>, - instance: &Instance<'tcx>, - options: TypeIdOptions, -) -> u32 { - // 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(); - hash.write(typeid_itanium_cxx_abi::typeid_for_instance(tcx, instance, options).as_bytes()); - hash.finish() as u32 -} 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 51e2c96120caa..a0c4ce1bde4e0 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 @@ -12,8 +12,8 @@ use rustc_data_structures::fx::FxHashMap; use rustc_hir as hir; use rustc_middle::ty::layout::IntegerExt; use rustc_middle::ty::{ - self, Const, ExistentialPredicate, FloatTy, FnSig, Instance, IntTy, List, Region, RegionKind, - TermKind, Ty, TyCtxt, UintTy, + self, Const, ExistentialPredicate, FloatTy, FnSig, IntTy, List, Region, RegionKind, TermKind, + Ty, TyCtxt, UintTy, }; use rustc_middle::ty::{GenericArg, GenericArgKind, GenericArgsRef}; use rustc_span::def_id::DefId; @@ -1074,58 +1074,3 @@ pub fn typeid_for_fnabi<'tcx>( typeid } - -/// Returns a type metadata identifier for the specified Instance using the Itanium C++ ABI with -/// vendor extended type qualifiers and types for Rust types that are not used at the FFI boundary. -pub fn typeid_for_instance<'tcx>( - tcx: TyCtxt<'tcx>, - instance: &Instance<'tcx>, - options: TypeIdOptions, -) -> String { - let fn_abi = tcx - .fn_abi_of_instance(tcx.param_env(instance.def_id()).and((*instance, ty::List::empty()))) - .unwrap_or_else(|instance| { - bug!("typeid_for_instance: couldn't get fn_abi of instance {:?}", instance) - }); - - // If this instance is a method and self is a reference, get the impl it belongs to - let impl_def_id = tcx.impl_of_method(instance.def_id()); - if impl_def_id.is_some() && !fn_abi.args.is_empty() && fn_abi.args[0].layout.ty.is_ref() { - // If this impl is not an inherent impl, get the trait it implements - if let Some(trait_ref) = tcx.impl_trait_ref(impl_def_id.unwrap()) { - // Transform the concrete self into a reference to a trait object - let existential_predicate = trait_ref.map_bound(|trait_ref| { - ty::ExistentialPredicate::Trait(ty::ExistentialTraitRef::erase_self_ty( - tcx, trait_ref, - )) - }); - let existential_predicates = tcx.mk_poly_existential_predicates(&[ty::Binder::dummy( - existential_predicate.skip_binder(), - )]); - // Is the concrete self mutable? - let self_ty = if fn_abi.args[0].layout.ty.is_mutable_ptr() { - Ty::new_mut_ref( - tcx, - tcx.lifetimes.re_erased, - Ty::new_dynamic(tcx, existential_predicates, tcx.lifetimes.re_erased, ty::Dyn), - ) - } else { - Ty::new_imm_ref( - tcx, - tcx.lifetimes.re_erased, - Ty::new_dynamic(tcx, existential_predicates, tcx.lifetimes.re_erased, ty::Dyn), - ) - }; - - // Replace the concrete self in an fn_abi clone by the reference to a trait object - let mut fn_abi = fn_abi.clone(); - // HACK(rcvalle): It is okay to not replace or update the entire ArgAbi here because the - // other fields are never used. - fn_abi.args[0].layout.ty = self_ty; - - return typeid_for_fnabi(tcx, &fn_abi, options); - } - } - - typeid_for_fnabi(tcx, fn_abi, options) -} diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 66177d551ba2d..37f5ba4a99263 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -268,7 +268,7 @@ impl Uniform { /// `rest.unit` register type gets repeated often enough to cover `rest.size`. This describes the /// actual type used for the call; the Rust type of the argument is then transmuted to this ABI type /// (and all data in the padding between the registers is dropped). -#[derive(Clone, PartialEq, Eq, Hash, Debug, HashStable_Generic)] +#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, HashStable_Generic)] pub struct CastTarget { pub prefix: [Option; 8], pub rest: Uniform, diff --git a/tests/ui/sanitizer/cfi-address-taken.rs b/tests/ui/sanitizer/cfi-address-taken.rs new file mode 100644 index 0000000000000..7f5b70eb531fb --- /dev/null +++ b/tests/ui/sanitizer/cfi-address-taken.rs @@ -0,0 +1,30 @@ +// Check that the type of trait methods on a concrete type are not abstracted + +//@ needs-sanitizer-cfi +//@ compile-flags: --crate-type=bin -Cprefer-dynamic=off -Clto -Zsanitizer=cfi +//@ compile-flags: -C codegen-units=1 -C opt-level=0 +//@ run-pass + +trait Foo { + fn foo(&self); +} + +struct S; + +impl Foo for S { + fn foo(&self) {} +} + +struct S2 { + f: fn(&S) +} + +impl S2 { + fn foo(&self, s: &S) { + (self.f)(s) + } +} + +fn main() { + S2 { f: ::foo }.foo(&S) +} From 252fcd196499d461fe963577f53c9de417342cef Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Sat, 17 Feb 2024 14:47:44 +0000 Subject: [PATCH 09/13] CFI: Skip non-passed arguments Rust will occasionally rely on fn((), X) -> Y being compatible with fn(X) -> Y, since () is a non-passed argument. Relax CFI by choosing not to encode non-passed arguments. --- .../src/typeid/typeid_itanium_cxx_abi.rs | 18 +-- ...i-emit-type-metadata-id-itanium-cxx-abi.rs | 113 ++---------------- .../sanitizer/cfi-normalize-integers.rs | 6 +- tests/ui/sanitizer/cfi-closure-fn-ptr-cast.rs | 11 ++ 4 files changed, 33 insertions(+), 115 deletions(-) create mode 100644 tests/ui/sanitizer/cfi-closure-fn-ptr-cast.rs 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 a0c4ce1bde4e0..beff86b627602 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 @@ -18,7 +18,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 std::fmt::Write as _; @@ -1041,18 +1041,22 @@ 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, transform_ty_options); - typeid.push_str(&encode_ty(tcx, ty, &mut dict, encode_ty_options)); - } - } else { + 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, 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'); } } 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, 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.rs b/tests/codegen/sanitizer/cfi-emit-type-metadata-id-itanium-cxx-abi.rs index 5f49909712f96..3094ba31695b7 100644 --- a/tests/codegen/sanitizer/cfi-emit-type-metadata-id-itanium-cxx-abi.rs +++ b/tests/codegen/sanitizer/cfi-emit-type-metadata-id-itanium-cxx-abi.rs @@ -154,7 +154,7 @@ pub struct Type14(T); pub fn foo0(_: ()) { } // CHECK: define{{.*}}foo0{{.*}}!type ![[TYPE0:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo1(_: (), _: c_void) { } -// CHECK: define{{.*}}foo1{{.*}}!type ![[TYPE1:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} +// CHECK: define{{.*}}foo1{{.*}}!type ![[TYPE0:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo2(_: (), _: c_void, _: c_void) { } // CHECK: define{{.*}}foo2{{.*}}!type ![[TYPE2:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo3(_: *mut ()) { } @@ -368,71 +368,11 @@ pub fn foo106(_: &dyn Send, _: &dyn Send) { } pub fn foo107(_: &dyn Send, _: &dyn Send, _: &dyn Send) { } // CHECK: define{{.*}}foo107{{.*}}!type ![[TYPE107:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo108(_: Type1) { } -// CHECK: define{{.*}}foo108{{.*}}!type ![[TYPE108:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} +// CHECK: define{{.*}}foo108{{.*}}!type ![[TYPE0:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo109(_: Type1, _: Type1) { } -// CHECK: define{{.*}}foo109{{.*}}!type ![[TYPE109:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} +// CHECK: define{{.*}}foo109{{.*}}!type ![[TYPE0:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo110(_: Type1, _: Type1, _: Type1) { } -// CHECK: define{{.*}}foo110{{.*}}!type ![[TYPE110:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo111(_: Type2) { } -// CHECK: define{{.*}}foo111{{.*}}!type ![[TYPE111:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo112(_: Type2, _: Type2) { } -// CHECK: define{{.*}}foo112{{.*}}!type ![[TYPE112:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo113(_: Type2, _: Type2, _: Type2) { } -// CHECK: define{{.*}}foo113{{.*}}!type ![[TYPE113:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo114(_: Type3) { } -// CHECK: define{{.*}}foo114{{.*}}!type ![[TYPE114:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo115(_: Type3, _: Type3) { } -// CHECK: define{{.*}}foo115{{.*}}!type ![[TYPE115:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo116(_: Type3, _: Type3, _: Type3) { } -// CHECK: define{{.*}}foo116{{.*}}!type ![[TYPE116:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo117(_: Type4) { } -// CHECK: define{{.*}}foo117{{.*}}!type ![[TYPE117:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo118(_: Type4, _: Type4) { } -// CHECK: define{{.*}}foo118{{.*}}!type ![[TYPE118:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo119(_: Type4, _: Type4, _: Type4) { } -// CHECK: define{{.*}}foo119{{.*}}!type ![[TYPE119:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo120(_: Type5) { } -// CHECK: define{{.*}}foo120{{.*}}!type ![[TYPE120:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo121(_: Type5, _: Type5) { } -// CHECK: define{{.*}}foo121{{.*}}!type ![[TYPE121:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo122(_: Type5, _: Type5, _: Type5) { } -// CHECK: define{{.*}}foo122{{.*}}!type ![[TYPE122:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo123(_: Type6) { } -// CHECK: define{{.*}}foo123{{.*}}!type ![[TYPE123:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo124(_: Type6, _: Type6) { } -// CHECK: define{{.*}}foo124{{.*}}!type ![[TYPE124:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo125(_: Type6, _: Type6, _: Type6) { } -// CHECK: define{{.*}}foo125{{.*}}!type ![[TYPE125:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo126(_: Type7) { } -// CHECK: define{{.*}}foo126{{.*}}!type ![[TYPE126:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo127(_: Type7, _: Type7) { } -// CHECK: define{{.*}}foo127{{.*}}!type ![[TYPE127:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo128(_: Type7, _: Type7, _: Type7) { } -// CHECK: define{{.*}}foo128{{.*}}!type ![[TYPE128:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo129(_: Type8) { } -// CHECK: define{{.*}}foo129{{.*}}!type ![[TYPE129:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo130(_: Type8, _: Type8) { } -// CHECK: define{{.*}}foo130{{.*}}!type ![[TYPE130:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo131(_: Type8, _: Type8, _: Type8) { } -// CHECK: define{{.*}}foo131{{.*}}!type ![[TYPE131:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo132(_: Type9) { } -// CHECK: define{{.*}}foo132{{.*}}!type ![[TYPE132:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo133(_: Type9, _: Type9) { } -// CHECK: define{{.*}}foo133{{.*}}!type ![[TYPE133:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo134(_: Type9, _: Type9, _: Type9) { } -// CHECK: define{{.*}}foo134{{.*}}!type ![[TYPE134:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo135(_: Type10) { } -// CHECK: define{{.*}}foo135{{.*}}!type ![[TYPE135:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo136(_: Type10, _: Type10) { } -// CHECK: define{{.*}}foo136{{.*}}!type ![[TYPE136:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo137(_: Type10, _: Type10, _: Type10) { } -// CHECK: define{{.*}}foo137{{.*}}!type ![[TYPE137:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo138(_: Type11) { } -// CHECK: define{{.*}}foo138{{.*}}!type ![[TYPE138:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo139(_: Type11, _: Type11) { } -// CHECK: define{{.*}}foo139{{.*}}!type ![[TYPE139:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -pub fn foo140(_: Type11, _: Type11, _: Type11) { } -// CHECK: define{{.*}}foo140{{.*}}!type ![[TYPE140:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} +// CHECK: define{{.*}}foo110{{.*}}!type ![[TYPE0:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo141(_: Type12) { } // CHECK: define{{.*}}foo141{{.*}}!type ![[TYPE141:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo142(_: Type12, _: Type12) { } @@ -446,15 +386,14 @@ pub fn foo145(_: Type13, _: Type13) { } pub fn foo146(_: Type13, _: Type13, _: Type13) { } // CHECK: define{{.*}}foo146{{.*}}!type ![[TYPE146:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo147(_: Type14) { } -// CHECK: define{{.*}}foo147{{.*}}!type ![[TYPE147:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} +// CHECK: define{{.*}}foo147{{.*}}!type ![[TYPE0:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo148(_: Type14, _: Type14) { } -// CHECK: define{{.*}}foo148{{.*}}!type ![[TYPE148:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} +// CHECK: define{{.*}}foo148{{.*}}!type ![[TYPE0:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} pub fn foo149(_: Type14, _: Type14, _: Type14) { } -// CHECK: define{{.*}}foo149{{.*}}!type ![[TYPE149:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} +// CHECK: define{{.*}}foo149{{.*}}!type ![[TYPE0:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} // CHECK: ![[TYPE0]] = !{i64 0, !"_ZTSFvvE"} -// CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvvvE"} -// CHECK: ![[TYPE2]] = !{i64 0, !"_ZTSFvvvvE"} +// CHECK: ![[TYPE2]] = !{i64 0, !"_ZTSFvvvE"} // CHECK: ![[TYPE3]] = !{i64 0, !"_ZTSFvPvE"} // CHECK: ![[TYPE4]] = !{i64 0, !"_ZTSFvPvS_E"} // CHECK: ![[TYPE5]] = !{i64 0, !"_ZTSFvPvS_S_E"} @@ -560,45 +499,9 @@ pub fn foo149(_: Type14, _: Type14, _: Type14) { } // CHECK: ![[TYPE105]] = !{i64 0, !"_ZTSFvu3refIu3dynIu{{[0-9]+}}NtNtC{{[[:print:]]+}}_4core6marker4Sendu6regionEEE"} // CHECK: ![[TYPE106]] = !{i64 0, !"_ZTSFvu3refIu3dynIu{{[0-9]+}}NtNtC{{[[:print:]]+}}_4core6marker4Sendu6regionEES2_E"} // CHECK: ![[TYPE107]] = !{i64 0, !"_ZTSFvu3refIu3dynIu{{[0-9]+}}NtNtC{{[[:print:]]+}}_4core6marker4Sendu6regionEES2_S2_E"} -// CHECK: ![[TYPE108]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NCNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn111{{[{}][{}]}}closure{{[}][}]}}Iu2i8PFvvEvEE"} -// CHECK: ![[TYPE109]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NCNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn111{{[{}][{}]}}closure{{[}][}]}}Iu2i8PFvvEvES1_E"} -// CHECK: ![[TYPE110]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NCNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn111{{[{}][{}]}}closure{{[}][}]}}Iu2i8PFvvEvES1_S1_E"} -// CHECK: ![[TYPE111]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NcNtNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn13Foo15{{[{}][{}]}}constructor{{[}][}]}}E"} -// CHECK: ![[TYPE112]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NcNtNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn13Foo15{{[{}][{}]}}constructor{{[}][}]}}S_E"} -// CHECK: ![[TYPE113]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NcNtNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn13Foo15{{[{}][{}]}}constructor{{[}][}]}}S_S_E"} -// CHECK: ![[TYPE114]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNFNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn110{{[{}][{}]}}extern{{[}][}]}}3fooE"} -// CHECK: ![[TYPE115]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNFNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn110{{[{}][{}]}}extern{{[}][}]}}3fooS_E"} -// CHECK: ![[TYPE116]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNFNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn110{{[{}][{}]}}extern{{[}][}]}}3fooS_S_E"} -// CHECK: ![[TYPE117]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNCNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn1s0_11{{[{}][{}]}}closure{{[}][}]}}3FooE"} -// CHECK: ![[TYPE118]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNCNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn1s0_11{{[{}][{}]}}closure{{[}][}]}}3FooS_E"} -// CHECK: ![[TYPE119]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNCNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn1s0_11{{[{}][{}]}}closure{{[}][}]}}3FooS_S_E"} -// CHECK: ![[TYPE120]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNkNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn112{{[{}][{}]}}constant{{[}][}]}}3FooE"} -// CHECK: ![[TYPE121]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNkNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn112{{[{}][{}]}}constant{{[}][}]}}3FooS_E"} -// CHECK: ![[TYPE122]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNkNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn112{{[{}][{}]}}constant{{[}][}]}}3FooS_S_E"} -// CHECK: ![[TYPE123]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNINvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn18{{[{}][{}]}}impl{{[}][}]}}3fooIu3i32EE"} -// CHECK: ![[TYPE124]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNINvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn18{{[{}][{}]}}impl{{[}][}]}}3fooIu3i32ES0_E"} -// CHECK: ![[TYPE125]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNINvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn18{{[{}][{}]}}impl{{[}][}]}}3fooIu3i32ES0_S0_E"} -// CHECK: ![[TYPE126]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]6Trait13fooIu3dynIu{{[0-9]+}}NtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]6Trait1Iu5paramEu6regionEu3i32EE"} -// CHECK: ![[TYPE127]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]6Trait13fooIu3dynIu{{[0-9]+}}NtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]6Trait1Iu5paramEu6regionEu3i32ES4_E"} -// CHECK: ![[TYPE128]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]6Trait13fooIu3dynIu{{[0-9]+}}NtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]6Trait1Iu5paramEu6regionEu3i32ES4_S4_E"} -// CHECK: ![[TYPE129]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]6Trait13fooIu3i32S_EE"} -// CHECK: ![[TYPE130]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]6Trait13fooIu3i32S_ES0_E"} -// CHECK: ![[TYPE131]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]6Trait13fooIu3i32S_ES0_S0_E"} -// CHECK: ![[TYPE132]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]6Trait13fooIu{{[0-9]+}}NtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]7Struct1Iu3i32ES_EE"} -// CHECK: ![[TYPE133]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]6Trait13fooIu{{[0-9]+}}NtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]7Struct1Iu3i32ES_ES1_E"} -// CHECK: ![[TYPE134]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NvNtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]6Trait13fooIu{{[0-9]+}}NtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]7Struct1Iu3i32ES_ES1_S1_E"} -// CHECK: ![[TYPE135]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn13QuxIu3i32Lu5usize32EEE"} -// CHECK: ![[TYPE136]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn13QuxIu3i32Lu5usize32EES2_E"} -// CHECK: ![[TYPE137]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn13QuxIu3i32Lu5usize32EES2_S2_E"} -// CHECK: ![[TYPE138]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NcNtNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn15Quuux15{{[{}][{}]}}constructor{{[}][}]}}Iu6regionS_EE"} -// CHECK: ![[TYPE139]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NcNtNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn15Quuux15{{[{}][{}]}}constructor{{[}][}]}}Iu6regionS_ES0_E"} -// CHECK: ![[TYPE140]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NcNtNvC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3fn15Quuux15{{[{}][{}]}}constructor{{[}][}]}}Iu6regionS_ES0_S0_E"} // CHECK: ![[TYPE141]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3FooE"} // CHECK: ![[TYPE142]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3FooS_E"} // CHECK: ![[TYPE143]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3FooS_S_E"} // CHECK: ![[TYPE144]] = !{i64 0, !"_ZTSFvu3refIvEE"} // CHECK: ![[TYPE145]] = !{i64 0, !"_ZTSFvu3refIvES_E"} // CHECK: ![[TYPE146]] = !{i64 0, !"_ZTSFvu3refIvES_S_E"} -// CHECK: ![[TYPE147]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3BarE"} -// CHECK: ![[TYPE148]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3BarS_E"} -// CHECK: ![[TYPE149]] = !{i64 0, !"_ZTSFvu{{[0-9]+}}NtC{{[[:print:]]+}}_[[ITANIUMED_FILENAME]]3BarS_S_E"} diff --git a/tests/codegen/sanitizer/cfi-normalize-integers.rs b/tests/codegen/sanitizer/cfi-normalize-integers.rs index 210814eb9ae1f..801ed312be5b1 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, !"_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"} +// 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"} diff --git a/tests/ui/sanitizer/cfi-closure-fn-ptr-cast.rs b/tests/ui/sanitizer/cfi-closure-fn-ptr-cast.rs new file mode 100644 index 0000000000000..dd62c82f8a9ca --- /dev/null +++ b/tests/ui/sanitizer/cfi-closure-fn-ptr-cast.rs @@ -0,0 +1,11 @@ +// Tests that converting a closure to a function pointer works + +//@ needs-sanitizer-cfi +//@ compile-flags: --crate-type=bin -Cprefer-dynamic=off -Clto -Zsanitizer=cfi +//@ compile-flags:-C codegen-units=1 -C opt-level=0 +//@ run-pass + +pub fn main() { + let f: &fn() = &((|| ()) as _); + f(); +} From b30fedb585d3623dcc324cf902c72d63d971b7ac Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Mon, 19 Feb 2024 18:39:35 +0000 Subject: [PATCH 10/13] CFI: Handle dyn with no principal In user-facing Rust, `dyn` always has at least one predicate following it. Unfortunately, because we filter out marker traits and `dyn Sync` is, for example, legal, this results in us having `dyn` types with no predicates on occasion. This patch handles cases where there are no predicates in a `dyn` type. --- compiler/rustc_middle/src/ty/predicate.rs | 2 +- .../src/typeid/typeid_itanium_cxx_abi.rs | 6 +++++- tests/ui/sanitizer/cfi-drop-no-principal.rs | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 tests/ui/sanitizer/cfi-drop-no-principal.rs diff --git a/compiler/rustc_middle/src/ty/predicate.rs b/compiler/rustc_middle/src/ty/predicate.rs index 8dd95daed36a7..fd3f2139b87d4 100644 --- a/compiler/rustc_middle/src/ty/predicate.rs +++ b/compiler/rustc_middle/src/ty/predicate.rs @@ -290,7 +290,7 @@ impl<'tcx> ty::List> { /// have a "trivial" vtable consisting of just the size, alignment, /// and destructor. pub fn principal(&self) -> Option>> { - self[0] + self.get(0)? .map_bound(|this| match this { ExistentialPredicate::Trait(tr) => Some(tr), _ => None, 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 beff86b627602..de88a116b8721 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 @@ -759,7 +759,11 @@ fn transform_predicates<'tcx>( ty::ExistentialPredicate::AutoTrait(..) => Some(predicate), }) .collect(); - tcx.mk_poly_existential_predicates(&predicates) + if predicates.len() == 0 { + List::empty() + } else { + tcx.mk_poly_existential_predicates(&predicates) + } } /// Transforms args for being encoded and used in the substitution dictionary. diff --git a/tests/ui/sanitizer/cfi-drop-no-principal.rs b/tests/ui/sanitizer/cfi-drop-no-principal.rs new file mode 100644 index 0000000000000..87f7029d30185 --- /dev/null +++ b/tests/ui/sanitizer/cfi-drop-no-principal.rs @@ -0,0 +1,16 @@ +// Check that dropping a trait object without a principal trait succeeds + +//@ needs-sanitizer-cfi +//@ compile-flags: --crate-type=bin -Cprefer-dynamic=off -Clto -Zsanitizer=cfi +//@ compile-flags: -C codegen-units=1 -C opt-level=0 +//@ run-pass +// Check that trait objects without a principal can be dropped. + +struct CustomDrop; +impl Drop for CustomDrop { + fn drop(&mut self) {} +} + +fn main() { + let _ = Box::new(CustomDrop) as Box; +} From 09d1d3ef29724030a0e607a117b9fe1b58b68626 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Sun, 3 Mar 2024 03:51:05 +0000 Subject: [PATCH 11/13] CFI: Support self_cell-like recursion Current `transform_ty` attempts to avoid cycles when normalizing `#[repr(transparent)]` types to their interior, but runs afoul of this pattern used in `self_cell`: ``` struct X { x: u8, p: PhantomData, } #[repr(transparent)] struct Y(X); ``` When attempting to normalize Y, it will still cycle indefinitely. By using a types-visited list, this will instead get expanded exactly one layer deep to X, and then stop, not attempting to normalize `Y` any further. --- .../src/typeid/typeid_itanium_cxx_abi.rs | 69 ++++++++++++------- tests/ui/sanitizer/cfi-self-ref.rs | 31 +++++++++ 2 files changed, 75 insertions(+), 25 deletions(-) create mode 100644 tests/ui/sanitizer/cfi-self-ref.rs 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 de88a116b8721..a33a6d3563461 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 @@ -178,14 +178,14 @@ fn encode_fnsig<'tcx>( // Encode the return type let transform_ty_options = TransformTyOptions::from_bits(options.bits()) .unwrap_or_else(|| bug!("encode_fnsig: invalid option(s) `{:?}`", options.bits())); - let ty = transform_ty(tcx, fn_sig.output(), transform_ty_options); + let ty = transform_ty(tcx, fn_sig.output(), &[], transform_ty_options); s.push_str(&encode_ty(tcx, ty, dict, encode_ty_options)); // Encode the parameter types let tys = fn_sig.inputs(); if !tys.is_empty() { for ty in tys { - let ty = transform_ty(tcx, *ty, transform_ty_options); + let ty = transform_ty(tcx, *ty, &[], transform_ty_options); s.push_str(&encode_ty(tcx, ty, dict, encode_ty_options)); } @@ -770,11 +770,12 @@ fn transform_predicates<'tcx>( fn transform_args<'tcx>( tcx: TyCtxt<'tcx>, args: GenericArgsRef<'tcx>, + parents: &[Ty<'tcx>], options: TransformTyOptions, ) -> GenericArgsRef<'tcx> { let args = args.iter().map(|arg| match arg.unpack() { GenericArgKind::Type(ty) if ty.is_c_void(tcx) => Ty::new_unit(tcx).into(), - GenericArgKind::Type(ty) => transform_ty(tcx, ty, options).into(), + GenericArgKind::Type(ty) => transform_ty(tcx, ty, parents, options).into(), _ => arg, }); tcx.mk_args_from_iter(args) @@ -784,9 +785,12 @@ fn transform_args<'tcx>( // c_void types into unit types unconditionally, generalizes pointers if // TransformTyOptions::GENERALIZE_POINTERS option is set, and normalizes integers if // TransformTyOptions::NORMALIZE_INTEGERS option is set. -fn transform_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, options: TransformTyOptions) -> Ty<'tcx> { - let mut ty = ty; - +fn transform_ty<'tcx>( + tcx: TyCtxt<'tcx>, + mut ty: Ty<'tcx>, + parents: &[Ty<'tcx>], + options: TransformTyOptions, +) -> Ty<'tcx> { match ty.kind() { ty::Float(..) | ty::Str | ty::Never | ty::Foreign(..) | ty::CoroutineWitness(..) => {} @@ -846,17 +850,20 @@ fn transform_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, options: TransformTyOptio _ if ty.is_unit() => {} ty::Tuple(tys) => { - ty = Ty::new_tup_from_iter(tcx, tys.iter().map(|ty| transform_ty(tcx, ty, options))); + ty = Ty::new_tup_from_iter( + tcx, + tys.iter().map(|ty| transform_ty(tcx, ty, &parents, options)), + ); } ty::Array(ty0, len) => { let len = len.eval_target_usize(tcx, ty::ParamEnv::reveal_all()); - ty = Ty::new_array(tcx, transform_ty(tcx, *ty0, options), len); + ty = Ty::new_array(tcx, transform_ty(tcx, *ty0, &parents, options), len); } ty::Slice(ty0) => { - ty = Ty::new_slice(tcx, transform_ty(tcx, *ty0, options)); + ty = Ty::new_slice(tcx, transform_ty(tcx, *ty0, &parents, options)); } ty::Adt(adt_def, args) => { @@ -865,7 +872,8 @@ fn transform_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, options: TransformTyOptio } else if options.contains(TransformTyOptions::GENERALIZE_REPR_C) && adt_def.repr().c() { ty = Ty::new_adt(tcx, *adt_def, ty::List::empty()); - } else if adt_def.repr().transparent() && adt_def.is_struct() { + } else if adt_def.repr().transparent() && adt_def.is_struct() && !parents.contains(&ty) + { // Don't transform repr(transparent) types with an user-defined CFI encoding to // preserve the user-defined CFI encoding. if let Some(_) = tcx.get_attr(adt_def.did(), sym::cfi_encoding) { @@ -884,38 +892,48 @@ fn transform_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, options: TransformTyOptio // Generalize any repr(transparent) user-defined type that is either a pointer // or reference, and either references itself or any other type that contains or // references itself, to avoid a reference cycle. + + // If the self reference is not through a pointer, for example, due + // to using `PhantomData`, need to skip normalizing it if we hit it again. + let mut parents = parents.to_vec(); + parents.push(ty); if ty0.is_any_ptr() && ty0.contains(ty) { ty = transform_ty( tcx, ty0, + &parents, options | TransformTyOptions::GENERALIZE_POINTERS, ); } else { - ty = transform_ty(tcx, ty0, options); + ty = transform_ty(tcx, ty0, &parents, options); } } else { // Transform repr(transparent) types without non-ZST field into () ty = Ty::new_unit(tcx); } } else { - ty = Ty::new_adt(tcx, *adt_def, transform_args(tcx, args, options)); + ty = Ty::new_adt(tcx, *adt_def, transform_args(tcx, args, &parents, options)); } } ty::FnDef(def_id, args) => { - ty = Ty::new_fn_def(tcx, *def_id, transform_args(tcx, args, options)); + ty = Ty::new_fn_def(tcx, *def_id, transform_args(tcx, args, &parents, options)); } ty::Closure(def_id, args) => { - ty = Ty::new_closure(tcx, *def_id, transform_args(tcx, args, options)); + ty = Ty::new_closure(tcx, *def_id, transform_args(tcx, args, &parents, options)); } ty::CoroutineClosure(def_id, args) => { - ty = Ty::new_coroutine_closure(tcx, *def_id, transform_args(tcx, args, options)); + ty = Ty::new_coroutine_closure( + tcx, + *def_id, + transform_args(tcx, args, &parents, options), + ); } ty::Coroutine(def_id, args) => { - ty = Ty::new_coroutine(tcx, *def_id, transform_args(tcx, args, options)); + ty = Ty::new_coroutine(tcx, *def_id, transform_args(tcx, args, &parents, options)); } ty::Ref(region, ty0, ..) => { @@ -927,9 +945,9 @@ fn transform_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, options: TransformTyOptio } } else { if ty.is_mutable_ptr() { - ty = Ty::new_mut_ref(tcx, *region, transform_ty(tcx, *ty0, options)); + ty = Ty::new_mut_ref(tcx, *region, transform_ty(tcx, *ty0, &parents, options)); } else { - ty = Ty::new_imm_ref(tcx, *region, transform_ty(tcx, *ty0, options)); + ty = Ty::new_imm_ref(tcx, *region, transform_ty(tcx, *ty0, &parents, options)); } } } @@ -943,9 +961,9 @@ fn transform_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, options: TransformTyOptio } } else { if ty.is_mutable_ptr() { - ty = Ty::new_mut_ptr(tcx, transform_ty(tcx, tm.ty, options)); + ty = Ty::new_mut_ptr(tcx, transform_ty(tcx, tm.ty, &parents, options)); } else { - ty = Ty::new_imm_ptr(tcx, transform_ty(tcx, tm.ty, options)); + ty = Ty::new_imm_ptr(tcx, transform_ty(tcx, tm.ty, &parents, options)); } } } @@ -958,9 +976,9 @@ fn transform_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, options: TransformTyOptio .skip_binder() .inputs() .iter() - .map(|ty| transform_ty(tcx, *ty, options)) + .map(|ty| transform_ty(tcx, *ty, &parents, options)) .collect(); - let output = transform_ty(tcx, fn_sig.skip_binder().output(), options); + let output = transform_ty(tcx, fn_sig.skip_binder().output(), &parents, options); ty = Ty::new_fn_ptr( tcx, ty::Binder::bind_with_vars( @@ -990,6 +1008,7 @@ fn transform_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, options: TransformTyOptio ty = transform_ty( tcx, tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), ty), + &parents, options, ); } @@ -1040,7 +1059,7 @@ pub fn typeid_for_fnabi<'tcx>( // Encode the return type let transform_ty_options = TransformTyOptions::from_bits(options.bits()) .unwrap_or_else(|| bug!("typeid_for_fnabi: invalid option(s) `{:?}`", options.bits())); - let ty = transform_ty(tcx, fn_abi.ret.layout.ty, transform_ty_options); + let ty = transform_ty(tcx, fn_abi.ret.layout.ty, &[], transform_ty_options); typeid.push_str(&encode_ty(tcx, ty, &mut dict, encode_ty_options)); // Encode the parameter types @@ -1048,7 +1067,7 @@ pub fn typeid_for_fnabi<'tcx>( 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, transform_ty_options); + let ty = transform_ty(tcx, arg.layout.ty, &[], transform_ty_options); typeid.push_str(&encode_ty(tcx, ty, &mut dict, encode_ty_options)); } if !pushed_arg { @@ -1061,7 +1080,7 @@ pub fn typeid_for_fnabi<'tcx>( if fn_abi.args[n].mode == PassMode::Ignore { continue; } - let ty = transform_ty(tcx, fn_abi.args[n].layout.ty, transform_ty_options); + let ty = transform_ty(tcx, fn_abi.args[n].layout.ty, &[], transform_ty_options); typeid.push_str(&encode_ty(tcx, ty, &mut dict, encode_ty_options)); } diff --git a/tests/ui/sanitizer/cfi-self-ref.rs b/tests/ui/sanitizer/cfi-self-ref.rs new file mode 100644 index 0000000000000..710a05c8a7962 --- /dev/null +++ b/tests/ui/sanitizer/cfi-self-ref.rs @@ -0,0 +1,31 @@ +// Check that encoding self-referential types works with #[repr(transparent)] + +//@ needs-sanitizer-cfi +//@ compile-flags: --crate-type=bin -Cprefer-dynamic=off -Clto -Zsanitizer=cfi +//@ compile-flags: -C codegen-units=1 -C opt-level=0 +//@ build-pass + +use std::marker::PhantomData; + +struct X { + x: u8, + p: PhantomData, +} + +#[repr(transparent)] +struct Y(X); + +trait Fooable { + fn foo(&self, y: Y); +} + +struct Bar; + +impl Fooable for Bar { + fn foo(&self, _: Y) {} +} + +fn main() { + let x = &Bar as &dyn Fooable; + x.foo(Y(X {x: 0, p: PhantomData})); +} From 4912a322f1e24370f4151d31a42c25a2bce19a92 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Mon, 19 Feb 2024 17:40:38 +0000 Subject: [PATCH 12/13] CFI: Generate super vtables explicitly CFI shimming means they're not gauranteed to be pre-generated. Traditionally, the base vtable has all the elements of the supertrait vtable, and so visiting the base vtable implies you don't need to visit the supertrait vtable. However, with CFI the base vtable entries will have invocation type `dyn Child`, and the parent vtable will have invocation type `dyn Parent`, so they aren't actually the same instance, and both must be visited. --- compiler/rustc_monomorphize/src/collector.rs | 55 +++++++++++++++----- tests/ui/sanitizer/cfi-super-vtable.rs | 39 ++++++++++++++ 2 files changed, 81 insertions(+), 13 deletions(-) create mode 100644 tests/ui/sanitizer/cfi-super-vtable.rs diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 9950b6a960b66..5a06bdb1a8644 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -191,6 +191,7 @@ use rustc_span::source_map::{dummy_spanned, respan, Spanned}; use rustc_span::symbol::{sym, Ident}; use rustc_span::{Span, DUMMY_SP}; use rustc_target::abi::Size; +use std::iter; use std::path::PathBuf; use crate::errors::{ @@ -1206,23 +1207,51 @@ fn create_mono_items_for_vtable_methods<'tcx>( assert!(!poly_trait_ref.has_escaping_bound_vars()); // Walk all methods of the trait, including those of its supertraits - let entries = tcx.vtable_entries(poly_trait_ref); - let methods = entries - .iter() - .filter_map(|entry| match entry { + for entry in tcx.vtable_entries(poly_trait_ref) { + match entry { VtblEntry::MetadataDropInPlace | VtblEntry::MetadataSize | VtblEntry::MetadataAlign - | VtblEntry::Vacant => None, - VtblEntry::TraitVPtr(_) => { - // all super trait items already covered, so skip them. - None + | VtblEntry::Vacant => (), + VtblEntry::TraitVPtr(super_trait) => { + // If CFI shims are enabled, the super_trait will use a different invocation + // type than the instances selected for this trait. This means we must walk + // the super_trait pointer visit its instances as well. + if tcx.sess.cfi_shims() { + let pep: ty::PolyExistentialPredicate<'tcx> = + super_trait.map_bound(|t| { + ty::ExistentialPredicate::Trait( + ty::ExistentialTraitRef::erase_self_ty(tcx, t), + ) + }); + let existential_predicates = tcx + .mk_poly_existential_predicates_from_iter( + iter::once(pep).chain(trait_ty.iter().skip(1)), + ); + let super_trait_ty = Ty::new_dynamic( + tcx, + existential_predicates, + tcx.lifetimes.re_erased, + ty::Dyn, + ); + + create_mono_items_for_vtable_methods( + tcx, + super_trait_ty, + impl_ty, + source, + output, + ); + } + } + VtblEntry::Method(instance) => { + let instance = instance.cfi_shim(tcx, invoke_trait); + if should_codegen_locally(tcx, &instance) { + output.push(create_fn_mono_item(tcx, instance, source)); + } } - VtblEntry::Method(instance) => Some(instance.cfi_shim(tcx, invoke_trait)) - .filter(|instance| should_codegen_locally(tcx, instance)), - }) - .map(|item| create_fn_mono_item(tcx, item, source)); - output.extend(methods); + } + } }; // Also add the destructor. diff --git a/tests/ui/sanitizer/cfi-super-vtable.rs b/tests/ui/sanitizer/cfi-super-vtable.rs new file mode 100644 index 0000000000000..2764ffc5426b4 --- /dev/null +++ b/tests/ui/sanitizer/cfi-super-vtable.rs @@ -0,0 +1,39 @@ +// Check that super-traits with vptrs have their shims generated + +//@ needs-sanitizer-cfi +//@ compile-flags: --crate-type=bin -Cprefer-dynamic=off -Clto -Zsanitizer=cfi +//@ compile-flags: -C codegen-units=1 -C opt-level=0 +//@ build-pass + +trait Parent1 { + fn p1(&self); +} + +trait Parent2 { + fn p2(&self); +} + +// We need two parent traits to force the vtable upcasting code to choose to add a pointer to +// another vtable to the child. This vtable is generated even if trait upcasting is not in use. +trait Child : Parent1 + Parent2 { + fn c(&self); +} + +struct Foo; + +impl Parent1 for Foo { + fn p1(&self) {} +} + +impl Parent2 for Foo { + fn p2(&self) {} +} + +impl Child for Foo { + fn c(&self) {} +} + +fn main() { + let x = &Foo as &dyn Child; + x.c(); +} From 50a0a86274adc7756f60d244816d1fe3554d9d0f Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Sun, 3 Mar 2024 18:38:32 +0000 Subject: [PATCH 13/13] CFI: Strip auto traits from Virtual receivers As the instance being called is behind a vtable, it cannot depend on auto traits on the receiver (unless the principal trait requires them, in which case the additional constraint is not needed). Removing this causes the type signature of the `Virtual` instance to match the type signature of the `CfiShim`-wrapped entry in the vtable. --- compiler/rustc_codegen_ssa/src/mir/block.rs | 4 +-- compiler/rustc_middle/src/query/mod.rs | 7 +++++ compiler/rustc_ty_utils/src/instance.rs | 26 ++++++++++++++++--- .../ui/sanitizer/cfi-marker-trait-objects.rs | 17 ++++++++++++ 4 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 tests/ui/sanitizer/cfi-marker-trait-objects.rs diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 95a587b8181f6..e854abeb52690 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -495,7 +495,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // let virtual_drop = Instance { def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0), - args: drop_fn.args, + args: bx.tcx().strip_receiver_auto(drop_fn.args), }; debug!("ty = {:?}", ty); debug!("drop_fn = {:?}", drop_fn); @@ -535,7 +535,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // SO THEN WE CAN USE THE ABOVE CODE. let virtual_drop = Instance { def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0), - args: drop_fn.args, + args: bx.tcx().strip_receiver_auto(drop_fn.args), }; debug!("ty = {:?}", ty); debug!("drop_fn = {:?}", drop_fn); diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index b52b681ae6b11..b1d90d9a3543a 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -2256,6 +2256,13 @@ rustc_queries! { query trait_object_ty(trait_ref: ty::PolyTraitRef<'tcx>) -> Ty<'tcx> { desc { "Compute the trait object type for calling a method on a trait" } } + + /// Strip auto traits off the first parameter in the parametr list. Intended for use when + /// constructing `InstanceDef::Virtual`, as auto traits won't be part of the vtable's `Self` + /// types. + query strip_receiver_auto(args: ty::GenericArgsRef<'tcx>) -> ty::GenericArgsRef<'tcx> { + desc { "Strip auto traits off the first type arg" } + } } rustc_query_append! { define_callbacks! } diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs index 381681fb1f416..d5b56c18f2041 100644 --- a/compiler/rustc_ty_utils/src/instance.rs +++ b/compiler/rustc_ty_utils/src/instance.rs @@ -4,7 +4,7 @@ use rustc_infer::infer::TyCtxtInferExt; use rustc_middle::query::Providers; use rustc_middle::traits::{BuiltinImplSource, CodegenObligationError}; use rustc_middle::ty::GenericArgsRef; -use rustc_middle::ty::{self, Instance, TyCtxt, TypeVisitableExt}; +use rustc_middle::ty::{self, Instance, Ty, TyCtxt, TypeVisitableExt}; use rustc_span::sym; use rustc_trait_selection::traits; use traits::{translate_args, Reveal}; @@ -197,7 +197,7 @@ fn resolve_associated_item<'tcx>( traits::get_vtable_index_of_object_method(tcx, *vtable_base, trait_item_id).map( |index| Instance { def: ty::InstanceDef::Virtual(trait_item_id, index), - args: rcvr_args, + args: tcx.strip_receiver_auto(rcvr_args), }, ) } @@ -337,6 +337,26 @@ fn resolve_associated_item<'tcx>( }) } +fn strip_receiver_auto<'tcx>( + tcx: TyCtxt<'tcx>, + args: ty::GenericArgsRef<'tcx>, +) -> ty::GenericArgsRef<'tcx> { + let ty = args.type_at(0); + let ty::Dynamic(preds, lifetime, kind) = ty.kind() else { + bug!("Tried to strip auto traits from non-dynamic type {ty}"); + }; + let filtered_preds = + if preds.principal().is_some() { + tcx.mk_poly_existential_predicates_from_iter(preds.into_iter().filter(|pred| { + !matches!(pred.skip_binder(), ty::ExistentialPredicate::AutoTrait(..)) + })) + } else { + ty::List::empty() + }; + let new_rcvr = Ty::new_dynamic(tcx, filtered_preds, *lifetime, *kind); + tcx.mk_args_trait(new_rcvr, args.into_iter().skip(1)) +} + pub(crate) fn provide(providers: &mut Providers) { - *providers = Providers { resolve_instance, ..*providers }; + *providers = Providers { resolve_instance, strip_receiver_auto, ..*providers }; } diff --git a/tests/ui/sanitizer/cfi-marker-trait-objects.rs b/tests/ui/sanitizer/cfi-marker-trait-objects.rs new file mode 100644 index 0000000000000..cf9be8328076c --- /dev/null +++ b/tests/ui/sanitizer/cfi-marker-trait-objects.rs @@ -0,0 +1,17 @@ +// Test that we can promote closures / fns to trait objects, and call them despite a marker trait. + +//@ needs-sanitizer-cfi +//@ compile-flags: --crate-type=bin -Cprefer-dynamic=off -Clto -Zsanitizer=cfi +//@ compile-flags: -C codegen-units=1 -C opt-level=0 +//@ run-pass + + +fn foo() {} + +static FOO: &'static (dyn Fn() + Sync) = &foo; +static BAR: &(dyn Fn() -> i32 + Sync) = &|| 3; + +fn main() { + FOO(); + BAR(); +}