From f837c48f0d8da1a205b7ab97846bc90f8d497271 Mon Sep 17 00:00:00 2001 From: Ramon de C Valle Date: Tue, 11 Jul 2023 16:19:42 -0700 Subject: [PATCH] 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. --- compiler/rustc_codegen_llvm/src/back/write.rs | 3 ++ compiler/rustc_codegen_llvm/src/builder.rs | 43 ++++++++++--------- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 4 ++ .../rustc_llvm/llvm-wrapper/PassWrapper.cpp | 16 +++++++ .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 11 +++++ compiler/rustc_session/messages.ftl | 4 +- compiler/rustc_session/src/errors.rs | 4 ++ compiler/rustc_session/src/session.rs | 12 ++++-- tests/ui/lto/issue-100772.rs | 4 +- .../issue-111184-generator-witness.rs | 4 +- .../ui/sanitize/sanitizer-cfi-requires-lto.rs | 2 +- .../sanitizer-cfi-requires-lto.stderr | 2 +- ...-rustc-lto-requires-single-codegen-unit.rs | 8 ++++ ...tc-lto-requires-single-codegen-unit.stderr | 4 ++ 14 files changed, 90 insertions(+), 31 deletions(-) create mode 100644 tests/ui/sanitize/sanitizer-cfi-with-rustc-lto-requires-single-codegen-unit.rs create mode 100644 tests/ui/sanitize/sanitizer-cfi-with-rustc-lto-requires-single-codegen-unit.stderr diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs index 53b4296802ef7..d12f7615361af 100644 --- a/compiler/rustc_codegen_llvm/src/back/write.rs +++ b/compiler/rustc_codegen_llvm/src/back/write.rs @@ -449,6 +449,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, @@ -484,6 +486,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, diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 43258078bd770..169123d36f7e0 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -1525,9 +1525,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; } @@ -1539,7 +1539,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. @@ -1563,25 +1563,26 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> { fn_abi: Option<&FnAbi<'tcx, Ty<'tcx>>>, llfn: &'ll Value, ) -> Option> { - 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 } } diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 6ef3418cc5f77..5c680d1e638a6 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -477,6 +477,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, @@ -1083,6 +1085,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; @@ -2319,6 +2322,7 @@ extern "C" { TM: &'a TargetMachine, OptLevel: PassBuilderOptLevel, OptStage: OptStage, + IsLinkerPluginLTO: bool, NoPrepopulatePasses: bool, VerifyIR: bool, UseThinLTOBuffers: bool, diff --git a/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp index c43a02724773a..3efa888e57fae 100644 --- a/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp @@ -29,6 +29,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" @@ -599,6 +600,8 @@ enum class LLVMRustOptStage { struct LLVMRustSanitizerOptions { bool SanitizeAddress; bool SanitizeAddressRecover; + bool SanitizeCFI; + bool SanitizeKCFI; bool SanitizeMemory; bool SanitizeMemoryRecover; int SanitizeMemoryTrackOrigins; @@ -615,6 +618,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, @@ -722,6 +726,18 @@ LLVMRustOptimize( std::vector> 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) { diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index ea04899ab6872..bc2b9f908f4fe 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1952,3 +1952,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(V)->getType()->isPointerTy()) { + if (auto *GV = dyn_cast(unwrap(V))) { + if (GV->getValueType()->isFunctionTy()) + return false; + } + return true; + } + return false; +} diff --git a/compiler/rustc_session/messages.ftl b/compiler/rustc_session/messages.ftl index 4897bd8d5daec..7c1e6954df20a 100644 --- a/compiler/rustc_session/messages.ftl +++ b/compiler/rustc_session/messages.ftl @@ -86,7 +86,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 diff --git a/compiler/rustc_session/src/errors.rs b/compiler/rustc_session/src/errors.rs index 4a3e668da111a..42c7b929bb1b3 100644 --- a/compiler/rustc_session/src/errors.rs +++ b/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; diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index ea5beb6f8bebf..e6db759032ebe 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -1616,13 +1616,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 { diff --git a/tests/ui/lto/issue-100772.rs b/tests/ui/lto/issue-100772.rs index d6b06719277eb..eeb511962362b 100644 --- a/tests/ui/lto/issue-100772.rs +++ b/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 diff --git a/tests/ui/sanitize/issue-111184-generator-witness.rs b/tests/ui/sanitize/issue-111184-generator-witness.rs index 8f4118057cec4..d36d8bce561f0 100644 --- a/tests/ui/sanitize/issue-111184-generator-witness.rs +++ b/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; diff --git a/tests/ui/sanitize/sanitizer-cfi-requires-lto.rs b/tests/ui/sanitize/sanitizer-cfi-requires-lto.rs index 29e32889fcc77..e9a49dd3ff114 100644 --- a/tests/ui/sanitize/sanitizer-cfi-requires-lto.rs +++ b/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 diff --git a/tests/ui/sanitize/sanitizer-cfi-requires-lto.stderr b/tests/ui/sanitize/sanitizer-cfi-requires-lto.stderr index 5e706b513b914..8cd9c544417f3 100644 --- a/tests/ui/sanitize/sanitizer-cfi-requires-lto.stderr +++ b/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 diff --git a/tests/ui/sanitize/sanitizer-cfi-with-rustc-lto-requires-single-codegen-unit.rs b/tests/ui/sanitize/sanitizer-cfi-with-rustc-lto-requires-single-codegen-unit.rs new file mode 100644 index 0000000000000..a13c12c17878a --- /dev/null +++ b/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] diff --git a/tests/ui/sanitize/sanitizer-cfi-with-rustc-lto-requires-single-codegen-unit.stderr b/tests/ui/sanitize/sanitizer-cfi-with-rustc-lto-requires-single-codegen-unit.stderr new file mode 100644 index 0000000000000..136f49360849f --- /dev/null +++ b/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 +