Revert "CFI: Fix SIGILL reached via trait objects"

We no longer need the special instance resolution this added, and it can
be broken in edge cases (specifically with a FnPtr shim, which will
cause the calculation of fn_abi to fail).

* We keep the Clone impls it added, because they have since become used
  by other portions of the compiler.
* Add a test for the address-taken calls that this previously broke.

This reverts commit 7c7b22e.

Author: maurer

compiler/rustc_codegen_llvm/src/callee.rs

@@ -67,14 +67,13 @@ pub fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>) ->
                     true,
                 ),
                 fn_abi,
-                Some(instance),
             );
             unsafe {
                 llvm::LLVMSetDLLStorageClass(llfn, llvm::DLLStorageClass::DllImport);
             }
             llfn
         } else {
-            cx.declare_fn(sym, fn_abi, Some(instance))
+            cx.declare_fn(sym, fn_abi)
         };
         debug!("get_fn: not casting pointer!");
 

compiler/rustc_codegen_llvm/src/declare.rs

@@ -19,11 +19,8 @@ use crate::llvm::AttributePlace::Function;
 use crate::type_::Type;
 use crate::value::Value;
 use rustc_codegen_ssa::traits::TypeMembershipMethods;
-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 rustc_middle::ty::Ty;
+use rustc_symbol_mangling::typeid::{kcfi_typeid_for_fnabi, typeid_for_fnabi, TypeIdOptions};
 use smallvec::SmallVec;
 
 /// Declare a function.
@@ -119,12 +116,7 @@ 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>>,
-        instance: Option<Instance<'tcx>>,
-    ) -> &'ll Value {
+    pub fn declare_fn(&self, name: &str, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> &'ll Value {
         debug!("declare_rust_fn(name={:?}, fn_abi={:?})", name, fn_abi);
 
         // Function addresses in Rust are never significant, allowing functions to
@@ -140,35 +132,18 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
         fn_abi.apply_attrs_llfn(self, llfn);
 
         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);
-            } 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);
-            }
+            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() {
@@ -181,13 +156,8 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
                 options.insert(TypeIdOptions::NORMALIZE_INTEGERS);
             }
 
-            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);
-            }
+            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

@@ -939,7 +939,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, None);
+    let llfn = cx.declare_fn(name, fn_abi);
     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.args.has_infer());
 
         let fn_abi = self.fn_abi_of_instance(instance, ty::List::empty());
-        let lldecl = self.declare_fn(symbol_name, fn_abi, Some(instance));
+        let lldecl = self.declare_fn(symbol_name, fn_abi);
         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::{Instance, Ty, TyCtxt};
+use rustc_middle::ty::{Ty, TyCtxt};
 use rustc_target::abi::call::FnAbi;
 use std::hash::Hasher;
 use twox_hash::XxHash64;
@@ -30,15 +30,6 @@ pub fn typeid_for_fnabi<'tcx>(
     typeid_itanium_cxx_abi::typeid_for_fnabi(tcx, fn_abi, 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>,
@@ -51,16 +42,3 @@ pub fn kcfi_typeid_for_fnabi<'tcx>(
     hash.write(typeid_itanium_cxx_abi::typeid_for_fnabi(tcx, fn_abi, 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

@@ -12,8 +12,8 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_hir as hir;
 use rustc_middle::ty::layout::IntegerExt;
 use rustc_middle::ty::{
-    self, Const, ExistentialPredicate, FloatTy, FnSig, Instance, IntTy, List, Region, RegionKind,
-    TermKind, Ty, TyCtxt, UintTy,
+    self, Const, ExistentialPredicate, FloatTy, FnSig, IntTy, List, Region, RegionKind, TermKind,
+    Ty, TyCtxt, UintTy,
 };
 use rustc_middle::ty::{GenericArg, GenericArgKind, GenericArgsRef};
 use rustc_span::def_id::DefId;
@@ -1074,58 +1074,3 @@ pub fn typeid_for_fnabi<'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() {
-                Ty::new_mut_ref(
-                    tcx,
-                    tcx.lifetimes.re_erased,
-                    Ty::new_dynamic(tcx, existential_predicates, tcx.lifetimes.re_erased, ty::Dyn),
-                )
-            } else {
-                Ty::new_imm_ref(
-                    tcx,
-                    tcx.lifetimes.re_erased,
-                    Ty::new_dynamic(tcx, 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

@@ -268,7 +268,7 @@ impl Uniform {
 /// `rest.unit` register type gets repeated often enough to cover `rest.size`. This describes the
 /// actual type used for the call; the Rust type of the argument is then transmuted to this ABI type
 /// (and all data in the padding between the registers is dropped).
-#[derive(Clone, PartialEq, Eq, Hash, Debug, HashStable_Generic)]
+#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, HashStable_Generic)]
 pub struct CastTarget {
     pub prefix: [Option<Reg>; 8],
     pub rest: Uniform,

tests/ui/sanitizer/cfi-address-taken.rs

@@ -0,0 +1,30 @@
+// Check that the type of trait methods on a concrete type are not abstracted
+
+//@ needs-sanitizer-cfi
+//@ compile-flags: --crate-type=bin -Cprefer-dynamic=off -Clto -Zsanitizer=cfi
+//@ compile-flags: -C codegen-units=1 -C opt-level=0
+//@ run-pass
+
+trait Foo {
+    fn foo(&self);
+}
+
+struct S;
+
+impl Foo for S {
+    fn foo(&self) {}
+}
+
+struct S2 {
+    f: fn(&S)
+}
+
+impl S2 {
+    fn foo(&self, s: &S) {
+        (self.f)(s)
+    }
+}
+
+fn main() {
+    S2 { f: <S as Foo>::foo }.foo(&S)
+}