Skip to content

Commit f837c48

Browse files
committed
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.
1 parent 3c554f5 commit f837c48

14 files changed

+90
-31
lines changed

Diff for: compiler/rustc_codegen_llvm/src/back/write.rs

+3
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,8 @@ pub(crate) unsafe fn llvm_optimize(
449449
Some(llvm::SanitizerOptions {
450450
sanitize_address: config.sanitizer.contains(SanitizerSet::ADDRESS),
451451
sanitize_address_recover: config.sanitizer_recover.contains(SanitizerSet::ADDRESS),
452+
sanitize_cfi: config.sanitizer.contains(SanitizerSet::CFI),
453+
sanitize_kcfi: config.sanitizer.contains(SanitizerSet::KCFI),
452454
sanitize_memory: config.sanitizer.contains(SanitizerSet::MEMORY),
453455
sanitize_memory_recover: config.sanitizer_recover.contains(SanitizerSet::MEMORY),
454456
sanitize_memory_track_origins: config.sanitizer_memory_track_origins as c_int,
@@ -484,6 +486,7 @@ pub(crate) unsafe fn llvm_optimize(
484486
&*module.module_llvm.tm,
485487
to_pass_builder_opt_level(opt_level),
486488
opt_stage,
489+
cgcx.opts.cg.linker_plugin_lto.enabled(),
487490
config.no_prepopulate_passes,
488491
config.verify_llvm_ir,
489492
using_thin_buffers,

Diff for: compiler/rustc_codegen_llvm/src/builder.rs

+22-21
Original file line numberDiff line numberDiff line change
@@ -1525,9 +1525,9 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
15251525
fn_abi: Option<&FnAbi<'tcx, Ty<'tcx>>>,
15261526
llfn: &'ll Value,
15271527
) {
1528-
let is_indirect_call = unsafe { llvm::LLVMIsAFunction(llfn).is_none() };
1529-
if is_indirect_call && fn_abi.is_some() && self.tcx.sess.is_sanitizer_cfi_enabled() {
1530-
if fn_attrs.is_some() && fn_attrs.unwrap().no_sanitize.contains(SanitizerSet::CFI) {
1528+
let is_indirect_call = unsafe { llvm::LLVMRustIsNonGVFunctionPointerTy(llfn) };
1529+
if self.tcx.sess.is_sanitizer_cfi_enabled() && let Some(fn_abi) = fn_abi && is_indirect_call {
1530+
if let Some(fn_attrs) = fn_attrs && fn_attrs.no_sanitize.contains(SanitizerSet::CFI) {
15311531
return;
15321532
}
15331533

@@ -1539,7 +1539,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
15391539
options.insert(TypeIdOptions::NORMALIZE_INTEGERS);
15401540
}
15411541

1542-
let typeid = typeid_for_fnabi(self.tcx, fn_abi.unwrap(), options);
1542+
let typeid = typeid_for_fnabi(self.tcx, fn_abi, options);
15431543
let typeid_metadata = self.cx.typeid_metadata(typeid).unwrap();
15441544

15451545
// Test whether the function pointer is associated with the type identifier.
@@ -1563,25 +1563,26 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
15631563
fn_abi: Option<&FnAbi<'tcx, Ty<'tcx>>>,
15641564
llfn: &'ll Value,
15651565
) -> Option<llvm::OperandBundleDef<'ll>> {
1566-
let is_indirect_call = unsafe { llvm::LLVMIsAFunction(llfn).is_none() };
1567-
let kcfi_bundle = if is_indirect_call && self.tcx.sess.is_sanitizer_kcfi_enabled() {
1568-
if fn_attrs.is_some() && fn_attrs.unwrap().no_sanitize.contains(SanitizerSet::KCFI) {
1569-
return None;
1570-
}
1566+
let is_indirect_call = unsafe { llvm::LLVMRustIsNonGVFunctionPointerTy(llfn) };
1567+
let kcfi_bundle =
1568+
if self.tcx.sess.is_sanitizer_kcfi_enabled() && let Some(fn_abi) = fn_abi && is_indirect_call {
1569+
if let Some(fn_attrs) = fn_attrs && fn_attrs.no_sanitize.contains(SanitizerSet::KCFI) {
1570+
return None;
1571+
}
15711572

1572-
let mut options = TypeIdOptions::empty();
1573-
if self.tcx.sess.is_sanitizer_cfi_generalize_pointers_enabled() {
1574-
options.insert(TypeIdOptions::GENERALIZE_POINTERS);
1575-
}
1576-
if self.tcx.sess.is_sanitizer_cfi_normalize_integers_enabled() {
1577-
options.insert(TypeIdOptions::NORMALIZE_INTEGERS);
1578-
}
1573+
let mut options = TypeIdOptions::empty();
1574+
if self.tcx.sess.is_sanitizer_cfi_generalize_pointers_enabled() {
1575+
options.insert(TypeIdOptions::GENERALIZE_POINTERS);
1576+
}
1577+
if self.tcx.sess.is_sanitizer_cfi_normalize_integers_enabled() {
1578+
options.insert(TypeIdOptions::NORMALIZE_INTEGERS);
1579+
}
15791580

1580-
let kcfi_typeid = kcfi_typeid_for_fnabi(self.tcx, fn_abi.unwrap(), options);
1581-
Some(llvm::OperandBundleDef::new("kcfi", &[self.const_u32(kcfi_typeid)]))
1582-
} else {
1583-
None
1584-
};
1581+
let kcfi_typeid = kcfi_typeid_for_fnabi(self.tcx, fn_abi, options);
1582+
Some(llvm::OperandBundleDef::new("kcfi", &[self.const_u32(kcfi_typeid)]))
1583+
} else {
1584+
None
1585+
};
15851586
kcfi_bundle
15861587
}
15871588
}

Diff for: compiler/rustc_codegen_llvm/src/llvm/ffi.rs

+4
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,8 @@ pub enum OptStage {
477477
pub struct SanitizerOptions {
478478
pub sanitize_address: bool,
479479
pub sanitize_address_recover: bool,
480+
pub sanitize_cfi: bool,
481+
pub sanitize_kcfi: bool,
480482
pub sanitize_memory: bool,
481483
pub sanitize_memory_recover: bool,
482484
pub sanitize_memory_track_origins: c_int,
@@ -1083,6 +1085,7 @@ extern "C" {
10831085
pub fn LLVMRustGlobalAddMetadata<'a>(Val: &'a Value, KindID: c_uint, Metadata: &'a Metadata);
10841086
pub fn LLVMValueAsMetadata(Node: &Value) -> &Metadata;
10851087
pub fn LLVMIsAFunction(Val: &Value) -> Option<&Value>;
1088+
pub fn LLVMRustIsNonGVFunctionPointerTy(Val: &Value) -> bool;
10861089

10871090
// Operations on constants of any type
10881091
pub fn LLVMConstNull(Ty: &Type) -> &Value;
@@ -2319,6 +2322,7 @@ extern "C" {
23192322
TM: &'a TargetMachine,
23202323
OptLevel: PassBuilderOptLevel,
23212324
OptStage: OptStage,
2325+
IsLinkerPluginLTO: bool,
23222326
NoPrepopulatePasses: bool,
23232327
VerifyIR: bool,
23242328
UseThinLTOBuffers: bool,

Diff for: compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp

+16
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "llvm/Transforms/IPO/AlwaysInliner.h"
3030
#include "llvm/Transforms/IPO/FunctionImport.h"
3131
#include "llvm/Transforms/IPO/Internalize.h"
32+
#include "llvm/Transforms/IPO/LowerTypeTests.h"
3233
#include "llvm/Transforms/IPO/ThinLTOBitcodeWriter.h"
3334
#include "llvm/Transforms/Utils/AddDiscriminators.h"
3435
#include "llvm/Transforms/Utils/FunctionImportUtils.h"
@@ -599,6 +600,8 @@ enum class LLVMRustOptStage {
599600
struct LLVMRustSanitizerOptions {
600601
bool SanitizeAddress;
601602
bool SanitizeAddressRecover;
603+
bool SanitizeCFI;
604+
bool SanitizeKCFI;
602605
bool SanitizeMemory;
603606
bool SanitizeMemoryRecover;
604607
int SanitizeMemoryTrackOrigins;
@@ -615,6 +618,7 @@ LLVMRustOptimize(
615618
LLVMTargetMachineRef TMRef,
616619
LLVMRustPassBuilderOptLevel OptLevelRust,
617620
LLVMRustOptStage OptStage,
621+
bool IsLinkerPluginLTO,
618622
bool NoPrepopulatePasses, bool VerifyIR, bool UseThinLTOBuffers,
619623
bool MergeFunctions, bool UnrollLoops, bool SLPVectorize, bool LoopVectorize,
620624
bool DisableSimplifyLibCalls, bool EmitLifetimeMarkers,
@@ -722,6 +726,18 @@ LLVMRustOptimize(
722726
std::vector<std::function<void(ModulePassManager &, OptimizationLevel)>>
723727
OptimizerLastEPCallbacks;
724728

729+
if (!IsLinkerPluginLTO
730+
&& SanitizerOptions && SanitizerOptions->SanitizeCFI
731+
&& !NoPrepopulatePasses) {
732+
PipelineStartEPCallbacks.push_back(
733+
[](ModulePassManager &MPM, OptimizationLevel Level) {
734+
MPM.addPass(LowerTypeTestsPass(/*ExportSummary=*/nullptr,
735+
/*ImportSummary=*/nullptr,
736+
/*DropTypeTests=*/false));
737+
}
738+
);
739+
}
740+
725741
if (VerifyIR) {
726742
PipelineStartEPCallbacks.push_back(
727743
[VerifyIR](ModulePassManager &MPM, OptimizationLevel Level) {

Diff for: compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

+11
Original file line numberDiff line numberDiff line change
@@ -1952,3 +1952,14 @@ extern "C" int32_t LLVMRustGetElementTypeArgIndex(LLVMValueRef CallSite) {
19521952
extern "C" bool LLVMRustIsBitcode(char *ptr, size_t len) {
19531953
return identify_magic(StringRef(ptr, len)) == file_magic::bitcode;
19541954
}
1955+
1956+
extern "C" bool LLVMRustIsNonGVFunctionPointerTy(LLVMValueRef V) {
1957+
if (unwrap<Value>(V)->getType()->isPointerTy()) {
1958+
if (auto *GV = dyn_cast<GlobalValue>(unwrap<Value>(V))) {
1959+
if (GV->getValueType()->isFunctionTy())
1960+
return false;
1961+
}
1962+
return true;
1963+
}
1964+
return false;
1965+
}

Diff for: compiler/rustc_session/messages.ftl

+3-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,9 @@ session_sanitizer_cfi_generalize_pointers_requires_cfi = `-Zsanitizer-cfi-genera
8686
8787
session_sanitizer_cfi_normalize_integers_requires_cfi = `-Zsanitizer-cfi-normalize-integers` requires `-Zsanitizer=cfi` or `-Zsanitizer=kcfi`
8888
89-
session_sanitizer_cfi_requires_lto = `-Zsanitizer=cfi` requires `-Clto`, `-Clto=thin`, or `-Clinker-plugin-lto`
89+
session_sanitizer_cfi_requires_lto = `-Zsanitizer=cfi` requires `-Clto` or `-Clinker-plugin-lto`
90+
91+
session_sanitizer_cfi_requires_single_codegen_unit = `-Zsanitizer=cfi` with `-Clto` requires `-Ccodegen-units=1`
9092
9193
session_sanitizer_not_supported = {$us} sanitizer is not supported for this target
9294

Diff for: compiler/rustc_session/src/errors.rs

+4
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,10 @@ pub struct CannotEnableCrtStaticLinux;
114114
#[diag(session_sanitizer_cfi_requires_lto)]
115115
pub struct SanitizerCfiRequiresLto;
116116

117+
#[derive(Diagnostic)]
118+
#[diag(session_sanitizer_cfi_requires_single_codegen_unit)]
119+
pub struct SanitizerCfiRequiresSingleCodegenUnit;
120+
117121
#[derive(Diagnostic)]
118122
#[diag(session_sanitizer_cfi_canonical_jump_tables_requires_cfi)]
119123
pub struct SanitizerCfiCanonicalJumpTablesRequiresCfi;

Diff for: compiler/rustc_session/src/session.rs

+9-3
Original file line numberDiff line numberDiff line change
@@ -1616,13 +1616,19 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
16161616

16171617
// LLVM CFI requires LTO.
16181618
if sess.is_sanitizer_cfi_enabled()
1619-
&& !(sess.lto() == config::Lto::Fat
1620-
|| sess.lto() == config::Lto::Thin
1621-
|| sess.opts.cg.linker_plugin_lto.enabled())
1619+
&& !(sess.lto() == config::Lto::Fat || sess.opts.cg.linker_plugin_lto.enabled())
16221620
{
16231621
sess.emit_err(errors::SanitizerCfiRequiresLto);
16241622
}
16251623

1624+
// LLVM CFI using rustc LTO requires a single codegen unit.
1625+
if sess.is_sanitizer_cfi_enabled()
1626+
&& sess.lto() == config::Lto::Fat
1627+
&& !(sess.codegen_units().as_usize() == 1)
1628+
{
1629+
sess.emit_err(errors::SanitizerCfiRequiresSingleCodegenUnit);
1630+
}
1631+
16261632
// LLVM CFI is incompatible with LLVM KCFI.
16271633
if sess.is_sanitizer_cfi_enabled() && sess.is_sanitizer_kcfi_enabled() {
16281634
sess.emit_err(errors::CannotMixAndMatchSanitizers {

Diff for: tests/ui/lto/issue-100772.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
// run-pass
1+
// build-pass
22
// needs-sanitizer-cfi
3-
// compile-flags: -Clto -Ctarget-feature=-crt-static -Zsanitizer=cfi
3+
// compile-flags: -Ccodegen-units=1 -Clto -Ctarget-feature=-crt-static -Zsanitizer=cfi
44
// no-prefer-dynamic
55
// only-x86_64-unknown-linux-gnu
66

Diff for: tests/ui/sanitize/issue-111184-generator-witness.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22
// encode_ty and caused the compiler to ICE.
33
//
44
// needs-sanitizer-cfi
5-
// compile-flags: -Clto -Ctarget-feature=-crt-static -Zsanitizer=cfi --edition=2021
5+
// compile-flags: -Ccodegen-units=1 -Clto -Ctarget-feature=-crt-static -Zsanitizer=cfi --edition=2021
66
// no-prefer-dynamic
77
// only-x86_64-unknown-linux-gnu
8-
// run-pass
8+
// build-pass
99

1010
use std::future::Future;
1111

Diff for: tests/ui/sanitize/sanitizer-cfi-requires-lto.rs

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

Diff for: tests/ui/sanitize/sanitizer-cfi-requires-lto.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error: `-Zsanitizer=cfi` requires `-Clto`, `-Clto=thin`, or `-Clinker-plugin-lto`
1+
error: `-Zsanitizer=cfi` requires `-Clto` or `-Clinker-plugin-lto`
22

33
error: aborting due to previous error
44

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Verifies that `-Zsanitizer=cfi` with `-Clto` or `-Clto=thin` requires `-Ccodegen-units=1`.
2+
//
3+
// needs-sanitizer-cfi
4+
// compile-flags: -Ccodegen-units=2 -Clto -Ctarget-feature=-crt-static -Zsanitizer=cfi
5+
6+
#![feature(no_core)]
7+
#![no_core]
8+
#![no_main]
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
error: `-Zsanitizer=cfi` with `-Clto` requires `-Ccodegen-units=1`
2+
3+
error: aborting due to previous error
4+

0 commit comments

Comments
 (0)