Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit b688b69

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of rust-lang#83080 - tmiasko:inline-coverage, r=wesleywiser
Make source-based code coverage compatible with MIR inlining When codegenning code coverage use the instance that coverage data was originally generated for, to ensure basic level of compatibility with MIR inlining. Fixes rust-lang#83061
2 parents c99200f + 0d84e0b commit b688b69

25 files changed

+1100
-88
lines changed

‎compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ fn save_function_record(
254254
///
255255
/// 1. The file name of an "Unreachable" function must match the file name of the existing
256256
/// codegenned (covered) function to which the unreachable code regions will be added.
257-
/// 2. The function to which the unreachable code regions will be added must not be a genaric
257+
/// 2. The function to which the unreachable code regions will be added must not be a generic
258258
/// function (must not have type parameters) because the coverage tools will get confused
259259
/// if the codegenned function has more than one instantiation and additional `CodeRegion`s
260260
/// attached to only one of those instantiations.
@@ -284,7 +284,7 @@ fn add_unreachable_coverage<'tcx>(
284284
let all_def_ids: DefIdSet =
285285
tcx.mir_keys(LOCAL_CRATE).iter().map(|local_def_id| local_def_id.to_def_id()).collect();
286286

287-
let (codegenned_def_ids, _) = tcx.collect_and_partition_mono_items(LOCAL_CRATE);
287+
let codegenned_def_ids = tcx.codegened_and_inlined_items(LOCAL_CRATE);
288288

289289
let mut unreachable_def_ids_by_file: FxHashMap<Symbol, Vec<DefId>> = FxHashMap::default();
290290
for &non_codegenned_def_id in all_def_ids.difference(codegenned_def_ids) {

‎compiler/rustc_codegen_ssa/src/coverageinfo/map.rs‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_middle::mir::coverage::{
88
use rustc_middle::ty::Instance;
99
use rustc_middle::ty::TyCtxt;
1010

11-
#[derive(Clone, Debug)]
11+
#[derive(Clone, Debug, PartialEq)]
1212
pub struct Expression {
1313
lhs: ExpressionOperandId,
1414
op: Op,
@@ -64,7 +64,9 @@ impl<'tcx> FunctionCoverage<'tcx> {
6464

6565
/// Adds a code region to be counted by an injected counter intrinsic.
6666
pub fn add_counter(&mut self, id: CounterValueReference, region: CodeRegion) {
67-
self.counters[id].replace(region).expect_none("add_counter called with duplicate `id`");
67+
if let Some(previous_region) = self.counters[id].replace(region.clone()) {
68+
assert_eq!(previous_region, region, "add_counter: code region for id changed");
69+
}
6870
}
6971

7072
/// Both counters and "counter expressions" (or simply, "expressions") can be operands in other
@@ -94,9 +96,18 @@ impl<'tcx> FunctionCoverage<'tcx> {
9496
expression_id, lhs, op, rhs, region
9597
);
9698
let expression_index = self.expression_index(u32::from(expression_id));
97-
self.expressions[expression_index]
98-
.replace(Expression { lhs, op, rhs, region })
99-
.expect_none("add_counter_expression called with duplicate `id_descending_from_max`");
99+
if let Some(previous_expression) = self.expressions[expression_index].replace(Expression {
100+
lhs,
101+
op,
102+
rhs,
103+
region: region.clone(),
104+
}) {
105+
assert_eq!(
106+
previous_expression,
107+
Expression { lhs, op, rhs, region },
108+
"add_counter_expression: expression for id changed"
109+
);
110+
}
100111
}
101112

102113
/// Add a region that will be marked as "unreachable", with a constant "zero counter".

‎compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,38 @@ use crate::traits::*;
22

33
use rustc_middle::mir::coverage::*;
44
use rustc_middle::mir::Coverage;
5+
use rustc_middle::mir::SourceScope;
56

67
use super::FunctionCx;
78

89
impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
9-
pub fn codegen_coverage(&self, bx: &mut Bx, coverage: Coverage) {
10+
pub fn codegen_coverage(&self, bx: &mut Bx, coverage: Coverage, scope: SourceScope) {
11+
// Determine the instance that coverage data was originally generated for.
12+
let scope_data = &self.mir.source_scopes[scope];
13+
let instance = if let Some((inlined_instance, _)) = scope_data.inlined {
14+
self.monomorphize(inlined_instance)
15+
} else if let Some(inlined_scope) = scope_data.inlined_parent_scope {
16+
self.monomorphize(self.mir.source_scopes[inlined_scope].inlined.unwrap().0)
17+
} else {
18+
self.instance
19+
};
20+
1021
let Coverage { kind, code_region } = coverage;
1122
match kind {
1223
CoverageKind::Counter { function_source_hash, id } => {
13-
if bx.set_function_source_hash(self.instance, function_source_hash) {
24+
if bx.set_function_source_hash(instance, function_source_hash) {
1425
// If `set_function_source_hash()` returned true, the coverage map is enabled,
1526
// so continue adding the counter.
1627
if let Some(code_region) = code_region {
1728
// Note: Some counters do not have code regions, but may still be referenced
1829
// from expressions. In that case, don't add the counter to the coverage map,
1930
// but do inject the counter intrinsic.
20-
bx.add_coverage_counter(self.instance, id, code_region);
31+
bx.add_coverage_counter(instance, id, code_region);
2132
}
2233

23-
let coverageinfo = bx.tcx().coverageinfo(self.instance.def_id());
34+
let coverageinfo = bx.tcx().coverageinfo(instance.def_id());
2435

25-
let fn_name = bx.create_pgo_func_name_var(self.instance);
36+
let fn_name = bx.create_pgo_func_name_var(instance);
2637
let hash = bx.const_u64(function_source_hash);
2738
let num_counters = bx.const_u32(coverageinfo.num_counters);
2839
let index = bx.const_u32(u32::from(id));
@@ -34,11 +45,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
3445
}
3546
}
3647
CoverageKind::Expression { id, lhs, op, rhs } => {
37-
bx.add_coverage_counter_expression(self.instance, id, lhs, op, rhs, code_region);
48+
bx.add_coverage_counter_expression(instance, id, lhs, op, rhs, code_region);
3849
}
3950
CoverageKind::Unreachable => {
4051
bx.add_coverage_unreachable(
41-
self.instance,
52+
instance,
4253
code_region.expect("unreachable regions always have code regions"),
4354
);
4455
}

‎compiler/rustc_codegen_ssa/src/mir/statement.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
112112
bx
113113
}
114114
mir::StatementKind::Coverage(box ref coverage) => {
115-
self.codegen_coverage(&mut bx, coverage.clone());
115+
self.codegen_coverage(&mut bx, coverage.clone(), statement.source_info.scope);
116116
bx
117117
}
118118
mir::StatementKind::CopyNonOverlapping(box mir::CopyNonOverlapping {

‎compiler/rustc_middle/src/query/mod.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1407,6 +1407,14 @@ rustc_queries! {
14071407
query is_codegened_item(def_id: DefId) -> bool {
14081408
desc { |tcx| "determining whether `{}` needs codegen", tcx.def_path_str(def_id) }
14091409
}
1410+
1411+
/// All items participating in code generation together with items inlined into them.
1412+
query codegened_and_inlined_items(_: CrateNum)
1413+
-> &'tcx DefIdSet {
1414+
eval_always
1415+
desc { "codegened_and_inlined_items" }
1416+
}
1417+
14101418
query codegen_unit(_: Symbol) -> &'tcx CodegenUnit<'tcx> {
14111419
desc { "codegen_unit" }
14121420
}

‎compiler/rustc_mir/src/monomorphize/partitioning/mod.rs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,8 +424,33 @@ fn collect_and_partition_mono_items<'tcx>(
424424
(tcx.arena.alloc(mono_items), codegen_units)
425425
}
426426

427+
fn codegened_and_inlined_items<'tcx>(tcx: TyCtxt<'tcx>, cnum: CrateNum) -> &'tcx DefIdSet {
428+
let (items, cgus) = tcx.collect_and_partition_mono_items(cnum);
429+
let mut visited = DefIdSet::default();
430+
let mut result = items.clone();
431+
432+
for cgu in cgus {
433+
for (item, _) in cgu.items() {
434+
if let MonoItem::Fn(ref instance) = item {
435+
let did = instance.def_id();
436+
if !visited.insert(did) {
437+
continue;
438+
}
439+
for scope in &tcx.instance_mir(instance.def).source_scopes {
440+
if let Some((ref inlined, _)) = scope.inlined {
441+
result.insert(inlined.def_id());
442+
}
443+
}
444+
}
445+
}
446+
}
447+
448+
tcx.arena.alloc(result)
449+
}
450+
427451
pub fn provide(providers: &mut Providers) {
428452
providers.collect_and_partition_mono_items = collect_and_partition_mono_items;
453+
providers.codegened_and_inlined_items = codegened_and_inlined_items;
429454

430455
providers.is_codegened_item = |tcx, def_id| {
431456
let (all_mono_items, _) = tcx.collect_and_partition_mono_items(LOCAL_CRATE);

‎compiler/rustc_mir/src/transform/coverage/query.rs‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
use super::*;
22

33
use rustc_middle::mir::coverage::*;
4-
use rustc_middle::mir::visit::Visitor;
5-
use rustc_middle::mir::{self, Coverage, CoverageInfo, Location};
4+
use rustc_middle::mir::{self, Body, Coverage, CoverageInfo};
65
use rustc_middle::ty::query::Providers;
76
use rustc_middle::ty::{self, TyCtxt};
87
use rustc_span::def_id::DefId;
@@ -85,10 +84,21 @@ impl CoverageVisitor {
8584
}
8685
}
8786
}
88-
}
8987

90-
impl Visitor<'_> for CoverageVisitor {
91-
fn visit_coverage(&mut self, coverage: &Coverage, _location: Location) {
88+
fn visit_body(&mut self, body: &Body<'_>) {
89+
for bb_data in body.basic_blocks().iter() {
90+
for statement in bb_data.statements.iter() {
91+
if let StatementKind::Coverage(box ref coverage) = statement.kind {
92+
if is_inlined(body, statement) {
93+
continue;
94+
}
95+
self.visit_coverage(coverage);
96+
}
97+
}
98+
}
99+
}
100+
101+
fn visit_coverage(&mut self, coverage: &Coverage) {
92102
if self.add_missing_operands {
93103
match coverage.kind {
94104
CoverageKind::Expression { lhs, rhs, .. } => {
@@ -129,10 +139,14 @@ fn coverageinfo_from_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo
129139
}
130140

131141
fn covered_file_name<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Option<Symbol> {
132-
for bb_data in mir_body(tcx, def_id).basic_blocks().iter() {
142+
let body = mir_body(tcx, def_id);
143+
for bb_data in body.basic_blocks().iter() {
133144
for statement in bb_data.statements.iter() {
134145
if let StatementKind::Coverage(box ref coverage) = statement.kind {
135146
if let Some(code_region) = coverage.code_region.as_ref() {
147+
if is_inlined(body, statement) {
148+
continue;
149+
}
136150
return Some(code_region.file_name);
137151
}
138152
}
@@ -151,17 +165,26 @@ fn mir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx mir::Body<'tcx> {
151165
}
152166

153167
fn covered_code_regions<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Vec<&'tcx CodeRegion> {
154-
mir_body(tcx, def_id)
155-
.basic_blocks()
168+
let body = mir_body(tcx, def_id);
169+
body.basic_blocks()
156170
.iter()
157171
.map(|data| {
158172
data.statements.iter().filter_map(|statement| match statement.kind {
159173
StatementKind::Coverage(box ref coverage) => {
160-
coverage.code_region.as_ref() // may be None
174+
if is_inlined(body, statement) {
175+
None
176+
} else {
177+
coverage.code_region.as_ref() // may be None
178+
}
161179
}
162180
_ => None,
163181
})
164182
})
165183
.flatten()
166184
.collect()
167185
}
186+
187+
fn is_inlined(body: &Body<'_>, statement: &Statement<'_>) -> bool {
188+
let scope_data = &body.source_scopes[statement.source_info.scope];
189+
scope_data.inlined.is_some() || scope_data.inlined_parent_scope.is_some()
190+
}

‎compiler/rustc_mir/src/transform/inline.rs‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,6 @@ struct CallSite<'tcx> {
3939

4040
/// Returns true if MIR inlining is enabled in the current compilation session.
4141
crate fn is_enabled(tcx: TyCtxt<'_>) -> bool {
42-
if tcx.sess.opts.debugging_opts.instrument_coverage {
43-
// Since `Inline` happens after `InstrumentCoverage`, the function-specific coverage
44-
// counters can be invalidated, such as by merging coverage counter statements from
45-
// a pre-inlined function into a different function. This kind of change is invalid,
46-
// so inlining must be skipped. Note: This check is performed here so inlining can
47-
// be disabled without preventing other optimizations (regardless of `mir_opt_level`).
48-
return false;
49-
}
50-
5142
if let Some(enabled) = tcx.sess.opts.debugging_opts.inline_mir {
5243
return enabled;
5344
}

‎compiler/rustc_session/src/config.rs‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,25 +1937,6 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
19371937
}
19381938
Some(SymbolManglingVersion::V0) => {}
19391939
}
1940-
1941-
if let Some(mir_opt_level) = debugging_opts.mir_opt_level {
1942-
if mir_opt_level > 1 {
1943-
// Functions inlined during MIR transform can, at best, make it impossible to
1944-
// effectively cover inlined functions, and, at worst, break coverage map generation
1945-
// during LLVM codegen. For example, function counter IDs are only unique within a
1946-
// function. Inlining after these counters are injected can produce duplicate counters,
1947-
// resulting in an invalid coverage map (and ICE); so this option combination is not
1948-
// allowed.
1949-
early_warn(
1950-
error_format,
1951-
&format!(
1952-
"`-Z mir-opt-level={}` (or any level > 1) enables function inlining, which \
1953-
is incompatible with `-Z instrument-coverage`. Inlining will be disabled.",
1954-
mir_opt_level,
1955-
),
1956-
);
1957-
}
1958-
}
19591940
}
19601941

19611942
if let Ok(graphviz_font) = std::env::var("RUSTC_GRAPHVIZ_FONT") {

‎src/test/run-make-fulldeps/coverage-reports/Makefile‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,13 @@ endif
8282
%: $(SOURCEDIR)/lib/%.rs
8383
# Compile the test library with coverage instrumentation
8484
$(RUSTC) $(SOURCEDIR)/lib/$@.rs \
85-
$$( grep -q '^\/\/ require-rust-edition-2018' $(SOURCEDIR)/lib/$@.rs && echo "--edition=2018" ) \
85+
$$( sed -nE 's#^// compile-flags:(.*)#\1# p' $(SOURCEDIR)/lib/$@.rs) \
8686
--crate-type rlib -Zinstrument-coverage
8787

8888
%: $(SOURCEDIR)/%.rs
8989
# Compile the test program with coverage instrumentation
9090
$(RUSTC) $(SOURCEDIR)/$@.rs \
91-
$$( grep -q '^\/\/ require-rust-edition-2018' $(SOURCEDIR)/$@.rs && echo "--edition=2018" ) \
91+
$$( sed -nE 's#^// compile-flags:(.*)#\1# p' $(SOURCEDIR)/$@.rs) \
9292
-L "$(TMPDIR)" -Zinstrument-coverage
9393

9494
# Run it in order to generate some profiling data,
@@ -107,7 +107,7 @@ endif
107107
# Run it through rustdoc as well to cover doctests
108108
LLVM_PROFILE_FILE="$(TMPDIR)"/$@-%p.profraw \
109109
$(RUSTDOC) --crate-name workaround_for_79771 --test $(SOURCEDIR)/$@.rs \
110-
$$( grep -q '^\/\/ require-rust-edition-2018' $(SOURCEDIR)/$@.rs && echo "--edition=2018" ) \
110+
$$( sed -nE 's#^// compile-flags:(.*)#\1# p' $(SOURCEDIR)/$@.rs) \
111111
-L "$(TMPDIR)" -Zinstrument-coverage \
112112
-Z unstable-options --persist-doctests=$(TMPDIR)/rustdoc-$@
113113

0 commit comments

Comments
 (0)