1
Fork 0

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.
This commit is contained in:
Matthias Krüger 2023-08-08 21:44:43 +02:00 committed by GitHub
commit c097e48082
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 90 additions and 31 deletions

View file

@ -472,6 +472,8 @@ pub(crate) unsafe fn llvm_optimize(
Some(llvm::SanitizerOptions { Some(llvm::SanitizerOptions {
sanitize_address: config.sanitizer.contains(SanitizerSet::ADDRESS), sanitize_address: config.sanitizer.contains(SanitizerSet::ADDRESS),
sanitize_address_recover: config.sanitizer_recover.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: config.sanitizer.contains(SanitizerSet::MEMORY),
sanitize_memory_recover: config.sanitizer_recover.contains(SanitizerSet::MEMORY), sanitize_memory_recover: config.sanitizer_recover.contains(SanitizerSet::MEMORY),
sanitize_memory_track_origins: config.sanitizer_memory_track_origins as c_int, 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, &*module.module_llvm.tm,
to_pass_builder_opt_level(opt_level), to_pass_builder_opt_level(opt_level),
opt_stage, opt_stage,
cgcx.opts.cg.linker_plugin_lto.enabled(),
config.no_prepopulate_passes, config.no_prepopulate_passes,
config.verify_llvm_ir, config.verify_llvm_ir,
using_thin_buffers, using_thin_buffers,

View file

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

View file

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

View file

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

View file

@ -2033,3 +2033,14 @@ extern "C" int32_t LLVMRustGetElementTypeArgIndex(LLVMValueRef CallSite) {
extern "C" bool LLVMRustIsBitcode(char *ptr, size_t len) { extern "C" bool LLVMRustIsBitcode(char *ptr, size_t len) {
return identify_magic(StringRef(ptr, len)) == file_magic::bitcode; 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;
}

View file

@ -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_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 session_sanitizer_not_supported = {$us} sanitizer is not supported for this target

View file

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

View file

@ -1619,13 +1619,19 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
// LLVM CFI requires LTO. // LLVM CFI requires LTO.
if sess.is_sanitizer_cfi_enabled() if sess.is_sanitizer_cfi_enabled()
&& !(sess.lto() == config::Lto::Fat && !(sess.lto() == config::Lto::Fat || sess.opts.cg.linker_plugin_lto.enabled())
|| sess.lto() == config::Lto::Thin
|| sess.opts.cg.linker_plugin_lto.enabled())
{ {
sess.emit_err(errors::SanitizerCfiRequiresLto); 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. // LLVM CFI is incompatible with LLVM KCFI.
if sess.is_sanitizer_cfi_enabled() && sess.is_sanitizer_kcfi_enabled() { if sess.is_sanitizer_cfi_enabled() && sess.is_sanitizer_kcfi_enabled() {
sess.emit_err(errors::CannotMixAndMatchSanitizers { sess.emit_err(errors::CannotMixAndMatchSanitizers {

View file

@ -1,6 +1,6 @@
// run-pass // build-pass
// needs-sanitizer-cfi // 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 // no-prefer-dynamic
// only-x86_64-unknown-linux-gnu // only-x86_64-unknown-linux-gnu

View file

@ -2,10 +2,10 @@
// encode_ty and caused the compiler to ICE. // encode_ty and caused the compiler to ICE.
// //
// needs-sanitizer-cfi // 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 // no-prefer-dynamic
// only-x86_64-unknown-linux-gnu // only-x86_64-unknown-linux-gnu
// run-pass // build-pass
use std::future::Future; use std::future::Future;

View file

@ -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 // needs-sanitizer-cfi
// compile-flags: -Cno-prepopulate-passes -Ctarget-feature=-crt-static -Zsanitizer=cfi // compile-flags: -Cno-prepopulate-passes -Ctarget-feature=-crt-static -Zsanitizer=cfi

View file

@ -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 error: aborting due to previous error

View file

@ -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]

View file

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