Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CFI: Restore typeid_for_instance default behavior #123487

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"}
Loading