From bbb63d4554b03feee481bc799a04f183abaff1d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Wed, 13 May 2020 00:00:00 +0000 Subject: [PATCH] Consistently use LLVM lifetime markers during codegen Ensure that inliner inserts lifetime markers if they have been emitted during codegen. Otherwise if allocas from inlined functions are merged together, lifetime markers from one function might invalidate load & stores performed by the other one. --- src/librustc_codegen_llvm/back/write.rs | 5 +-- src/librustc_codegen_llvm/builder.rs | 10 +----- src/librustc_codegen_llvm/llvm/ffi.rs | 1 + src/librustc_codegen_ssa/back/write.rs | 2 ++ src/librustc_session/session.rs | 10 ++++++ src/rustllvm/PassWrapper.cpp | 4 +-- .../sanitize/issue-72154-lifetime-markers.rs | 31 +++++++++++++++++++ 7 files changed, 50 insertions(+), 13 deletions(-) create mode 100644 src/test/ui/sanitize/issue-72154-lifetime-markers.rs diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index dd9ada0b95daa..a08235b304dc5 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -394,6 +394,7 @@ pub(crate) unsafe fn optimize_with_new_llvm_pass_manager( config.vectorize_slp, config.vectorize_loop, config.no_builtins, + config.emit_lifetime_markers, sanitizer_options.as_ref(), pgo_gen_path.as_ref().map_or(std::ptr::null(), |s| s.as_ptr()), pgo_use_path.as_ref().map_or(std::ptr::null(), |s| s.as_ptr()), @@ -934,10 +935,10 @@ pub unsafe fn with_llvm_pmb( llvm::LLVMPassManagerBuilderUseInlinerWithThreshold(builder, 25); } (llvm::CodeGenOptLevel::None, ..) => { - llvm::LLVMRustAddAlwaysInlinePass(builder, false); + llvm::LLVMRustAddAlwaysInlinePass(builder, config.emit_lifetime_markers); } (llvm::CodeGenOptLevel::Less, ..) => { - llvm::LLVMRustAddAlwaysInlinePass(builder, true); + llvm::LLVMRustAddAlwaysInlinePass(builder, config.emit_lifetime_markers); } (llvm::CodeGenOptLevel::Default, ..) => { llvm::LLVMPassManagerBuilderUseInlinerWithThreshold(builder, 225); diff --git a/src/librustc_codegen_llvm/builder.rs b/src/librustc_codegen_llvm/builder.rs index 89bd96c1fe214..f5ae9824df894 100644 --- a/src/librustc_codegen_llvm/builder.rs +++ b/src/librustc_codegen_llvm/builder.rs @@ -18,7 +18,6 @@ use rustc_data_structures::small_c_str::SmallCStr; use rustc_hir::def_id::DefId; use rustc_middle::ty::layout::TyAndLayout; use rustc_middle::ty::{self, Ty, TyCtxt}; -use rustc_session::config::{self, Sanitizer}; use rustc_target::abi::{self, Align, Size}; use rustc_target::spec::{HasTargetSpec, Target}; use std::borrow::Cow; @@ -1243,14 +1242,7 @@ impl Builder<'a, 'll, 'tcx> { return; } - let opts = &self.cx.sess().opts; - let emit = match opts.debugging_opts.sanitizer { - // Some sanitizer use lifetime intrinsics. When they are in use, - // emit lifetime intrinsics regardless of optimization level. - Some(Sanitizer::Address | Sanitizer::Memory) => true, - _ => opts.optimize != config::OptLevel::No, - }; - if !emit { + if !self.cx().sess().emit_lifetime_markers() { return; } diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 0d466c2cd745a..713cb2324f7f7 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -2000,6 +2000,7 @@ extern "C" { SLPVectorize: bool, LoopVectorize: bool, DisableSimplifyLibCalls: bool, + EmitLifetimeMarkers: bool, SanitizerOptions: Option<&SanitizerOptions>, PGOGenPath: *const c_char, PGOUsePath: *const c_char, diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index 6210559251def..a88fef5aaacf2 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/src/librustc_codegen_ssa/back/write.rs @@ -110,6 +110,7 @@ pub struct ModuleConfig { pub merge_functions: bool, pub inline_threshold: Option, pub new_llvm_pass_manager: bool, + pub emit_lifetime_markers: bool, } impl ModuleConfig { @@ -244,6 +245,7 @@ impl ModuleConfig { inline_threshold: sess.opts.cg.inline_threshold, new_llvm_pass_manager: sess.opts.debugging_opts.new_llvm_pass_manager, + emit_lifetime_markers: sess.emit_lifetime_markers(), } } diff --git a/src/librustc_session/session.rs b/src/librustc_session/session.rs index b39b15dc24428..cb5bd37442a7e 100644 --- a/src/librustc_session/session.rs +++ b/src/librustc_session/session.rs @@ -936,6 +936,16 @@ impl Session { // then try to skip it where possible. dbg_opts.plt.unwrap_or(needs_plt || !full_relro) } + + /// Checks if LLVM lifetime markers should be emitted. + pub fn emit_lifetime_markers(&self) -> bool { + match self.opts.debugging_opts.sanitizer { + // AddressSanitizer uses lifetimes to detect use after scope bugs. + // MemorySanitizer uses lifetimes to detect use of uninitialized stack variables. + Some(Sanitizer::Address | Sanitizer::Memory) => true, + _ => self.opts.optimize != config::OptLevel::No, + } + } } pub fn build_session( diff --git a/src/rustllvm/PassWrapper.cpp b/src/rustllvm/PassWrapper.cpp index 84bde9a52f7ce..b17fa57c5c1ef 100644 --- a/src/rustllvm/PassWrapper.cpp +++ b/src/rustllvm/PassWrapper.cpp @@ -717,7 +717,7 @@ LLVMRustOptimizeWithNewPassManager( LLVMRustOptStage OptStage, bool NoPrepopulatePasses, bool VerifyIR, bool UseThinLTOBuffers, bool MergeFunctions, bool UnrollLoops, bool SLPVectorize, bool LoopVectorize, - bool DisableSimplifyLibCalls, + bool DisableSimplifyLibCalls, bool EmitLifetimeMarkers, LLVMRustSanitizerOptions *SanitizerOptions, const char *PGOGenPath, const char *PGOUsePath, void* LlvmSelfProfiler, @@ -853,7 +853,7 @@ LLVMRustOptimizeWithNewPassManager( MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM))); } - MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/false)); + MPM.addPass(AlwaysInlinerPass(EmitLifetimeMarkers)); #if LLVM_VERSION_GE(10, 0) if (PGOOpt) { diff --git a/src/test/ui/sanitize/issue-72154-lifetime-markers.rs b/src/test/ui/sanitize/issue-72154-lifetime-markers.rs new file mode 100644 index 0000000000000..458f99143b648 --- /dev/null +++ b/src/test/ui/sanitize/issue-72154-lifetime-markers.rs @@ -0,0 +1,31 @@ +// Regression test for issue 72154, where the use of AddressSanitizer enabled +// emission of lifetime markers during codegen, while at the same time asking +// always inliner pass not to insert them. This eventually lead to a +// miscompilation which was subsequently detected by AddressSanitizer as UB. +// +// needs-sanitizer-support +// only-x86_64 +// +// compile-flags: -Copt-level=0 -Zsanitizer=address +// run-pass + +pub struct Wrap { + pub t: [usize; 1] +} + +impl Wrap { + #[inline(always)] + pub fn new(t: [usize; 1]) -> Self { + Wrap { t } + } +} + +#[inline(always)] +pub fn assume_init() -> [usize; 1] { + [1234] +} + +fn main() { + let x: [usize; 1] = assume_init(); + Wrap::new(x); +}