Skip to content

Commit ec050f2

Browse files
committed
WIP: Injects counters before every block (experimental only)
Uses the span from the start of the first statement to the end of the Terminator. Includes "cleanup" subgraphs. This simple approach does introduce some poor coverage results. For example, a function that only calls macros (like `println!`, for example) may only have counters injected at the macro site, and these may never get counted, for some reason. (I'm seeing some functions with coverage counts of zero, even though they clearly should be called.) I'm developing a follow-up change to this one that injects counters more precisely, by inspecting the Statement and Terminator types, and making different decisions regarding what blocks should be counted, and what spans to use to generate the code regions.
1 parent fa86fc4 commit ec050f2

File tree

4 files changed

+368
-75
lines changed

4 files changed

+368
-75
lines changed

src/librustc_mir/transform/instrument_coverage.rs

+48-15
Original file line numberDiff line numberDiff line change
@@ -174,21 +174,27 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
174174
let body_span = self.hir_body.value.span;
175175
debug!("instrumenting {:?}, span: {:?}", self.mir_def_id, body_span);
176176

177-
// FIXME(richkadel): As a first step, counters are only injected at the top of each
178-
// function. The complete solution will inject counters at each conditional code branch.
179-
let _ignore = mir_body;
180-
let id = self.next_counter();
181-
let function_source_hash = self.function_source_hash();
182-
let code_region = body_span;
183-
let scope = rustc_middle::mir::OUTERMOST_SOURCE_SCOPE;
184-
let is_cleanup = false;
185-
let next_block = rustc_middle::mir::START_BLOCK;
186-
self.inject_call(
187-
self.make_counter(id, function_source_hash, code_region),
188-
scope,
189-
is_cleanup,
190-
next_block,
191-
);
177+
// Experimentally: Instrument every block (including cleanup).
178+
let mut source_info_by_bb = Vec::with_capacity(mir_body.basic_blocks().len());
179+
for (bb, data) in traversal::preorder(mir_body) {
180+
debug!("Injecting before: {:?}", data);
181+
let source_info = self.get_block_source_info(data);
182+
let scope = source_info.scope;
183+
let code_region = source_info.span;
184+
source_info_by_bb.push((code_region, scope, data.is_cleanup, bb));
185+
}
186+
187+
// Inject counters for the selected code regions
188+
for (code_region, scope, is_cleanup, next_block) in source_info_by_bb {
189+
let id = self.next_counter();
190+
let function_source_hash = self.function_source_hash();
191+
self.inject_call(
192+
self.make_counter(id, function_source_hash, code_region),
193+
scope,
194+
is_cleanup,
195+
next_block,
196+
);
197+
}
192198

193199
// FIXME(richkadel): The next step to implement source based coverage analysis will be
194200
// instrumenting branches within functions, and some regions will be counted by "counter
@@ -335,6 +341,33 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
335341
self.mir_body.basic_blocks_mut().swap(next_block, new_block);
336342
}
337343

344+
fn get_block_source_info(&self, data: &BasicBlockData<'tcx>) -> SourceInfo {
345+
if let Some(statement) = data.statements.get(0) {
346+
let start = &statement.source_info;
347+
let end = &data.terminator().source_info;
348+
let scope = if start.scope == end.scope {
349+
start.scope
350+
} else if self.mir_body.is_sub_scope(start.scope, end.scope) {
351+
end.scope
352+
} else if self.mir_body.is_sub_scope(end.scope, start.scope) {
353+
start.scope
354+
} else {
355+
rustc_middle::mir::OUTERMOST_SOURCE_SCOPE
356+
};
357+
let span = start.span.to(end.span);
358+
// FIXME(richkadel): See the embedded `FIXME` comments in `Span::to()`, and note,
359+
// the function may return only the span of `start` or `end`, if the contexts are
360+
// not equal. When attempting to encompass the `Span`s of `BasicBlock`s, as is
361+
// done here, it is apparently not unusual to have different contexts. Upcoming
362+
// (near term) iterations of coverage injection will be more deliberate about
363+
// selecting the spans for code regions, from the analysis of `Statement`s and
364+
// `Terminator`s, so this issue should be revisited at that time.
365+
SourceInfo { span, scope }
366+
} else {
367+
data.terminator().source_info
368+
}
369+
}
370+
338371
fn const_u32(&self, value: u32, span: Span) -> Operand<'tcx> {
339372
Operand::const_from_scalar(self.tcx, self.tcx.types.u32, Scalar::from_u32(value), span)
340373
}

src/test/mir-opt/instrument_coverage/rustc.bar.InstrumentCoverage.diff

+10-10
Original file line numberDiff line numberDiff line change
@@ -3,40 +3,40 @@
33

44
fn bar() -> bool {
55
let mut _0: bool; // return place in scope 0 at $DIR/instrument_coverage.rs:18:13: 18:17
6-
+ let mut _1: (); // in scope 0 at $DIR/instrument_coverage.rs:18:18: 18:18
6+
+ let mut _1: (); // in scope 0 at $DIR/instrument_coverage.rs:19:5: 19:5
77

88
bb0: {
9-
+ StorageLive(_1); // scope 0 at $DIR/instrument_coverage.rs:18:18: 18:18
10-
+ _1 = const std::intrinsics::count_code_region(const 10208505205182607101_u64, const 0_u32, const 501_u32, const 513_u32) -> bb2; // scope 0 at $DIR/instrument_coverage.rs:18:18: 18:18
9+
+ StorageLive(_1); // scope 0 at $DIR/instrument_coverage.rs:19:5: 19:5
10+
+ _1 = const std::intrinsics::count_code_region(const 10208505205182607101_u64, const 0_u32, const 507_u32, const 513_u32) -> bb2; // scope 0 at $DIR/instrument_coverage.rs:19:5: 19:5
1111
+ // ty::Const
1212
+ // + ty: unsafe extern "rust-intrinsic" fn(u64, u32, u32, u32) {std::intrinsics::count_code_region}
1313
+ // + val: Value(Scalar(<ZST>))
1414
+ // mir::Constant
15-
+ // + span: $DIR/instrument_coverage.rs:18:18: 18:18
15+
+ // + span: $DIR/instrument_coverage.rs:19:5: 19:5
1616
+ // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(u64, u32, u32, u32) {std::intrinsics::count_code_region}, val: Value(Scalar(<ZST>)) }
1717
+ // ty::Const
1818
+ // + ty: u64
1919
+ // + val: Value(Scalar(0x8dabe565aaa2aefd))
2020
+ // mir::Constant
21-
+ // + span: $DIR/instrument_coverage.rs:18:18: 18:18
21+
+ // + span: $DIR/instrument_coverage.rs:19:5: 19:5
2222
+ // + literal: Const { ty: u64, val: Value(Scalar(0x8dabe565aaa2aefd)) }
2323
+ // ty::Const
2424
+ // + ty: u32
2525
+ // + val: Value(Scalar(0x00000000))
2626
+ // mir::Constant
27-
+ // + span: $DIR/instrument_coverage.rs:18:18: 18:18
27+
+ // + span: $DIR/instrument_coverage.rs:19:5: 19:5
2828
+ // + literal: Const { ty: u32, val: Value(Scalar(0x00000000)) }
2929
+ // ty::Const
3030
+ // + ty: u32
31-
+ // + val: Value(Scalar(0x000001f5))
31+
+ // + val: Value(Scalar(0x000001fb))
3232
+ // mir::Constant
33-
+ // + span: $DIR/instrument_coverage.rs:18:18: 18:18
34-
+ // + literal: Const { ty: u32, val: Value(Scalar(0x000001f5)) }
33+
+ // + span: $DIR/instrument_coverage.rs:19:5: 19:5
34+
+ // + literal: Const { ty: u32, val: Value(Scalar(0x000001fb)) }
3535
+ // ty::Const
3636
+ // + ty: u32
3737
+ // + val: Value(Scalar(0x00000201))
3838
+ // mir::Constant
39-
+ // + span: $DIR/instrument_coverage.rs:18:18: 18:18
39+
+ // + span: $DIR/instrument_coverage.rs:19:5: 19:5
4040
+ // + literal: Const { ty: u32, val: Value(Scalar(0x00000201)) }
4141
+ }
4242
+

0 commit comments

Comments
 (0)