Skip to content

Commit

Permalink
CFI: Restore typeid_for_instance default behavior
Browse files Browse the repository at this point in the history
Restore typeid_for_instance default behavior of performing self type
erasure, since it's the most common case and what it does most of the
time. Using concrete self (or not performing self type erasure) is for
assigning a secondary type id, and secondary type ids are only assigned
when they're unique and to methods, and also are only tested for when
methods are used as function pointers.
  • Loading branch information
rcvalle committed Apr 5, 2024
1 parent 9cbaa01 commit 2498a9d
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 13 deletions.
6 changes: 2 additions & 4 deletions compiler/rustc_codegen_llvm/src/declare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
for options in [
TypeIdOptions::GENERALIZE_POINTERS,
TypeIdOptions::NORMALIZE_INTEGERS,
TypeIdOptions::ERASE_SELF_TYPE,
TypeIdOptions::USE_CONCRETE_SELF,
]
.into_iter()
.powerset()
Expand All @@ -173,9 +173,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {

if self.tcx.sess.is_sanitizer_kcfi_enabled() {
// LLVM KCFI does not support multiple !kcfi_type attachments
// Default to erasing the self type. If we need the concrete type, there will be a
// hint in the instance.
let mut options = TypeIdOptions::ERASE_SELF_TYPE;
let mut options = TypeIdOptions::empty();
if self.tcx.sess.is_sanitizer_cfi_generalize_pointers_enabled() {
options.insert(TypeIdOptions::GENERALIZE_POINTERS);
}
Expand Down
30 changes: 24 additions & 6 deletions compiler/rustc_symbol_mangling/src/typeid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,14 @@ bitflags! {
/// `-fsanitize-cfi-icall-experimental-normalize-integers` option for cross-language LLVM
/// CFI and KCFI support.
const NORMALIZE_INTEGERS = 4;
/// Generalize the instance by erasing the concrete `Self` type where possible.
/// Only has an effect on `{kcfi_,}typeid_for_instance`.
const ERASE_SELF_TYPE = 8;
/// Do not perform self type erasure for attaching a secondary type id to methods with their
/// concrete self so they can be used as function pointers.
///
/// (This applies to typeid_for_instance only and should be used to attach a secondary type
/// id to methods during their declaration/definition so they match the type ids returned by
/// either typeid_for_instance or typeid_for_fnabi at call sites during code generation for
/// type membership tests when methods are used as function pointers.)
const USE_CONCRETE_SELF = 8;
}
}

Expand Down Expand Up @@ -69,10 +74,23 @@ pub fn kcfi_typeid_for_instance<'tcx>(
instance: Instance<'tcx>,
mut options: TypeIdOptions,
) -> u32 {
// If we receive a `ReifyShim` intended to produce a function pointer, we need to remain
// concrete - abstraction is for vtables.
// KCFI support for Rust shares most of its implementation with the CFI support, with some key
// differences:
//
// 1. KCFI performs type tests differently and are implemented as different LLVM passes than CFI
// to not require LTO.
// 2. KCFI has the limitation that a function or method may have one type id assigned only.
//
// Because of the limitation listed above (2), the current KCFI implementation (not CFI) does
// reifying of types (i.e., adds shims/trampolines for indirect calls in these cases) for:
//
// * Supporting casting between function items, closures, and Fn trait objects.
// * Supporting methods being cast as function pointers.
//
// This was implemented for KCFI support in #123106 and #123052 (which introduced the
// ReifyReason). The tracking issue for KCFI support for Rust is #123479.
if matches!(instance.def, InstanceDef::ReifyShim(_, Some(ReifyReason::FnPtr))) {
options.remove(TypeIdOptions::ERASE_SELF_TYPE);
options.insert(TypeIdOptions::USE_CONCRETE_SELF);
}
// 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.)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1098,7 +1098,7 @@ pub fn typeid_for_instance<'tcx>(
instance.args = tcx.mk_args_trait(invoke_ty, trait_ref.args.into_iter().skip(1));
}

if options.contains(EncodeTyOptions::ERASE_SELF_TYPE) {
if !options.contains(EncodeTyOptions::USE_CONCRETE_SELF) {
if let Some(impl_id) = tcx.impl_of_method(instance.def_id())
&& let Some(trait_ref) = tcx.impl_trait_ref(impl_id)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ impl Trait1 for Type1 {
}


// CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NtC{{[[:print:]]+}}_{{[[:print:]]+}}5Type1EE"}
// CHECK: ![[TYPE2]] = !{i64 0, !"_ZTSFvu3refIu3dynIu{{[0-9]+}}NtC{{[[:print:]]+}}_{{[[:print:]]+}}6Trait1u6regionEEE"}
// CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvu3refIu3dynIu{{[0-9]+}}NtC{{[[:print:]]+}}_{{[[:print:]]+}}6Trait1u6regionEEE"}
// CHECK: ![[TYPE2]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NtC{{[[:print:]]+}}_{{[[:print:]]+}}5Type1EE"}

0 comments on commit 2498a9d

Please sign in to comment.