Rollup merge of #123487 - rcvalle:rust-cfi-restore-typeid-for-instance, r=compiler-errors

CFI: Restore typeid_for_instance default behavior

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.

Author: GuillaumeGomez

compiler/rustc_codegen_llvm/src/declare.rs

@@ -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()
@@ -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);
             }

compiler/rustc_symbol_mangling/src/typeid.rs

@@ -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;
     }
 }
 
@@ -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.)

compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs

@@ -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)
         {

tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-method-secondary-typeid.rs

@@ -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"}