Rollup merge of #113593 - rcvalle:rust-cfi-fix-90546, r=wesleywiser

CFI: Fix error compiling core with LLVM CFI enabled

Fix #90546 by filtering out global value function pointer types from the type tests, and adding the LowerTypeTests pass to the rustc LTO optimization pipelines.

Author: matthiaskrgr

compiler/rustc_codegen_llvm/src/back/write.rs

@@ -472,6 +472,8 @@ pub(crate) unsafe fn llvm_optimize(
         Some(llvm::SanitizerOptions {
             sanitize_address: config.sanitizer.contains(SanitizerSet::ADDRESS),
             sanitize_address_recover: config.sanitizer_recover.contains(SanitizerSet::ADDRESS),
+            sanitize_cfi: config.sanitizer.contains(SanitizerSet::CFI),
+            sanitize_kcfi: config.sanitizer.contains(SanitizerSet::KCFI),
             sanitize_memory: config.sanitizer.contains(SanitizerSet::MEMORY),
             sanitize_memory_recover: config.sanitizer_recover.contains(SanitizerSet::MEMORY),
             sanitize_memory_track_origins: config.sanitizer_memory_track_origins as c_int,
@@ -507,6 +509,7 @@ pub(crate) unsafe fn llvm_optimize(
         &*module.module_llvm.tm,
         to_pass_builder_opt_level(opt_level),
         opt_stage,
+        cgcx.opts.cg.linker_plugin_lto.enabled(),
         config.no_prepopulate_passes,
         config.verify_llvm_ir,
         using_thin_buffers,

compiler/rustc_codegen_llvm/src/builder.rs

@@ -1512,9 +1512,9 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
         fn_abi: Option<&FnAbi<'tcx, Ty<'tcx>>>,
         llfn: &'ll Value,
     ) {
-        let is_indirect_call = unsafe { llvm::LLVMIsAFunction(llfn).is_none() };
-        if is_indirect_call && fn_abi.is_some() && self.tcx.sess.is_sanitizer_cfi_enabled() {
-            if fn_attrs.is_some() && fn_attrs.unwrap().no_sanitize.contains(SanitizerSet::CFI) {
+        let is_indirect_call = unsafe { llvm::LLVMRustIsNonGVFunctionPointerTy(llfn) };
+        if self.tcx.sess.is_sanitizer_cfi_enabled() && let Some(fn_abi) = fn_abi && is_indirect_call {
+            if let Some(fn_attrs) = fn_attrs && fn_attrs.no_sanitize.contains(SanitizerSet::CFI) {
                 return;
             }
 
@@ -1526,7 +1526,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
                 options.insert(TypeIdOptions::NORMALIZE_INTEGERS);
             }
 
-            let typeid = typeid_for_fnabi(self.tcx, fn_abi.unwrap(), options);
+            let typeid = typeid_for_fnabi(self.tcx, fn_abi, options);
             let typeid_metadata = self.cx.typeid_metadata(typeid).unwrap();
 
             // Test whether the function pointer is associated with the type identifier.
@@ -1550,25 +1550,26 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
         fn_abi: Option<&FnAbi<'tcx, Ty<'tcx>>>,
         llfn: &'ll Value,
     ) -> Option<llvm::OperandBundleDef<'ll>> {
-        let is_indirect_call = unsafe { llvm::LLVMIsAFunction(llfn).is_none() };
-        let kcfi_bundle = if is_indirect_call && self.tcx.sess.is_sanitizer_kcfi_enabled() {
-            if fn_attrs.is_some() && fn_attrs.unwrap().no_sanitize.contains(SanitizerSet::KCFI) {
-                return None;
-            }
+        let is_indirect_call = unsafe { llvm::LLVMRustIsNonGVFunctionPointerTy(llfn) };
+        let kcfi_bundle =
+            if self.tcx.sess.is_sanitizer_kcfi_enabled() && let Some(fn_abi) = fn_abi && is_indirect_call {
+                if let Some(fn_attrs) = fn_attrs && fn_attrs.no_sanitize.contains(SanitizerSet::KCFI) {
+                    return None;
+                }
 
-            let mut options = TypeIdOptions::empty();
-            if self.tcx.sess.is_sanitizer_cfi_generalize_pointers_enabled() {
-                options.insert(TypeIdOptions::GENERALIZE_POINTERS);
-            }
-            if self.tcx.sess.is_sanitizer_cfi_normalize_integers_enabled() {
-                options.insert(TypeIdOptions::NORMALIZE_INTEGERS);
-            }
+                let mut options = TypeIdOptions::empty();
+                if self.tcx.sess.is_sanitizer_cfi_generalize_pointers_enabled() {
+                    options.insert(TypeIdOptions::GENERALIZE_POINTERS);
+                }
+                if self.tcx.sess.is_sanitizer_cfi_normalize_integers_enabled() {
+                    options.insert(TypeIdOptions::NORMALIZE_INTEGERS);
+                }
 
-            let kcfi_typeid = kcfi_typeid_for_fnabi(self.tcx, fn_abi.unwrap(), options);
-            Some(llvm::OperandBundleDef::new("kcfi", &[self.const_u32(kcfi_typeid)]))
-        } else {
-            None
-        };
+                let kcfi_typeid = kcfi_typeid_for_fnabi(self.tcx, fn_abi, options);
+                Some(llvm::OperandBundleDef::new("kcfi", &[self.const_u32(kcfi_typeid)]))
+            } else {
+                None
+            };
         kcfi_bundle
     }
 }

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

@@ -475,6 +475,8 @@ pub enum OptStage {
 pub struct SanitizerOptions {
     pub sanitize_address: bool,
     pub sanitize_address_recover: bool,
+    pub sanitize_cfi: bool,
+    pub sanitize_kcfi: bool,
     pub sanitize_memory: bool,
     pub sanitize_memory_recover: bool,
     pub sanitize_memory_track_origins: c_int,
@@ -894,6 +896,7 @@ extern "C" {
     pub fn LLVMRustGlobalAddMetadata<'a>(Val: &'a Value, KindID: c_uint, Metadata: &'a Metadata);
     pub fn LLVMValueAsMetadata(Node: &Value) -> &Metadata;
     pub fn LLVMIsAFunction(Val: &Value) -> Option<&Value>;
+    pub fn LLVMRustIsNonGVFunctionPointerTy(Val: &Value) -> bool;
 
     // Operations on constants of any type
     pub fn LLVMConstNull(Ty: &Type) -> &Value;
@@ -2138,6 +2141,7 @@ extern "C" {
         TM: &'a TargetMachine,
         OptLevel: PassBuilderOptLevel,
         OptStage: OptStage,
+        IsLinkerPluginLTO: bool,
         NoPrepopulatePasses: bool,
         VerifyIR: bool,
         UseThinLTOBuffers: bool,

compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp

@@ -30,6 +30,7 @@
 #include "llvm/Transforms/IPO/AlwaysInliner.h"
 #include "llvm/Transforms/IPO/FunctionImport.h"
 #include "llvm/Transforms/IPO/Internalize.h"
+#include "llvm/Transforms/IPO/LowerTypeTests.h"
 #include "llvm/Transforms/IPO/ThinLTOBitcodeWriter.h"
 #include "llvm/Transforms/Utils/AddDiscriminators.h"
 #include "llvm/Transforms/Utils/FunctionImportUtils.h"
@@ -609,6 +610,8 @@ enum class LLVMRustOptStage {
 struct LLVMRustSanitizerOptions {
   bool SanitizeAddress;
   bool SanitizeAddressRecover;
+  bool SanitizeCFI;
+  bool SanitizeKCFI;
   bool SanitizeMemory;
   bool SanitizeMemoryRecover;
   int  SanitizeMemoryTrackOrigins;
@@ -625,6 +628,7 @@ LLVMRustOptimize(
     LLVMTargetMachineRef TMRef,
     LLVMRustPassBuilderOptLevel OptLevelRust,
     LLVMRustOptStage OptStage,
+    bool IsLinkerPluginLTO,
     bool NoPrepopulatePasses, bool VerifyIR, bool UseThinLTOBuffers,
     bool MergeFunctions, bool UnrollLoops, bool SLPVectorize, bool LoopVectorize,
     bool DisableSimplifyLibCalls, bool EmitLifetimeMarkers,
@@ -736,6 +740,18 @@ LLVMRustOptimize(
   std::vector<std::function<void(ModulePassManager &, OptimizationLevel)>>
       OptimizerLastEPCallbacks;
 
+  if (!IsLinkerPluginLTO
+      && SanitizerOptions && SanitizerOptions->SanitizeCFI
+      && !NoPrepopulatePasses) {
+    PipelineStartEPCallbacks.push_back(
+      [](ModulePassManager &MPM, OptimizationLevel Level) {
+        MPM.addPass(LowerTypeTestsPass(/*ExportSummary=*/nullptr,
+                                       /*ImportSummary=*/nullptr,
+                                       /*DropTypeTests=*/false));
+      }
+    );
+  }
+
   if (VerifyIR) {
     PipelineStartEPCallbacks.push_back(
       [VerifyIR](ModulePassManager &MPM, OptimizationLevel Level) {

compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

@@ -2033,3 +2033,14 @@ extern "C" int32_t LLVMRustGetElementTypeArgIndex(LLVMValueRef CallSite) {
 extern "C" bool LLVMRustIsBitcode(char *ptr, size_t len) {
   return identify_magic(StringRef(ptr, len)) == file_magic::bitcode;
 }
+
+extern "C" bool LLVMRustIsNonGVFunctionPointerTy(LLVMValueRef V) {
+  if (unwrap<Value>(V)->getType()->isPointerTy()) {
+    if (auto *GV = dyn_cast<GlobalValue>(unwrap<Value>(V))) {
+      if (GV->getValueType()->isFunctionTy())
+        return false;
+    }
+    return true;
+  }
+  return false;
+}

compiler/rustc_session/messages.ftl

@@ -89,7 +89,9 @@ session_sanitizer_cfi_generalize_pointers_requires_cfi = `-Zsanitizer-cfi-genera
 
 session_sanitizer_cfi_normalize_integers_requires_cfi = `-Zsanitizer-cfi-normalize-integers` requires `-Zsanitizer=cfi` or `-Zsanitizer=kcfi`
 
-session_sanitizer_cfi_requires_lto = `-Zsanitizer=cfi` requires `-Clto`, `-Clto=thin`, or `-Clinker-plugin-lto`
+session_sanitizer_cfi_requires_lto = `-Zsanitizer=cfi` requires `-Clto` or `-Clinker-plugin-lto`
+
+session_sanitizer_cfi_requires_single_codegen_unit = `-Zsanitizer=cfi` with `-Clto` requires `-Ccodegen-units=1`
 
 session_sanitizer_not_supported = {$us} sanitizer is not supported for this target
 

compiler/rustc_session/src/errors.rs

@@ -114,6 +114,10 @@ pub struct CannotEnableCrtStaticLinux;
 #[diag(session_sanitizer_cfi_requires_lto)]
 pub struct SanitizerCfiRequiresLto;
 
+#[derive(Diagnostic)]
+#[diag(session_sanitizer_cfi_requires_single_codegen_unit)]
+pub struct SanitizerCfiRequiresSingleCodegenUnit;
+
 #[derive(Diagnostic)]
 #[diag(session_sanitizer_cfi_canonical_jump_tables_requires_cfi)]
 pub struct SanitizerCfiCanonicalJumpTablesRequiresCfi;

compiler/rustc_session/src/session.rs

@@ -1619,13 +1619,19 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
 
     // LLVM CFI requires LTO.
     if sess.is_sanitizer_cfi_enabled()
-        && !(sess.lto() == config::Lto::Fat
-            || sess.lto() == config::Lto::Thin
-            || sess.opts.cg.linker_plugin_lto.enabled())
+        && !(sess.lto() == config::Lto::Fat || sess.opts.cg.linker_plugin_lto.enabled())
     {
         sess.emit_err(errors::SanitizerCfiRequiresLto);
     }
 
+    // LLVM CFI using rustc LTO requires a single codegen unit.
+    if sess.is_sanitizer_cfi_enabled()
+        && sess.lto() == config::Lto::Fat
+        && !(sess.codegen_units().as_usize() == 1)
+    {
+        sess.emit_err(errors::SanitizerCfiRequiresSingleCodegenUnit);
+    }
+
     // LLVM CFI is incompatible with LLVM KCFI.
     if sess.is_sanitizer_cfi_enabled() && sess.is_sanitizer_kcfi_enabled() {
         sess.emit_err(errors::CannotMixAndMatchSanitizers {

tests/ui/lto/issue-100772.rs

@@ -1,6 +1,6 @@
-// run-pass
+// build-pass
 // needs-sanitizer-cfi
-// compile-flags: -Clto -Ctarget-feature=-crt-static -Zsanitizer=cfi
+// compile-flags: -Ccodegen-units=1 -Clto -Ctarget-feature=-crt-static -Zsanitizer=cfi
 // no-prefer-dynamic
 // only-x86_64-unknown-linux-gnu
 

tests/ui/sanitize/issue-111184-generator-witness.rs

@@ -2,10 +2,10 @@
 // encode_ty and caused the compiler to ICE.
 //
 // needs-sanitizer-cfi
-// compile-flags: -Clto -Ctarget-feature=-crt-static -Zsanitizer=cfi --edition=2021
+// compile-flags: -Ccodegen-units=1 -Clto -Ctarget-feature=-crt-static -Zsanitizer=cfi --edition=2021
 // no-prefer-dynamic
 // only-x86_64-unknown-linux-gnu
-// run-pass
+// build-pass
 
 use std::future::Future;
 

tests/ui/sanitize/sanitizer-cfi-requires-lto.rs

@@ -1,4 +1,4 @@
-// Verifies that `-Zsanitizer=cfi` requires `-Clto`, `-Clto=thin`, or `-Clinker-plugin-lto`.
+// Verifies that `-Zsanitizer=cfi` requires `-Clto` or `-Clinker-plugin-lto`.
 //
 // needs-sanitizer-cfi
 // compile-flags: -Cno-prepopulate-passes -Ctarget-feature=-crt-static -Zsanitizer=cfi

tests/ui/sanitize/sanitizer-cfi-requires-lto.stderr

@@ -1,4 +1,4 @@
-error: `-Zsanitizer=cfi` requires `-Clto`, `-Clto=thin`, or `-Clinker-plugin-lto`
+error: `-Zsanitizer=cfi` requires `-Clto` or `-Clinker-plugin-lto`
 
 error: aborting due to previous error
 

tests/ui/sanitize/sanitizer-cfi-with-rustc-lto-requires-single-codegen-unit.rs

@@ -0,0 +1,8 @@
+// Verifies that `-Zsanitizer=cfi` with `-Clto` or `-Clto=thin` requires `-Ccodegen-units=1`.
+//
+// needs-sanitizer-cfi
+// compile-flags: -Ccodegen-units=2 -Clto -Ctarget-feature=-crt-static -Zsanitizer=cfi
+
+#![feature(no_core)]
+#![no_core]
+#![no_main]

tests/ui/sanitize/sanitizer-cfi-with-rustc-lto-requires-single-codegen-unit.stderr

@@ -0,0 +1,4 @@
+error: `-Zsanitizer=cfi` with `-Clto` requires `-Ccodegen-units=1`
+
+error: aborting due to previous error
+