Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Completes support for coverage in external crates #75037

Merged
merged 1 commit into from
Aug 5, 2020
Merged

Completes support for coverage in external crates #75037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 27 additions & 9 deletions library/core/src/intrinsics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1950,15 +1950,20 @@ extern "rust-intrinsic" {
pub fn ptr_offset_from<T>(ptr: *const T, base: *const T) -> isize;

/// Internal placeholder for injecting code coverage counters when the "instrument-coverage"
/// option is enabled. The placeholder is replaced with `llvm.instrprof.increment` during code
/// generation.
/// option is enabled. The source code region information is extracted prior to code generation,
/// and added to the "coverage map", which is injected into the generated code as additional
/// data. This intrinsic then triggers the generation of LLVM intrinsic call
/// `instrprof.increment`, using the remaining args (`function_source_hash` and `index`).
#[cfg(not(bootstrap))]
#[lang = "count_code_region"]
pub fn count_code_region(
function_source_hash: u64,
index: u32,
start_byte_pos: u32,
end_byte_pos: u32,
file_name: &'static str,
start_line: u32,
start_col: u32,
end_line: u32,
end_col: u32,
);

/// Internal marker for code coverage expressions, injected into the MIR when the
Expand All @@ -1973,8 +1978,11 @@ extern "rust-intrinsic" {
index: u32,
left_index: u32,
right_index: u32,
start_byte_pos: u32,
end_byte_pos: u32,
file_name: &'static str,
start_line: u32,
start_col: u32,
end_line: u32,
end_col: u32,
);

/// This marker identifies a code region and two other counters or counter expressions
Expand All @@ -1986,14 +1994,24 @@ extern "rust-intrinsic" {
index: u32,
left_index: u32,
right_index: u32,
start_byte_pos: u32,
end_byte_pos: u32,
file_name: &'static str,
start_line: u32,
start_col: u32,
end_line: u32,
end_col: u32,
);

/// This marker identifies a code region to be added to the "coverage map" to indicate source
/// code that can never be reached.
/// (See `coverage_counter_add` for more information.)
pub fn coverage_unreachable(start_byte_pos: u32, end_byte_pos: u32);
#[cfg(not(bootstrap))]
pub fn coverage_unreachable(
file_name: &'static str,
start_line: u32,
start_col: u32,
end_line: u32,
end_col: u32,
);

/// See documentation of `<*const T>::guaranteed_eq` for details.
#[rustc_const_unstable(feature = "const_raw_ptr_comparison", issue = "53020")]
Expand Down
24 changes: 12 additions & 12 deletions src/librustc_codegen_llvm/coverageinfo/mapgen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ impl CoverageMapGenerator {
fn write_coverage_mappings(
&mut self,
expressions: Vec<CounterExpression>,
counter_regions: impl Iterator<Item = (Counter, &'a Region)>,
counter_regions: impl Iterator<Item = (Counter, &'tcx Region<'tcx>)>,
coverage_mappings_buffer: &RustString,
) {
let mut counter_regions = counter_regions.collect::<Vec<_>>();
Expand All @@ -102,7 +102,7 @@ impl CoverageMapGenerator {

let mut virtual_file_mapping = Vec::new();
let mut mapping_regions = Vec::new();
let mut current_file_path = None;
let mut current_file_name = None;
let mut current_file_id = 0;

// Convert the list of (Counter, Region) pairs to an array of `CounterMappingRegion`, sorted
Expand All @@ -112,22 +112,22 @@ impl CoverageMapGenerator {
// `filenames` array.
counter_regions.sort_unstable_by_key(|(_counter, region)| *region);
for (counter, region) in counter_regions {
let (file_path, start_line, start_col, end_line, end_col) = region.file_start_and_end();
let same_file = current_file_path.as_ref().map_or(false, |p| p == file_path);
let Region { file_name, start_line, start_col, end_line, end_col } = *region;
let same_file = current_file_name.as_ref().map_or(false, |p| p == file_name);
if !same_file {
if current_file_path.is_some() {
if current_file_name.is_some() {
current_file_id += 1;
}
current_file_path = Some(file_path.clone());
let filename = CString::new(file_path.to_string_lossy().to_string())
.expect("null error converting filename to C string");
debug!(" file_id: {} = '{:?}'", current_file_id, filename);
let filenames_index = match self.filename_to_index.get(&filename) {
current_file_name = Some(file_name.to_string());
let c_filename =
CString::new(file_name).expect("null error converting filename to C string");
debug!(" file_id: {} = '{:?}'", current_file_id, c_filename);
let filenames_index = match self.filename_to_index.get(&c_filename) {
Some(index) => *index,
None => {
let index = self.filenames.len() as u32;
self.filenames.push(filename.clone());
self.filename_to_index.insert(filename.clone(), index);
self.filenames.push(c_filename.clone());
self.filename_to_index.insert(c_filename.clone(), index);
index
}
};
Expand Down
40 changes: 13 additions & 27 deletions src/librustc_codegen_llvm/coverageinfo/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::common::CodegenCx;
use libc::c_uint;
use llvm::coverageinfo::CounterMappingRegion;
use log::debug;
use rustc_codegen_ssa::coverageinfo::map::{CounterExpression, ExprKind, FunctionCoverage};
use rustc_codegen_ssa::coverageinfo::map::{CounterExpression, ExprKind, FunctionCoverage, Region};
use rustc_codegen_ssa::traits::{
BaseTypeMethods, CoverageInfoBuilderMethods, CoverageInfoMethods, StaticMethods,
};
Expand Down Expand Up @@ -49,19 +49,18 @@ impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
instance: Instance<'tcx>,
function_source_hash: u64,
id: u32,
start_byte_pos: u32,
end_byte_pos: u32,
region: Region<'tcx>,
) {
debug!(
"adding counter to coverage_regions: instance={:?}, function_source_hash={}, id={}, \
byte range {}..{}",
instance, function_source_hash, id, start_byte_pos, end_byte_pos,
at {:?}",
instance, function_source_hash, id, region,
);
let mut coverage_regions = self.coverage_context().function_coverage_map.borrow_mut();
coverage_regions
.entry(instance)
.or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
.add_counter(function_source_hash, id, start_byte_pos, end_byte_pos);
.add_counter(function_source_hash, id, region);
}

fn add_counter_expression_region(
Expand All @@ -71,43 +70,30 @@ impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
lhs: u32,
op: ExprKind,
rhs: u32,
start_byte_pos: u32,
end_byte_pos: u32,
region: Region<'tcx>,
) {
debug!(
"adding counter expression to coverage_regions: instance={:?}, id={}, {} {:?} {}, \
byte range {}..{}",
instance, id_descending_from_max, lhs, op, rhs, start_byte_pos, end_byte_pos,
at {:?}",
instance, id_descending_from_max, lhs, op, rhs, region,
);
let mut coverage_regions = self.coverage_context().function_coverage_map.borrow_mut();
coverage_regions
.entry(instance)
.or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
.add_counter_expression(
id_descending_from_max,
lhs,
op,
rhs,
start_byte_pos,
end_byte_pos,
);
.add_counter_expression(id_descending_from_max, lhs, op, rhs, region);
}

fn add_unreachable_region(
&mut self,
instance: Instance<'tcx>,
start_byte_pos: u32,
end_byte_pos: u32,
) {
fn add_unreachable_region(&mut self, instance: Instance<'tcx>, region: Region<'tcx>) {
debug!(
"adding unreachable code to coverage_regions: instance={:?}, byte range {}..{}",
instance, start_byte_pos, end_byte_pos,
"adding unreachable code to coverage_regions: instance={:?}, at {:?}",
instance, region,
);
let mut coverage_regions = self.coverage_context().function_coverage_map.borrow_mut();
coverage_regions
.entry(instance)
.or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
.add_unreachable_region(start_byte_pos, end_byte_pos);
.add_unreachable_region(region);
}
}

Expand Down
121 changes: 61 additions & 60 deletions src/librustc_codegen_llvm/intrinsic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_ast::ast;
use rustc_codegen_ssa::base::{compare_simd_types, to_immediate, wants_msvc_seh};
use rustc_codegen_ssa::common::span_invalid_monomorphization_error;
use rustc_codegen_ssa::common::{IntPredicate, TypeKind};
use rustc_codegen_ssa::coverageinfo::ExprKind;
use rustc_codegen_ssa::coverageinfo;
use rustc_codegen_ssa::glue;
use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue};
use rustc_codegen_ssa::mir::place::PlaceRef;
Expand Down Expand Up @@ -93,64 +93,64 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
let mut is_codegen_intrinsic = true;
// Set `is_codegen_intrinsic` to `false` to bypass `codegen_intrinsic_call()`.

if self.tcx.sess.opts.debugging_opts.instrument_coverage {
// If the intrinsic is from the local MIR, add the coverage information to the Codegen
// context, to be encoded into the local crate's coverage map.
if caller_instance.def_id().is_local() {
// FIXME(richkadel): Make sure to add coverage analysis tests on a crate with
// external crate dependencies, where:
// 1. Both binary and dependent crates are compiled with `-Zinstrument-coverage`
// 2. Only binary is compiled with `-Zinstrument-coverage`
// 3. Only dependent crates are compiled with `-Zinstrument-coverage`
match intrinsic {
sym::count_code_region => {
use coverage::count_code_region_args::*;
self.add_counter_region(
caller_instance,
op_to_u64(&args[FUNCTION_SOURCE_HASH]),
op_to_u32(&args[COUNTER_ID]),
op_to_u32(&args[START_BYTE_POS]),
op_to_u32(&args[END_BYTE_POS]),
);
}
sym::coverage_counter_add | sym::coverage_counter_subtract => {
use coverage::coverage_counter_expression_args::*;
self.add_counter_expression_region(
caller_instance,
op_to_u32(&args[EXPRESSION_ID]),
op_to_u32(&args[LEFT_ID]),
if intrinsic == sym::coverage_counter_add {
ExprKind::Add
} else {
ExprKind::Subtract
},
op_to_u32(&args[RIGHT_ID]),
op_to_u32(&args[START_BYTE_POS]),
op_to_u32(&args[END_BYTE_POS]),
);
}
sym::coverage_unreachable => {
use coverage::coverage_unreachable_args::*;
self.add_unreachable_region(
caller_instance,
op_to_u32(&args[START_BYTE_POS]),
op_to_u32(&args[END_BYTE_POS]),
);
}
_ => {}
}
// FIXME(richkadel): Make sure to add coverage analysis tests on a crate with
// external crate dependencies, where:
// 1. Both binary and dependent crates are compiled with `-Zinstrument-coverage`
// 2. Only binary is compiled with `-Zinstrument-coverage`
// 3. Only dependent crates are compiled with `-Zinstrument-coverage`
match intrinsic {
sym::count_code_region => {
use coverage::count_code_region_args::*;
self.add_counter_region(
caller_instance,
op_to_u64(&args[FUNCTION_SOURCE_HASH]),
op_to_u32(&args[COUNTER_ID]),
coverageinfo::Region::new(
op_to_str_slice(&args[FILE_NAME]),
op_to_u32(&args[START_LINE]),
op_to_u32(&args[START_COL]),
op_to_u32(&args[END_LINE]),
op_to_u32(&args[END_COL]),
),
);
}

// Only the `count_code_region` coverage intrinsic is translated into an actual LLVM
// intrinsic call (local or not); otherwise, set `is_codegen_intrinsic` to `false`.
match intrinsic {
sym::coverage_counter_add
| sym::coverage_counter_subtract
| sym::coverage_unreachable => {
is_codegen_intrinsic = false;
}
_ => {}
sym::coverage_counter_add | sym::coverage_counter_subtract => {
is_codegen_intrinsic = false;
use coverage::coverage_counter_expression_args::*;
self.add_counter_expression_region(
caller_instance,
op_to_u32(&args[EXPRESSION_ID]),
op_to_u32(&args[LEFT_ID]),
if intrinsic == sym::coverage_counter_add {
coverageinfo::ExprKind::Add
} else {
coverageinfo::ExprKind::Subtract
},
op_to_u32(&args[RIGHT_ID]),
coverageinfo::Region::new(
op_to_str_slice(&args[FILE_NAME]),
op_to_u32(&args[START_LINE]),
op_to_u32(&args[START_COL]),
op_to_u32(&args[END_LINE]),
op_to_u32(&args[END_COL]),
),
);
}
sym::coverage_unreachable => {
is_codegen_intrinsic = false;
use coverage::coverage_unreachable_args::*;
self.add_unreachable_region(
caller_instance,
coverageinfo::Region::new(
op_to_str_slice(&args[FILE_NAME]),
op_to_u32(&args[START_LINE]),
op_to_u32(&args[START_COL]),
op_to_u32(&args[END_LINE]),
op_to_u32(&args[END_COL]),
),
);
}
_ => {}
}
is_codegen_intrinsic
}
Expand Down Expand Up @@ -215,9 +215,6 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
self.call(llfn, &[], None)
}
sym::count_code_region => {
// FIXME(richkadel): The current implementation assumes the MIR for the given
// caller_instance represents a single function. Validate and/or correct if inlining
// and/or monomorphization invalidates these assumptions.
let coverageinfo = tcx.coverageinfo(caller_instance.def_id());
let mangled_fn = tcx.symbol_name(caller_instance);
let (mangled_fn_name, _len_val) = self.const_str(Symbol::intern(mangled_fn.name));
Expand Down Expand Up @@ -2283,6 +2280,10 @@ fn float_type_width(ty: Ty<'_>) -> Option<u64> {
}
}

fn op_to_str_slice<'tcx>(op: &Operand<'tcx>) -> &'tcx str {
Operand::value_from_const(op).try_to_str_slice().expect("Value is &str")
}

fn op_to_u32<'tcx>(op: &Operand<'tcx>) -> u32 {
Operand::scalar_from_const(op).to_u32().expect("Scalar is u32")
}
Expand Down
Loading