Skip to content

Commit 79dd275

Browse files
maurermaurer
committed
Track reason for creating a ReifyShim
KCFI needs to be able to tell which kind of `ReifyShim` it is examining in order to decide whether to use a concrete type (`FnPtr` case) or an abstract case (`Vtable` case). You can *almost* tell this from context, but there is one case where you can't - if a trait has a method which is *not* `#[track_caller]`, with an impl that *is* `#[track_caller]`, both the vtable and a function pointer created from that method will be `ReifyShim(def_id)`. Currently, the reason is optional to ensure no additional unique `ReifyShim`s are added without KCFI on. However, the case in which an extra `ReifyShim` is created is sufficiently rare that this may be worth revisiting to reduce complexity.
1 parent 70714e3 commit 79dd275

File tree

9 files changed

+45
-19
lines changed

9 files changed

+45
-19
lines changed

‎compiler/rustc_middle/src/mir/visit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ macro_rules! make_mir_visitor {
341341

342342
ty::InstanceDef::Intrinsic(_def_id) |
343343
ty::InstanceDef::VTableShim(_def_id) |
344-
ty::InstanceDef::ReifyShim(_def_id) |
344+
ty::InstanceDef::ReifyShim(_def_id, _) |
345345
ty::InstanceDef::Virtual(_def_id, _) |
346346
ty::InstanceDef::ThreadLocalShim(_def_id) |
347347
ty::InstanceDef::ClosureOnceShim { call_once: _def_id, track_caller: _ } |

‎compiler/rustc_middle/src/ty/instance.rs

+25-8
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ pub struct Instance<'tcx> {
3131
pub args: GenericArgsRef<'tcx>,
3232
}
3333

34+
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
35+
#[derive(TyEncodable, TyDecodable, HashStable)]
36+
pub enum ReifyReason {
37+
FnPtr,
38+
Vtable,
39+
}
40+
3441
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
3542
#[derive(TyEncodable, TyDecodable, HashStable, TypeFoldable, TypeVisitable, Lift)]
3643
pub enum InstanceDef<'tcx> {
@@ -67,7 +74,13 @@ pub enum InstanceDef<'tcx> {
6774
/// Because this is a required part of the function's ABI but can't be tracked
6875
/// as a property of the function pointer, we use a single "caller location"
6976
/// (the definition of the function itself).
70-
ReifyShim(DefId),
77+
///
78+
/// The second field encodes *why* this shim was created. This allows distinguishing between
79+
/// a `ReifyShim` that appears in a vtable vs one that appears as a function pointer.
80+
///
81+
/// This field will only be populated if we are compiling in a mode that needs these shims
82+
/// to be separable, currently only when KCFI is enabled.
83+
ReifyShim(DefId, Option<ReifyReason>),
7184

7285
/// `<fn() as FnTrait>::call_*` (generated `FnTrait` implementation for `fn()` pointers).
7386
///
@@ -194,7 +207,7 @@ impl<'tcx> InstanceDef<'tcx> {
194207
match self {
195208
InstanceDef::Item(def_id)
196209
| InstanceDef::VTableShim(def_id)
197-
| InstanceDef::ReifyShim(def_id)
210+
| InstanceDef::ReifyShim(def_id, _)
198211
| InstanceDef::FnPtrShim(def_id, _)
199212
| InstanceDef::Virtual(def_id, _)
200213
| InstanceDef::Intrinsic(def_id)
@@ -354,7 +367,9 @@ fn fmt_instance(
354367
match instance.def {
355368
InstanceDef::Item(_) => Ok(()),
356369
InstanceDef::VTableShim(_) => write!(f, " - shim(vtable)"),
357-
InstanceDef::ReifyShim(_) => write!(f, " - shim(reify)"),
370+
InstanceDef::ReifyShim(_, None) => write!(f, " - shim(reify)"),
371+
InstanceDef::ReifyShim(_, Some(ReifyReason::FnPtr)) => write!(f, " - shim(reify-fnptr)"),
372+
InstanceDef::ReifyShim(_, Some(ReifyReason::Vtable)) => write!(f, " - shim(reify-vtable)"),
358373
InstanceDef::ThreadLocalShim(_) => write!(f, " - shim(tls)"),
359374
InstanceDef::Intrinsic(_) => write!(f, " - intrinsic"),
360375
InstanceDef::Virtual(_, num) => write!(f, " - virtual#{num}"),
@@ -476,15 +491,16 @@ impl<'tcx> Instance<'tcx> {
476491
debug!("resolve(def_id={:?}, args={:?})", def_id, args);
477492
// Use either `resolve_closure` or `resolve_for_vtable`
478493
assert!(!tcx.is_closure_like(def_id), "Called `resolve_for_fn_ptr` on closure: {def_id:?}");
494+
let reason = tcx.sess.is_sanitizer_kcfi_enabled().then_some(ReifyReason::FnPtr);
479495
Instance::resolve(tcx, param_env, def_id, args).ok().flatten().map(|mut resolved| {
480496
match resolved.def {
481497
InstanceDef::Item(def) if resolved.def.requires_caller_location(tcx) => {
482498
debug!(" => fn pointer created for function with #[track_caller]");
483-
resolved.def = InstanceDef::ReifyShim(def);
499+
resolved.def = InstanceDef::ReifyShim(def, reason);
484500
}
485501
InstanceDef::Virtual(def_id, _) => {
486502
debug!(" => fn pointer created for virtual call");
487-
resolved.def = InstanceDef::ReifyShim(def_id);
503+
resolved.def = InstanceDef::ReifyShim(def_id, reason);
488504
}
489505
_ => {}
490506
}
@@ -508,6 +524,7 @@ impl<'tcx> Instance<'tcx> {
508524
debug!(" => associated item with unsizeable self: Self");
509525
Some(Instance { def: InstanceDef::VTableShim(def_id), args })
510526
} else {
527+
let reason = tcx.sess.is_sanitizer_kcfi_enabled().then_some(ReifyReason::Vtable);
511528
Instance::resolve(tcx, param_env, def_id, args).ok().flatten().map(|mut resolved| {
512529
match resolved.def {
513530
InstanceDef::Item(def) => {
@@ -544,18 +561,18 @@ impl<'tcx> Instance<'tcx> {
544561
// Create a shim for the `FnOnce/FnMut/Fn` method we are calling
545562
// - unlike functions, invoking a closure always goes through a
546563
// trait.
547-
resolved = Instance { def: InstanceDef::ReifyShim(def_id), args };
564+
resolved = Instance { def: InstanceDef::ReifyShim(def_id, reason), args };
548565
} else {
549566
debug!(
550567
" => vtable fn pointer created for function with #[track_caller]: {:?}", def
551568
);
552-
resolved.def = InstanceDef::ReifyShim(def);
569+
resolved.def = InstanceDef::ReifyShim(def, reason);
553570
}
554571
}
555572
}
556573
InstanceDef::Virtual(def_id, _) => {
557574
debug!(" => vtable fn pointer created for virtual call");
558-
resolved.def = InstanceDef::ReifyShim(def_id);
575+
resolved.def = InstanceDef::ReifyShim(def_id, reason)
559576
}
560577
_ => {}
561578
}

‎compiler/rustc_middle/src/ty/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ pub use self::context::{
8888
tls, CtxtInterners, CurrentGcx, DeducedParamAttrs, Feed, FreeRegionInfo, GlobalCtxt, Lift,
8989
TyCtxt, TyCtxtFeed,
9090
};
91-
pub use self::instance::{Instance, InstanceDef, ShortInstance, UnusedGenericParams};
91+
pub use self::instance::{Instance, InstanceDef, ReifyReason, ShortInstance, UnusedGenericParams};
9292
pub use self::list::List;
9393
pub use self::parameterized::ParameterizedOverTcx;
9494
pub use self::predicate::{

‎compiler/rustc_middle/src/ty/structural_impls.rs

+1
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,7 @@ TrivialTypeTraversalAndLiftImpls! {
449449
crate::ty::ClosureKind,
450450
crate::ty::ParamConst,
451451
crate::ty::ParamTy,
452+
crate::ty::instance::ReifyReason,
452453
interpret::AllocId,
453454
interpret::CtfeProvenance,
454455
interpret::Scalar,

‎compiler/rustc_mir_transform/src/inline.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ impl<'tcx> Inliner<'tcx> {
324324
// do not need to catch this here, we can wait until the inliner decides to continue
325325
// inlining a second time.
326326
InstanceDef::VTableShim(_)
327-
| InstanceDef::ReifyShim(_)
327+
| InstanceDef::ReifyShim(..)
328328
| InstanceDef::FnPtrShim(..)
329329
| InstanceDef::ClosureOnceShim { .. }
330330
| InstanceDef::ConstructCoroutineInClosureShim { .. }

‎compiler/rustc_mir_transform/src/inline/cycle.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ pub(crate) fn mir_callgraph_reachable<'tcx>(
8484
// again, a function item can end up getting inlined. Thus we'll be able to cause
8585
// a cycle that way
8686
InstanceDef::VTableShim(_)
87-
| InstanceDef::ReifyShim(_)
87+
| InstanceDef::ReifyShim(..)
8888
| InstanceDef::FnPtrShim(..)
8989
| InstanceDef::ClosureOnceShim { .. }
9090
| InstanceDef::ConstructCoroutineInClosureShim { .. }

‎compiler/rustc_mir_transform/src/shim.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
5555
// a virtual call, or a direct call to a function for which
5656
// indirect calls must be codegen'd differently than direct ones
5757
// (such as `#[track_caller]`).
58-
ty::InstanceDef::ReifyShim(def_id) => {
58+
ty::InstanceDef::ReifyShim(def_id, _) => {
5959
build_call_shim(tcx, instance, None, CallKind::Direct(def_id))
6060
}
6161
ty::InstanceDef::ClosureOnceShim { call_once: _, track_caller: _ } => {

‎compiler/rustc_symbol_mangling/src/legacy.rs

+9-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use rustc_data_structures::stable_hasher::{Hash64, HashStable, StableHasher};
22
use rustc_hir::def_id::CrateNum;
33
use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData};
44
use rustc_middle::ty::print::{PrettyPrinter, Print, PrintError, Printer};
5-
use rustc_middle::ty::{self, Instance, Ty, TyCtxt, TypeVisitableExt};
5+
use rustc_middle::ty::{self, Instance, ReifyReason, Ty, TyCtxt, TypeVisitableExt};
66
use rustc_middle::ty::{GenericArg, GenericArgKind};
77

88
use std::fmt::{self, Write};
@@ -71,8 +71,14 @@ pub(super) fn mangle<'tcx>(
7171
ty::InstanceDef::VTableShim(..) => {
7272
printer.write_str("{{vtable-shim}}").unwrap();
7373
}
74-
ty::InstanceDef::ReifyShim(..) => {
75-
printer.write_str("{{reify-shim}}").unwrap();
74+
ty::InstanceDef::ReifyShim(_, reason) => {
75+
printer.write_str("{{reify-shim").unwrap();
76+
match reason {
77+
Some(ReifyReason::FnPtr) => printer.write_str("-fnptr").unwrap(),
78+
Some(ReifyReason::Vtable) => printer.write_str("-vtable").unwrap(),
79+
None => (),
80+
}
81+
printer.write_str("}}").unwrap();
7682
}
7783
// FIXME(async_closures): This shouldn't be needed when we fix
7884
// `Instance::ty`/`Instance::def_id`.

‎compiler/rustc_symbol_mangling/src/v0.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData};
88
use rustc_middle::ty::layout::IntegerExt;
99
use rustc_middle::ty::print::{Print, PrintError, Printer};
1010
use rustc_middle::ty::{
11-
self, EarlyBinder, FloatTy, Instance, IntTy, Ty, TyCtxt, TypeVisitable, TypeVisitableExt,
12-
UintTy,
11+
self, EarlyBinder, FloatTy, Instance, IntTy, ReifyReason, Ty, TyCtxt, TypeVisitable,
12+
TypeVisitableExt, UintTy,
1313
};
1414
use rustc_middle::ty::{GenericArg, GenericArgKind};
1515
use rustc_span::symbol::kw;
@@ -44,7 +44,9 @@ pub(super) fn mangle<'tcx>(
4444
let shim_kind = match instance.def {
4545
ty::InstanceDef::ThreadLocalShim(_) => Some("tls"),
4646
ty::InstanceDef::VTableShim(_) => Some("vtable"),
47-
ty::InstanceDef::ReifyShim(_) => Some("reify"),
47+
ty::InstanceDef::ReifyShim(_, None) => Some("reify"),
48+
ty::InstanceDef::ReifyShim(_, Some(ReifyReason::FnPtr)) => Some("reify-fnptr"),
49+
ty::InstanceDef::ReifyShim(_, Some(ReifyReason::Vtable)) => Some("reify-vtable"),
4850

4951
ty::InstanceDef::ConstructCoroutineInClosureShim { .. }
5052
| ty::InstanceDef::CoroutineKindShim { .. } => Some("fn_once"),

0 commit comments

Comments
 (0)