From bcf755562a1e7c2ed4f6a292b3c82afc9b5b5fe6 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 15 Mar 2021 16:32:45 -0700 Subject: [PATCH 1/4] coverage bug fixes and optimization support Adjusted LLVM codegen for code compiled with `-Zinstrument-coverage` to address multiple, somewhat related issues. Fixed a significant flaw in prior coverage solution: Every counter generated a new counter variable, but there should have only been one counter variable per function. This appears to have bloated .profraw files significantly. (For a small program, it increased the size by about 40%. I have not tested large programs, but there is anecdotal evidence that profraw files were way too large. This is a good fix, regardless, but hopefully it also addresses related issues. Fixes: #82144 Invalid LLVM coverage data produced when compiled with -C opt-level=1 Existing tests now work up to at least `opt-level=3`. This required a detailed analysis of the LLVM IR, comparisons with Clang C++ LLVM IR when compiled with coverage, and a lot of trial and error with codegen adjustments. The biggest hurdle was figuring out how to continue to support coverage results for unused functions and generics. Rust's coverage results have three advantages over Clang's coverage results: 1. Rust's coverage map does not include any overlapping code regions, making coverage counting unambiguous. 2. Rust generates coverage results (showing zero counts) for all unused functions, including generics. (Clang does not generate coverage for uninstantiated template functions.) 3. Rust's unused functions produce minimal stubbed functions in LLVM IR, sufficient for including in the coverage results; while Clang must generate the complete LLVM IR for each unused function, even though it will never be called. This PR removes the previous hack of attempting to inject coverage into some other existing function instance, and generates dedicated instances for each unused function. This change, and a few other adjustments (similar to what is required for `-C link-dead-code`, but with lower impact), makes it possible to support LLVM optimizations. Fixes: #79651 Coverage report: "Unexecuted instantiation:..." for a generic function from multiple crates Fixed by removing the aforementioned hack. Some "Unexecuted instantiation" notices are unavoidable, as explained in the `used_crate.rs` test, but `-Zinstrument-coverage` has new options to back off support for either unused generics, or all unused functions, which avoids the notice, at the cost of less coverage of unused functions. Fixes: #82875 Invalid LLVM coverage data produced with crate brotli_decompressor Fixed by disabling the LLVM function attribute that forces inlining, if `-Z instrument-coverage` is enabled. This attribute is applied to Rust functions with `#[inline(always)], and in some cases, the forced inlining breaks coverage instrumentation and reports. --- compiler/rustc_codegen_llvm/src/base.rs | 2 +- compiler/rustc_codegen_llvm/src/context.rs | 8 +- .../src/coverageinfo/mapgen.rs | 228 +++++++--------- .../src/coverageinfo/mod.rs | 141 +++++++++- compiler/rustc_codegen_ssa/src/back/link.rs | 2 +- .../src/back/symbol_export.rs | 4 +- compiler/rustc_codegen_ssa/src/back/write.rs | 2 +- .../rustc_codegen_ssa/src/coverageinfo/map.rs | 25 +- .../rustc_codegen_ssa/src/mir/coverageinfo.rs | 2 +- .../src/traits/coverageinfo.rs | 33 ++- compiler/rustc_codegen_ssa/src/traits/mod.rs | 4 +- compiler/rustc_interface/src/tests.rs | 3 +- compiler/rustc_metadata/src/creader.rs | 2 +- compiler/rustc_middle/src/mir/mod.rs | 7 +- compiler/rustc_middle/src/mir/mono.rs | 9 +- compiler/rustc_middle/src/query/mod.rs | 10 +- .../src/monomorphize/partitioning/mod.rs | 8 +- .../rustc_mir/src/transform/coverage/query.rs | 45 ++-- compiler/rustc_mir/src/transform/mod.rs | 7 +- compiler/rustc_session/src/config.rs | 42 ++- compiler/rustc_session/src/options.rs | 42 ++- compiler/rustc_session/src/session.rs | 15 ++ compiler/rustc_typeck/src/collect.rs | 14 +- .../coverage-llvmir/Makefile | 2 +- .../coverage-reports/Makefile | 8 +- .../expected_show_coverage.async.txt | 15 +- .../expected_show_coverage.closure.txt | 2 +- .../expected_show_coverage.doctest.txt | 6 + .../expected_show_coverage.generics.txt | 4 +- .../expected_show_coverage.partial_eq.txt | 5 + .../expected_show_coverage.unused.txt | 62 +++++ .../expected_show_coverage.uses_crate.txt | 93 ++++--- ...pected_show_coverage.uses_inline_crate.txt | 156 +++++++++++ .../coverage-spanview/Makefile | 16 +- ...osure#0}.-------.InstrumentCoverage.0.html | 18 +- .../async.j.-------.InstrumentCoverage.0.html | 26 +- ...osure#0}.-------.InstrumentCoverage.0.html | 5 +- ...osure#4}.-------.InstrumentCoverage.0.html | 7 +- ...used.foo.-------.InstrumentCoverage.0.html | 87 ++++++ ...sed.main.-------.InstrumentCoverage.0.html | 98 +++++++ ...sed_func.-------.InstrumentCoverage.0.html | 86 ++++++ ...ed_func2.-------.InstrumentCoverage.0.html | 86 ++++++ ...ed_func3.-------.InstrumentCoverage.0.html | 86 ++++++ ...ate_func.-------.InstrumentCoverage.0.html | 87 ++++++ ...function.-------.InstrumentCoverage.0.html | 115 ++++++++ ...function.-------.InstrumentCoverage.0.html | 133 ++++++++++ ...function.-------.InstrumentCoverage.0.html | 115 ++++++++ ...ib_crate.-------.InstrumentCoverage.0.html | 190 +++++++++++++ ...function.-------.InstrumentCoverage.0.html | 133 ++++++++++ ...function.-------.InstrumentCoverage.0.html | 110 ++++++++ ...function.-------.InstrumentCoverage.0.html | 110 ++++++++ ...function.-------.InstrumentCoverage.0.html | 133 ++++++++++ ...function.-------.InstrumentCoverage.0.html | 133 ++++++++++ ...function.-------.InstrumentCoverage.0.html | 133 ++++++++++ ...ate.main.-------.InstrumentCoverage.0.html | 249 ++++++++++++++++++ src/test/run-make-fulldeps/coverage/async.rs | 15 +- .../run-make-fulldeps/coverage/closure.rs | 2 +- .../coverage/lib/used_crate.rs | 68 +++-- .../coverage/lib/used_inline_crate.rs | 90 +++++++ src/test/run-make-fulldeps/coverage/unused.rs | 39 +++ .../run-make-fulldeps/coverage/uses_crate.rs | 2 +- .../coverage/uses_inline_crate.rs | 17 ++ 62 files changed, 3058 insertions(+), 339 deletions(-) create mode 100644 src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.unused.txt create mode 100644 src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_inline_crate.txt create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.foo.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.main.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_func.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_func2.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_func3.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_template_func.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.unused_function.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.unused_generic_function.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.unused_private_function.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.use_this_lib_crate.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_from_bin_crate_and_lib_crate_generic_function.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_function.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_inline_function.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_only_from_bin_crate_generic_function.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_only_from_this_lib_crate_generic_function.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_with_same_type_from_bin_crate_and_lib_crate_generic_function.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.uses_inline_crate/uses_inline_crate.main.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage/lib/used_inline_crate.rs create mode 100644 src/test/run-make-fulldeps/coverage/unused.rs create mode 100644 src/test/run-make-fulldeps/coverage/uses_inline_crate.rs diff --git a/compiler/rustc_codegen_llvm/src/base.rs b/compiler/rustc_codegen_llvm/src/base.rs index d5be3132dee10..db8abdd9b1353 100644 --- a/compiler/rustc_codegen_llvm/src/base.rs +++ b/compiler/rustc_codegen_llvm/src/base.rs @@ -143,7 +143,7 @@ pub fn compile_codegen_unit( // Finalize code coverage by injecting the coverage map. Note, the coverage map will // also be added to the `llvm.used` variable, created next. - if cx.sess().opts.debugging_opts.instrument_coverage { + if cx.sess().instrument_coverage() { cx.coverageinfo_finalize(); } diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index 21473f3b1143c..4d0498ac78373 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -79,7 +79,7 @@ pub struct CodegenCx<'ll, 'tcx> { pub pointee_infos: RefCell, Size), Option>>, pub isize_ty: &'ll Type, - pub coverage_cx: Option>, + pub coverage_cx: Option>, pub dbg_cx: Option>, eh_personality: Cell>, @@ -280,7 +280,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { let (llcx, llmod) = (&*llvm_module.llcx, llvm_module.llmod()); - let coverage_cx = if tcx.sess.opts.debugging_opts.instrument_coverage { + let coverage_cx = if tcx.sess.instrument_coverage() { let covctx = coverageinfo::CrateCoverageContext::new(); Some(covctx) } else { @@ -331,7 +331,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { } #[inline] - pub fn coverage_context(&'a self) -> Option<&'a coverageinfo::CrateCoverageContext<'tcx>> { + pub fn coverage_context(&'a self) -> Option<&'a coverageinfo::CrateCoverageContext<'ll, 'tcx>> { self.coverage_cx.as_ref() } } @@ -712,7 +712,7 @@ impl CodegenCx<'b, 'tcx> { ifn!("llvm.va_end", fn(i8p) -> void); ifn!("llvm.va_copy", fn(i8p, i8p) -> void); - if self.sess().opts.debugging_opts.instrument_coverage { + if self.sess().instrument_coverage() { ifn!("llvm.instrprof.increment", fn(i8p, t_i64, t_i32, t_i32) -> void); } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 352638aa88ee8..239f786a727bd 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -3,13 +3,12 @@ use crate::coverageinfo; use crate::llvm; use llvm::coverageinfo::CounterMappingRegion; -use rustc_codegen_ssa::coverageinfo::map::{Counter, CounterExpression, FunctionCoverage}; -use rustc_codegen_ssa::traits::ConstMethods; +use rustc_codegen_ssa::coverageinfo::map::{Counter, CounterExpression}; +use rustc_codegen_ssa::traits::{ConstMethods, CoverageInfoMethods}; use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet}; use rustc_hir::def_id::{DefId, DefIdSet, LOCAL_CRATE}; use rustc_llvm::RustString; use rustc_middle::mir::coverage::CodeRegion; -use rustc_middle::ty::{Instance, TyCtxt}; use rustc_span::Symbol; use std::ffi::CString; @@ -20,16 +19,17 @@ use tracing::debug; /// /// This Coverage Map complies with Coverage Mapping Format version 4 (zero-based encoded as 3), /// as defined at [LLVM Code Coverage Mapping Format](https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/docs/CoverageMappingFormat.rst#llvm-code-coverage-mapping-format) -/// and published in Rust's current (November 2020) fork of LLVM. This version is supported by the -/// LLVM coverage tools (`llvm-profdata` and `llvm-cov`) bundled with Rust's fork of LLVM. +/// and published in Rust's November 2020 fork of LLVM. This version is supported by the LLVM +/// coverage tools (`llvm-profdata` and `llvm-cov`) bundled with Rust's fork of LLVM. /// /// Consequently, Rust's bundled version of Clang also generates Coverage Maps compliant with -/// version 3. Clang's implementation of Coverage Map generation was referenced when implementing -/// this Rust version, and though the format documentation is very explicit and detailed, some -/// undocumented details in Clang's implementation (that may or may not be important) were also -/// replicated for Rust's Coverage Map. +/// the same version. Clang's implementation of Coverage Map generation was referenced when +/// implementing this Rust version, and though the format documentation is very explicit and +/// detailed, some undocumented details in Clang's implementation (that may or may not be important) +/// were also replicated for Rust's Coverage Map. pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { let tcx = cx.tcx; + // Ensure LLVM supports Coverage Map Version 4 (encoded as a zero-based value: 3). // If not, the LLVM Version must be less than 11. let version = coverageinfo::mapping_version(); @@ -39,17 +39,24 @@ pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { debug!("Generating coverage map for CodegenUnit: `{}`", cx.codegen_unit.name()); - let mut function_coverage_map = match cx.coverage_context() { + // In order to show that unused functions have coverage counts of zero (0), LLVM requires the + // functions exist. Generate synthetic functions with a (required) single counter, and add the + // MIR `Coverage` code regions to the `function_coverage_map`, before calling + // `ctx.take_function_coverage_map()`. + if !tcx.sess.instrument_coverage_except_unused_functions() { + add_unused_functions(cx); + } + + let function_coverage_map = match cx.coverage_context() { Some(ctx) => ctx.take_function_coverage_map(), None => return, }; + if function_coverage_map.is_empty() { // This module has no functions with coverage instrumentation return; } - add_unreachable_coverage(tcx, &mut function_coverage_map); - let mut mapgen = CoverageMapGenerator::new(); // Encode coverage mappings and generate function records @@ -57,7 +64,8 @@ pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { for (instance, function_coverage) in function_coverage_map { debug!("Generate function coverage for {}, {:?}", cx.codegen_unit.name(), instance); let mangled_function_name = tcx.symbol_name(instance).to_string(); - let function_source_hash = function_coverage.source_hash(); + let source_hash = function_coverage.source_hash(); + let is_used = function_coverage.is_used(); let (expressions, counter_regions) = function_coverage.get_expressions_and_counter_regions(); @@ -69,7 +77,7 @@ pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { "Every `FunctionCoverage` should have at least one counter" ); - function_data.push((mangled_function_name, function_source_hash, coverage_mapping_buffer)); + function_data.push((mangled_function_name, source_hash, is_used, coverage_mapping_buffer)); } // Encode all filenames referenced by counters/expressions in this module @@ -84,13 +92,14 @@ pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { // Generate the LLVM IR representation of the coverage map and store it in a well-known global let cov_data_val = mapgen.generate_coverage_map(cx, version, filenames_size, filenames_val); - for (mangled_function_name, function_source_hash, coverage_mapping_buffer) in function_data { + for (mangled_function_name, source_hash, is_used, coverage_mapping_buffer) in function_data { save_function_record( cx, mangled_function_name, - function_source_hash, + source_hash, filenames_ref, coverage_mapping_buffer, + is_used, ); } @@ -201,9 +210,10 @@ impl CoverageMapGenerator { fn save_function_record( cx: &CodegenCx<'ll, 'tcx>, mangled_function_name: String, - function_source_hash: u64, + source_hash: u64, filenames_ref: u64, coverage_mapping_buffer: Vec, + is_used: bool, ) { // Concatenate the encoded coverage mappings let coverage_mapping_size = coverage_mapping_buffer.len(); @@ -212,128 +222,120 @@ fn save_function_record( let func_name_hash = coverageinfo::hash_str(&mangled_function_name); let func_name_hash_val = cx.const_u64(func_name_hash); let coverage_mapping_size_val = cx.const_u32(coverage_mapping_size as u32); - let func_hash_val = cx.const_u64(function_source_hash); + let source_hash_val = cx.const_u64(source_hash); let filenames_ref_val = cx.const_u64(filenames_ref); let func_record_val = cx.const_struct( &[ func_name_hash_val, coverage_mapping_size_val, - func_hash_val, + source_hash_val, filenames_ref_val, coverage_mapping_val, ], /*packed=*/ true, ); - // At the present time, the coverage map for Rust assumes every instrumented function `is_used`. - // Note that Clang marks functions as "unused" in `CodeGenPGO::emitEmptyCounterMapping`. (See: - // https://github.com/rust-lang/llvm-project/blob/de02a75e398415bad4df27b4547c25b896c8bf3b/clang%2Flib%2FCodeGen%2FCodeGenPGO.cpp#L877-L878 - // for example.) - // - // It's not yet clear if or how this may be applied to Rust in the future, but the `is_used` - // argument is available and handled similarly. - let is_used = true; coverageinfo::save_func_record_to_mod(cx, func_name_hash, func_record_val, is_used); } /// When finalizing the coverage map, `FunctionCoverage` only has the `CodeRegion`s and counters for /// the functions that went through codegen; such as public functions and "used" functions /// (functions referenced by other "used" or public items). Any other functions considered unused, -/// or "Unreachable" were still parsed and processed through the MIR stage. +/// or "Unreachable", were still parsed and processed through the MIR stage, but were not +/// codegenned. (Note that `-Clink-dead-code` can force some unused code to be codegenned, but +/// that flag is known to cause other errors, when combined with `-Z instrument-coverage`; and +/// `-Clink-dead-code` will not generate code for unused generic functions.) /// -/// We can find the unreachable functions by the set difference of all MIR `DefId`s (`tcx` query -/// `mir_keys`) minus the codegenned `DefId`s (`tcx` query `collect_and_partition_mono_items`). +/// We can find the unused functions (including generic functions) by the set difference of all MIR +/// `DefId`s (`tcx` query `mir_keys`) minus the codegenned `DefId`s (`tcx` query +/// `collect_and_partition_mono_items`). /// /// *HOWEVER* the codegenned `DefId`s are partitioned across multiple `CodegenUnit`s (CGUs), and /// this function is processing a `function_coverage_map` for the functions (`Instance`/`DefId`) -/// allocated to only one of those CGUs. We must NOT inject any "Unreachable" functions's -/// `CodeRegion`s more than once, so we have to pick which CGU's `function_coverage_map` to add -/// each "Unreachable" function to. -/// -/// Some constraints: -/// -/// 1. The file name of an "Unreachable" function must match the file name of the existing -/// codegenned (covered) function to which the unreachable code regions will be added. -/// 2. The function to which the unreachable code regions will be added must not be a generic -/// function (must not have type parameters) because the coverage tools will get confused -/// if the codegenned function has more than one instantiation and additional `CodeRegion`s -/// attached to only one of those instantiations. -fn add_unreachable_coverage<'tcx>( - tcx: TyCtxt<'tcx>, - function_coverage_map: &mut FxHashMap, FunctionCoverage<'tcx>>, -) { +/// allocated to only one of those CGUs. We must NOT inject any unused functions's `CodeRegion`s +/// more than once, so we have to pick a CGUs `function_coverage_map` into which the unused +/// function will be inserted. +fn add_unused_functions<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { + let tcx = cx.tcx; + // FIXME(#79622): Can this solution be simplified and/or improved? Are there other sources // of compiler state data that might help (or better sources that could be exposed, but // aren't yet)? - // Note: If the crate *only* defines generic functions, there are no codegenerated non-generic - // functions to add any unreachable code to. In this case, the unreachable code regions will - // have no coverage, instead of having coverage with zero executions. - // - // This is probably still an improvement over Clang, which does not generate any coverage - // for uninstantiated template functions. - - let has_non_generic_def_ids = - function_coverage_map.keys().any(|instance| instance.def.attrs(tcx).len() == 0); - - if !has_non_generic_def_ids { - // There are no non-generic functions to add unreachable `CodeRegion`s to - return; - } + let ignore_unused_generics = tcx.sess.instrument_coverage_except_unused_generics(); - let all_def_ids: DefIdSet = - tcx.mir_keys(LOCAL_CRATE).iter().map(|local_def_id| local_def_id.to_def_id()).collect(); + let all_def_ids: DefIdSet = tcx + .mir_keys(LOCAL_CRATE) + .iter() + .filter_map(|local_def_id| { + let def_id = local_def_id.to_def_id(); + if ignore_unused_generics && tcx.generics_of(def_id).count() > 0 { + return None; + } + Some(local_def_id.to_def_id()) + }) + .collect(); let codegenned_def_ids = tcx.codegened_and_inlined_items(LOCAL_CRATE); - let mut unreachable_def_ids_by_file: FxHashMap> = FxHashMap::default(); + let mut unused_def_ids_by_file: FxHashMap> = FxHashMap::default(); for &non_codegenned_def_id in all_def_ids.difference(codegenned_def_ids) { - // Make sure the non-codegenned (unreachable) function has a file_name + // Make sure the non-codegenned (unused) function has a file_name if let Some(non_codegenned_file_name) = tcx.covered_file_name(non_codegenned_def_id) { - let def_ids = unreachable_def_ids_by_file - .entry(*non_codegenned_file_name) - .or_insert_with(Vec::new); + let def_ids = + unused_def_ids_by_file.entry(*non_codegenned_file_name).or_insert_with(Vec::new); def_ids.push(non_codegenned_def_id); } } - if unreachable_def_ids_by_file.is_empty() { - // There are no unreachable functions with file names to add (in any CGU) + if unused_def_ids_by_file.is_empty() { + // There are no unused functions with file names to add (in any CGU) return; } - // Since there may be multiple `CodegenUnit`s, some codegenned_def_ids may be codegenned in a - // different CGU, and will be added to the function_coverage_map for each CGU. Determine which - // function_coverage_map has the responsibility for publishing unreachable coverage - // based on file name: + // Each `CodegenUnit` (CGU) has its own function_coverage_map, and generates a specific binary + // with its own coverage map. + // + // Each covered function `Instance` can be included in only one coverage map, produced from a + // specific function_coverage_map, from a specific CGU. // - // For each covered file name, sort ONLY the non-generic codegenned_def_ids, and if - // covered_def_ids.contains(the first def_id) for a given file_name, add the unreachable code - // region in this function_coverage_map. Otherwise, ignore it and assume another CGU's - // function_coverage_map will be adding it (because it will be first for one, and only one, - // of them). + // Since unused functions did not generate code, they are not associated with any CGU yet. + // + // To avoid injecting the unused functions in multiple coverage maps (for multiple CGUs) + // determine which function_coverage_map has the responsibility for publishing unreachable + // coverage, based on file name: For each unused function, find the CGU that generates the + // first function (based on sorted `DefId`) from the same file. + // + // Add a new `FunctionCoverage` to the `function_coverage_map`, with unreachable code regions + // for each region in it's MIR. + + // Convert the `HashSet` of `codegenned_def_ids` to a sortable vector, and sort them. let mut sorted_codegenned_def_ids: Vec = codegenned_def_ids.iter().map(|def_id| *def_id).collect(); sorted_codegenned_def_ids.sort_unstable(); let mut first_covered_def_id_by_file: FxHashMap = FxHashMap::default(); for &def_id in sorted_codegenned_def_ids.iter() { - // Only consider non-generic functions, to potentially add unreachable code regions - if tcx.generics_of(def_id).count() == 0 { - if let Some(covered_file_name) = tcx.covered_file_name(def_id) { - // Only add files known to have unreachable functions - if unreachable_def_ids_by_file.contains_key(covered_file_name) { - first_covered_def_id_by_file.entry(*covered_file_name).or_insert(def_id); - } + if let Some(covered_file_name) = tcx.covered_file_name(def_id) { + // Only add files known to have unused functions + if unused_def_ids_by_file.contains_key(covered_file_name) { + first_covered_def_id_by_file.entry(*covered_file_name).or_insert(def_id); } } } // Get the set of def_ids with coverage regions, known by *this* CoverageContext. - let cgu_covered_def_ids: DefIdSet = - function_coverage_map.keys().map(|instance| instance.def.def_id()).collect(); + let cgu_covered_def_ids: DefIdSet = match cx.coverage_context() { + Some(ctx) => ctx + .function_coverage_map + .borrow() + .keys() + .map(|&instance| instance.def.def_id()) + .collect(), + None => return, + }; - let mut cgu_covered_files: FxHashSet = first_covered_def_id_by_file + let cgu_covered_files: FxHashSet = first_covered_def_id_by_file .iter() .filter_map( |(&file_name, def_id)| { @@ -342,49 +344,13 @@ fn add_unreachable_coverage<'tcx>( ) .collect(); - // Find the first covered, non-generic function (instance) for each cgu_covered_file. Take the - // unreachable code regions for that file, and add them to the function. - // - // There are three `for` loops here, but (a) the lists have already been reduced to the minimum - // required values, the lists are further reduced (by `remove()` calls) when elements are no - // longer needed, and there are several opportunities to branch out of loops early. - for (instance, function_coverage) in function_coverage_map.iter_mut() { - if instance.def.attrs(tcx).len() > 0 { - continue; - } - // The covered function is not generic... - let covered_def_id = instance.def.def_id(); - if let Some(covered_file_name) = tcx.covered_file_name(covered_def_id) { - if !cgu_covered_files.remove(&covered_file_name) { - continue; - } - // The covered function's file is one of the files with unreachable code regions, so - // all of the unreachable code regions for this file will be added to this function. - for def_id in - unreachable_def_ids_by_file.remove(&covered_file_name).into_iter().flatten() - { - // Note, this loop adds an unreachable code regions for each MIR-derived region. - // Alternatively, we could add a single code region for the maximum span of all - // code regions here. - // - // Observed downsides of this approach are: - // - // 1. The coverage results will appear inconsistent compared with the same (or - // similar) code in a function that is reached. - // 2. If the function is unreachable from one crate but reachable when compiling - // another referencing crate (such as a cross-crate reference to a - // generic function or inlined function), actual coverage regions overlaid - // on a single larger code span of `Zero` coverage can appear confusing or - // wrong. Chaning the unreachable coverage from a `code_region` to a - // `gap_region` can help, but still can look odd with `0` line counts for - // lines between executed (> 0) lines (such as for blank lines or comments). - for ®ion in tcx.covered_code_regions(def_id) { - function_coverage.add_unreachable_region(region.clone()); - } - } - if cgu_covered_files.is_empty() { - break; - } + // For each file for which this CGU is responsible for adding unused function coverage, + // get the `def_id`s for each unused function (if any), define a synthetic function with a + // single LLVM coverage counter, and add the function's coverage `CodeRegion`s. to the + // function_coverage_map. + for covered_file_name in cgu_covered_files { + for def_id in unused_def_ids_by_file.remove(&covered_file_name).into_iter().flatten() { + cx.define_unused_fn(def_id); } } } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index e47b8fde40fee..db61be407c954 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -1,5 +1,6 @@ use crate::llvm; +use crate::abi::{Abi, FnAbi}; use crate::builder::Builder; use crate::common::CodegenCx; @@ -7,33 +8,47 @@ use libc::c_uint; use llvm::coverageinfo::CounterMappingRegion; use rustc_codegen_ssa::coverageinfo::map::{CounterExpression, FunctionCoverage}; use rustc_codegen_ssa::traits::{ - BaseTypeMethods, CoverageInfoBuilderMethods, CoverageInfoMethods, MiscMethods, StaticMethods, + BaseTypeMethods, BuilderMethods, ConstMethods, CoverageInfoBuilderMethods, CoverageInfoMethods, + MiscMethods, StaticMethods, }; use rustc_data_structures::fx::FxHashMap; +use rustc_hir as hir; +use rustc_hir::def_id::DefId; use rustc_llvm::RustString; +use rustc_middle::bug; use rustc_middle::mir::coverage::{ CodeRegion, CounterValueReference, ExpressionOperandId, InjectedExpressionId, Op, }; +use rustc_middle::ty; +use rustc_middle::ty::layout::FnAbiExt; +use rustc_middle::ty::subst::InternalSubsts; use rustc_middle::ty::Instance; use std::cell::RefCell; use std::ffi::CString; +use std::iter; use tracing::debug; pub mod mapgen; +const UNUSED_FUNCTION_COUNTER_ID: CounterValueReference = CounterValueReference::START; + const VAR_ALIGN_BYTES: usize = 8; /// A context object for maintaining all state needed by the coverageinfo module. -pub struct CrateCoverageContext<'tcx> { +pub struct CrateCoverageContext<'ll, 'tcx> { // Coverage data for each instrumented function identified by DefId. pub(crate) function_coverage_map: RefCell, FunctionCoverage<'tcx>>>, + pub(crate) pgo_func_name_var_map: RefCell, &'ll llvm::Value>>, } -impl<'tcx> CrateCoverageContext<'tcx> { +impl<'ll, 'tcx> CrateCoverageContext<'ll, 'tcx> { pub fn new() -> Self { - Self { function_coverage_map: Default::default() } + Self { + function_coverage_map: Default::default(), + pgo_func_name_var_map: Default::default(), + } } pub fn take_function_coverage_map(&self) -> FxHashMap, FunctionCoverage<'tcx>> { @@ -41,23 +56,44 @@ impl<'tcx> CrateCoverageContext<'tcx> { } } -impl CoverageInfoMethods for CodegenCx<'ll, 'tcx> { +impl CoverageInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { fn coverageinfo_finalize(&self) { mapgen::finalize(self) } -} -impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> { - /// Calls llvm::createPGOFuncNameVar() with the given function instance's mangled function name. - /// The LLVM API returns an llvm::GlobalVariable containing the function name, with the specific - /// variable name and linkage required by LLVM InstrProf source-based coverage instrumentation. - fn create_pgo_func_name_var(&self, instance: Instance<'tcx>) -> Self::Value { - let llfn = self.cx.get_fn(instance); + fn get_pgo_func_name_var(&self, instance: Instance<'tcx>) -> &'ll llvm::Value { + if let Some(coverage_context) = self.coverage_context() { + debug!("getting pgo_func_name_var for instance={:?}", instance); + let mut pgo_func_name_var_map = coverage_context.pgo_func_name_var_map.borrow_mut(); + pgo_func_name_var_map + .entry(instance) + .or_insert_with(|| self.create_pgo_func_name_var(instance)) + } else { + bug!("Could not get the `coverage_context`"); + } + } + + /// Calls llvm::createPGOFuncNameVar() with the given function instance's + /// mangled function name. The LLVM API returns an llvm::GlobalVariable + /// containing the function name, with the specific variable name and + /// linkage required by LLVM InstrProf source-based coverage + /// instrumentation. Use `bx.get_pgo_func_name_var()` to ensure the variable + /// is only created once per `Instance`. + fn create_pgo_func_name_var(&self, instance: Instance<'tcx>) -> &'ll llvm::Value { let mangled_fn_name = CString::new(self.tcx.symbol_name(instance).name) .expect("error converting function name to C string"); + let llfn = self.get_fn(instance); unsafe { llvm::LLVMRustCoverageCreatePGOFuncNameVar(llfn, mangled_fn_name.as_ptr()) } } + fn define_unused_fn(&self, def_id: DefId) { + let instance = declare_unused_fn(self, &def_id); + codegen_unused_fn_and_counter(self, instance); + add_function_coverage(self, instance, def_id); + } +} + +impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> { fn set_function_source_hash( &mut self, instance: Instance<'tcx>, @@ -145,6 +181,86 @@ impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> { } } +fn declare_unused_fn(cx: &CodegenCx<'ll, 'tcx>, def_id: &DefId) -> Instance<'tcx> { + let tcx = cx.tcx; + + let instance = Instance::new( + *def_id, + InternalSubsts::for_item(tcx, *def_id, |param, _| { + if let ty::GenericParamDefKind::Lifetime = param.kind { + tcx.lifetimes.re_erased.into() + } else { + tcx.mk_param_from_def(param) + } + }), + ); + + let llfn = cx.declare_fn( + &tcx.symbol_name(instance).name, + &FnAbi::of_fn_ptr( + cx, + ty::Binder::dummy(tcx.mk_fn_sig( + iter::once(tcx.mk_unit()), + tcx.mk_unit(), + false, + hir::Unsafety::Unsafe, + Abi::Rust, + )), + &[], + ), + ); + + unsafe { + llvm::LLVMRustSetLinkage(llfn, llvm::Linkage::ExternalLinkage); + llvm::LLVMRustSetVisibility(llfn, llvm::Visibility::Hidden); + } + + cx.instances.borrow_mut().insert(instance, llfn); + + instance +} + +fn codegen_unused_fn_and_counter(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>) { + let llfn = cx.get_fn(instance); + let mut bx = Builder::new_block(cx, llfn, "unused_function"); + let fn_name = bx.get_pgo_func_name_var(instance); + let hash = bx.const_u64(0); + let num_counters = bx.const_u32(1); + let index = bx.const_u32(u32::from(UNUSED_FUNCTION_COUNTER_ID)); + debug!( + "codegen intrinsic instrprof.increment(fn_name={:?}, hash={:?}, num_counters={:?}, + index={:?}) for unused function: {:?}", + fn_name, hash, num_counters, index, instance + ); + bx.instrprof_increment(fn_name, hash, num_counters, index); + bx.ret_void(); +} + +fn add_function_coverage(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>, def_id: DefId) { + let tcx = cx.tcx; + + let mut function_coverage = FunctionCoverage::unused(tcx, instance); + for (index, &code_region) in tcx.covered_code_regions(def_id).iter().enumerate() { + if index == 0 { + // Insert at least one real counter so the LLVM CoverageMappingReader will find expected + // definitions. + function_coverage.add_counter(UNUSED_FUNCTION_COUNTER_ID, code_region.clone()); + } + // Add a Zero Counter for every code region. + // + // Even though the first coverage region already has an actual Counter, `llvm-cov` will not + // always report it. Re-adding an unreachable region (zero counter) for the same region + // seems to help produce the expected coverage. + function_coverage.add_unreachable_region(code_region.clone()); + } + + if let Some(coverage_context) = cx.coverage_context() { + coverage_context.function_coverage_map.borrow_mut().insert(instance, function_coverage); + } else { + bug!("Could not get the `coverage_context`"); + } +} + pub(crate) fn write_filenames_section_to_buffer<'a>( filenames: impl IntoIterator, buffer: &RustString, @@ -177,6 +293,7 @@ pub(crate) fn write_mapping_to_buffer( ); } } + pub(crate) fn hash_str(strval: &str) -> u64 { let strval = CString::new(strval).expect("null error converting hashable str to C string"); unsafe { llvm::LLVMRustCoverageHashCString(strval.as_ptr()) } diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index b11821b7db0a2..4d6bc8381300a 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1746,7 +1746,7 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( ); // OBJECT-FILES-NO, AUDIT-ORDER - if sess.opts.cg.profile_generate.enabled() || sess.opts.debugging_opts.instrument_coverage { + if sess.opts.cg.profile_generate.enabled() || sess.instrument_coverage() { cmd.pgo_gen(); } diff --git a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs index 9a6f8cde1b25d..abad8281d3a69 100644 --- a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs +++ b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs @@ -188,9 +188,7 @@ fn exported_symbols_provider_local( } } - if tcx.sess.opts.debugging_opts.instrument_coverage - || tcx.sess.opts.cg.profile_generate.enabled() - { + if tcx.sess.instrument_coverage() || tcx.sess.opts.cg.profile_generate.enabled() { // These are weak symbols that point to the profile version and the // profile name, which need to be treated as exported so LTO doesn't nix // them. diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 490b3d3311284..7a8d8fb13043b 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -176,7 +176,7 @@ impl ModuleConfig { // The rustc option `-Zinstrument_coverage` injects intrinsic calls to // `llvm.instrprof.increment()`, which requires the LLVM `instrprof` pass. - if sess.opts.debugging_opts.instrument_coverage { + if sess.instrument_coverage() { passes.push("instrprof".to_owned()); } passes diff --git a/compiler/rustc_codegen_ssa/src/coverageinfo/map.rs b/compiler/rustc_codegen_ssa/src/coverageinfo/map.rs index af6482fdbc24f..7a17bced1c0b0 100644 --- a/compiler/rustc_codegen_ssa/src/coverageinfo/map.rs +++ b/compiler/rustc_codegen_ssa/src/coverageinfo/map.rs @@ -31,27 +31,44 @@ pub struct Expression { pub struct FunctionCoverage<'tcx> { instance: Instance<'tcx>, source_hash: u64, + is_used: bool, counters: IndexVec>, expressions: IndexVec>, unreachable_regions: Vec, } impl<'tcx> FunctionCoverage<'tcx> { + /// Creates a new set of coverage data for a used (called) function. pub fn new(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> Self { + Self::create(tcx, instance, true) + } + + /// Creates a new set of coverage data for an unused (never called) function. + pub fn unused(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> Self { + Self::create(tcx, instance, false) + } + + fn create(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, is_used: bool) -> Self { let coverageinfo = tcx.coverageinfo(instance.def_id()); debug!( - "FunctionCoverage::new(instance={:?}) has coverageinfo={:?}", - instance, coverageinfo + "FunctionCoverage::new(instance={:?}) has coverageinfo={:?}. is_used={}", + instance, coverageinfo, is_used ); Self { instance, source_hash: 0, // will be set with the first `add_counter()` + is_used, counters: IndexVec::from_elem_n(None, coverageinfo.num_counters as usize), expressions: IndexVec::from_elem_n(None, coverageinfo.num_expressions as usize), unreachable_regions: Vec::new(), } } + /// Returns true for a used (called) function, and false for an unused function. + pub fn is_used(&self) -> bool { + self.is_used + } + /// Sets the function source hash value. If called multiple times for the same function, all /// calls should have the same hash value. pub fn set_function_source_hash(&mut self, source_hash: u64) { @@ -128,8 +145,8 @@ impl<'tcx> FunctionCoverage<'tcx> { &'a self, ) -> (Vec, impl Iterator) { assert!( - self.source_hash != 0, - "No counters provided the source_hash for function: {:?}", + self.source_hash != 0 || !self.is_used, + "No counters provided the source_hash for used function: {:?}", self.instance ); diff --git a/compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs b/compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs index 5ab1baafb57de..e2f75b2e337cc 100644 --- a/compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs +++ b/compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs @@ -33,7 +33,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let coverageinfo = bx.tcx().coverageinfo(instance.def_id()); - let fn_name = bx.create_pgo_func_name_var(instance); + let fn_name = bx.get_pgo_func_name_var(instance); let hash = bx.const_u64(function_source_hash); let num_counters = bx.const_u32(coverageinfo.num_counters); let index = bx.const_u32(u32::from(id)); diff --git a/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs b/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs index 95bddfb4b41d1..c715752839687 100644 --- a/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs +++ b/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs @@ -1,14 +1,41 @@ use super::BackendTypes; +use rustc_hir::def_id::DefId; use rustc_middle::mir::coverage::*; use rustc_middle::ty::Instance; -pub trait CoverageInfoMethods: BackendTypes { +pub trait CoverageInfoMethods<'tcx>: BackendTypes { fn coverageinfo_finalize(&self); -} -pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes { + /// Functions with MIR-based coverage are normally codegenned _only_ if + /// called. LLVM coverage tools typically expect every function to be + /// defined (even if unused), with at least one call to LLVM intrinsic + /// `instrprof.increment`. + /// + /// Codegen a small function that will never be called, with one counter + /// that will never be incremented. + /// + /// For used/called functions, the coverageinfo was already added to the + /// `function_coverage_map` (keyed by function `Instance`) during codegen. + /// But in this case, since the unused function was _not_ previously + /// codegenned, collect the coverage `CodeRegion`s from the MIR and add + /// them. The first `CodeRegion` is used to add a single counter, with the + /// same counter ID used in the injected `instrprof.increment` intrinsic + /// call. Since the function is never called, all other `CodeRegion`s can be + /// added as `unreachable_region`s. + fn define_unused_fn(&self, def_id: DefId); + + /// For LLVM codegen, returns a function-specific `Value` for a global + /// string, to hold the function name passed to LLVM intrinsic + /// `instrprof.increment()`. The `Value` is only created once per instance. + /// Multiple invocations with the same instance return the same `Value`. + fn get_pgo_func_name_var(&self, instance: Instance<'tcx>) -> Self::Value; + + /// Creates a new PGO function name variable. This should only be called + /// to fill in the unused function names array. fn create_pgo_func_name_var(&self, instance: Instance<'tcx>) -> Self::Value; +} +pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes { /// Returns true if the function source hash was added to the coverage map (even if it had /// already been added, for this instance). Returns false *only* if `-Z instrument-coverage` is /// not enabled (a coverage map is not being generated). diff --git a/compiler/rustc_codegen_ssa/src/traits/mod.rs b/compiler/rustc_codegen_ssa/src/traits/mod.rs index 8ada6c10479da..be2e0ea230f30 100644 --- a/compiler/rustc_codegen_ssa/src/traits/mod.rs +++ b/compiler/rustc_codegen_ssa/src/traits/mod.rs @@ -58,7 +58,7 @@ pub trait CodegenMethods<'tcx>: + MiscMethods<'tcx> + ConstMethods<'tcx> + StaticMethods - + CoverageInfoMethods + + CoverageInfoMethods<'tcx> + DebugInfoMethods<'tcx> + AsmMethods + PreDefineMethods<'tcx> @@ -74,7 +74,7 @@ impl<'tcx, T> CodegenMethods<'tcx> for T where + MiscMethods<'tcx> + ConstMethods<'tcx> + StaticMethods - + CoverageInfoMethods + + CoverageInfoMethods<'tcx> + DebugInfoMethods<'tcx> + AsmMethods + PreDefineMethods<'tcx> diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 93ba2e6a4f1e9..4c34bdddc0763 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -2,6 +2,7 @@ use crate::interface::parse_cfgspecs; use rustc_data_structures::fx::FxHashSet; use rustc_errors::{emitter::HumanReadableErrorType, registry, ColorConfig}; +use rustc_session::config::InstrumentCoverage; use rustc_session::config::Strip; use rustc_session::config::{build_configuration, build_session_options, to_crate_config}; use rustc_session::config::{rustc_optgroups, ErrorOutputType, ExternLocation, Options, Passes}; @@ -560,7 +561,7 @@ fn test_debugging_options_tracking_hash() { tracked!(inline_mir, Some(true)); tracked!(inline_mir_threshold, Some(123)); tracked!(inline_mir_hint_threshold, Some(123)); - tracked!(instrument_coverage, true); + tracked!(instrument_coverage, Some(InstrumentCoverage::All)); tracked!(instrument_mcount, true); tracked!(link_only, true); tracked!(merge_functions, Some(MergeFunctions::Disabled)); diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index b5506acf73522..9335384aa6c20 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -736,7 +736,7 @@ impl<'a> CrateLoader<'a> { } fn inject_profiler_runtime(&mut self, krate: &ast::Crate) { - if (self.sess.opts.debugging_opts.instrument_coverage + if (self.sess.instrument_coverage() || self.sess.opts.debugging_opts.profile || self.sess.opts.cg.profile_generate.enabled()) && !self.sess.opts.debugging_opts.no_profiler_runtime diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 90fda9ec91cea..d05269913ebe4 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1537,9 +1537,10 @@ pub enum StatementKind<'tcx> { AscribeUserType(Box<(Place<'tcx>, UserTypeProjection)>, ty::Variance), /// Marks the start of a "coverage region", injected with '-Zinstrument-coverage'. A - /// `CoverageInfo` statement carries metadata about the coverage region, used to inject a coverage - /// map into the binary. The `Counter` kind also generates executable code, to increment a - /// counter varible at runtime, each time the code region is executed. + /// `Coverage` statement carries metadata about the coverage region, used to inject a coverage + /// map into the binary. If `Coverage::kind` is a `Counter`, the statement also generates + /// executable code, to increment a counter varible at runtime, each time the code region is + /// executed. Coverage(Box), /// Denotes a call to the intrinsic function copy_overlapping, where `src_dst` denotes the diff --git a/compiler/rustc_middle/src/mir/mono.rs b/compiler/rustc_middle/src/mir/mono.rs index 6c2468b9ffe0b..0167655bee514 100644 --- a/compiler/rustc_middle/src/mir/mono.rs +++ b/compiler/rustc_middle/src/mir/mono.rs @@ -84,7 +84,14 @@ impl<'tcx> MonoItem<'tcx> { .debugging_opts .inline_in_all_cgus .unwrap_or_else(|| tcx.sess.opts.optimize != OptLevel::No) - && !tcx.sess.link_dead_code(); + && !tcx.sess.link_dead_code() + && !tcx.sess.instrument_coverage(); + // Disabled for `-Z instrument-coverage` because some LLVM optimizations can sometimes + // break coverage results. A test that failed at certain optimization levels is now + // validated at that optimization level (via `compile-flags` directive): + // * `src/test/run-make-fulldeps/coverage/closure.rs` broke with `-C opt-level=2`, and + // also required disabling `internalize_symbols` in + // `rustc_mir/monomorphize/partitioning/mod.rs` match *self { MonoItem::Fn(ref instance) => { diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index ae367db019b59..9be750264dd12 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -328,7 +328,10 @@ rustc_queries! { /// Returns the name of the file that contains the function body, if instrumented for coverage. query covered_file_name(key: DefId) -> Option { - desc { |tcx| "retrieving the covered file name, if instrumented, for `{}`", tcx.def_path_str(key) } + desc { + |tcx| "retrieving the covered file name, if instrumented, for `{}`", + tcx.def_path_str(key) + } storage(ArenaCacheSelector<'tcx>) cache_on_disk_if { key.is_local() } } @@ -336,7 +339,10 @@ rustc_queries! { /// Returns the `CodeRegions` for a function that has instrumented coverage, in case the /// function was optimized out before codegen, and before being added to the Coverage Map. query covered_code_regions(key: DefId) -> Vec<&'tcx mir::coverage::CodeRegion> { - desc { |tcx| "retrieving the covered `CodeRegion`s, if instrumented, for `{}`", tcx.def_path_str(key) } + desc { + |tcx| "retrieving the covered `CodeRegion`s, if instrumented, for `{}`", + tcx.def_path_str(key) + } storage(ArenaCacheSelector<'tcx>) cache_on_disk_if { key.is_local() } } diff --git a/compiler/rustc_mir/src/monomorphize/partitioning/mod.rs b/compiler/rustc_mir/src/monomorphize/partitioning/mod.rs index dc2379fd92b83..04f31ec3a33c7 100644 --- a/compiler/rustc_mir/src/monomorphize/partitioning/mod.rs +++ b/compiler/rustc_mir/src/monomorphize/partitioning/mod.rs @@ -196,7 +196,13 @@ pub fn partition<'tcx>( // Next we try to make as many symbols "internal" as possible, so LLVM has // more freedom to optimize. - if !tcx.sess.link_dead_code() { + if !tcx.sess.link_dead_code() && !tcx.sess.instrument_coverage() { + // Disabled for `-Z instrument-coverage` because some LLVM optimizations can sometimes + // break coverage results. Tests that failed at certain optimization levels are now + // validated at those optimization levels (via `compile-flags` directive); for example: + // * `src/test/run-make-fulldeps/coverage/async.rs` broke with `-C opt-level=1` + // * `src/test/run-make-fulldeps/coverage/closure.rs` broke with `-C opt-level=2`, and + // also required disabling `generate_gcu_internal_copies` in `rustc_middle/mir/mono.rs` let _prof_timer = tcx.prof.generic_activity("cgu_partitioning_internalize_symbols"); partitioner.internalize_symbols(cx, &mut post_inlining); } diff --git a/compiler/rustc_mir/src/transform/coverage/query.rs b/compiler/rustc_mir/src/transform/coverage/query.rs index de8447f1974e7..2ba9d1bdc0c00 100644 --- a/compiler/rustc_mir/src/transform/coverage/query.rs +++ b/compiler/rustc_mir/src/transform/coverage/query.rs @@ -6,10 +6,9 @@ use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self, TyCtxt}; use rustc_span::def_id::DefId; -/// The `query` provider for `CoverageInfo`, requested by `codegen_coverage()` (to inject each -/// counter) and `FunctionCoverage::new()` (to extract the coverage map metadata from the MIR). +/// A `query` provider for retrieving coverage information injected into MIR. pub(crate) fn provide(providers: &mut Providers) { - providers.coverageinfo = |tcx, def_id| coverageinfo_from_mir(tcx, def_id); + providers.coverageinfo = |tcx, def_id| coverageinfo(tcx, def_id); providers.covered_file_name = |tcx, def_id| covered_file_name(tcx, def_id); providers.covered_code_regions = |tcx, def_id| covered_code_regions(tcx, def_id); } @@ -121,7 +120,7 @@ impl CoverageVisitor { } } -fn coverageinfo_from_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo { +fn coverageinfo<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo { let mir_body = mir_body(tcx, def_id); let mut coverage_visitor = CoverageVisitor { @@ -139,29 +138,22 @@ fn coverageinfo_from_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo } fn covered_file_name<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Option { - let body = mir_body(tcx, def_id); - for bb_data in body.basic_blocks().iter() { - for statement in bb_data.statements.iter() { - if let StatementKind::Coverage(box ref coverage) = statement.kind { - if let Some(code_region) = coverage.code_region.as_ref() { - if is_inlined(body, statement) { - continue; + if tcx.is_mir_available(def_id) { + let body = mir_body(tcx, def_id); + for bb_data in body.basic_blocks().iter() { + for statement in bb_data.statements.iter() { + if let StatementKind::Coverage(box ref coverage) = statement.kind { + if let Some(code_region) = coverage.code_region.as_ref() { + if is_inlined(body, statement) { + continue; + } + return Some(code_region.file_name); } - return Some(code_region.file_name); } } } } - None -} - -/// This function ensures we obtain the correct MIR for the given item irrespective of -/// whether that means const mir or runtime mir. For `const fn` this opts for runtime -/// mir. -fn mir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx mir::Body<'tcx> { - let id = ty::WithOptConstParam::unknown(def_id); - let def = ty::InstanceDef::Item(id); - tcx.instance_mir(def) + return None; } fn covered_code_regions<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Vec<&'tcx CodeRegion> { @@ -188,3 +180,12 @@ fn is_inlined(body: &Body<'_>, statement: &Statement<'_>) -> bool { let scope_data = &body.source_scopes[statement.source_info.scope]; scope_data.inlined.is_some() || scope_data.inlined_parent_scope.is_some() } + +/// This function ensures we obtain the correct MIR for the given item irrespective of +/// whether that means const mir or runtime mir. For `const fn` this opts for runtime +/// mir. +fn mir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx mir::Body<'tcx> { + let id = ty::WithOptConstParam::unknown(def_id); + let def = ty::InstanceDef::Item(id); + tcx.instance_mir(def) +} diff --git a/compiler/rustc_mir/src/transform/mod.rs b/compiler/rustc_mir/src/transform/mod.rs index 13546442f6652..e16981a050e49 100644 --- a/compiler/rustc_mir/src/transform/mod.rs +++ b/compiler/rustc_mir/src/transform/mod.rs @@ -313,11 +313,8 @@ fn mir_promoted( &simplify::SimplifyCfg::new("promote-consts"), ]; - let opt_coverage: &[&dyn MirPass<'tcx>] = if tcx.sess.opts.debugging_opts.instrument_coverage { - &[&coverage::InstrumentCoverage] - } else { - &[] - }; + let opt_coverage: &[&dyn MirPass<'tcx>] = + if tcx.sess.instrument_coverage() { &[&coverage::InstrumentCoverage] } else { &[] }; run_passes(tcx, &mut body, MirPhase::ConstPromotion, &[promote, opt_coverage]); diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 75078a1231163..5027e882ff6d3 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -184,6 +184,37 @@ pub enum MirSpanview { Block, } +/// The different settings that the `-Z instrument-coverage` flag can have. +/// +/// Coverage instrumentation now supports combining `-Z instrument-coverage` +/// with compiler and linker optimization (enabled with `-O` or `-C opt-level=1` +/// and higher). Nevertheless, there are many variables, depending on options +/// selected, code structure, and enabled attributes. If errors are encountered, +/// either while compiling or when generating `llvm-cov show` reports, consider +/// lowering the optimization level, including or excluding `-C link-dead-code`, +/// or using `-Z instrument-coverage=except-unused-functions` or `-Z +/// instrument-coverage=except-unused-generics`. +/// +/// Note that `ExceptUnusedFunctions` means: When `mapgen.rs` generates the +/// coverage map, it will not attempt to generate synthetic functions for unused +/// (and not code-generated) functions (whether they are generic or not). As a +/// result, non-codegenned functions will not be included in the coverage map, +/// and will not appear, as covered or uncovered, in coverage reports. +/// +/// `ExceptUnusedGenerics` will add synthetic functions to the coverage map, +/// unless the function has type parameters. +#[derive(Clone, Copy, PartialEq, Hash, Debug)] +pub enum InstrumentCoverage { + /// Default `-Z instrument-coverage` or `-Z instrument-coverage=statement` + All, + /// `-Z instrument-coverage=except-unused-generics` + ExceptUnusedGenerics, + /// `-Z instrument-coverage=except-unused-functions` + ExceptUnusedFunctions, + /// `-Z instrument-coverage=off` (or `no`, etc.) + Off, +} + #[derive(Clone, PartialEq, Hash)] pub enum LinkerPluginLto { LinkerPlugin(PathBuf), @@ -1911,7 +1942,9 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options { ); } - if debugging_opts.instrument_coverage { + if debugging_opts.instrument_coverage.is_some() + && debugging_opts.instrument_coverage != Some(InstrumentCoverage::Off) + { if cg.profile_generate.enabled() || cg.profile_use.is_some() { early_error( error_format, @@ -2298,9 +2331,9 @@ impl PpMode { /// how the hash should be calculated when adding a new command-line argument. crate mod dep_tracking { use super::{ - CFGuard, CrateType, DebugInfo, ErrorOutputType, LinkerPluginLto, LtoCli, OptLevel, - OutputTypes, Passes, SanitizerSet, SourceFileHashAlgorithm, SwitchWithOptPath, - SymbolManglingVersion, TrimmedDefPaths, + CFGuard, CrateType, DebugInfo, ErrorOutputType, InstrumentCoverage, LinkerPluginLto, + LtoCli, OptLevel, OutputTypes, Passes, SanitizerSet, SourceFileHashAlgorithm, + SwitchWithOptPath, SymbolManglingVersion, TrimmedDefPaths, }; use crate::lint; use crate::options::WasiExecModel; @@ -2364,6 +2397,7 @@ crate mod dep_tracking { impl_dep_tracking_hash_via_hash!(Option); impl_dep_tracking_hash_via_hash!(Option); impl_dep_tracking_hash_via_hash!(Option); + impl_dep_tracking_hash_via_hash!(Option); impl_dep_tracking_hash_via_hash!(Option); impl_dep_tracking_hash_via_hash!(Option); impl_dep_tracking_hash_via_hash!(CrateType); diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index d9e5a186073b6..dd2507c87f62e 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -262,6 +262,7 @@ macro_rules! options { pub const parse_linker_flavor: &str = ::rustc_target::spec::LinkerFlavor::one_of(); pub const parse_optimization_fuel: &str = "crate=integer"; pub const parse_mir_spanview: &str = "`statement` (default), `terminator`, or `block`"; + pub const parse_instrument_coverage: &str = "`all` (default), `except-unused-generics`, `except-unused-functions`, or `off`"; pub const parse_unpretty: &str = "`string` or `string=string`"; pub const parse_treat_err_as_bug: &str = "either no value or a number bigger than 0"; pub const parse_lto: &str = @@ -592,6 +593,41 @@ macro_rules! options { true } + fn parse_instrument_coverage(slot: &mut Option, v: Option<&str>) -> bool { + if v.is_some() { + let mut bool_arg = None; + if parse_opt_bool(&mut bool_arg, v) { + *slot = if bool_arg.unwrap() { + Some(InstrumentCoverage::All) + } else { + None + }; + return true + } + } + + let v = match v { + None => { + *slot = Some(InstrumentCoverage::All); + return true; + } + Some(v) => v, + }; + + *slot = Some(match v { + "all" => InstrumentCoverage::All, + "except-unused-generics" | "except_unused_generics" => { + InstrumentCoverage::ExceptUnusedGenerics + } + "except-unused-functions" | "except_unused_functions" => { + InstrumentCoverage::ExceptUnusedFunctions + } + "off" | "no" | "n" | "false" | "0" => InstrumentCoverage::Off, + _ => return false, + }); + true + } + fn parse_treat_err_as_bug(slot: &mut Option, v: Option<&str>) -> bool { match v { Some(s) => { *slot = s.parse().ok(); slot.is_some() } @@ -967,12 +1003,14 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "control whether `#[inline]` functions are in all CGUs"), input_stats: bool = (false, parse_bool, [UNTRACKED], "gather statistics about the input (default: no)"), - instrument_coverage: bool = (false, parse_bool, [TRACKED], + instrument_coverage: Option = (None, parse_instrument_coverage, [TRACKED], "instrument the generated code to support LLVM source-based code coverage \ reports (note, the compiler build config must include `profiler = true`, \ and is mutually exclusive with `-C profile-generate`/`-C profile-use`); \ implies `-Z symbol-mangling-version=v0`; disables/overrides some Rust \ - optimizations (default: no)"), + optimizations. Optional values are: `=all` (default coverage), \ + `=except-unused-generics`, `=except-unused-functions`, or `=off` \ + (default: instrument-coverage=off)"), instrument_mcount: bool = (false, parse_bool, [TRACKED], "insert function instrument code for mcount-based tracing (default: no)"), keep_hygiene_data: bool = (false, parse_bool, [UNTRACKED], diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index fc57b6b8acedf..f8ec00eb8aa2d 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -1150,6 +1150,21 @@ impl Session { self.opts.cg.link_dead_code.unwrap_or(false) } + pub fn instrument_coverage(&self) -> bool { + self.opts.debugging_opts.instrument_coverage.unwrap_or(config::InstrumentCoverage::Off) + != config::InstrumentCoverage::Off + } + + pub fn instrument_coverage_except_unused_generics(&self) -> bool { + self.opts.debugging_opts.instrument_coverage.unwrap_or(config::InstrumentCoverage::Off) + == config::InstrumentCoverage::ExceptUnusedGenerics + } + + pub fn instrument_coverage_except_unused_functions(&self) -> bool { + self.opts.debugging_opts.instrument_coverage.unwrap_or(config::InstrumentCoverage::Off) + == config::InstrumentCoverage::ExceptUnusedFunctions + } + pub fn mark_attr_known(&self, attr: &Attribute) { self.known_attrs.lock().mark(attr) } diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index e5136355ca982..f69674ea422b0 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -2875,7 +2875,19 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { .emit(); InlineAttr::None } else if list_contains_name(&items[..], sym::always) { - InlineAttr::Always + if tcx.sess.instrument_coverage() { + // Forced inlining will discard functions marked with `#[inline(always)]`. + // If `-Z instrument-coverage` is enabled, the generated coverage map may + // hold references to functions that no longer exist, causing errors in + // coverage reports. (Note, this fixes #82875. I added some tests that + // also include `#[inline(always)]` functions, used and unused, and within + // and across crates, but could not reproduce the reported error in the + // `rustc` test suite. I am able to reproduce the error, following the steps + // described in #82875, and this change does fix that issue.) + InlineAttr::Hint + } else { + InlineAttr::Always + } } else if list_contains_name(&items[..], sym::never) { InlineAttr::Never } else { diff --git a/src/test/run-make-fulldeps/coverage-llvmir/Makefile b/src/test/run-make-fulldeps/coverage-llvmir/Makefile index 7d9121ee2f834..86af7e41ae15b 100644 --- a/src/test/run-make-fulldeps/coverage-llvmir/Makefile +++ b/src/test/run-make-fulldeps/coverage-llvmir/Makefile @@ -17,7 +17,7 @@ else COMDAT_IF_SUPPORTED=, comdat endif -DEFINE_INTERNAL=define internal +DEFINE_INTERNAL=define hidden ifdef IS_WINDOWS LLVM_FILECHECK_OPTIONS=\ diff --git a/src/test/run-make-fulldeps/coverage-reports/Makefile b/src/test/run-make-fulldeps/coverage-reports/Makefile index af75ec5e24d13..dd5e28b449cd4 100644 --- a/src/test/run-make-fulldeps/coverage-reports/Makefile +++ b/src/test/run-make-fulldeps/coverage-reports/Makefile @@ -59,7 +59,7 @@ endif # if and when we allow `llvm-cov` to produce results for multiple files. Note, the path separators # appear to be normalized to `/` in those files, thankfully.) LLVM_COV_IGNORE_FILES=\ - --ignore-filename-regex=uses_crate.rs + --ignore-filename-regex='(uses_crate.rs|uses_inline_crate.rs)' # When generating `expected_*` results (using `x.py test --bless`), the `--debug` flag is forced. # If assertions are disabled, the command will fail with an error, rather than attempt to generate @@ -82,13 +82,13 @@ endif %: $(SOURCEDIR)/lib/%.rs # Compile the test library with coverage instrumentation $(RUSTC) $(SOURCEDIR)/lib/$@.rs \ - $$( sed -nE 's#^// compile-flags:(.*)#\1# p' $(SOURCEDIR)/lib/$@.rs) \ + $$( sed -n 's/^\/\/ compile-flags: \([^#]*\).*/\1/p' $(SOURCEDIR)/lib/$@.rs ) \ --crate-type rlib -Zinstrument-coverage %: $(SOURCEDIR)/%.rs # Compile the test program with coverage instrumentation $(RUSTC) $(SOURCEDIR)/$@.rs \ - $$( sed -nE 's#^// compile-flags:(.*)#\1# p' $(SOURCEDIR)/$@.rs) \ + $$( sed -n 's/^\/\/ compile-flags: \([^#]*\).*/\1/p' $(SOURCEDIR)/$@.rs ) \ -L "$(TMPDIR)" -Zinstrument-coverage # Run it in order to generate some profiling data, @@ -107,7 +107,7 @@ endif # Run it through rustdoc as well to cover doctests LLVM_PROFILE_FILE="$(TMPDIR)"/$@-%p.profraw \ $(RUSTDOC) --crate-name workaround_for_79771 --test $(SOURCEDIR)/$@.rs \ - $$( sed -nE 's#^// compile-flags:(.*)#\1# p' $(SOURCEDIR)/$@.rs) \ + $$( sed -n 's/^\/\/ compile-flags: \([^#]*\).*/\1/p' $(SOURCEDIR)/$@.rs ) \ -L "$(TMPDIR)" -Zinstrument-coverage \ -Z unstable-options --persist-doctests=$(TMPDIR)/rustdoc-$@ diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt index 3f9403e6f70b8..d9097bb50e5a0 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt @@ -1,6 +1,6 @@ 1| |#![allow(unused_assignments, dead_code)] 2| | - 3| |// compile-flags: --edition=2018 + 3| |// compile-flags: --edition=2018 -C opt-level=1 # fix in rustc_mir/monomorphize/partitioning/mod.rs 4| | 5| 1|async fn c(x: u8) -> u8 { 6| 1| if x == 8 { @@ -10,7 +10,7 @@ 10| | } 11| 1|} 12| | - 13| 0|async fn d() -> u8 { 1 } + 13| |async fn d() -> u8 { 1 } // should have a coverage count `0` (see below) 14| | 15| 0|async fn e() -> u8 { 1 } // unused function; executor does not block on `g()` 16| | @@ -132,4 +132,15 @@ 126| | } 127| 1| } 128| |} + 129| | + 130| |// `llvm-cov show` shows no coverage results for the `d()`, even though the + 131| |// crate's LLVM IR shows the function exists and has an InstrProf PGO counter, + 132| |// and appears to be registered like all other counted functions. + 133| |// + 134| |// `llvm-cov show --debug` output shows there is at least one `Counter` for this + 135| |// line, but counters do not appear in the `Combined regions` section (unlike + 136| |// `f()`, which is similar, but does appear in `Combined regions`, and does show + 137| |// coverage). The only difference is, `f()` is awaited, but the call to await + 138| |// `d()` is not reached. (Note: `d()` will appear in coverage if the test is + 139| |// modified to cause it to be awaited.) diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure.txt index 7261cf0a3b0da..a39e3a16fc64b 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure.txt @@ -1,5 +1,5 @@ 1| |#![allow(unused_assignments, unused_variables)] - 2| | + 2| |// compile-flags: -C opt-level=2 # fix described in rustc_middle/mir/mono.rs 3| 1|fn main() { 4| 1| // Initialize test constants in a way that cannot be determined at compile time, to ensure 5| 1| // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt index 8f67170561a2a..7d2abe05973e1 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt @@ -23,6 +23,12 @@ 22| 1|//! ``` 23| 2|//! #[derive(Debug, PartialEq)] ^1 + ------------------ + | Unexecuted instantiation: ::ne + ------------------ + | ::eq: + | 23| 2|//! #[derive(Debug, PartialEq)] + ------------------ 24| 1|//! struct SomeError { 25| 1|//! msg: String, 26| 1|//! } diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.generics.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.generics.txt index 7b38ffb87cba8..6f28c08909387 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.generics.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.generics.txt @@ -29,12 +29,12 @@ 18| 2| println!("BOOM times {}!!!", self.strength); 19| 2| } ------------------ - | as core::ops::drop::Drop>::drop: + | as core::ops::drop::Drop>::drop: | 17| 1| fn drop(&mut self) { | 18| 1| println!("BOOM times {}!!!", self.strength); | 19| 1| } ------------------ - | as core::ops::drop::Drop>::drop: + | as core::ops::drop::Drop>::drop: | 17| 1| fn drop(&mut self) { | 18| 1| println!("BOOM times {}!!!", self.strength); | 19| 1| } diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.partial_eq.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.partial_eq.txt index 9cc9a4d47ad1c..4e4dde46b344b 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.partial_eq.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.partial_eq.txt @@ -3,6 +3,11 @@ 3| | 4| 2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] ^0 ^0 ^0 ^0 ^1 ^1 ^0^0 + ------------------ + | Unexecuted instantiation: ::ne + ------------------ + | Unexecuted instantiation: ::eq + ------------------ 5| |pub struct Version { 6| | major: usize, 7| | minor: usize, diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.unused.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.unused.txt new file mode 100644 index 0000000000000..15fcf21c0ef3c --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.unused.txt @@ -0,0 +1,62 @@ + 1| 2|fn foo(x: T) { + 2| 2| let mut i = 0; + 3| 22| while i < 10 { + 4| 20| i != 0 || i != 0; + ^2 + 5| 20| i += 1; + 6| | } + 7| 2|} + ------------------ + | unused::foo::: + | 1| 1|fn foo(x: T) { + | 2| 1| let mut i = 0; + | 3| 11| while i < 10 { + | 4| 10| i != 0 || i != 0; + | ^1 + | 5| 10| i += 1; + | 6| | } + | 7| 1|} + ------------------ + | unused::foo::: + | 1| 1|fn foo(x: T) { + | 2| 1| let mut i = 0; + | 3| 11| while i < 10 { + | 4| 10| i != 0 || i != 0; + | ^1 + | 5| 10| i += 1; + | 6| | } + | 7| 1|} + ------------------ + 8| | + 9| 0|fn unused_template_func(x: T) { + 10| 0| let mut i = 0; + 11| 0| while i < 10 { + 12| 0| i != 0 || i != 0; + 13| 0| i += 1; + 14| | } + 15| 0|} + 16| | + 17| 0|fn unused_func(mut a: u32) { + 18| 0| if a != 0 { + 19| 0| a += 1; + 20| 0| } + 21| 0|} + 22| | + 23| 0|fn unused_func2(mut a: u32) { + 24| 0| if a != 0 { + 25| 0| a += 1; + 26| 0| } + 27| 0|} + 28| | + 29| 0|fn unused_func3(mut a: u32) { + 30| 0| if a != 0 { + 31| 0| a += 1; + 32| 0| } + 33| 0|} + 34| | + 35| 1|fn main() -> Result<(), u8> { + 36| 1| foo::(0); + 37| 1| foo::(0.0); + 38| 1| Ok(()) + 39| 1|} + diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt index bc2f673349a67..380869d62a8b6 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt @@ -1,5 +1,5 @@ 1| |#![allow(unused_assignments, unused_variables)] - 2| | + 2| |// compile-flags: -C opt-level=3 # validates coverage now works with optimizations 3| |use std::fmt::Debug; 4| | 5| 1|pub fn used_function() { @@ -29,7 +29,9 @@ | 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg); | 19| 1|} ------------------ - 20| | + | Unexecuted instantiation: used_crate::used_only_from_bin_crate_generic_function::<_> + ------------------ + 20| |// Expect for above function: `Unexecuted instantiation` (see below) 21| 2|pub fn used_only_from_this_lib_crate_generic_function(arg: T) { 22| 2| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg); 23| 2|} @@ -63,6 +65,17 @@ 29| 2|pub fn used_with_same_type_from_bin_crate_and_lib_crate_generic_function(arg: T) { 30| 2| println!("used_with_same_type_from_bin_crate_and_lib_crate_generic_function with {:?}", arg); 31| 2|} + ------------------ + | used_crate::used_with_same_type_from_bin_crate_and_lib_crate_generic_function::<&str>: + | 29| 1|pub fn used_with_same_type_from_bin_crate_and_lib_crate_generic_function(arg: T) { + | 30| 1| println!("used_with_same_type_from_bin_crate_and_lib_crate_generic_function with {:?}", arg); + | 31| 1|} + ------------------ + | used_crate::used_with_same_type_from_bin_crate_and_lib_crate_generic_function::<&str>: + | 29| 1|pub fn used_with_same_type_from_bin_crate_and_lib_crate_generic_function(arg: T) { + | 30| 1| println!("used_with_same_type_from_bin_crate_and_lib_crate_generic_function with {:?}", arg); + | 31| 1|} + ------------------ 32| | 33| 0|pub fn unused_generic_function(arg: T) { 34| 0| println!("unused_generic_function with {:?}", arg); @@ -94,46 +107,42 @@ 60| 1| used_only_from_this_lib_crate_generic_function("used ONLY from library used_crate.rs"); 61| 1|} 62| | - 63| |// FIXME(#79651): `used_from_bin_crate_and_lib_crate_generic_function()` is covered and executed - 64| |// `2` times, but the coverage output also shows (at the bottom of the coverage report): - 65| |// ------------------ - 66| |// | Unexecuted instantiation: - 67| |// ------------------ - 68| |// - 69| |// Note, the function name shown in the error seems to change depending on the structure of the - 70| |// code, for some reason, including: - 71| |// - 72| |// * used_crate::used_from_bin_crate_and_lib_crate_generic_function::<&str> - 73| |// * used_crate::use_this_lib_crate + 63| |// FIXME(#79651): "Unexecuted instantiation" errors appear in coverage results, + 64| |// for example: + 65| |// + 66| |// | Unexecuted instantiation: used_crate::used_only_from_bin_crate_generic_function::<_> + 67| |// + 68| |// These notices appear when `llvm-cov` shows instantiations. This may be a + 69| |// default option, but it can be suppressed with: + 70| |// + 71| |// ```shell + 72| |// $ `llvm-cov show --show-instantiations=0 ...` + 73| |// ``` 74| |// - 75| |// The `Unexecuted instantiation` error may be related to more than one generic function. Since the - 76| |// reporting is not consistent, it may not be obvious if there are multiple problems here; however, - 77| |// `used_crate::used_from_bin_crate_and_lib_crate_generic_function::<&str>` (which I have seen - 78| |// with this error) is the only generic function missing instantiaion coverage counts. + 75| |// The notice is triggered because the function is unused by the library itself, + 76| |// and when the library is compiled, a synthetic function is generated, so + 77| |// unused function coverage can be reported. Coverage can be skipped for unused + 78| |// generic functions with: 79| |// - 80| |// The `&str` variant was called from within this `lib` crate, and the `bin` crate also calls this - 81| |// function, but with `T` type `&Vec`. - 82| |// - 83| |// I believe the reason is that one or both crates are generating `Zero` counters for what it - 84| |// believes are "Unreachable" instantiations, but those instantiations are counted from the - 85| |// coverage map in the other crate. - 86| |// - 87| |// See `add_unreachable_coverage()` in `mapgen.rs` for more on how these `Zero` counters are added - 88| |// for what the funciton believes are `DefId`s that did not get codegenned. I suspect the issue - 89| |// may be related to this process, but this needs to be confirmed. It may not be possible to know - 90| |// for sure if a function is truly unused and should be reported with `Zero` coverage if it may - 91| |// still get used from an external crate. (Something to look at: If the `DefId` in MIR corresponds - 92| |// _only_ to the generic function without type parameters, is the `DefId` in the codegenned set, - 93| |// instantiated with one of the type parameters (in either or both crates) a *different* `DefId`? - 94| |// If so, `add_unreachable_coverage()` would assume the MIR `DefId` was uncovered, and would add - 95| |// unreachable coverage. - 96| |// - 97| |// I didn't think they could be different, but if they can, we would need to find the `DefId` for - 98| |// the generic function MIR and include it in the set of "codegenned" DefIds if any instantiation - 99| |// of that generic function does exist. - 100| |// - 101| |// Note, however, for `used_with_same_type_from_bin_crate_and_lib_crate_generic_function()` both - 102| |// crates use this function with the same type variant. The function does not have multiple - 103| |// instantiations, so the coverage analysis is not confused. No "Unexecuted instantiations" errors - 104| |// are reported. + 80| |// ```shell + 81| |// $ `rustc -Z instrument-coverage=except-unused-generics ...` + 82| |// ``` + 83| |// + 84| |// Even though this function is used by `uses_crate.rs` (and + 85| |// counted), with substitutions for `T`, those instantiations are only generated + 86| |// when the generic function is actually used (from the binary, not from this + 87| |// library crate). So the test result shows coverage for all instantiated + 88| |// versions and their generic type substitutions, plus the `Unexecuted + 89| |// instantiation` message for the non-substituted version. This is valid, but + 90| |// unfortunately a little confusing. + 91| |// + 92| |// The library crate has its own coverage map, and the only way to show unused + 93| |// coverage of a generic function is to include the generic function in the + 94| |// coverage map, marked as an "unused function". If the library were used by + 95| |// another binary that never used this generic function, then it would be valid + 96| |// to show the unused generic, with unknown substitution (`_`). + 97| |// + 98| |// The alternative is to exclude all generics from being included in the "unused + 99| |// functions" list, which would then omit coverage results for + 100| |// `unused_generic_function()`, below. diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_inline_crate.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_inline_crate.txt new file mode 100644 index 0000000000000..0853dc9c014e5 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_inline_crate.txt @@ -0,0 +1,156 @@ + 1| |#![allow(unused_assignments, unused_variables)] + 2| | + 3| |// compile-flags: -C opt-level=3 # validates coverage now works with optimizations + 4| | + 5| |use std::fmt::Debug; + 6| | + 7| 1|pub fn used_function() { + 8| | // Initialize test constants in a way that cannot be determined at compile time, to ensure + 9| | // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + 10| | // dependent conditions. + 11| 1| let is_true = std::env::args().len() == 1; + 12| 1| let mut countdown = 0; + 13| 1| if is_true { + 14| 1| countdown = 10; + 15| 1| } + ^0 + 16| 1| use_this_lib_crate(); + 17| 1|} + 18| | + 19| |#[inline(always)] + 20| 1|pub fn used_inline_function() { + 21| | // Initialize test constants in a way that cannot be determined at compile time, to ensure + 22| | // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + 23| | // dependent conditions. + 24| 1| let is_true = std::env::args().len() == 1; + 25| 1| let mut countdown = 0; + 26| 1| if is_true { + 27| 1| countdown = 10; + 28| 1| } + ^0 + 29| 1| use_this_lib_crate(); + 30| 1|} + ------------------ + | used_inline_crate::used_inline_function: + | 20| 1|pub fn used_inline_function() { + | 21| | // Initialize test constants in a way that cannot be determined at compile time, to ensure + | 22| | // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + | 23| | // dependent conditions. + | 24| 1| let is_true = std::env::args().len() == 1; + | 25| 1| let mut countdown = 0; + | 26| 1| if is_true { + | 27| 1| countdown = 10; + | 28| 1| } + | ^0 + | 29| 1| use_this_lib_crate(); + | 30| 1|} + ------------------ + | Unexecuted instantiation: used_inline_crate::used_inline_function + ------------------ + 31| |// Expect for above function: + 32| |// + 33| |// | Unexecuted instantiation: used_crate::used_only_from_bin_crate_generic_function::<_> + 34| |// + 35| |// With `#[inline(always)]` this function is instantiated twice, in both the library crate (which + 36| |// does not use it) and the `uses_inline_crate` binary (which does use/call it). + 37| | + 38| |#[inline(always)] + 39| 2|pub fn used_only_from_bin_crate_generic_function(arg: T) { + 40| 2| println!("used_only_from_bin_crate_generic_function with {:?}", arg); + 41| 2|} + ------------------ + | used_inline_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec>: + | 39| 1|pub fn used_only_from_bin_crate_generic_function(arg: T) { + | 40| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg); + | 41| 1|} + ------------------ + | used_inline_crate::used_only_from_bin_crate_generic_function::<&str>: + | 39| 1|pub fn used_only_from_bin_crate_generic_function(arg: T) { + | 40| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg); + | 41| 1|} + ------------------ + | Unexecuted instantiation: used_inline_crate::used_only_from_bin_crate_generic_function::<_> + ------------------ + 42| |// Expect for above function: `Unexecuted instantiation` (see notes in `used_crate.rs`) + 43| | + 44| |#[inline(always)] + 45| 4|pub fn used_only_from_this_lib_crate_generic_function(arg: T) { + 46| 4| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg); + 47| 4|} + ------------------ + | used_inline_crate::used_only_from_this_lib_crate_generic_function::>: + | 45| 2|pub fn used_only_from_this_lib_crate_generic_function(arg: T) { + | 46| 2| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg); + | 47| 2|} + ------------------ + | used_inline_crate::used_only_from_this_lib_crate_generic_function::<&str>: + | 45| 2|pub fn used_only_from_this_lib_crate_generic_function(arg: T) { + | 46| 2| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg); + | 47| 2|} + ------------------ + 48| | + 49| |#[inline(always)] + 50| 3|pub fn used_from_bin_crate_and_lib_crate_generic_function(arg: T) { + 51| 3| println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg); + 52| 3|} + ------------------ + | used_inline_crate::used_from_bin_crate_and_lib_crate_generic_function::>: + | 50| 1|pub fn used_from_bin_crate_and_lib_crate_generic_function(arg: T) { + | 51| 1| println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg); + | 52| 1|} + ------------------ + | used_inline_crate::used_from_bin_crate_and_lib_crate_generic_function::<&str>: + | 50| 2|pub fn used_from_bin_crate_and_lib_crate_generic_function(arg: T) { + | 51| 2| println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg); + | 52| 2|} + ------------------ + 53| | + 54| |#[inline(always)] + 55| 3|pub fn used_with_same_type_from_bin_crate_and_lib_crate_generic_function(arg: T) { + 56| 3| println!("used_with_same_type_from_bin_crate_and_lib_crate_generic_function with {:?}", arg); + 57| 3|} + ------------------ + | used_inline_crate::used_with_same_type_from_bin_crate_and_lib_crate_generic_function::<&str>: + | 55| 1|pub fn used_with_same_type_from_bin_crate_and_lib_crate_generic_function(arg: T) { + | 56| 1| println!("used_with_same_type_from_bin_crate_and_lib_crate_generic_function with {:?}", arg); + | 57| 1|} + ------------------ + | used_inline_crate::used_with_same_type_from_bin_crate_and_lib_crate_generic_function::<&str>: + | 55| 2|pub fn used_with_same_type_from_bin_crate_and_lib_crate_generic_function(arg: T) { + | 56| 2| println!("used_with_same_type_from_bin_crate_and_lib_crate_generic_function with {:?}", arg); + | 57| 2|} + ------------------ + 58| | + 59| |#[inline(always)] + 60| 0|pub fn unused_generic_function(arg: T) { + 61| 0| println!("unused_generic_function with {:?}", arg); + 62| 0|} + 63| | + 64| |#[inline(always)] + 65| 0|pub fn unused_function() { + 66| 0| let is_true = std::env::args().len() == 1; + 67| 0| let mut countdown = 2; + 68| 0| if !is_true { + 69| 0| countdown = 20; + 70| 0| } + 71| 0|} + 72| | + 73| |#[inline(always)] + 74| 0|fn unused_private_function() { + 75| 0| let is_true = std::env::args().len() == 1; + 76| 0| let mut countdown = 2; + 77| 0| if !is_true { + 78| 0| countdown = 20; + 79| 0| } + 80| 0|} + 81| | + 82| 2|fn use_this_lib_crate() { + 83| 2| used_from_bin_crate_and_lib_crate_generic_function("used from library used_crate.rs"); + 84| 2| used_with_same_type_from_bin_crate_and_lib_crate_generic_function( + 85| 2| "used from library used_crate.rs", + 86| 2| ); + 87| 2| let some_vec = vec![5, 6, 7, 8]; + 88| 2| used_only_from_this_lib_crate_generic_function(some_vec); + 89| 2| used_only_from_this_lib_crate_generic_function("used ONLY from library used_crate.rs"); + 90| 2|} + diff --git a/src/test/run-make-fulldeps/coverage-spanview/Makefile b/src/test/run-make-fulldeps/coverage-spanview/Makefile index b0bfa7074db94..b3d1009dbd092 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/Makefile +++ b/src/test/run-make-fulldeps/coverage-spanview/Makefile @@ -38,13 +38,11 @@ endif %: $(SOURCEDIR)/lib/%.rs # Compile the test library with coverage instrumentation $(RUSTC) $(SOURCEDIR)/lib/$@.rs \ - $$( sed -nE 's#^// compile-flags:(.*)#\1# p' $(SOURCEDIR)/lib/$@.rs) \ + $$( sed -n 's/^\/\/ compile-flags: \([^#]*\).*/\1/p' $(SOURCEDIR)/lib/$@.rs ) \ --crate-type rlib \ -Ztrim-diagnostic-paths=no \ - -Zinstrument-coverage \ - -Zdump-mir=InstrumentCoverage \ - -Zdump-mir-spanview \ - -Zdump-mir-dir="$(TMPDIR)"/mir_dump.$@ + -Zdump-mir=InstrumentCoverage -Zdump-mir-spanview -Zdump-mir-dir="$(TMPDIR)"/mir_dump.$@ \ + -Zinstrument-coverage for path in "$(TMPDIR)"/mir_dump.$@/*; do \ file="$$(basename "$$path")"; \ @@ -68,13 +66,11 @@ endif %: $(SOURCEDIR)/%.rs # Compile the test program with coverage instrumentation $(RUSTC) $(SOURCEDIR)/$@.rs \ - $$( sed -nE 's#^// compile-flags:(.*)#\1# p' $(SOURCEDIR)/$@.rs) \ + $$( sed -n 's/^\/\/ compile-flags: \([^#]*\).*/\1/p' $(SOURCEDIR)/$@.rs ) \ -L "$(TMPDIR)" \ -Ztrim-diagnostic-paths=no \ - -Zinstrument-coverage \ - -Zdump-mir=InstrumentCoverage \ - -Zdump-mir-spanview \ - -Zdump-mir-dir="$(TMPDIR)"/mir_dump.$@ + -Zdump-mir=InstrumentCoverage -Zdump-mir-spanview -Zdump-mir-dir="$(TMPDIR)"/mir_dump.$@ \ + -Zinstrument-coverage for path in "$(TMPDIR)"/mir_dump.$@/*; do \ file="$$(basename "$$path")"; \ diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.async/async.i-{closure#0}.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.async/async.i-{closure#0}.-------.InstrumentCoverage.0.html index 49297870fa0a7..1c63875a8be9b 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.async/async.i-{closure#0}.-------.InstrumentCoverage.0.html +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.async/async.i-{closure#0}.-------.InstrumentCoverage.0.html @@ -74,18 +74,16 @@ // (b) the open brace for the function body, counted once when the body is // executed asynchronously. match x⦉@0,3,4 { - @18,20⦊y⦉@18,20 if @17,19⦊y⦉@17,19 if @0,3,4⦊c(x)⦉@0,3,4.await == @10,13,15,16,17⦊y + 1⦉@10,13,15,16,17 => { @18,20⦊d()⦉@18,20.await; } - @48⦊y⦉@48 if @1,33,34⦊f()⦉@1,33,34.await == @40,43,45,46,47⦊y + 1⦉@40,43,45,46,47 => @48⦊()⦉@48, +43:28-43:33: @15[4]: _25 = Add(move _26, const 1_u8)">@10,13,15,16⦊y + 1⦉@10,13,15,16 => { @17,19⦊d()⦉@17,19.await; } + @46⦊y⦉@46 if @1,32,33⦊f()⦉@1,32,33.await == @39,42,44,45⦊y + 1⦉@39,42,44,45 => @46⦊()⦉@46, _ => @2⦊()⦉@2, } -}@50,51⦊⦉@50,51 +}@48,49⦊⦉@48,49 diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.async/async.j.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.async/async.j.-------.InstrumentCoverage.0.html index 7cc751074a077..2b43c7bd25d90 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.async/async.j.-------.InstrumentCoverage.0.html +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.async/async.j.-------.InstrumentCoverage.0.html @@ -69,7 +69,7 @@ -
@0,3,4,5⦊fn j(x: u8) { +
@0,3,4⦊fn j(x: u8) { // non-async versions of `c()`, `d()`, and `f()` to make it similar to async `i()`. fn c(x: u8) -> u8 { if x == 8 { @@ -88,21 +88,19 @@ } fn d() -> u8 { 1 } fn f() -> u8 { 1 } - match x⦉@0,3,4,5 { - @6,8⦊y⦉@6,8 if match x⦉@0,3,4 { + @5,7⦊y⦉@5,7 if @0,3,4,5⦊c(x) == y + 1⦉@0,3,4,5 => @6,8⦊{ d(); }⦉@6,8 - @12⦊y⦉@12 if @1,9,10,11⦊f() == y + 1⦉@1,9,10,11 => @12⦊()⦉@12, +69:22-69:27: @4[4]: _7 = Add(move _8, const 1_u8) +69:14-69:27: @4[6]: _4 = Eq(move _5, move _7)">@0,3,4⦊c(x) == y + 1⦉@0,3,4 => @5,7⦊{ d(); }⦉@5,7 + @10⦊y⦉@10 if @1,8,9⦊f() == y + 1⦉@1,8,9 => @10⦊()⦉@10, _ => @2⦊()⦉@2, } -}@14⦊⦉@14
+}@12⦊⦉@12
diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.async/async.m-{closure#0}.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.async/async.m-{closure#0}.-------.InstrumentCoverage.0.html index 9cf86ce34a04d..5cec484a964da 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.async/async.m-{closure#0}.-------.InstrumentCoverage.0.html +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.async/async.m-{closure#0}.-------.InstrumentCoverage.0.html @@ -72,8 +72,7 @@
@0,1⦊{ x - 1 }⦉@0,1
+91:27-91:32: @0[5]: _0 = Sub(move _4, const 1_u8) +91:34-91:34: @0.Return: return">@0⦊{ x - 1 }⦉@0 diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.closure/closure.main-{closure#4}.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.closure/closure.main-{closure#4}.-------.InstrumentCoverage.0.html index df0172bdf7067..420135e143dbf 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.closure/closure.main-{closure#4}.-------.InstrumentCoverage.0.html +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.closure/closure.main-{closure#4}.-------.InstrumentCoverage.0.html @@ -69,9 +69,8 @@ -
| _unused_arg: u8 | @0,1⦊countdown += 1⦉@0,1
+
| _unused_arg: u8 | @0⦊countdown += 1⦉@0
diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.foo.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.foo.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..45fec46f55ca4 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.foo.-------.InstrumentCoverage.0.html @@ -0,0 +1,87 @@ + + + + +unused.foo - Coverage Spans + + + +
@0⦊fn foo<T>(x: T) { + let mut i = 0⦉@0; + while @1,2⦊i < 10⦉@1,2 { + @3,5⦊i != 0⦉@3,5 || @8⦊i != 0⦉@8; + @9,10⦊i += 1⦉@9,10; + } +}@4,11⦊⦉@4,11
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.main.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.main.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..864f6c4feb152 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.main.-------.InstrumentCoverage.0.html @@ -0,0 +1,98 @@ + + + + +unused.main - Coverage Spans + + + +
@0,1,2⦊fn main() -> Result<(), u8> { + foo::<u32>(0); + foo::<f32>(0.0); + Ok(()) +}⦉@0,1,2
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_func.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_func.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..52beea080232e --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_func.-------.InstrumentCoverage.0.html @@ -0,0 +1,86 @@ + + + + +unused.unused_func - Coverage Spans + + + +
@0⦊fn unused_func(mut a: u32) { + if a != 0⦉@0 @1,3⦊{ + a += 1; + }⦉@1,3@2⦊⦉@2 +}@4⦊⦉@4
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_func2.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_func2.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..816ef73ea6b06 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_func2.-------.InstrumentCoverage.0.html @@ -0,0 +1,86 @@ + + + + +unused.unused_func2 - Coverage Spans + + + +
@0⦊fn unused_func2(mut a: u32) { + if a != 0⦉@0 @1,3⦊{ + a += 1; + }⦉@1,3@2⦊⦉@2 +}@4⦊⦉@4
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_func3.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_func3.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..739f9f42db359 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_func3.-------.InstrumentCoverage.0.html @@ -0,0 +1,86 @@ + + + + +unused.unused_func3 - Coverage Spans + + + +
@0⦊fn unused_func3(mut a: u32) { + if a != 0⦉@0 @1,3⦊{ + a += 1; + }⦉@1,3@2⦊⦉@2 +}@4⦊⦉@4
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_template_func.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_template_func.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..9b32bcb47f6e5 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_template_func.-------.InstrumentCoverage.0.html @@ -0,0 +1,87 @@ + + + + +unused.unused_template_func - Coverage Spans + + + +
@0⦊fn unused_template_func<T>(x: T) { + let mut i = 0⦉@0; + while @1,2⦊i < 10⦉@1,2 { + @3,5⦊i != 0⦉@3,5 || @8⦊i != 0⦉@8; + @9,10⦊i += 1⦉@9,10; + } +}@4,11⦊⦉@4,11
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.unused_function.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.unused_function.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..322eaf07677f4 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.unused_function.-------.InstrumentCoverage.0.html @@ -0,0 +1,115 @@ + + + + +used_inline_crate.unused_function - Coverage Spans + + + +
@0,1,2,3⦊pub fn unused_function() { + let is_true = std::env::args().len() == 1; + let mut countdown = 2; + if !is_true⦉@0,1,2,3 @4⦊{ + countdown = 20; + }⦉@4@5⦊⦉@5 +}@6⦊⦉@6
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.unused_generic_function.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.unused_generic_function.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..9e3052ccac155 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.unused_generic_function.-------.InstrumentCoverage.0.html @@ -0,0 +1,133 @@ + + + + +used_inline_crate.unused_generic_function - Coverage Spans + + + +
@0,1,2,3,4⦊pub fn unused_generic_function<T: Debug>(arg: T) { + println!("unused_generic_function with {:?}", arg); +}⦉@0,1,2,3,4
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.unused_private_function.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.unused_private_function.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..e9c381db94025 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.unused_private_function.-------.InstrumentCoverage.0.html @@ -0,0 +1,115 @@ + + + + +used_inline_crate.unused_private_function - Coverage Spans + + + +
@0,1,2,3⦊fn unused_private_function() { + let is_true = std::env::args().len() == 1; + let mut countdown = 2; + if !is_true⦉@0,1,2,3 @4⦊{ + countdown = 20; + }⦉@4@5⦊⦉@5 +}@6⦊⦉@6
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.use_this_lib_crate.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.use_this_lib_crate.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..056f618a403c9 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.use_this_lib_crate.-------.InstrumentCoverage.0.html @@ -0,0 +1,190 @@ + + + + +used_inline_crate.use_this_lib_crate - Coverage Spans + + + +
@0,1,2,3,4,5,6,7,8⦊fn use_this_lib_crate() { + used_from_bin_crate_and_lib_crate_generic_function("used from library used_crate.rs"); + used_with_same_type_from_bin_crate_and_lib_crate_generic_function( + "used from library used_crate.rs", + ); + let some_vec = vec![5, 6, 7, 8]; + used_only_from_this_lib_crate_generic_function(some_vec); + used_only_from_this_lib_crate_generic_function("used ONLY from library used_crate.rs"); +}⦉@0,1,2,3,4,5,6,7,8
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_from_bin_crate_and_lib_crate_generic_function.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_from_bin_crate_and_lib_crate_generic_function.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..0d88b0bc60e34 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_from_bin_crate_and_lib_crate_generic_function.-------.InstrumentCoverage.0.html @@ -0,0 +1,133 @@ + + + + +used_inline_crate.used_from_bin_crate_and_lib_crate_generic_function - Coverage Spans + + + +
@0,1,2,3,4⦊pub fn used_from_bin_crate_and_lib_crate_generic_function<T: Debug>(arg: T) { + println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg); +}⦉@0,1,2,3,4
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_function.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_function.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..d722d9f46ecff --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_function.-------.InstrumentCoverage.0.html @@ -0,0 +1,110 @@ + + + + +used_inline_crate.used_function - Coverage Spans + + + +
@0,1,2,3⦊pub fn used_function() ⦉@0,1,2,3{ + // Initialize test constants in a way that cannot be determined at compile time, to ensure + // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + // dependent conditions. + let @0,1,2,3⦊is_true = std::env::args().len() == 1; + let mut countdown = 0; + if is_true⦉@0,1,2,3 @4⦊{ + countdown = 10; + }⦉@4@5⦊⦉@5 + @6,7⦊use_this_lib_crate(); +}⦉@6,7
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_inline_function.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_inline_function.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..a15c31bb13fd0 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_inline_function.-------.InstrumentCoverage.0.html @@ -0,0 +1,110 @@ + + + + +used_inline_crate.used_inline_function - Coverage Spans + + + +
@0,1,2,3⦊pub fn used_inline_function() ⦉@0,1,2,3{ + // Initialize test constants in a way that cannot be determined at compile time, to ensure + // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + // dependent conditions. + let @0,1,2,3⦊is_true = std::env::args().len() == 1; + let mut countdown = 0; + if is_true⦉@0,1,2,3 @4⦊{ + countdown = 10; + }⦉@4@5⦊⦉@5 + @6,7⦊use_this_lib_crate(); +}⦉@6,7
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_only_from_bin_crate_generic_function.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_only_from_bin_crate_generic_function.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..252ff76d41696 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_only_from_bin_crate_generic_function.-------.InstrumentCoverage.0.html @@ -0,0 +1,133 @@ + + + + +used_inline_crate.used_only_from_bin_crate_generic_function - Coverage Spans + + + +
@0,1,2,3,4⦊pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) { + println!("used_only_from_bin_crate_generic_function with {:?}", arg); +}⦉@0,1,2,3,4
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_only_from_this_lib_crate_generic_function.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_only_from_this_lib_crate_generic_function.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..f8fb4990ad9a6 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_only_from_this_lib_crate_generic_function.-------.InstrumentCoverage.0.html @@ -0,0 +1,133 @@ + + + + +used_inline_crate.used_only_from_this_lib_crate_generic_function - Coverage Spans + + + +
@0,1,2,3,4⦊pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) { + println!("used_only_from_this_lib_crate_generic_function with {:?}", arg); +}⦉@0,1,2,3,4
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_with_same_type_from_bin_crate_and_lib_crate_generic_function.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_with_same_type_from_bin_crate_and_lib_crate_generic_function.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..db5e24d9b1dd6 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.used_inline_crate/used_inline_crate.used_with_same_type_from_bin_crate_and_lib_crate_generic_function.-------.InstrumentCoverage.0.html @@ -0,0 +1,133 @@ + + + + +used_inline_crate.used_with_same_type_from_bin_crate_and_lib_crate_generic_function - Coverage Spans + + + +
@0,1,2,3,4⦊pub fn used_with_same_type_from_bin_crate_and_lib_crate_generic_function<T: Debug>(arg: T) { + println!("used_with_same_type_from_bin_crate_and_lib_crate_generic_function with {:?}", arg); +}⦉@0,1,2,3,4
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.uses_inline_crate/uses_inline_crate.main.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.uses_inline_crate/uses_inline_crate.main.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..179940f6bd5b7 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.uses_inline_crate/uses_inline_crate.main.-------.InstrumentCoverage.0.html @@ -0,0 +1,249 @@ + + + + +uses_inline_crate.main - Coverage Spans + + + +
@0,1,2,3,4,5,6,7,8,9,10⦊fn main() { + used_inline_crate::used_function(); + used_inline_crate::used_inline_function(); + let some_vec = vec![1, 2, 3, 4]; + used_inline_crate::used_only_from_bin_crate_generic_function(&some_vec); + used_inline_crate::used_only_from_bin_crate_generic_function("used from bin uses_crate.rs"); + used_inline_crate::used_from_bin_crate_and_lib_crate_generic_function(some_vec); + used_inline_crate::used_with_same_type_from_bin_crate_and_lib_crate_generic_function( + "interesting?", + ); +}⦉@0,1,2,3,4,5,6,7,8,9,10
+ + diff --git a/src/test/run-make-fulldeps/coverage/async.rs b/src/test/run-make-fulldeps/coverage/async.rs index d5ec32deac125..f08a7b44fbd1a 100644 --- a/src/test/run-make-fulldeps/coverage/async.rs +++ b/src/test/run-make-fulldeps/coverage/async.rs @@ -1,6 +1,6 @@ #![allow(unused_assignments, dead_code)] -// compile-flags: --edition=2018 +// compile-flags: --edition=2018 -C opt-level=1 # fix in rustc_mir/monomorphize/partitioning/mod.rs async fn c(x: u8) -> u8 { if x == 8 { @@ -10,7 +10,7 @@ async fn c(x: u8) -> u8 { } } -async fn d() -> u8 { 1 } +async fn d() -> u8 { 1 } // should have a coverage count `0` (see below) async fn e() -> u8 { 1 } // unused function; executor does not block on `g()` @@ -126,3 +126,14 @@ mod executor { } } } + +// `llvm-cov show` shows no coverage results for the `d()`, even though the +// crate's LLVM IR shows the function exists and has an InstrProf PGO counter, +// and appears to be registered like all other counted functions. +// +// `llvm-cov show --debug` output shows there is at least one `Counter` for this +// line, but counters do not appear in the `Combined regions` section (unlike +// `f()`, which is similar, but does appear in `Combined regions`, and does show +// coverage). The only difference is, `f()` is awaited, but the call to await +// `d()` is not reached. (Note: `d()` will appear in coverage if the test is +// modified to cause it to be awaited.) diff --git a/src/test/run-make-fulldeps/coverage/closure.rs b/src/test/run-make-fulldeps/coverage/closure.rs index 914cb0fe051c7..796512f0c71ed 100644 --- a/src/test/run-make-fulldeps/coverage/closure.rs +++ b/src/test/run-make-fulldeps/coverage/closure.rs @@ -1,5 +1,5 @@ #![allow(unused_assignments, unused_variables)] - +// compile-flags: -C opt-level=2 # fix described in rustc_middle/mir/mono.rs fn main() { // Initialize test constants in a way that cannot be determined at compile time, to ensure // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from diff --git a/src/test/run-make-fulldeps/coverage/lib/used_crate.rs b/src/test/run-make-fulldeps/coverage/lib/used_crate.rs index e5555f9193576..eaa93115ae8d2 100644 --- a/src/test/run-make-fulldeps/coverage/lib/used_crate.rs +++ b/src/test/run-make-fulldeps/coverage/lib/used_crate.rs @@ -1,5 +1,5 @@ #![allow(unused_assignments, unused_variables)] - +// compile-flags: -C opt-level=3 # validates coverage now works with optimizations use std::fmt::Debug; pub fn used_function() { @@ -17,7 +17,7 @@ pub fn used_function() { pub fn used_only_from_bin_crate_generic_function(arg: T) { println!("used_only_from_bin_crate_generic_function with {:?}", arg); } - +// Expect for above function: `Unexecuted instantiation` (see below) pub fn used_only_from_this_lib_crate_generic_function(arg: T) { println!("used_only_from_this_lib_crate_generic_function with {:?}", arg); } @@ -60,45 +60,41 @@ fn use_this_lib_crate() { used_only_from_this_lib_crate_generic_function("used ONLY from library used_crate.rs"); } -// FIXME(#79651): `used_from_bin_crate_and_lib_crate_generic_function()` is covered and executed -// `2` times, but the coverage output also shows (at the bottom of the coverage report): -// ------------------ -// | Unexecuted instantiation: -// ------------------ +// FIXME(#79651): "Unexecuted instantiation" errors appear in coverage results, +// for example: // -// Note, the function name shown in the error seems to change depending on the structure of the -// code, for some reason, including: +// | Unexecuted instantiation: used_crate::used_only_from_bin_crate_generic_function::<_> // -// * used_crate::used_from_bin_crate_and_lib_crate_generic_function::<&str> -// * used_crate::use_this_lib_crate +// These notices appear when `llvm-cov` shows instantiations. This may be a +// default option, but it can be suppressed with: // -// The `Unexecuted instantiation` error may be related to more than one generic function. Since the -// reporting is not consistent, it may not be obvious if there are multiple problems here; however, -// `used_crate::used_from_bin_crate_and_lib_crate_generic_function::<&str>` (which I have seen -// with this error) is the only generic function missing instantiaion coverage counts. +// ```shell +// $ `llvm-cov show --show-instantiations=0 ...` +// ``` // -// The `&str` variant was called from within this `lib` crate, and the `bin` crate also calls this -// function, but with `T` type `&Vec`. +// The notice is triggered because the function is unused by the library itself, +// and when the library is compiled, a synthetic function is generated, so +// unused function coverage can be reported. Coverage can be skipped for unused +// generic functions with: // -// I believe the reason is that one or both crates are generating `Zero` counters for what it -// believes are "Unreachable" instantiations, but those instantiations are counted from the -// coverage map in the other crate. +// ```shell +// $ `rustc -Z instrument-coverage=except-unused-generics ...` +// ``` // -// See `add_unreachable_coverage()` in `mapgen.rs` for more on how these `Zero` counters are added -// for what the funciton believes are `DefId`s that did not get codegenned. I suspect the issue -// may be related to this process, but this needs to be confirmed. It may not be possible to know -// for sure if a function is truly unused and should be reported with `Zero` coverage if it may -// still get used from an external crate. (Something to look at: If the `DefId` in MIR corresponds -// _only_ to the generic function without type parameters, is the `DefId` in the codegenned set, -// instantiated with one of the type parameters (in either or both crates) a *different* `DefId`? -// If so, `add_unreachable_coverage()` would assume the MIR `DefId` was uncovered, and would add -// unreachable coverage. +// Even though this function is used by `uses_crate.rs` (and +// counted), with substitutions for `T`, those instantiations are only generated +// when the generic function is actually used (from the binary, not from this +// library crate). So the test result shows coverage for all instantiated +// versions and their generic type substitutions, plus the `Unexecuted +// instantiation` message for the non-substituted version. This is valid, but +// unfortunately a little confusing. // -// I didn't think they could be different, but if they can, we would need to find the `DefId` for -// the generic function MIR and include it in the set of "codegenned" DefIds if any instantiation -// of that generic function does exist. +// The library crate has its own coverage map, and the only way to show unused +// coverage of a generic function is to include the generic function in the +// coverage map, marked as an "unused function". If the library were used by +// another binary that never used this generic function, then it would be valid +// to show the unused generic, with unknown substitution (`_`). // -// Note, however, for `used_with_same_type_from_bin_crate_and_lib_crate_generic_function()` both -// crates use this function with the same type variant. The function does not have multiple -// instantiations, so the coverage analysis is not confused. No "Unexecuted instantiations" errors -// are reported. +// The alternative is to exclude all generics from being included in the "unused +// functions" list, which would then omit coverage results for +// `unused_generic_function()`, below. diff --git a/src/test/run-make-fulldeps/coverage/lib/used_inline_crate.rs b/src/test/run-make-fulldeps/coverage/lib/used_inline_crate.rs new file mode 100644 index 0000000000000..f4c3dd46f7686 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage/lib/used_inline_crate.rs @@ -0,0 +1,90 @@ +#![allow(unused_assignments, unused_variables)] + +// compile-flags: -C opt-level=3 # validates coverage now works with optimizations + +use std::fmt::Debug; + +pub fn used_function() { + // Initialize test constants in a way that cannot be determined at compile time, to ensure + // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + // dependent conditions. + let is_true = std::env::args().len() == 1; + let mut countdown = 0; + if is_true { + countdown = 10; + } + use_this_lib_crate(); +} + +#[inline(always)] +pub fn used_inline_function() { + // Initialize test constants in a way that cannot be determined at compile time, to ensure + // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + // dependent conditions. + let is_true = std::env::args().len() == 1; + let mut countdown = 0; + if is_true { + countdown = 10; + } + use_this_lib_crate(); +} +// Expect for above function: +// +// | Unexecuted instantiation: used_crate::used_only_from_bin_crate_generic_function::<_> +// +// With `#[inline(always)]` this function is instantiated twice, in both the library crate (which +// does not use it) and the `uses_inline_crate` binary (which does use/call it). + +#[inline(always)] +pub fn used_only_from_bin_crate_generic_function(arg: T) { + println!("used_only_from_bin_crate_generic_function with {:?}", arg); +} +// Expect for above function: `Unexecuted instantiation` (see notes in `used_crate.rs`) + +#[inline(always)] +pub fn used_only_from_this_lib_crate_generic_function(arg: T) { + println!("used_only_from_this_lib_crate_generic_function with {:?}", arg); +} + +#[inline(always)] +pub fn used_from_bin_crate_and_lib_crate_generic_function(arg: T) { + println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg); +} + +#[inline(always)] +pub fn used_with_same_type_from_bin_crate_and_lib_crate_generic_function(arg: T) { + println!("used_with_same_type_from_bin_crate_and_lib_crate_generic_function with {:?}", arg); +} + +#[inline(always)] +pub fn unused_generic_function(arg: T) { + println!("unused_generic_function with {:?}", arg); +} + +#[inline(always)] +pub fn unused_function() { + let is_true = std::env::args().len() == 1; + let mut countdown = 2; + if !is_true { + countdown = 20; + } +} + +#[inline(always)] +fn unused_private_function() { + let is_true = std::env::args().len() == 1; + let mut countdown = 2; + if !is_true { + countdown = 20; + } +} + +fn use_this_lib_crate() { + used_from_bin_crate_and_lib_crate_generic_function("used from library used_crate.rs"); + used_with_same_type_from_bin_crate_and_lib_crate_generic_function( + "used from library used_crate.rs", + ); + let some_vec = vec![5, 6, 7, 8]; + used_only_from_this_lib_crate_generic_function(some_vec); + used_only_from_this_lib_crate_generic_function("used ONLY from library used_crate.rs"); +} diff --git a/src/test/run-make-fulldeps/coverage/unused.rs b/src/test/run-make-fulldeps/coverage/unused.rs new file mode 100644 index 0000000000000..fb6113eb01c2d --- /dev/null +++ b/src/test/run-make-fulldeps/coverage/unused.rs @@ -0,0 +1,39 @@ +fn foo(x: T) { + let mut i = 0; + while i < 10 { + i != 0 || i != 0; + i += 1; + } +} + +fn unused_template_func(x: T) { + let mut i = 0; + while i < 10 { + i != 0 || i != 0; + i += 1; + } +} + +fn unused_func(mut a: u32) { + if a != 0 { + a += 1; + } +} + +fn unused_func2(mut a: u32) { + if a != 0 { + a += 1; + } +} + +fn unused_func3(mut a: u32) { + if a != 0 { + a += 1; + } +} + +fn main() -> Result<(), u8> { + foo::(0); + foo::(0.0); + Ok(()) +} diff --git a/src/test/run-make-fulldeps/coverage/uses_crate.rs b/src/test/run-make-fulldeps/coverage/uses_crate.rs index 8d24b1ca3e67b..20cb05fe5b0ad 100644 --- a/src/test/run-make-fulldeps/coverage/uses_crate.rs +++ b/src/test/run-make-fulldeps/coverage/uses_crate.rs @@ -1,5 +1,5 @@ #![allow(unused_assignments, unused_variables)] - +// compile-flags: -C opt-level=3 # validates coverage now works with optimizations extern crate used_crate; fn main() { diff --git a/src/test/run-make-fulldeps/coverage/uses_inline_crate.rs b/src/test/run-make-fulldeps/coverage/uses_inline_crate.rs new file mode 100644 index 0000000000000..a7fe8532be31b --- /dev/null +++ b/src/test/run-make-fulldeps/coverage/uses_inline_crate.rs @@ -0,0 +1,17 @@ +#![allow(unused_assignments, unused_variables)] + +// compile-flags: -C opt-level=3 # validates coverage now works with optimizations + +extern crate used_inline_crate; + +fn main() { + used_inline_crate::used_function(); + used_inline_crate::used_inline_function(); + let some_vec = vec![1, 2, 3, 4]; + used_inline_crate::used_only_from_bin_crate_generic_function(&some_vec); + used_inline_crate::used_only_from_bin_crate_generic_function("used from bin uses_crate.rs"); + used_inline_crate::used_from_bin_crate_and_lib_crate_generic_function(some_vec); + used_inline_crate::used_with_same_type_from_bin_crate_and_lib_crate_generic_function( + "interesting?", + ); +} From 5a484a1aed9265f44980a748ebca41037b2eb923 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Fri, 19 Mar 2021 20:46:15 -0700 Subject: [PATCH 2/4] gave unused_fn WeakAnyLinkage; moved create_pgo_func_name_var The sample json5format tests produce coverage results again (and work with opt-level 3!) --- .../src/coverageinfo/mod.rs | 37 ++++++++++--------- .../src/traits/coverageinfo.rs | 4 -- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index db61be407c954..2854717bb6e74 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -67,25 +67,12 @@ impl CoverageInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { let mut pgo_func_name_var_map = coverage_context.pgo_func_name_var_map.borrow_mut(); pgo_func_name_var_map .entry(instance) - .or_insert_with(|| self.create_pgo_func_name_var(instance)) + .or_insert_with(|| create_pgo_func_name_var(self, instance)) } else { bug!("Could not get the `coverage_context`"); } } - /// Calls llvm::createPGOFuncNameVar() with the given function instance's - /// mangled function name. The LLVM API returns an llvm::GlobalVariable - /// containing the function name, with the specific variable name and - /// linkage required by LLVM InstrProf source-based coverage - /// instrumentation. Use `bx.get_pgo_func_name_var()` to ensure the variable - /// is only created once per `Instance`. - fn create_pgo_func_name_var(&self, instance: Instance<'tcx>) -> &'ll llvm::Value { - let mangled_fn_name = CString::new(self.tcx.symbol_name(instance).name) - .expect("error converting function name to C string"); - let llfn = self.get_fn(instance); - unsafe { llvm::LLVMRustCoverageCreatePGOFuncNameVar(llfn, mangled_fn_name.as_ptr()) } - } - fn define_unused_fn(&self, def_id: DefId) { let instance = declare_unused_fn(self, &def_id); codegen_unused_fn_and_counter(self, instance); @@ -210,10 +197,8 @@ fn declare_unused_fn(cx: &CodegenCx<'ll, 'tcx>, def_id: &DefId) -> Instance<'tcx ), ); - unsafe { - llvm::LLVMRustSetLinkage(llfn, llvm::Linkage::ExternalLinkage); - llvm::LLVMRustSetVisibility(llfn, llvm::Visibility::Hidden); - } + llvm::set_linkage(llfn, llvm::Linkage::WeakAnyLinkage); + llvm::set_visibility(llfn, llvm::Visibility::Hidden); cx.instances.borrow_mut().insert(instance, llfn); @@ -261,6 +246,22 @@ fn add_function_coverage(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>, de } } +/// Calls llvm::createPGOFuncNameVar() with the given function instance's +/// mangled function name. The LLVM API returns an llvm::GlobalVariable +/// containing the function name, with the specific variable name and linkage +/// required by LLVM InstrProf source-based coverage instrumentation. Use +/// `bx.get_pgo_func_name_var()` to ensure the variable is only created once per +/// `Instance`. +fn create_pgo_func_name_var( + cx: &CodegenCx<'ll, 'tcx>, + instance: Instance<'tcx>, +) -> &'ll llvm::Value { + let mangled_fn_name = CString::new(cx.tcx.symbol_name(instance).name) + .expect("error converting function name to C string"); + let llfn = cx.get_fn(instance); + unsafe { llvm::LLVMRustCoverageCreatePGOFuncNameVar(llfn, mangled_fn_name.as_ptr()) } +} + pub(crate) fn write_filenames_section_to_buffer<'a>( filenames: impl IntoIterator, buffer: &RustString, diff --git a/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs b/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs index c715752839687..b2b0b9eac9179 100644 --- a/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs +++ b/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs @@ -29,10 +29,6 @@ pub trait CoverageInfoMethods<'tcx>: BackendTypes { /// `instrprof.increment()`. The `Value` is only created once per instance. /// Multiple invocations with the same instance return the same `Value`. fn get_pgo_func_name_var(&self, instance: Instance<'tcx>) -> Self::Value; - - /// Creates a new PGO function name variable. This should only be called - /// to fill in the unused function names array. - fn create_pgo_func_name_var(&self, instance: Instance<'tcx>) -> Self::Value; } pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes { From 94a3454b0318e506e167f67aa52df1d054c6500b Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Tue, 23 Mar 2021 00:33:57 -0700 Subject: [PATCH 3/4] Change def_id filter to use requires_monomorphization() Per @wesleywiser's comment: https://github.com/rust-lang/rust/pull/83307#discussion_r599223342 --- compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 239f786a727bd..2ac814bf22838 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -269,7 +269,7 @@ fn add_unused_functions<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { .iter() .filter_map(|local_def_id| { let def_id = local_def_id.to_def_id(); - if ignore_unused_generics && tcx.generics_of(def_id).count() > 0 { + if ignore_unused_generics && tcx.generics_of(def_id).requires_monomorphization(tcx) { return None; } Some(local_def_id.to_def_id()) From 0859cec65255e34221b3b443f4bdd751549fd4c3 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Tue, 23 Mar 2021 14:25:52 -0700 Subject: [PATCH 4/4] Changes from review comments --- .../src/coverageinfo/mod.rs | 26 ++++++++++++++++--- .../src/traits/coverageinfo.rs | 19 +++----------- compiler/rustc_typeck/src/collect.rs | 14 +++++----- 3 files changed, 33 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 2854717bb6e74..32f4fc76b3d1f 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -73,10 +73,26 @@ impl CoverageInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { } } + /// Functions with MIR-based coverage are normally codegenned _only_ if + /// called. LLVM coverage tools typically expect every function to be + /// defined (even if unused), with at least one call to LLVM intrinsic + /// `instrprof.increment`. + /// + /// Codegen a small function that will never be called, with one counter + /// that will never be incremented. + /// + /// For used/called functions, the coverageinfo was already added to the + /// `function_coverage_map` (keyed by function `Instance`) during codegen. + /// But in this case, since the unused function was _not_ previously + /// codegenned, collect the coverage `CodeRegion`s from the MIR and add + /// them. The first `CodeRegion` is used to add a single counter, with the + /// same counter ID used in the injected `instrprof.increment` intrinsic + /// call. Since the function is never called, all other `CodeRegion`s can be + /// added as `unreachable_region`s. fn define_unused_fn(&self, def_id: DefId) { let instance = declare_unused_fn(self, &def_id); codegen_unused_fn_and_counter(self, instance); - add_function_coverage(self, instance, def_id); + add_unused_function_coverage(self, instance, def_id); } } @@ -200,7 +216,7 @@ fn declare_unused_fn(cx: &CodegenCx<'ll, 'tcx>, def_id: &DefId) -> Instance<'tcx llvm::set_linkage(llfn, llvm::Linkage::WeakAnyLinkage); llvm::set_visibility(llfn, llvm::Visibility::Hidden); - cx.instances.borrow_mut().insert(instance, llfn); + assert!(cx.instances.borrow_mut().insert(instance, llfn).is_none()); instance } @@ -221,7 +237,11 @@ fn codegen_unused_fn_and_counter(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<' bx.ret_void(); } -fn add_function_coverage(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>, def_id: DefId) { +fn add_unused_function_coverage( + cx: &CodegenCx<'ll, 'tcx>, + instance: Instance<'tcx>, + def_id: DefId, +) { let tcx = cx.tcx; let mut function_coverage = FunctionCoverage::unused(tcx, instance); diff --git a/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs b/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs index b2b0b9eac9179..cbf570dba4c3e 100644 --- a/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs +++ b/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs @@ -6,22 +6,11 @@ use rustc_middle::ty::Instance; pub trait CoverageInfoMethods<'tcx>: BackendTypes { fn coverageinfo_finalize(&self); - /// Functions with MIR-based coverage are normally codegenned _only_ if - /// called. LLVM coverage tools typically expect every function to be - /// defined (even if unused), with at least one call to LLVM intrinsic - /// `instrprof.increment`. - /// /// Codegen a small function that will never be called, with one counter - /// that will never be incremented. - /// - /// For used/called functions, the coverageinfo was already added to the - /// `function_coverage_map` (keyed by function `Instance`) during codegen. - /// But in this case, since the unused function was _not_ previously - /// codegenned, collect the coverage `CodeRegion`s from the MIR and add - /// them. The first `CodeRegion` is used to add a single counter, with the - /// same counter ID used in the injected `instrprof.increment` intrinsic - /// call. Since the function is never called, all other `CodeRegion`s can be - /// added as `unreachable_region`s. + /// that will never be incremented, that gives LLVM coverage tools a + /// function definition it needs in order to resolve coverage map references + /// to unused functions. This is necessary so unused functions will appear + /// as uncovered (coverage execution count `0`) in LLVM coverage reports. fn define_unused_fn(&self, def_id: DefId); /// For LLVM codegen, returns a function-specific `Value` for a global diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index f69674ea422b0..fbcc6720780c2 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -2876,14 +2876,12 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { InlineAttr::None } else if list_contains_name(&items[..], sym::always) { if tcx.sess.instrument_coverage() { - // Forced inlining will discard functions marked with `#[inline(always)]`. - // If `-Z instrument-coverage` is enabled, the generated coverage map may - // hold references to functions that no longer exist, causing errors in - // coverage reports. (Note, this fixes #82875. I added some tests that - // also include `#[inline(always)]` functions, used and unused, and within - // and across crates, but could not reproduce the reported error in the - // `rustc` test suite. I am able to reproduce the error, following the steps - // described in #82875, and this change does fix that issue.) + // Fixes Issue #82875. Forced inlining allows LLVM to discard functions + // marked with `#[inline(always)]`, which can break coverage reporting if + // that function was referenced from a coverage map. + // + // FIXME(#83429): Is there a better place, e.g., in codegen, to check and + // convert `Always` to `Hint`? InlineAttr::Hint } else { InlineAttr::Always