CFI: Fix methods as function pointer cast

Fix casting between methods and function pointers by assigning a
secondary type id to methods with their concrete self so they can be
used as function pointers.

Author: rcvalle

compiler/rustc_codegen_llvm/src/declare.rs

@@ -18,7 +18,9 @@ use crate::llvm;
 use crate::llvm::AttributePlace::Function;
 use crate::type_::Type;
 use crate::value::Value;
+use itertools::Itertools;
 use rustc_codegen_ssa::traits::TypeMembershipMethods;
+use rustc_data_structures::fx::FxIndexSet;
 use rustc_middle::ty::{Instance, Ty};
 use rustc_symbol_mangling::typeid::{
     kcfi_typeid_for_fnabi, kcfi_typeid_for_instance, typeid_for_fnabi, typeid_for_instance,
@@ -141,33 +143,48 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
 
         if self.tcx.sess.is_sanitizer_cfi_enabled() {
             if let Some(instance) = instance {
-                let typeid = typeid_for_instance(self.tcx, instance, TypeIdOptions::empty());
-                self.set_type_metadata(llfn, typeid);
-                let typeid =
-                    typeid_for_instance(self.tcx, instance, TypeIdOptions::GENERALIZE_POINTERS);
-                self.add_type_metadata(llfn, typeid);
-                let typeid =
-                    typeid_for_instance(self.tcx, instance, TypeIdOptions::NORMALIZE_INTEGERS);
-                self.add_type_metadata(llfn, typeid);
-                let typeid = typeid_for_instance(
-                    self.tcx,
-                    instance,
-                    TypeIdOptions::GENERALIZE_POINTERS | TypeIdOptions::NORMALIZE_INTEGERS,
-                );
-                self.add_type_metadata(llfn, typeid);
+                let mut typeids = FxIndexSet::default();
+                for options in
+                    [TypeIdOptions::GENERALIZE_POINTERS, TypeIdOptions::NORMALIZE_INTEGERS]
+                        .into_iter()
+                        .powerset()
+                        .map(TypeIdOptions::from_iter)
+                {
+                    let typeid = typeid_for_instance(self.tcx, instance, options);
+                    typeids.insert(typeid.clone());
+                    self.add_type_metadata(llfn, typeid);
+                }
+                // Attach a secondary type id to methods with their concrete self so they can be
+                // used as function pointers. The secondary type ids tend to repeat a lot (e.g.,
+                // they repeat for all non methods). Always attaching them makes it difficult to
+                // maintain tests and debug by looking at the .ll files and correlating typeid
+                // attachments with the actual type metadata so add secondary type ids only when
+                // they are unique. This also makes it easy to identify methods by when they have a
+                // secondary type id.
+                for options in [
+                    TypeIdOptions::NO_SELF_TYPE_ERASURE,
+                    TypeIdOptions::GENERALIZE_POINTERS,
+                    TypeIdOptions::NORMALIZE_INTEGERS,
+                ]
+                .into_iter()
+                .powerset()
+                .map(TypeIdOptions::from_iter)
+                {
+                    let typeid = typeid_for_instance(self.tcx, instance, options);
+                    if typeids.insert(typeid.clone()) {
+                        self.add_type_metadata(llfn, typeid);
+                    }
+                }
             } else {
-                let typeid = typeid_for_fnabi(self.tcx, fn_abi, TypeIdOptions::empty());
-                self.set_type_metadata(llfn, typeid);
-                let typeid = typeid_for_fnabi(self.tcx, fn_abi, TypeIdOptions::GENERALIZE_POINTERS);
-                self.add_type_metadata(llfn, typeid);
-                let typeid = typeid_for_fnabi(self.tcx, fn_abi, TypeIdOptions::NORMALIZE_INTEGERS);
-                self.add_type_metadata(llfn, typeid);
-                let typeid = typeid_for_fnabi(
-                    self.tcx,
-                    fn_abi,
-                    TypeIdOptions::GENERALIZE_POINTERS | TypeIdOptions::NORMALIZE_INTEGERS,
-                );
-                self.add_type_metadata(llfn, typeid);
+                for options in
+                    [TypeIdOptions::GENERALIZE_POINTERS, TypeIdOptions::NORMALIZE_INTEGERS]
+                        .into_iter()
+                        .powerset()
+                        .map(TypeIdOptions::from_iter)
+                {
+                    let typeid = typeid_for_fnabi(self.tcx, fn_abi, options);
+                    self.add_type_metadata(llfn, typeid);
+                }
             }
         }
 

compiler/rustc_symbol_mangling/src/typeid.rs

@@ -13,9 +13,20 @@ bitflags! {
     /// Options for typeid_for_fnabi.
     #[derive(Clone, Copy, Debug)]
     pub struct TypeIdOptions: u32 {
+        /// Generalizes pointers for compatibility with Clang
+        /// `-fsanitize-cfi-icall-generalize-pointers` option for cross-language LLVM CFI and KCFI
+        /// support.
         const GENERALIZE_POINTERS = 1;
+        /// Generalizes repr(C) user-defined type for extern function types with the "C" calling
+        /// convention (or extern types) for cross-language LLVM CFI and  KCFI support.
         const GENERALIZE_REPR_C = 2;
+        /// Normalizes integers for compatibility with Clang
+        /// `-fsanitize-cfi-icall-experimental-normalize-integers` option for cross-language LLVM
+        /// CFI and  KCFI support.
         const NORMALIZE_INTEGERS = 4;
+        /// 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.
+        const NO_SELF_TYPE_ERASURE = 8;
     }
 }
 

compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs

@@ -1116,7 +1116,8 @@ pub fn typeid_for_instance<'tcx>(
         instance.args = strip_receiver_auto(tcx, instance.args)
     }
 
-    if let Some(impl_id) = tcx.impl_of_method(instance.def_id())
+    if !options.contains(EncodeTyOptions::NO_SELF_TYPE_ERASURE)
+        && let Some(impl_id) = tcx.impl_of_method(instance.def_id())
         && let Some(trait_ref) = tcx.impl_trait_ref(impl_id)
     {
         let impl_method = tcx.associated_item(instance.def_id());

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

@@ -0,0 +1,22 @@
+// Verifies that a secondary type metadata identifier is assigned to methods with their concrete
+// self so they can be used as function pointers.
+//
+//@ needs-sanitizer-cfi
+//@ compile-flags: -Clto -Cno-prepopulate-passes -Copt-level=0 -Zsanitizer=cfi -Ctarget-feature=-crt-static
+
+#![crate_type="lib"]
+
+trait Trait1 {
+    fn foo(&self);
+}
+
+struct Type1;
+
+impl Trait1 for Type1 {
+    fn foo(&self) {}
+    // CHECK: define{{.*}}3foo{{.*}}!type ![[TYPE1:[0-9]+]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} !type ![[TYPE2:[0-9]+]] !type !{{[0-9]+}}
+}
+
+
+// CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvu3refIu3dynIu{{[0-9]+}}NtC{{[[:print:]]+}}_{{[[:print:]]+}}6Trait1u6regionEEE"}
+// CHECK: ![[TYPE2]] = !{i64 0, !"_ZTSFvu3refIu{{[0-9]+}}NtC{{[[:print:]]+}}_{{[[:print:]]+}}5Type1EE"}

tests/ui/sanitizer/cfi-method-fn-ptr-cast.rs

@@ -0,0 +1,23 @@
+// Verifies that casting a method to a function pointer works.
+//
+// FIXME(#122848): Remove only-linux when fixed.
+//@ only-linux
+//@ needs-sanitizer-cfi
+//@ compile-flags: -Clto -Copt-level=0 -Cprefer-dynamic=off -Ctarget-feature=-crt-static -Zsanitizer=cfi
+//@ run-pass
+
+trait Trait1 {
+    fn foo(&self);
+}
+
+struct Type1;
+
+impl Trait1 for Type1 {
+    fn foo(&self) {}
+}
+
+fn main() {
+    let type1 = Type1 {};
+    let f = <Type1 as Trait1>::foo;
+    f(&type1);
+}