Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit b04e6b6

Browse files
borsbors
committed
Auto merge of rust-lang#123052 - maurer:addr-taken, r=<try>
CFI: Support function pointers for trait methods Adds support for both CFI and KCFI for function pointers to trait methods by attaching both concrete and abstract types to functions. KCFI does this through generation of a `ReifyShim` on any function pointer for a method that could go into a vtable, and keeping this separate from `ReifyShim`s that are *intended* for vtable us by setting a `ReifyReason` on them. CFI does this by setting both the concrete and abstract type on every instance. This should land after rust-lang#123024 or a similar PR, as it diverges the implementation of CFI vs KCFI. r? `@compiler-errors`
2 parents 35936c4 + f64da91 commit b04e6b6

File tree

19 files changed

+112
-46
lines changed

19 files changed

+112
-46
lines changed

‎compiler/rustc_codegen_llvm/src/declare.rs‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use crate::llvm;
1818
use crate::llvm::AttributePlace::Function;
1919
use crate::type_::Type;
2020
use crate::value::Value;
21+
use itertools::Itertools;
2122
use rustc_codegen_ssa::traits::TypeMembershipMethods;
2223
use rustc_middle::ty::{Instance, Ty};
2324
use rustc_symbol_mangling::typeid::{
@@ -143,18 +144,18 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
143144
if let Some(instance) = instance {
144145
let typeid = typeid_for_instance(self.tcx, &instance, TypeIdOptions::empty());
145146
self.set_type_metadata(llfn, typeid);
146-
let typeid =
147-
typeid_for_instance(self.tcx, &instance, TypeIdOptions::GENERALIZE_POINTERS);
148-
self.add_type_metadata(llfn, typeid);
149-
let typeid =
150-
typeid_for_instance(self.tcx, &instance, TypeIdOptions::NORMALIZE_INTEGERS);
151-
self.add_type_metadata(llfn, typeid);
152-
let typeid = typeid_for_instance(
153-
self.tcx,
154-
&instance,
155-
TypeIdOptions::GENERALIZE_POINTERS | TypeIdOptions::NORMALIZE_INTEGERS,
156-
);
157-
self.add_type_metadata(llfn, typeid);
147+
for options in [
148+
TypeIdOptions::GENERALIZE_POINTERS,
149+
TypeIdOptions::NORMALIZE_INTEGERS,
150+
TypeIdOptions::CONCRETE,
151+
]
152+
.into_iter()
153+
.powerset()
154+
.map(TypeIdOptions::from_iter)
155+
{
156+
let typeid = typeid_for_instance(self.tcx, &instance, options);
157+
self.add_type_metadata(llfn, typeid);
158+
}
158159
} else {
159160
let typeid = typeid_for_fnabi(self.tcx, fn_abi, TypeIdOptions::empty());
160161
self.set_type_metadata(llfn, typeid);

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

Lines changed: 1 addition & 1 deletion
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‎

Lines changed: 33 additions & 8 deletions
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,24 @@ 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);
504+
}
505+
// FIXME(maurer) only shim it if it is a vtable-safe function
506+
_ if tcx.sess.is_sanitizer_kcfi_enabled()
507+
&& tcx.associated_item(def_id).trait_item_def_id.is_some() =>
508+
{
509+
// If this function could also go in a vtable, we need to `ReifyShim` it with
510+
// KCFI because it can only attach one type per function.
511+
resolved.def = InstanceDef::ReifyShim(resolved.def_id(), reason)
488512
}
489513
_ => {}
490514
}
@@ -508,6 +532,7 @@ impl<'tcx> Instance<'tcx> {
508532
debug!(" => associated item with unsizeable self: Self");
509533
Some(Instance { def: InstanceDef::VTableShim(def_id), args })
510534
} else {
535+
let reason = tcx.sess.is_sanitizer_kcfi_enabled().then_some(ReifyReason::Vtable);
511536
Instance::resolve(tcx, param_env, def_id, args).ok().flatten().map(|mut resolved| {
512537
match resolved.def {
513538
InstanceDef::Item(def) => {
@@ -544,18 +569,18 @@ impl<'tcx> Instance<'tcx> {
544569
// Create a shim for the `FnOnce/FnMut/Fn` method we are calling
545570
// - unlike functions, invoking a closure always goes through a
546571
// trait.
547-
resolved = Instance { def: InstanceDef::ReifyShim(def_id), args };
572+
resolved = Instance { def: InstanceDef::ReifyShim(def_id, reason), args };
548573
} else {
549574
debug!(
550575
" => vtable fn pointer created for function with #[track_caller]: {:?}", def
551576
);
552-
resolved.def = InstanceDef::ReifyShim(def);
577+
resolved.def = InstanceDef::ReifyShim(def, reason);
553578
}
554579
}
555580
}
556581
InstanceDef::Virtual(def_id, _) => {
557582
debug!(" => vtable fn pointer created for virtual call");
558-
resolved.def = InstanceDef::ReifyShim(def_id);
583+
resolved.def = InstanceDef::ReifyShim(def_id, reason)
559584
}
560585
_ => {}
561586
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ pub use self::context::{
8989
tls, CtxtInterners, DeducedParamAttrs, Feed, FreeRegionInfo, GlobalCtxt, Lift, TyCtxt,
9090
TyCtxtFeed,
9191
};
92-
pub use self::instance::{Instance, InstanceDef, ShortInstance, UnusedGenericParams};
92+
pub use self::instance::{Instance, InstanceDef, ReifyReason, ShortInstance, UnusedGenericParams};
9393
pub use self::list::List;
9494
pub use self::parameterized::ParameterizedOverTcx;
9595
pub use self::predicate::{

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,7 @@ TrivialTypeTraversalAndLiftImpls! {
448448
crate::ty::ClosureKind,
449449
crate::ty::ParamConst,
450450
crate::ty::ParamTy,
451+
crate::ty::instance::ReifyReason,
451452
interpret::AllocId,
452453
interpret::CtfeProvenance,
453454
interpret::Scalar,

‎compiler/rustc_mir_transform/src/inline.rs‎

Lines changed: 1 addition & 1 deletion
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‎

Lines changed: 1 addition & 1 deletion
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‎

Lines changed: 1 addition & 1 deletion
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‎

Lines changed: 9 additions & 3 deletions
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/typeid.rs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
/// For more information about LLVM CFI and cross-language LLVM CFI support for the Rust compiler,
55
/// see design document in the tracking issue #89653.
66
use bitflags::bitflags;
7-
use rustc_middle::ty::{Instance, Ty, TyCtxt};
7+
use rustc_middle::ty::{Instance, InstanceDef, ReifyReason, Ty, TyCtxt};
88
use rustc_target::abi::call::FnAbi;
99
use std::hash::Hasher;
1010
use twox_hash::XxHash64;
@@ -16,6 +16,8 @@ bitflags! {
1616
const GENERALIZE_POINTERS = 1;
1717
const GENERALIZE_REPR_C = 2;
1818
const NORMALIZE_INTEGERS = 4;
19+
// Only relevant for `typeid_for_instance`
20+
const CONCRETE = 8;
1921
}
2022
}
2123

@@ -56,8 +58,13 @@ pub fn kcfi_typeid_for_fnabi<'tcx>(
5658
pub fn kcfi_typeid_for_instance<'tcx>(
5759
tcx: TyCtxt<'tcx>,
5860
instance: &Instance<'tcx>,
59-
options: TypeIdOptions,
61+
mut options: TypeIdOptions,
6062
) -> u32 {
63+
// If we receive a `ReifyShim` intended to produce a function pointer, we need to remain
64+
// concrete - abstraction is for vtables.
65+
if matches!(instance.def, InstanceDef::ReifyShim(_, Some(ReifyReason::FnPtr))) {
66+
options |= TypeIdOptions::CONCRETE
67+
}
6168
// A KCFI type metadata identifier is a 32-bit constant produced by taking the lower half of the
6269
// xxHash64 of the type metadata identifier. (See llvm/llvm-project@cff5bef.)
6370
let mut hash: XxHash64 = Default::default();

0 commit comments

Comments
 (0)