CFI: Fix SIGILL reached via trait objects

Fix #106547 by transforming the concrete self into a reference to a
trait object before emitting type metadata identifiers for trait
methods.

Author: rcvalle

compiler/rustc_codegen_llvm/src/callee.rs

@@ -94,11 +94,11 @@ pub fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>) ->
             // LLVM will prefix the name with `__imp_`. Ideally, we'd like the
             // existing logic below to set the Storage Class, but it has an
             // exemption for MinGW for backwards compatability.
-            let llfn = cx.declare_fn(&common::i686_decorated_name(&dllimport, common::is_mingw_gnu_toolchain(&tcx.sess.target), true), fn_abi);
+            let llfn = cx.declare_fn(&common::i686_decorated_name(&dllimport, common::is_mingw_gnu_toolchain(&tcx.sess.target), true), fn_abi, Some(instance));
             unsafe { llvm::LLVMSetDLLStorageClass(llfn, llvm::DLLStorageClass::DllImport); }
             llfn
         } else {
-            cx.declare_fn(sym, fn_abi)
+            cx.declare_fn(sym, fn_abi, Some(instance))
         };
         debug!("get_fn: not casting pointer!");
 

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

@@ -207,6 +207,7 @@ fn declare_unused_fn<'tcx>(cx: &CodegenCx<'_, 'tcx>, def_id: DefId) -> Instance<
             )),
             ty::List::empty(),
         ),
+        None,
     );
 
     llvm::set_linkage(llfn, llvm::Linkage::PrivateLinkage);

compiler/rustc_codegen_llvm/src/declare.rs

@@ -19,8 +19,11 @@ use crate::llvm::AttributePlace::Function;
 use crate::type_::Type;
 use crate::value::Value;
 use rustc_codegen_ssa::traits::TypeMembershipMethods;
-use rustc_middle::ty::Ty;
-use rustc_symbol_mangling::typeid::{kcfi_typeid_for_fnabi, typeid_for_fnabi, TypeIdOptions};
+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,
+    TypeIdOptions,
+};
 use smallvec::SmallVec;
 
 /// Declare a function.
@@ -116,7 +119,12 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
     ///
     /// If there’s a value with the same name already declared, the function will
     /// update the declaration and return existing Value instead.
-    pub fn declare_fn(&self, name: &str, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> &'ll Value {
+    pub fn declare_fn(
+        &self,
+        name: &str,
+        fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
+        instance: Option<Instance<'tcx>>,
+    ) -> &'ll Value {
         debug!("declare_rust_fn(name={:?}, fn_abi={:?})", name, fn_abi);
 
         // Function addresses in Rust are never significant, allowing functions to
@@ -132,18 +140,35 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
         fn_abi.apply_attrs_llfn(self, llfn);
 
         if self.tcx.sess.is_sanitizer_cfi_enabled() {
-            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);
+            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);
+            } 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);
+            }
         }
 
         if self.tcx.sess.is_sanitizer_kcfi_enabled() {
@@ -156,8 +181,13 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
                 options.insert(TypeIdOptions::NORMALIZE_INTEGERS);
             }
 
-            let kcfi_typeid = kcfi_typeid_for_fnabi(self.tcx, fn_abi, options);
-            self.set_kcfi_type_metadata(llfn, kcfi_typeid);
+            if let Some(instance) = instance {
+                let kcfi_typeid = kcfi_typeid_for_instance(self.tcx, &instance, options);
+                self.set_kcfi_type_metadata(llfn, kcfi_typeid);
+            } else {
+                let kcfi_typeid = kcfi_typeid_for_fnabi(self.tcx, fn_abi, options);
+                self.set_kcfi_type_metadata(llfn, kcfi_typeid);
+            }
         }
 
         llfn

compiler/rustc_codegen_llvm/src/intrinsic.rs

@@ -772,7 +772,7 @@ fn gen_fn<'ll, 'tcx>(
 ) -> (&'ll Type, &'ll Value) {
     let fn_abi = cx.fn_abi_of_fn_ptr(rust_fn_sig, ty::List::empty());
     let llty = fn_abi.llvm_type(cx);
-    let llfn = cx.declare_fn(name, fn_abi);
+    let llfn = cx.declare_fn(name, fn_abi, None);
     cx.set_frame_pointer_type(llfn);
     cx.apply_target_cpu_attr(llfn);
     // FIXME(eddyb) find a nicer way to do this.

compiler/rustc_codegen_llvm/src/mono_item.rs

@@ -51,7 +51,7 @@ impl<'tcx> PreDefineMethods<'tcx> for CodegenCx<'_, 'tcx> {
         assert!(!instance.substs.has_infer());
 
         let fn_abi = self.fn_abi_of_instance(instance, ty::List::empty());
-        let lldecl = self.declare_fn(symbol_name, fn_abi);
+        let lldecl = self.declare_fn(symbol_name, fn_abi, Some(instance));
         unsafe { llvm::LLVMRustSetLinkage(lldecl, base::linkage_to_llvm(linkage)) };
         let attrs = self.tcx.codegen_fn_attrs(instance.def_id());
         base::set_link_section(lldecl, attrs);

compiler/rustc_symbol_mangling/src/typeid.rs

@@ -4,7 +4,7 @@
 /// For more information about LLVM CFI and cross-language LLVM CFI support for the Rust compiler,
 /// see design document in the tracking issue #89653.
 use bitflags::bitflags;
-use rustc_middle::ty::{FnSig, Ty, TyCtxt};
+use rustc_middle::ty::{FnSig, Instance, Ty, TyCtxt};
 use rustc_target::abi::call::FnAbi;
 use std::hash::Hasher;
 use twox_hash::XxHash64;
@@ -38,6 +38,15 @@ pub fn typeid_for_fnsig<'tcx>(
     typeid_itanium_cxx_abi::typeid_for_fnsig(tcx, fn_sig, options)
 }
 
+/// Returns a type metadata identifier for the specified Instance.
+pub fn typeid_for_instance<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    instance: &Instance<'tcx>,
+    options: TypeIdOptions,
+) -> String {
+    typeid_itanium_cxx_abi::typeid_for_instance(tcx, instance, options)
+}
+
 /// Returns a KCFI type metadata identifier for the specified FnAbi.
 pub fn kcfi_typeid_for_fnabi<'tcx>(
     tcx: TyCtxt<'tcx>,
@@ -63,3 +72,16 @@ pub fn kcfi_typeid_for_fnsig<'tcx>(
     hash.write(typeid_itanium_cxx_abi::typeid_for_fnsig(tcx, fn_sig, options).as_bytes());
     hash.finish() as u32
 }
+
+/// Returns a KCFI type metadata identifier for the specified Instance.
+pub fn kcfi_typeid_for_instance<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    instance: &Instance<'tcx>,
+    options: TypeIdOptions,
+) -> u32 {
+    // 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.)
+    let mut hash: XxHash64 = Default::default();
+    hash.write(typeid_itanium_cxx_abi::typeid_for_instance(tcx, instance, options).as_bytes());
+    hash.finish() as u32
+}

compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs

@@ -14,8 +14,8 @@ use rustc_errors::DiagnosticMessage;
 use rustc_hir as hir;
 use rustc_middle::ty::subst::{GenericArg, GenericArgKind, SubstsRef};
 use rustc_middle::ty::{
-    self, Const, ExistentialPredicate, FloatTy, FnSig, IntTy, List, Region, RegionKind, TermKind,
-    Ty, TyCtxt, UintTy,
+    self, Const, ExistentialPredicate, FloatTy, FnSig, Instance, IntTy, List, Region, RegionKind,
+    TermKind, Ty, TyCtxt, UintTy,
 };
 use rustc_span::def_id::DefId;
 use rustc_span::sym;
@@ -1010,3 +1010,56 @@ pub fn typeid_for_fnsig<'tcx>(
 
     typeid
 }
+
+/// Returns a type metadata identifier for the specified Instance using the Itanium C++ ABI with
+/// vendor extended type qualifiers and types for Rust types that are not used at the FFI boundary.
+pub fn typeid_for_instance<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    instance: &Instance<'tcx>,
+    options: TypeIdOptions,
+) -> String {
+    let fn_abi = tcx
+        .fn_abi_of_instance(tcx.param_env(instance.def_id()).and((*instance, ty::List::empty())))
+        .unwrap_or_else(|instance| {
+            bug!("typeid_for_instance: couldn't get fn_abi of instance {:?}", instance)
+        });
+
+    // If this instance is a method and self is a reference, get the impl it belongs to
+    let impl_def_id = tcx.impl_of_method(instance.def_id());
+    if impl_def_id.is_some() && !fn_abi.args.is_empty() && fn_abi.args[0].layout.ty.is_ref() {
+        // If this impl is not an inherent impl, get the trait it implements
+        if let Some(trait_ref) = tcx.impl_trait_ref(impl_def_id.unwrap()) {
+            // Transform the concrete self into a reference to a trait object
+            let existential_predicate = trait_ref.map_bound(|trait_ref| {
+                ty::ExistentialPredicate::Trait(ty::ExistentialTraitRef::erase_self_ty(
+                    tcx, trait_ref,
+                ))
+            });
+            let existential_predicates = tcx.mk_poly_existential_predicates(&[ty::Binder::dummy(
+                existential_predicate.skip_binder(),
+            )]);
+            // Is the concrete self mutable?
+            let self_ty = if fn_abi.args[0].layout.ty.is_mutable_ptr() {
+                tcx.mk_mut_ref(
+                    tcx.lifetimes.re_erased,
+                    tcx.mk_dynamic(existential_predicates, tcx.lifetimes.re_erased, ty::Dyn),
+                )
+            } else {
+                tcx.mk_imm_ref(
+                    tcx.lifetimes.re_erased,
+                    tcx.mk_dynamic(existential_predicates, tcx.lifetimes.re_erased, ty::Dyn),
+                )
+            };
+
+            // Replace the concrete self in an fn_abi clone by the reference to a trait object
+            let mut fn_abi = fn_abi.clone();
+            // HACK(rcvalle): It is okay to not replace or update the entire ArgAbi here because the
+            //   other fields are never used.
+            fn_abi.args[0].layout.ty = self_ty;
+
+            return typeid_for_fnabi(tcx, &fn_abi, options);
+        }
+    }
+
+    typeid_for_fnabi(tcx, &fn_abi, options)
+}

compiler/rustc_target/src/abi/call/mod.rs

@@ -28,7 +28,7 @@ mod x86;
 mod x86_64;
 mod x86_win64;
 
-#[derive(PartialEq, Eq, Hash, Debug, HashStable_Generic)]
+#[derive(Clone, PartialEq, Eq, Hash, Debug, HashStable_Generic)]
 pub enum PassMode {
     /// Ignore the argument.
     ///
@@ -211,7 +211,7 @@ impl Uniform {
     }
 }
 
-#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, HashStable_Generic)]
+#[derive(Clone, PartialEq, Eq, Hash, Debug, HashStable_Generic)]
 pub struct CastTarget {
     pub prefix: [Option<Reg>; 8],
     pub rest: Uniform,
@@ -458,7 +458,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
 
 /// Information about how to pass an argument to,
 /// or return a value from, a function, under some ABI.
-#[derive(PartialEq, Eq, Hash, Debug, HashStable_Generic)]
+#[derive(Clone, PartialEq, Eq, Hash, Debug, HashStable_Generic)]
 pub struct ArgAbi<'a, Ty> {
     pub layout: TyAndLayout<'a, Ty>,
     pub mode: PassMode,
@@ -605,7 +605,7 @@ pub enum Conv {
 ///
 /// I will do my best to describe this structure, but these
 /// comments are reverse-engineered and may be inaccurate. -NDM
-#[derive(PartialEq, Eq, Hash, Debug, HashStable_Generic)]
+#[derive(Clone, PartialEq, Eq, Hash, Debug, HashStable_Generic)]
 pub struct FnAbi<'a, Ty> {
     /// The LLVM types of each argument.
     pub args: Box<[ArgAbi<'a, Ty>]>,

tests/codegen/sanitizer-cfi-emit-type-metadata-trait-objects.rs

@@ -0,0 +1,44 @@
+// Verifies that type metadata identifiers for trait objects are emitted correctly.
+//
+// needs-sanitizer-cfi
+// compile-flags: -Clto -Cno-prepopulate-passes -Ctarget-feature=-crt-static -Zsanitizer=cfi
+
+#![crate_type="lib"]
+
+trait Trait1 {
+    fn foo(&self);
+}
+
+struct Type1;
+
+impl Trait1 for Type1 {
+    fn foo(&self) {
+    }
+}
+
+pub fn foo() {
+    let a = Type1;
+    a.foo();
+    // CHECK-LABEL: define{{.*}}foo{{.*}}!type !{{[0-9]+}}
+    // CHECK:       call <sanitizer_cfi_emit_type_metadata_trait_objects::Type1 as sanitizer_cfi_emit_type_metadata_trait_objects::Trait1>::foo
+}
+
+pub fn bar() {
+    let a = Type1;
+    let b = &a as &dyn Trait1;
+    b.foo();
+    // CHECK-LABEL: define{{.*}}bar{{.*}}!type !{{[0-9]+}}
+    // CHECK:       call i1 @llvm.type.test({{i8\*|ptr}} {{%f|%0|%1}}, metadata !"[[TYPE1:[[:print:]]+]]")
+}
+
+pub fn baz() {
+    let a = Type1;
+    let b = &a as &dyn Trait1;
+    a.foo();
+    b.foo();
+    // CHECK-LABEL: define{{.*}}baz{{.*}}!type !{{[0-9]+}}
+    // CHECK:       call <sanitizer_cfi_emit_type_metadata_trait_objects::Type1 as sanitizer_cfi_emit_type_metadata_trait_objects::Trait1>::foo
+    // CHECK:       call i1 @llvm.type.test({{i8\*|ptr}} {{%f|%0|%1}}, metadata !"[[TYPE1:[[:print:]]+]]")
+}
+
+// CHECK: !{{[0-9]+}} = !{i64 0, !"[[TYPE1]]"}

tests/codegen/sanitizer-kcfi-emit-type-metadata-trait-objects.rs

@@ -0,0 +1,69 @@
+// Verifies that type metadata identifiers for trait objects are emitted correctly.
+//
+// revisions: aarch64 x86_64
+// [aarch64] compile-flags: --target aarch64-unknown-none
+// [aarch64] needs-llvm-components: aarch64
+// [x86_64] compile-flags: --target x86_64-unknown-none
+// [x86_64] needs-llvm-components:
+// compile-flags: -Cno-prepopulate-passes -Zsanitizer=kcfi -Copt-level=0
+
+#![crate_type="lib"]
+#![feature(arbitrary_self_types, no_core, lang_items)]
+#![no_core]
+
+#[lang="sized"]
+trait Sized { }
+#[lang="copy"]
+trait Copy { }
+#[lang="receiver"]
+trait Receiver { }
+#[lang="dispatch_from_dyn"]
+trait DispatchFromDyn<T> { }
+impl<'a, T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<&'a U> for &'a T {}
+#[lang = "unsize"]
+trait Unsize<T: ?Sized> { }
+#[lang = "coerce_unsized"]
+pub trait CoerceUnsized<T: ?Sized> { }
+impl<'a, 'b: 'a, T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<&'a U> for &'b T {}
+#[lang="freeze"]
+trait Freeze { }
+#[lang="drop_in_place"]
+fn drop_in_place_fn<T>() { }
+
+trait Trait1 {
+    fn foo(&self);
+}
+
+struct Type1;
+
+impl Trait1 for Type1 {
+    fn foo(&self) {
+    }
+}
+
+pub fn foo() {
+    let a = Type1;
+    a.foo();
+    // CHECK-LABEL: define{{.*}}foo{{.*}}!{{<unknown kind #36>|kcfi_type}} !{{[0-9]+}}
+    // CHECK:       call <sanitizer_kcfi_emit_type_metadata_trait_objects::Type1 as sanitizer_kcfi_emit_type_metadata_trait_objects::Trait1>::foo
+}
+
+pub fn bar() {
+    let a = Type1;
+    let b = &a as &dyn Trait1;
+    b.foo();
+    // CHECK-LABEL: define{{.*}}bar{{.*}}!{{<unknown kind #36>|kcfi_type}} !{{[0-9]+}}
+    // CHECK:       call void %0({{\{\}\*|ptr}} align 1 {{%b\.0|%_1}}){{.*}}[ "kcfi"(i32 [[TYPE1:[[:print:]]+]]) ]
+}
+
+pub fn baz() {
+    let a = Type1;
+    let b = &a as &dyn Trait1;
+    a.foo();
+    b.foo();
+    // CHECK-LABEL: define{{.*}}baz{{.*}}!{{<unknown kind #36>|kcfi_type}} !{{[0-9]+}}
+    // CHECK:       call <sanitizer_kcfi_emit_type_metadata_trait_objects::Type1 as sanitizer_kcfi_emit_type_metadata_trait_objects::Trait1>::foo
+    // CHECK:       call void %0({{\{\}\*|ptr}} align 1 {{%b\.0|%_1}}){{.*}}[ "kcfi"(i32 [[TYPE1:[[:print:]]+]]) ]
+}
+
+// CHECK: !{{[0-9]+}} = !{i32 [[TYPE1]]}