From 1a0278e1d17f66ddc5975f38387746f85c6602ce Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 20 Jan 2022 20:23:27 -0500 Subject: Work around missing code coverage data causing llvm-cov failures If we do not add code coverage instrumentation to the `Body` of a function, then when we go to generate the function record for it, we won't write any data and this later causes llvm-cov to fail when processing data for the entire coverage report. I've identified two main cases where we do not currently add code coverage instrumentation to the `Body` of a function: 1. If the function has a single `BasicBlock` and it ends with a `TerminatorKind::Unreachable`. 2. If the function is created using a proc macro of some kind. For case 1, this typically not important as this most often occurs as the result of function definitions that take or return uninhabited types. These kinds of functions, by definition, cannot even be called so they logically should not be counted in code coverage statistics. For case 2, I haven't looked into this very much but I've noticed while testing this patch that (other than functions which are covered by case 1) the skipped function coverage debug message is occasionally triggered in large crate graphs by functions generated from a proc macro. This may have something to do with weird spans being generated by the proc macro but this is just a guess. I think it's reasonable to land this change since currently, we fail to generate *any* results from llvm-cov when a function has no coverage instrumentation applied to it. With this change, we get coverage data for all functions other than the two cases discussed above. --- compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 32f18419753..3014d2f1930 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -9,6 +9,7 @@ use rustc_data_structures::fx::FxIndexSet; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefIdSet; use rustc_llvm::RustString; +use rustc_middle::bug; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::coverage::CodeRegion; use rustc_middle::ty::TyCtxt; @@ -76,10 +77,18 @@ pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { let coverage_mapping_buffer = llvm::build_byte_buffer(|coverage_mapping_buffer| { mapgen.write_coverage_mapping(expressions, counter_regions, coverage_mapping_buffer); }); - debug_assert!( - !coverage_mapping_buffer.is_empty(), - "Every `FunctionCoverage` should have at least one counter" - ); + + if coverage_mapping_buffer.is_empty() { + if function_coverage.is_used() { + bug!( + "A used function should have had coverage mapping data but did not: {}", + mangled_function_name + ); + } else { + debug!("unused function had no coverage mapping data: {}", mangled_function_name); + continue; + } + } function_data.push((mangled_function_name, source_hash, is_used, coverage_mapping_buffer)); } -- cgit 1.4.1-3-g733a5 From e02e9582d253ffaa04c9137f732efb0b80f98907 Mon Sep 17 00:00:00 2001 From: Jacob Bramley Date: Wed, 19 Jan 2022 14:51:59 +0000 Subject: Use error-on-mismatch policy for PAuth module flags. This agrees with Clang, and avoids an error when using LTO with mixed C/Rust. LLVM considers different behaviour flags to be a mismatch, even when the flag value itself is the same. This also makes the flag setting explicit for all uses of LLVMRustAddModuleFlag. --- compiler/rustc_codegen_llvm/src/context.rs | 31 ++++++++++++++++++------ compiler/rustc_codegen_llvm/src/debuginfo/mod.rs | 15 ++++++++++-- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 31 +++++++++++++++++++++++- compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp | 9 ++++--- 4 files changed, 73 insertions(+), 13 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src') diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index bb16bc5dccd..8672459b5da 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -215,16 +215,19 @@ pub unsafe fn create_module<'ll>( // to ensure intrinsic calls don't use it. if !sess.needs_plt() { let avoid_plt = "RtLibUseGOT\0".as_ptr().cast(); - llvm::LLVMRustAddModuleFlag(llmod, avoid_plt, 1); + llvm::LLVMRustAddModuleFlag(llmod, llvm::LLVMModFlagBehavior::Warning, avoid_plt, 1); } if sess.is_sanitizer_cfi_enabled() { // FIXME(rcvalle): Add support for non canonical jump tables. let canonical_jump_tables = "CFI Canonical Jump Tables\0".as_ptr().cast(); - // FIXME(rcvalle): Add it with Override behavior flag--LLVMRustAddModuleFlag adds it with - // Warning behavior flag. Add support for specifying the behavior flag to - // LLVMRustAddModuleFlag. - llvm::LLVMRustAddModuleFlag(llmod, canonical_jump_tables, 1); + // FIXME(rcvalle): Add it with Override behavior flag. + llvm::LLVMRustAddModuleFlag( + llmod, + llvm::LLVMModFlagBehavior::Warning, + canonical_jump_tables, + 1, + ); } // Control Flow Guard is currently only supported by the MSVC linker on Windows. @@ -233,11 +236,21 @@ pub unsafe fn create_module<'ll>( CFGuard::Disabled => {} CFGuard::NoChecks => { // Set `cfguard=1` module flag to emit metadata only. - llvm::LLVMRustAddModuleFlag(llmod, "cfguard\0".as_ptr() as *const _, 1) + llvm::LLVMRustAddModuleFlag( + llmod, + llvm::LLVMModFlagBehavior::Warning, + "cfguard\0".as_ptr() as *const _, + 1, + ) } CFGuard::Checks => { // Set `cfguard=2` module flag to emit metadata and checks. - llvm::LLVMRustAddModuleFlag(llmod, "cfguard\0".as_ptr() as *const _, 2) + llvm::LLVMRustAddModuleFlag( + llmod, + llvm::LLVMModFlagBehavior::Warning, + "cfguard\0".as_ptr() as *const _, + 2, + ) } } } @@ -247,24 +260,28 @@ pub unsafe fn create_module<'ll>( llvm::LLVMRustAddModuleFlag( llmod, + llvm::LLVMModFlagBehavior::Error, "branch-target-enforcement\0".as_ptr().cast(), bti.into(), ); llvm::LLVMRustAddModuleFlag( llmod, + llvm::LLVMModFlagBehavior::Error, "sign-return-address\0".as_ptr().cast(), pac.is_some().into(), ); let pac_opts = pac.unwrap_or(PacRet { leaf: false, key: PAuthKey::A }); llvm::LLVMRustAddModuleFlag( llmod, + llvm::LLVMModFlagBehavior::Error, "sign-return-address-all\0".as_ptr().cast(), pac_opts.leaf.into(), ); let is_bkey = if pac_opts.key == PAuthKey::A { false } else { true }; llvm::LLVMRustAddModuleFlag( llmod, + llvm::LLVMModFlagBehavior::Error, "sign-return-address-with-bkey\0".as_ptr().cast(), is_bkey.into(), ); diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index 61e49fab6ff..28eb8e2a0a4 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -108,18 +108,29 @@ impl<'a, 'tcx> CrateDebugContext<'a, 'tcx> { // This can be overridden using --llvm-opts -dwarf-version,N. // Android has the same issue (#22398) if let Some(version) = sess.target.dwarf_version { - llvm::LLVMRustAddModuleFlag(self.llmod, "Dwarf Version\0".as_ptr().cast(), version) + llvm::LLVMRustAddModuleFlag( + self.llmod, + llvm::LLVMModFlagBehavior::Warning, + "Dwarf Version\0".as_ptr().cast(), + version, + ) } // Indicate that we want CodeView debug information on MSVC if sess.target.is_like_msvc { - llvm::LLVMRustAddModuleFlag(self.llmod, "CodeView\0".as_ptr().cast(), 1) + llvm::LLVMRustAddModuleFlag( + self.llmod, + llvm::LLVMModFlagBehavior::Warning, + "CodeView\0".as_ptr().cast(), + 1, + ) } // Prevent bitcode readers from deleting the debug info. let ptr = "Debug Info Version\0".as_ptr(); llvm::LLVMRustAddModuleFlag( self.llmod, + llvm::LLVMModFlagBehavior::Warning, ptr.cast(), llvm::LLVMRustDebugMetadataVersion(), ); diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index a1c7d2b4f61..2b102188790 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -61,6 +61,26 @@ pub enum LLVMMachineType { ARM = 0x01c0, } +/// LLVM's Module::ModFlagBehavior, defined in llvm/include/llvm/IR/Module.h. +/// +/// When merging modules (e.g. during LTO), their metadata flags are combined. Conflicts are +/// resolved according to the merge behaviors specified here. Flags differing only in merge +/// behavior are still considered to be in conflict. +/// +/// In order for Rust-C LTO to work, we must specify behaviors compatible with Clang. Notably, +/// 'Error' and 'Warning' cannot be mixed for a given flag. +#[derive(Copy, Clone, PartialEq)] +#[repr(C)] +pub enum LLVMModFlagBehavior { + Error = 1, + Warning = 2, + Require = 3, + Override = 4, + Append = 5, + AppendUnique = 6, + Max = 7, +} + // Consts for the LLVM CallConv type, pre-cast to usize. /// LLVM CallingConv::ID. Should we wrap this? @@ -1895,7 +1915,16 @@ extern "C" { pub fn LLVMRustIsRustLLVM() -> bool; - pub fn LLVMRustAddModuleFlag(M: &Module, name: *const c_char, value: u32); + /// Add LLVM module flags. + /// + /// In order for Rust-C LTO to work, module flags must be compatible with Clang. What + /// "compatible" means depends on the merge behaviors involved. + pub fn LLVMRustAddModuleFlag( + M: &Module, + merge_behavior: LLVMModFlagBehavior, + name: *const c_char, + value: u32, + ); pub fn LLVMRustMetadataAsValue<'a>(C: &'a Context, MD: &'a Metadata) -> &'a Value; diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index c21e4acbefe..dcd6327c92f 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -722,9 +722,12 @@ extern "C" bool LLVMRustIsRustLLVM() { #endif } -extern "C" void LLVMRustAddModuleFlag(LLVMModuleRef M, const char *Name, - uint32_t Value) { - unwrap(M)->addModuleFlag(Module::Warning, Name, Value); +extern "C" void LLVMRustAddModuleFlag( + LLVMModuleRef M, + Module::ModFlagBehavior MergeBehavior, + const char *Name, + uint32_t Value) { + unwrap(M)->addModuleFlag(MergeBehavior, Name, Value); } extern "C" LLVMValueRef LLVMRustMetadataAsValue(LLVMContextRef C, LLVMMetadataRef MD) { -- cgit 1.4.1-3-g733a5