From b260126ebb17a5b514b8a1ab8e7d7d2a374a75a8 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 2 Apr 2024 13:53:03 +0800 Subject: [PATCH 01/29] coverage. Add mcdc coverage option --- compiler/rustc_interface/src/tests.rs | 2 +- compiler/rustc_session/src/config.rs | 2 ++ compiler/rustc_session/src/options.rs | 34 +++++++++++++------ compiler/rustc_session/src/session.rs | 4 +++ src/doc/rustc/src/instrument-coverage.md | 4 +-- .../src/compiler-flags/coverage-options.md | 2 +- .../coverage-options.bad.stderr | 2 +- .../instrument-coverage/coverage-options.rs | 6 ++++ 8 files changed, 41 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index d2fb65b5d4f8e..3de1b98aa4e56 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -759,7 +759,7 @@ fn test_unstable_options_tracking_hash() { ); tracked!(codegen_backend, Some("abc".to_string())); tracked!(collapse_macro_debuginfo, CollapseMacroDebuginfo::Yes); - tracked!(coverage_options, CoverageOptions { branch: true }); + tracked!(coverage_options, CoverageOptions { branch: true, mcdc: true }); tracked!(crate_attr, vec!["abc".to_string()]); tracked!(cross_crate_inline_threshold, InliningThreshold::Always); tracked!(debug_info_for_profiling, true); diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index a4919b25fe35e..05e19f7783f84 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -148,6 +148,8 @@ pub enum InstrumentCoverage { pub struct CoverageOptions { /// Add branch coverage instrumentation. pub branch: bool, + /// Add mcdc coverage instrumentation. + pub mcdc: bool, } /// Settings for `-Z instrument-xray` flag. diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 5e7c246509771..e3eaaf3eb455c 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -396,7 +396,7 @@ mod desc { pub const parse_optimization_fuel: &str = "crate=integer"; pub const parse_dump_mono_stats: &str = "`markdown` (default) or `json`"; pub const parse_instrument_coverage: &str = parse_bool; - pub const parse_coverage_options: &str = "`branch` or `no-branch`"; + pub const parse_coverage_options: &str = "either `no-branch`, `branch` or `mcdc`"; pub const parse_instrument_xray: &str = "either a boolean (`yes`, `no`, `on`, `off`, etc), or a comma separated list of settings: `always` or `never` (mutually exclusive), `ignore-loops`, `instruction-threshold=N`, `skip-entry`, `skip-exit`"; pub const parse_unpretty: &str = "`string` or `string=string`"; pub const parse_treat_err_as_bug: &str = "either no value or a non-negative number"; @@ -943,16 +943,30 @@ mod parse { pub(crate) fn parse_coverage_options(slot: &mut CoverageOptions, v: Option<&str>) -> bool { let Some(v) = v else { return true }; - for option in v.split(',') { - let (option, enabled) = match option.strip_prefix("no-") { - Some(without_no) => (without_no, false), - None => (option, true), - }; - let slot = match option { - "branch" => &mut slot.branch, + let set_branch_option = |slot: &mut CoverageOptions, option: &str| { + match option { + "no-branch" => slot.branch = false, + "branch" => slot.branch = true, + "mcdc" => { + slot.mcdc = true; + slot.branch = true + } _ => return false, - }; - *slot = enabled; + } + true + }; + + // Once an option is parsed we removed it from the array so that conflicting options such as "branch,no-branch" could be detected. + let mut parsers_set: [Option<&dyn Fn(&mut CoverageOptions, &str) -> bool>; 1] = + [Some(&set_branch_option)]; + + for option in v.split(',') { + if !parsers_set + .iter_mut() + .any(|p| p.is_some_and(|parser| parser(slot, option)).then(|| p.take()).is_some()) + { + return false; + } } true diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 22ca8a3cf3ec2..7729befbe7ad8 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -352,6 +352,10 @@ impl Session { self.instrument_coverage() && self.opts.unstable_opts.coverage_options.branch } + pub fn instrument_coverage_mcdc(&self) -> bool { + self.instrument_coverage() && self.opts.unstable_opts.coverage_options.mcdc + } + pub fn is_sanitizer_cfi_enabled(&self) -> bool { self.opts.unstable_opts.sanitizer.contains(SanitizerSet::CFI) } diff --git a/src/doc/rustc/src/instrument-coverage.md b/src/doc/rustc/src/instrument-coverage.md index 185a3ba5dbd41..bbd81e7437c67 100644 --- a/src/doc/rustc/src/instrument-coverage.md +++ b/src/doc/rustc/src/instrument-coverage.md @@ -351,8 +351,8 @@ $ llvm-cov report \ This unstable option provides finer control over some aspects of coverage instrumentation. Pass one or more of the following values, separated by commas. -- `branch` or `no-branch` - - Enables or disables branch coverage instrumentation. +- Either `no-branch`, `branch` or `mcdc` + - `branch` enables branch coverage instrumentation and `mcdc` further enables modified condition/decision coverage instrumentation. `no-branch` disables branch coverage instrumentation, which is same as do not pass `branch` or `mcdc`. ## Other references diff --git a/src/doc/unstable-book/src/compiler-flags/coverage-options.md b/src/doc/unstable-book/src/compiler-flags/coverage-options.md index 450573cc6c746..7972e73119bfe 100644 --- a/src/doc/unstable-book/src/compiler-flags/coverage-options.md +++ b/src/doc/unstable-book/src/compiler-flags/coverage-options.md @@ -5,4 +5,4 @@ This option controls details of the coverage instrumentation performed by Multiple options can be passed, separated by commas. Valid options are: -- `branch` or `no-branch`: Enables or disables branch coverage instrumentation. +- `no-branch`, `branch` or `mcdc`: `branch` enables branch coverage instrumentation and `mcdc` further enables modified condition/decision coverage instrumentation. `no-branch` disables branch coverage instrumentation, which is same as do not pass `branch` or `mcdc` diff --git a/tests/ui/instrument-coverage/coverage-options.bad.stderr b/tests/ui/instrument-coverage/coverage-options.bad.stderr index ca82dc5bdb93d..f6e5421f87845 100644 --- a/tests/ui/instrument-coverage/coverage-options.bad.stderr +++ b/tests/ui/instrument-coverage/coverage-options.bad.stderr @@ -1,2 +1,2 @@ -error: incorrect value `bad` for unstable option `coverage-options` - `branch` or `no-branch` was expected +error: incorrect value `bad` for unstable option `coverage-options` - either `no-branch`, `branch` or `mcdc` was expected diff --git a/tests/ui/instrument-coverage/coverage-options.rs b/tests/ui/instrument-coverage/coverage-options.rs index a62e0554f7690..50c01ed29b5ec 100644 --- a/tests/ui/instrument-coverage/coverage-options.rs +++ b/tests/ui/instrument-coverage/coverage-options.rs @@ -8,7 +8,13 @@ //@ [no-branch] check-pass //@ [no-branch] compile-flags: -Zcoverage-options=no-branch +//@ [mcdc] check-pass +//@ [mcdc] compile-flags: -Zcoverage-options=mcdc + //@ [bad] check-fail //@ [bad] compile-flags: -Zcoverage-options=bad +//@ [conflict] check-fail +//@ [conflict] compile-flags: -Zcoverage-options=no-branch,mcdc + fn main() {} From c7071c2423de3d79e017bba24e541626b2cca1ef Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 2 Apr 2024 13:57:04 +0800 Subject: [PATCH 02/29] coverage. Generate mcdc coverage mappings --- .../src/coverageinfo/ffi.rs | 115 +++++++++++++++- .../llvm-wrapper/CoverageMappingWrapper.cpp | 60 +++++++- compiler/rustc_middle/src/mir/coverage.rs | 72 +++++++++- compiler/rustc_middle/src/mir/pretty.rs | 18 ++- .../rustc_middle/src/ty/structural_impls.rs | 1 + .../rustc_mir_build/src/build/coverageinfo.rs | 130 +++++++++++++++++- .../rustc_mir_build/src/build/expr/into.rs | 3 + .../rustc_mir_build/src/build/matches/mod.rs | 2 + .../rustc_mir_transform/src/coverage/mod.rs | 59 ++++++-- .../rustc_mir_transform/src/coverage/spans.rs | 20 ++- .../src/coverage/spans/from_mir.rs | 13 +- 11 files changed, 453 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index 2af28146a51ea..62b8342f27958 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -1,4 +1,6 @@ -use rustc_middle::mir::coverage::{CodeRegion, CounterId, CovTerm, ExpressionId, MappingKind}; +use rustc_middle::mir::coverage::{ + CodeRegion, ConditionInfo, CounterId, CovTerm, DecisionInfo, ExpressionId, MappingKind, +}; /// Must match the layout of `LLVMRustCounterKind`. #[derive(Copy, Clone, Debug)] @@ -99,6 +101,63 @@ pub enum RegionKind { /// associated with two counters, each representing the number of times the /// expression evaluates to true or false. BranchRegion = 4, + + /// A DecisionRegion represents a top-level boolean expression and is + /// associated with a variable length bitmap index and condition number. + MCDCDecisionRegion = 5, + + /// A Branch Region can be extended to include IDs to facilitate MC/DC. + MCDCBranchRegion = 6, +} + +pub mod mcdc { + use rustc_middle::mir::coverage::{ConditionInfo, DecisionInfo}; + + /// Must match the layout of `LLVMRustMCDCDecisionParameters`. + #[repr(C)] + #[derive(Clone, Copy, Debug)] + pub struct DecisionParameters { + bitmap_idx: u32, + conditions_num: u16, + } + + // ConditionId in llvm is `unsigned int` at 18 while `int16_t` at [19](https://github.com/llvm/llvm-project/pull/81257) + type LLVMConditionId = i16; + + /// Must match the layout of `LLVMRustMCDCBranchParameters`. + #[repr(C)] + #[derive(Clone, Copy, Debug)] + pub struct BranchParameters { + condition_id: LLVMConditionId, + condition_ids: [LLVMConditionId; 2], + } + + /// Same layout with `LLVMRustMCDCParameters` + #[repr(C, u8)] + #[derive(Clone, Copy, Debug)] + pub enum Parameters { + None, + Decision(DecisionParameters), + Branch(BranchParameters), + } + + impl From for BranchParameters { + fn from(value: ConditionInfo) -> Self { + Self { + condition_id: value.condition_id.as_u32() as LLVMConditionId, + condition_ids: [ + value.false_next_id.as_u32() as LLVMConditionId, + value.true_next_id.as_u32() as LLVMConditionId, + ], + } + } + } + + impl From for DecisionParameters { + fn from(value: DecisionInfo) -> Self { + Self { bitmap_idx: value.bitmap_idx, conditions_num: value.conditions_num } + } + } } /// This struct provides LLVM's representation of a "CoverageMappingRegion", encoded into the @@ -122,6 +181,7 @@ pub struct CounterMappingRegion { /// for the false branch of the region. false_counter: Counter, + mcdc_params: mcdc::Parameters, /// An indirect reference to the source filename. In the LLVM Coverage Mapping Format, the /// file_id is an index into a function-specific `virtual_file_mapping` array of indexes /// that, in turn, are used to look up the filename for this region. @@ -164,9 +224,20 @@ impl CounterMappingRegion { end_line, end_col, ), - MappingKind::Branch { true_term, false_term } => Self::branch_region( - Counter::from_term(true_term), - Counter::from_term(false_term), + MappingKind::Branch { true_term, false_term, mcdc_params: condition_info } => { + Self::branch_region( + Counter::from_term(true_term), + Counter::from_term(false_term), + condition_info, + local_file_id, + start_line, + start_col, + end_line, + end_col, + ) + } + MappingKind::Decision(decision_info) => Self::decision_region( + decision_info, local_file_id, start_line, start_col, @@ -187,6 +258,7 @@ impl CounterMappingRegion { Self { counter, false_counter: Counter::ZERO, + mcdc_params: mcdc::Parameters::None, file_id, expanded_file_id: 0, start_line, @@ -200,22 +272,52 @@ impl CounterMappingRegion { pub(crate) fn branch_region( counter: Counter, false_counter: Counter, + condition_info: ConditionInfo, file_id: u32, start_line: u32, start_col: u32, end_line: u32, end_col: u32, ) -> Self { + let mcdc_params = mcdc::Parameters::Branch(condition_info.into()); + let kind = match mcdc_params { + mcdc::Parameters::None => RegionKind::BranchRegion, + mcdc::Parameters::Branch(_) => RegionKind::MCDCBranchRegion, + _ => unreachable!("invalid mcdc params for branch"), + }; Self { counter, false_counter, + mcdc_params, + file_id, + expanded_file_id: 0, + start_line, + start_col, + end_line, + end_col, + kind, + } + } + + pub(crate) fn decision_region( + decision_info: DecisionInfo, + file_id: u32, + start_line: u32, + start_col: u32, + end_line: u32, + end_col: u32, + ) -> Self { + Self { + counter: Counter::ZERO, + false_counter: Counter::ZERO, + mcdc_params: mcdc::Parameters::Decision(decision_info.into()), file_id, expanded_file_id: 0, start_line, start_col, end_line, end_col, - kind: RegionKind::BranchRegion, + kind: RegionKind::MCDCDecisionRegion, } } @@ -233,6 +335,7 @@ impl CounterMappingRegion { Self { counter: Counter::ZERO, false_counter: Counter::ZERO, + mcdc_params: mcdc::Parameters::None, file_id, expanded_file_id, start_line, @@ -256,6 +359,7 @@ impl CounterMappingRegion { Self { counter: Counter::ZERO, false_counter: Counter::ZERO, + mcdc_params: mcdc::Parameters::None, file_id, expanded_file_id: 0, start_line, @@ -280,6 +384,7 @@ impl CounterMappingRegion { Self { counter, false_counter: Counter::ZERO, + mcdc_params: mcdc::Parameters::None, file_id, expanded_file_id: 0, start_line, diff --git a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp index 0998b463a886c..4d721d22d9665 100644 --- a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp @@ -4,8 +4,6 @@ #include "llvm/ProfileData/Coverage/CoverageMappingWriter.h" #include "llvm/ProfileData/InstrProf.h" -#include - using namespace llvm; // FFI equivalent of enum `llvm::coverage::Counter::CounterKind` @@ -43,6 +41,8 @@ enum class LLVMRustCounterMappingRegionKind { SkippedRegion = 2, GapRegion = 3, BranchRegion = 4, + MCDCDecisionRegion = 5, + MCDCBranchRegion = 6 }; static coverage::CounterMappingRegion::RegionKind @@ -58,15 +58,69 @@ fromRust(LLVMRustCounterMappingRegionKind Kind) { return coverage::CounterMappingRegion::GapRegion; case LLVMRustCounterMappingRegionKind::BranchRegion: return coverage::CounterMappingRegion::BranchRegion; + case LLVMRustCounterMappingRegionKind::MCDCDecisionRegion: + return coverage::CounterMappingRegion::MCDCDecisionRegion; + case LLVMRustCounterMappingRegionKind::MCDCBranchRegion: + return coverage::CounterMappingRegion::MCDCBranchRegion; } report_fatal_error("Bad LLVMRustCounterMappingRegionKind!"); } +enum class LLVMRustMCDCParametersTag : uint8_t { + None = 0, + Decision = 1, + Branch = 2, +}; + +struct LLVMRustMCDCDecisionParameters { + uint32_t BitmapIdx; + uint16_t NumConditions; +}; + +struct LLVMRustMCDCBranchParameters { + int16_t ConditionID; + int16_t ConditionIDs[2]; +}; + +union LLVMRustMCDCParametersPayload { + LLVMRustMCDCDecisionParameters DecisionParameters; + LLVMRustMCDCBranchParameters BranchParameters; +}; + +struct LLVMRustMCDCParameters { + LLVMRustMCDCParametersTag Tag; + LLVMRustMCDCParametersPayload Payload; +}; + +static coverage::CounterMappingRegion::MCDCParameters +fromRust(LLVMRustMCDCParameters Params) { + switch (Params.Tag) { + case LLVMRustMCDCParametersTag::None: + return coverage::CounterMappingRegion::MCDCParameters{}; + case LLVMRustMCDCParametersTag::Decision: + return coverage::CounterMappingRegion::MCDCParameters{ + .BitmapIdx = + static_cast(Params.Payload.DecisionParameters.BitmapIdx), + .NumConditions = static_cast( + Params.Payload.DecisionParameters.NumConditions)}; + case LLVMRustMCDCParametersTag::Branch: + return coverage::CounterMappingRegion::MCDCParameters{ + .ID = static_cast( + Params.Payload.BranchParameters.ConditionID), + .FalseID = static_cast( + Params.Payload.BranchParameters.ConditionIDs[0]), + .TrueID = static_cast( + Params.Payload.BranchParameters.ConditionIDs[1])}; + } + report_fatal_error("Bad LLVMRustMCDCParametersTag!"); +} + // FFI equivalent of struct `llvm::coverage::CounterMappingRegion` // https://github.com/rust-lang/llvm-project/blob/ea6fa9c2/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L211-L304 struct LLVMRustCounterMappingRegion { LLVMRustCounter Count; LLVMRustCounter FalseCount; + LLVMRustMCDCParameters MCDCParameters; uint32_t FileID; uint32_t ExpandedFileID; uint32_t LineStart; @@ -135,7 +189,7 @@ extern "C" void LLVMRustCoverageWriteMappingToBuffer( MappingRegions.emplace_back( fromRust(Region.Count), fromRust(Region.FalseCount), #if LLVM_VERSION_GE(18, 0) && LLVM_VERSION_LT(19, 0) - coverage::CounterMappingRegion::MCDCParameters{}, + fromRust(Region.MCDCParameters), #endif Region.FileID, Region.ExpandedFileID, // File IDs, then region info. Region.LineStart, Region.ColumnStart, Region.LineEnd, Region.ColumnEnd, diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 582a180668816..efcbf303617b6 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -51,6 +51,23 @@ rustc_index::newtype_index! { pub struct ExpressionId {} } +rustc_index::newtype_index! { + /// ID of a mcdc condition. Used by llvm to check mcdc coverage. + /// + /// Note that LLVM handles condition IDs as `int16_t`, so there is no need + /// to use a larger representation on the Rust side. + #[derive(HashStable)] + #[encodable] + #[orderable] + #[max = 0xFFFF] + #[debug_format = "ConditionId({})"] + pub struct ConditionId {} +} + +impl ConditionId { + pub const NONE: Self = Self::from_u32(0); +} + /// Enum that can hold a constant zero value, the ID of an physical coverage /// counter, or the ID of a coverage-counter expression. /// @@ -171,17 +188,21 @@ pub enum MappingKind { /// Associates a normal region of code with a counter/expression/zero. Code(CovTerm), /// Associates a branch region with separate counters for true and false. - Branch { true_term: CovTerm, false_term: CovTerm }, + Branch { true_term: CovTerm, false_term: CovTerm, mcdc_params: ConditionInfo }, + /// Associates a decision region with a bitmap and number of conditions. + Decision(DecisionInfo), } impl MappingKind { /// Iterator over all coverage terms in this mapping kind. pub fn terms(&self) -> impl Iterator { - let one = |a| std::iter::once(a).chain(None); - let two = |a, b| std::iter::once(a).chain(Some(b)); + let zero = || None.into_iter().chain(None); + let one = |a| Some(a).into_iter().chain(None); + let two = |a, b| Some(a).into_iter().chain(Some(b)); match *self { Self::Code(term) => one(term), - Self::Branch { true_term, false_term } => two(true_term, false_term), + Self::Branch { true_term, false_term, .. } => two(true_term, false_term), + Self::Decision(_) => zero(), } } @@ -190,9 +211,12 @@ impl MappingKind { pub fn map_terms(&self, map_fn: impl Fn(CovTerm) -> CovTerm) -> Self { match *self { Self::Code(term) => Self::Code(map_fn(term)), - Self::Branch { true_term, false_term } => { - Self::Branch { true_term: map_fn(true_term), false_term: map_fn(false_term) } - } + Self::Branch { true_term, false_term, mcdc_params } => Self::Branch { + true_term: map_fn(true_term), + false_term: map_fn(false_term), + mcdc_params, + }, + Self::Decision(param) => Self::Decision(param), } } } @@ -226,12 +250,46 @@ pub struct BranchInfo { /// data structures without having to scan the entire body first. pub num_block_markers: usize, pub branch_spans: Vec, + pub decision_spans: Vec, } #[derive(Clone, Debug)] #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub struct BranchSpan { pub span: Span, + pub condition_info: ConditionInfo, pub true_marker: BlockMarkerId, pub false_marker: BlockMarkerId, } + +#[derive(Copy, Clone, Debug)] +#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] +pub struct ConditionInfo { + pub condition_id: ConditionId, + pub true_next_id: ConditionId, + pub false_next_id: ConditionId, +} + +impl Default for ConditionInfo { + fn default() -> Self { + Self { + condition_id: ConditionId::NONE, + true_next_id: ConditionId::NONE, + false_next_id: ConditionId::NONE, + } + } +} + +#[derive(Copy, Clone, Debug)] +#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] +pub struct DecisionInfo { + pub bitmap_idx: u32, + pub conditions_num: u16, +} + +#[derive(Clone, Debug)] +#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] +pub struct DecisionSpan { + pub span: Span, + pub conditions_num: u16, +} diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 41df2e3b5875a..faa70606f186b 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -477,11 +477,19 @@ fn write_coverage_branch_info( ) -> io::Result<()> { let coverage::BranchInfo { branch_spans, .. } = branch_info; - for coverage::BranchSpan { span, true_marker, false_marker } in branch_spans { - writeln!( - w, - "{INDENT}coverage branch {{ true: {true_marker:?}, false: {false_marker:?} }} => {span:?}", - )?; + for coverage::BranchSpan { span, true_marker, false_marker, condition_info } in branch_spans { + if condition_info.condition_id == coverage::ConditionId::NONE { + writeln!( + w, + "{INDENT}coverage branch {{ true: {true_marker:?}, false: {false_marker:?} }} => {span:?}", + )?; + } else { + let id = condition_info.condition_id; + writeln!( + w, + "{INDENT}coverage branch {{ condition_id: {id:?}, true: {true_marker:?}, false: {false_marker:?} }} => {span:?}", + )?; + } } if !branch_spans.is_empty() { writeln!(w)?; diff --git a/compiler/rustc_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs index 8231c0214cb71..11584320f2852 100644 --- a/compiler/rustc_middle/src/ty/structural_impls.rs +++ b/compiler/rustc_middle/src/ty/structural_impls.rs @@ -409,6 +409,7 @@ TrivialTypeTraversalImpls! { crate::mir::coverage::BlockMarkerId, crate::mir::coverage::CounterId, crate::mir::coverage::ExpressionId, + crate::mir::coverage::ConditionId, crate::mir::Local, crate::mir::Promoted, crate::traits::Reveal, diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index ab0043906b19f..3b43b1e792136 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -1,10 +1,13 @@ use std::assert_matches::assert_matches; use std::collections::hash_map::Entry; +use std::collections::VecDeque; use rustc_data_structures::fx::FxHashMap; -use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind}; +use rustc_middle::mir::coverage::{ + BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageKind, DecisionSpan, +}; use rustc_middle::mir::{self, BasicBlock, UnOp}; -use rustc_middle::thir::{ExprId, ExprKind, Thir}; +use rustc_middle::thir::{ExprId, ExprKind, LogicalOp, Thir}; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::LocalDefId; @@ -16,6 +19,8 @@ pub(crate) struct BranchInfoBuilder { num_block_markers: usize, branch_spans: Vec, + + mcdc_state: Option, } #[derive(Clone, Copy)] @@ -33,7 +38,12 @@ impl BranchInfoBuilder { /// is enabled and `def_id` represents a function that is eligible for coverage. pub(crate) fn new_if_enabled(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option { if tcx.sess.instrument_coverage_branch() && tcx.is_eligible_for_coverage(def_id) { - Some(Self { nots: FxHashMap::default(), num_block_markers: 0, branch_spans: vec![] }) + Some(Self { + nots: FxHashMap::default(), + num_block_markers: 0, + branch_spans: vec![], + mcdc_state: MCDCState::new_if_enabled(tcx), + }) } else { None } @@ -79,6 +89,10 @@ impl BranchInfoBuilder { } } + fn get_mcdc_state_mut(&mut self) -> Option<&mut MCDCState> { + self.mcdc_state.as_mut() + } + fn next_block_marker_id(&mut self) -> BlockMarkerId { let id = BlockMarkerId::from_usize(self.num_block_markers); self.num_block_markers += 1; @@ -86,14 +100,96 @@ impl BranchInfoBuilder { } pub(crate) fn into_done(self) -> Option> { - let Self { nots: _, num_block_markers, branch_spans } = self; + let Self { nots: _, num_block_markers, branch_spans, mcdc_state } = self; if num_block_markers == 0 { assert!(branch_spans.is_empty()); return None; } - Some(Box::new(mir::coverage::BranchInfo { num_block_markers, branch_spans })) + Some(Box::new(mir::coverage::BranchInfo { + num_block_markers, + branch_spans, + decision_spans: mcdc_state.map(|state| state.decisions).unwrap_or_default(), + })) + } +} + +struct MCDCState { + /// To construct condition evaluation tree. + decision_stack: VecDeque, + next_condition_id: usize, + decisions: Vec, + conditions_counter: u16, +} + +impl MCDCState { + fn new_if_enabled(tcx: TyCtxt<'_>) -> Option { + tcx.sess.instrument_coverage_mcdc().then(|| Self { + decision_stack: VecDeque::new(), + next_condition_id: 0, + decisions: vec![], + conditions_counter: 0, + }) + } + + fn record_conditions(&mut self, op: LogicalOp) { + let parent_condition = self.decision_stack.pop_back().unwrap_or_default(); + let lhs_id = if parent_condition.condition_id == ConditionId::NONE { + self.next_condition_id += 1; + ConditionId::from(self.next_condition_id) + } else { + parent_condition.condition_id + }; + + self.next_condition_id += 1; + let rhs_condition_id = ConditionId::from(self.next_condition_id); + + let (lhs, rhs) = match op { + LogicalOp::And => { + let lhs = ConditionInfo { + condition_id: lhs_id, + true_next_id: rhs_condition_id, + false_next_id: parent_condition.false_next_id, + }; + let rhs = ConditionInfo { + condition_id: rhs_condition_id, + true_next_id: parent_condition.true_next_id, + false_next_id: parent_condition.false_next_id, + }; + (lhs, rhs) + } + LogicalOp::Or => { + let lhs = ConditionInfo { + condition_id: lhs_id, + true_next_id: parent_condition.true_next_id, + false_next_id: rhs_condition_id, + }; + let rhs = ConditionInfo { + condition_id: rhs_condition_id, + true_next_id: parent_condition.true_next_id, + false_next_id: parent_condition.false_next_id, + }; + (lhs, rhs) + } + }; + // We visit expressions tree in pre-order, so place the left-hand side on the top. + self.decision_stack.push_back(rhs); + self.decision_stack.push_back(lhs); + } + + fn pop_condition_info(&mut self) -> Option { + let condition = self.decision_stack.pop_back(); + if condition.is_some() { + self.conditions_counter += 1; + if self.decision_stack.is_empty() + && let Some(decision) = self.decisions.last_mut() + { + decision.conditions_num = self.conditions_counter; + self.conditions_counter = 0; + } + } + condition } } @@ -122,6 +218,12 @@ impl Builder<'_, '_> { // Now that we have `source_info`, we can upgrade to a &mut reference. let branch_info = self.coverage_branch_info.as_mut().expect("upgrading & to &mut"); + let condition_info = branch_info + .mcdc_state + .as_mut() + .and_then(MCDCState::pop_condition_info) + .unwrap_or_default(); + let mut inject_branch_marker = |block: BasicBlock| { let id = branch_info.next_block_marker_id(); @@ -139,8 +241,26 @@ impl Builder<'_, '_> { branch_info.branch_spans.push(BranchSpan { span: source_info.span, + condition_info, true_marker, false_marker, }); } + + pub(crate) fn visit_coverage_decision(&mut self, expr_id: ExprId) { + if let Some(mcdc_state) = + self.coverage_branch_info.as_mut().and_then(BranchInfoBuilder::get_mcdc_state_mut) + { + mcdc_state + .decisions + .push(DecisionSpan { span: self.thir[expr_id].span, conditions_num: 0 }); + } + } + pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp) { + if let Some(mcdc_state) = + self.coverage_branch_info.as_mut().and_then(BranchInfoBuilder::get_mcdc_state_mut) + { + mcdc_state.record_conditions(logical_op); + } + } } diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index c8360b6a5fdd2..a0e77b56ead88 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -61,6 +61,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let then_span = this.thir[then].span; let then_source_info = this.source_info(then_span); let condition_scope = this.local_scope(); + this.visit_coverage_decision(cond); let then_and_else_blocks = this.in_scope( (if_then_scope, then_source_info), @@ -148,6 +149,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ExprKind::LogicalOp { op, lhs, rhs } => { let condition_scope = this.local_scope(); let source_info = this.source_info(expr.span); + + this.visit_coverage_decision(expr_id); // We first evaluate the left-hand side of the predicate ... let (then_block, else_block) = this.in_if_then_scope(condition_scope, expr.span, |this| { diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 367c391b45a49..ba67d9654c896 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -77,11 +77,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { match expr.kind { ExprKind::LogicalOp { op: LogicalOp::And, lhs, rhs } => { + this.visit_coverage_branch_operation(LogicalOp::And); let lhs_then_block = unpack!(this.then_else_break_inner(block, lhs, args)); let rhs_then_block = unpack!(this.then_else_break_inner(lhs_then_block, rhs, args)); rhs_then_block.unit() } ExprKind::LogicalOp { op: LogicalOp::Or, lhs, rhs } => { + this.visit_coverage_branch_operation(LogicalOp::Or); let local_scope = this.local_scope(); let (lhs_success_block, failure_block) = this.in_if_then_scope(local_scope, expr_span, |this| { diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index d382d2c03c24a..e1228188f1e9a 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -7,6 +7,8 @@ mod spans; #[cfg(test)] mod tests; +use std::collections::VecDeque; + use self::counters::{CounterIncrementSite, CoverageCounters}; use self::graph::{BasicCoverageBlock, CoverageGraph}; use self::spans::{BcbMapping, BcbMappingKind, CoverageSpans}; @@ -136,20 +138,59 @@ fn create_mappings<'tcx>( .as_term() }; - coverage_spans - .all_bcb_mappings() - .filter_map(|&BcbMapping { kind: bcb_mapping_kind, span }| { + let mut mappings = Vec::new(); + + let mut ignored_conditions = VecDeque::new(); + let mut next_decision_bitmap_idx = 0; + let mut decision_begin_condition: usize = 1; + for decision in coverage_spans.decisions() { + let conditions_num = decision.conditions_num as usize; + if conditions_num > 6 { + for cond_offset in 0..conditions_num { + ignored_conditions + .push_back(ConditionId::from(decision_begin_condition + cond_offset as usize)); + } + todo!("emit a warning"); + } else { + let kind = MappingKind::Decision(DecisionInfo { + bitmap_idx: next_decision_bitmap_idx, + conditions_num: decision.conditions_num, + }); + next_decision_bitmap_idx += (1 << conditions_num).max(8); + decision_begin_condition += conditions_num; + if let Some(code_region) = + make_code_region(source_map, file_name, decision.span, body_span) + { + mappings.push(Mapping { kind, code_region }); + } + } + } + + mappings.extend(coverage_spans.all_bcb_mappings().filter_map( + |&BcbMapping { kind: bcb_mapping_kind, span }| { let kind = match bcb_mapping_kind { BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(bcb)), - BcbMappingKind::Branch { true_bcb, false_bcb } => MappingKind::Branch { - true_term: term_for_bcb(true_bcb), - false_term: term_for_bcb(false_bcb), - }, + BcbMappingKind::Branch { true_bcb, false_bcb, mut mcdc_params } => { + if ignored_conditions + .front() + .is_some_and(|cond| *cond == mcdc_params.condition_id) + { + ignored_conditions.pop_front(); + mcdc_params = ConditionInfo::default(); + } + MappingKind::Branch { + true_term: term_for_bcb(true_bcb), + false_term: term_for_bcb(false_bcb), + mcdc_params, + } + } }; let code_region = make_code_region(source_map, file_name, span, body_span)?; Some(Mapping { kind, code_region }) - }) - .collect::>() + }, + )); + + mappings } /// For each BCB node or BCB edge that has an associated coverage counter, diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 672de1fbe60fd..aef5e324a84e3 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -1,6 +1,7 @@ use rustc_data_structures::graph::WithNumNodes; use rustc_index::bit_set::BitSet; use rustc_middle::mir; +use rustc_middle::mir::coverage::{ConditionInfo, DecisionSpan}; use rustc_span::{BytePos, Span}; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph, START_BCB}; @@ -14,7 +15,11 @@ pub(super) enum BcbMappingKind { /// Associates an ordinary executable code span with its corresponding BCB. Code(BasicCoverageBlock), /// Associates a branch span with BCBs for its true and false arms. - Branch { true_bcb: BasicCoverageBlock, false_bcb: BasicCoverageBlock }, + Branch { + true_bcb: BasicCoverageBlock, + false_bcb: BasicCoverageBlock, + mcdc_params: ConditionInfo, + }, } #[derive(Debug)] @@ -26,6 +31,7 @@ pub(super) struct BcbMapping { pub(super) struct CoverageSpans { bcb_has_mappings: BitSet, mappings: Vec, + decisions: Vec, } impl CoverageSpans { @@ -36,6 +42,10 @@ impl CoverageSpans { pub(super) fn all_bcb_mappings(&self) -> impl Iterator { self.mappings.iter() } + + pub(super) fn decisions(&self) -> impl Iterator { + self.decisions.iter() + } } /// Extracts coverage-relevant spans from MIR, and associates them with @@ -88,14 +98,18 @@ pub(super) fn generate_coverage_spans( for &BcbMapping { kind, span: _ } in &mappings { match kind { BcbMappingKind::Code(bcb) => insert(bcb), - BcbMappingKind::Branch { true_bcb, false_bcb } => { + BcbMappingKind::Branch { true_bcb, false_bcb, .. } => { insert(true_bcb); insert(false_bcb); } } } - Some(CoverageSpans { bcb_has_mappings, mappings }) + Some(CoverageSpans { + bcb_has_mappings, + mappings, + decisions: from_mir::extract_decision_spans(mir_body).unwrap_or_default(), + }) } #[derive(Debug)] diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index adb0c9f1929d9..a96fa4c90cf96 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -1,7 +1,7 @@ use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashSet; use rustc_index::IndexVec; -use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind}; +use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind, DecisionSpan}; use rustc_middle::mir::{ self, AggregateKind, BasicBlock, FakeReadCause, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, @@ -387,7 +387,7 @@ pub(super) fn extract_branch_mappings( branch_info .branch_spans .iter() - .filter_map(|&BranchSpan { span: raw_span, true_marker, false_marker }| { + .filter_map(|&BranchSpan { span: raw_span, condition_info, true_marker, false_marker }| { // For now, ignore any branch span that was introduced by // expansion. This makes things like assert macros less noisy. if !raw_span.ctxt().outer_expn_data().is_root() { @@ -401,7 +401,14 @@ pub(super) fn extract_branch_mappings( let true_bcb = bcb_from_marker(true_marker)?; let false_bcb = bcb_from_marker(false_marker)?; - Some(BcbMapping { kind: BcbMappingKind::Branch { true_bcb, false_bcb }, span }) + Some(BcbMapping { + kind: BcbMappingKind::Branch { true_bcb, false_bcb, mcdc_params: condition_info }, + span, + }) }) .collect::>() } + +pub(super) fn extract_decision_spans(mir_body: &mir::Body<'_>) -> Option> { + mir_body.coverage_branch_info.as_deref().map(|info| info.decision_spans.clone()) +} From 94f2998b480ded1fc7c7a46a8bfd71b620b36955 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 2 Apr 2024 13:58:24 +0800 Subject: [PATCH 03/29] coverage. Add llvm wrapper to get mcdc intrinsic functions --- compiler/rustc_codegen_llvm/src/builder.rs | 108 ++++++++++++++++++ compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 4 + .../rustc_codegen_ssa/src/traits/builder.rs | 27 +++++ .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 15 +++ 4 files changed, 154 insertions(+) diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 1a32958d3627b..c40f5314cbfea 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -1238,6 +1238,114 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { } } + fn mcdc_parameters( + &mut self, + fn_name: Self::Value, + hash: Self::Value, + bitmap_bytes: Self::Value, + ) { + debug!("mcdc_parameters() with args ({:?}, {:?}, {:?})", fn_name, hash, bitmap_bytes); + + let llfn = unsafe { llvm::LLVMRustGetInstrProfMCDCParametersIntrinsic(self.cx().llmod) }; + let llty = self.cx.type_func( + &[self.cx.type_ptr(), self.cx.type_i64(), self.cx.type_i32()], + self.cx.type_void(), + ); + let args = &[fn_name, hash, bitmap_bytes]; + let args = self.check_call("call", llty, llfn, args); + + unsafe { + let _ = llvm::LLVMRustBuildCall( + self.llbuilder, + llty, + llfn, + args.as_ptr() as *const &llvm::Value, + args.len() as c_uint, + [].as_ptr(), + 0 as c_uint, + ); + } + } + + fn mcdc_tvbitmap_update( + &mut self, + fn_name: Self::Value, + hash: Self::Value, + bitmap_bytes: Self::Value, + bitmap_index: Self::Value, + mcdc_temp: Self::Value, + ) { + debug!( + "mcdc_tvbitmap_update() with args ({:?}, {:?}, {:?}, {:?}, {:?})", + fn_name, hash, bitmap_bytes, bitmap_index, mcdc_temp + ); + + let llfn = + unsafe { llvm::LLVMRustGetInstrProfMCDCTVBitmapUpdateIntrinsic(self.cx().llmod) }; + let llty = self.cx.type_func( + &[ + self.cx.type_ptr(), + self.cx.type_i64(), + self.cx.type_i32(), + self.cx.type_i32(), + self.cx.type_ptr(), + ], + self.cx.type_void(), + ); + let args = &[fn_name, hash, bitmap_bytes, bitmap_index, mcdc_temp]; + let args = self.check_call("call", llty, llfn, args); + unsafe { + let _ = llvm::LLVMRustBuildCall( + self.llbuilder, + llty, + llfn, + args.as_ptr() as *const &llvm::Value, + args.len() as c_uint, + [].as_ptr(), + 0 as c_uint, + ); + } + } + + fn mcdc_condbitmap_update( + &mut self, + fn_name: Self::Value, + hash: Self::Value, + cond_loc: Self::Value, + mcdc_temp: Self::Value, + bool_value: Self::Value, + ) { + let llfn = unsafe { llvm::LLVMRustGetInstrProfMCDCCondBitmapIntrinsic(self.cx().llmod) }; + let llty = self.cx.type_func( + &[ + self.cx.type_ptr(), + self.cx.type_i64(), + self.cx.type_i32(), + self.cx.type_ptr(), + self.cx.type_bool(), + ], + self.cx.type_void(), + ); + let args = &[fn_name, hash, cond_loc, mcdc_temp, bool_value]; + let args = self.check_call("call", llty, llfn, args); + unsafe { + let _ = llvm::LLVMRustBuildCall( + self.llbuilder, + llty, + llfn, + args.as_ptr() as *const &llvm::Value, + args.len() as c_uint, + [].as_ptr(), + 0 as c_uint, + ); + } + } + + fn mcdc_condbitmap_reset(&mut self, mcdc_temp: Self::Value) { + let i32_align = self.tcx().data_layout.i32_align.abi; + self.store(self.const_i32(0), mcdc_temp, i32_align); + } + fn call( &mut self, llty: &'ll Type, diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 284bc74d5c434..986119a4ff46b 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -1631,6 +1631,10 @@ extern "C" { // Miscellaneous instructions pub fn LLVMRustGetInstrProfIncrementIntrinsic(M: &Module) -> &Value; + pub fn LLVMRustGetInstrProfMCDCParametersIntrinsic(M: &Module) -> &Value; + pub fn LLVMRustGetInstrProfMCDCTVBitmapUpdateIntrinsic(M: &Module) -> &Value; + pub fn LLVMRustGetInstrProfMCDCCondBitmapIntrinsic(M: &Module) -> &Value; + pub fn LLVMRustBuildCall<'a>( B: &Builder<'a>, Ty: &'a Type, diff --git a/compiler/rustc_codegen_ssa/src/traits/builder.rs b/compiler/rustc_codegen_ssa/src/traits/builder.rs index 6c8dcc5b69001..dcd930f84a09c 100644 --- a/compiler/rustc_codegen_ssa/src/traits/builder.rs +++ b/compiler/rustc_codegen_ssa/src/traits/builder.rs @@ -382,6 +382,33 @@ pub trait BuilderMethods<'a, 'tcx>: index: Self::Value, ); + fn mcdc_parameters( + &mut self, + fn_name: Self::Value, + hash: Self::Value, + bitmap_bytes: Self::Value, + ); + + fn mcdc_condbitmap_update( + &mut self, + fn_name: Self::Value, + hash: Self::Value, + cond_loc: Self::Value, + mcdc_temp: Self::Value, + bool_value: Self::Value, + ); + + fn mcdc_tvbitmap_update( + &mut self, + fn_name: Self::Value, + hash: Self::Value, + bitmap_bytes: Self::Value, + bitmap_index: Self::Value, + mcdc_temp: Self::Value, + ); + + fn mcdc_condbitmap_reset(&mut self, mcdc_temp: Self::Value); + fn call( &mut self, llty: Self::Type, diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 8ec1f5a99e7ca..11f9661eec754 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1528,6 +1528,21 @@ extern "C" LLVMValueRef LLVMRustGetInstrProfIncrementIntrinsic(LLVMModuleRef M) (llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_increment)); } +extern "C" LLVMValueRef LLVMRustGetInstrProfMCDCParametersIntrinsic(LLVMModuleRef M) { + return wrap(llvm::Intrinsic::getDeclaration(unwrap(M), + (llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_mcdc_parameters)); +} + +extern "C" LLVMValueRef LLVMRustGetInstrProfMCDCTVBitmapUpdateIntrinsic(LLVMModuleRef M) { + return wrap(llvm::Intrinsic::getDeclaration(unwrap(M), + (llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_mcdc_tvbitmap_update)); +} + +extern "C" LLVMValueRef LLVMRustGetInstrProfMCDCCondBitmapIntrinsic(LLVMModuleRef M) { + return wrap(llvm::Intrinsic::getDeclaration(unwrap(M), + (llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_mcdc_condbitmap_update)); +} + extern "C" LLVMValueRef LLVMRustBuildMemCpy(LLVMBuilderRef B, LLVMValueRef Dst, unsigned DstAlign, LLVMValueRef Src, unsigned SrcAlign, From 248e1f681e1a9d6858d5bff631d1887720f3bb19 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 2 Apr 2024 14:15:57 +0800 Subject: [PATCH 04/29] coverage. Generate mcdc instrument when lowering to ir --- compiler/rustc_codegen_llvm/src/builder.rs | 19 +++-- .../src/coverageinfo/map_data.rs | 2 +- .../src/coverageinfo/mod.rs | 71 +++++++++++++++++- .../rustc_codegen_ssa/src/traits/builder.rs | 2 +- compiler/rustc_middle/src/mir/coverage.rs | 26 +++++-- compiler/rustc_middle/src/mir/query.rs | 4 ++ .../rustc_mir_build/src/build/coverageinfo.rs | 72 +++++++++++++++---- .../rustc_mir_build/src/build/expr/into.rs | 2 + .../rustc_mir_transform/src/coverage/mod.rs | 48 +++++-------- .../rustc_mir_transform/src/coverage/query.rs | 8 ++- .../src/coverage/spans/from_mir.rs | 6 +- 11 files changed, 203 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index c40f5314cbfea..6394637f3b231 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -17,7 +17,7 @@ use rustc_data_structures::small_c_str::SmallCStr; use rustc_hir::def_id::DefId; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs; use rustc_middle::ty::layout::{ - FnAbiError, FnAbiOfHelpers, FnAbiRequest, LayoutError, LayoutOfHelpers, TyAndLayout, + FnAbiError, FnAbiOfHelpers, FnAbiRequest, HasTyCtxt, LayoutError, LayoutOfHelpers, TyAndLayout, }; use rustc_middle::ty::{self, Instance, Ty, TyCtxt}; use rustc_session::config::OptLevel; @@ -1243,7 +1243,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { fn_name: Self::Value, hash: Self::Value, bitmap_bytes: Self::Value, - ) { + ) -> &'ll Value { debug!("mcdc_parameters() with args ({:?}, {:?}, {:?})", fn_name, hash, bitmap_bytes); let llfn = unsafe { llvm::LLVMRustGetInstrProfMCDCParametersIntrinsic(self.cx().llmod) }; @@ -1264,6 +1264,17 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { [].as_ptr(), 0 as c_uint, ); + // Create condition bitmap named `mcdc.addr`. + let mut bx = Builder::with_cx(self.cx); + bx.position_at_start(llvm::LLVMGetFirstBasicBlock(self.llfn())); + let cond_bitmap = { + let alloca = + llvm::LLVMBuildAlloca(bx.llbuilder, bx.cx.type_i32(), c"mcdc.addr".as_ptr()); + llvm::LLVMSetAlignment(alloca, 4); + alloca + }; + bx.store(self.const_i32(0), cond_bitmap, self.tcx().data_layout.i32_align.abi); + cond_bitmap } } @@ -1322,12 +1333,12 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { self.cx.type_i64(), self.cx.type_i32(), self.cx.type_ptr(), - self.cx.type_bool(), + self.cx.type_i1(), ], self.cx.type_void(), ); let args = &[fn_name, hash, cond_loc, mcdc_temp, bool_value]; - let args = self.check_call("call", llty, llfn, args); + self.check_call("call", llty, llfn, args); unsafe { let _ = llvm::LLVMRustBuildCall( self.llbuilder, diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs index d85d9411f03b6..291e53c2904f0 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs @@ -1,5 +1,4 @@ use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind}; - use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxIndexSet; use rustc_index::bit_set::BitSet; @@ -74,6 +73,7 @@ impl<'tcx> FunctionCoverageCollector<'tcx> { Self { function_coverage_info, is_used, + counters_seen: BitSet::new_empty(num_counters), expressions_seen, } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 140566e8da9bb..323107e27ebd1 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -13,7 +13,7 @@ use rustc_codegen_ssa::traits::{ use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_llvm::RustString; use rustc_middle::bug; -use rustc_middle::mir::coverage::CoverageKind; +use rustc_middle::mir::coverage::{CoverageKind, FunctionCoverageInfo}; use rustc_middle::ty::layout::HasTyCtxt; use rustc_middle::ty::Instance; use rustc_target::abi::Align; @@ -30,6 +30,7 @@ pub struct CrateCoverageContext<'ll, 'tcx> { pub(crate) function_coverage_map: RefCell, FunctionCoverageCollector<'tcx>>>, pub(crate) pgo_func_name_var_map: RefCell, &'ll llvm::Value>>, + pub(crate) mcdc_condition_bitmap_map: RefCell, &'ll llvm::Value>>, } impl<'ll, 'tcx> CrateCoverageContext<'ll, 'tcx> { @@ -37,6 +38,7 @@ impl<'ll, 'tcx> CrateCoverageContext<'ll, 'tcx> { Self { function_coverage_map: Default::default(), pgo_func_name_var_map: Default::default(), + mcdc_condition_bitmap_map: Default::default(), } } @@ -45,6 +47,12 @@ impl<'ll, 'tcx> CrateCoverageContext<'ll, 'tcx> { ) -> FxIndexMap, FunctionCoverageCollector<'tcx>> { self.function_coverage_map.replace(FxIndexMap::default()) } + + /// LLVM use a temp value to record evaluated mcdc test vector of each decision, which is called condition bitmap. + /// This value is named `mcdc.addr` (same as clang) and is a 32-bit integer. + fn try_get_mcdc_condition_bitmap(&self, instance: &Instance<'tcx>) -> Option<&'ll llvm::Value> { + self.mcdc_condition_bitmap_map.borrow().get(instance).copied() + } } // These methods used to be part of trait `CoverageInfoMethods`, which no longer @@ -90,6 +98,14 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { return; }; + match *kind { + // Ensure mcdc parameters are properly initialized for these operations. + CoverageKind::UpdateCondBitmap { .. } | CoverageKind::UpdateTestVector { .. } => { + ensure_mcdc_parameters(bx, instance, function_coverage_info) + } + _ => {} + } + let Some(coverage_context) = bx.coverage_context() else { return }; let mut coverage_map = coverage_context.function_coverage_map.borrow_mut(); let func_coverage = coverage_map @@ -131,10 +147,63 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { CoverageKind::ExpressionUsed { id } => { func_coverage.mark_expression_id_seen(id); } + + CoverageKind::UpdateCondBitmap { id, value } => { + drop(coverage_map); + let cond_bitmap = coverage_context + .try_get_mcdc_condition_bitmap(&instance) + .expect("mcdc cond bitmap must be set before being updated"); + let cond_loc = bx.const_i32(id.as_u32() as i32 - 1); + let bool_value = bx.const_bool(value); + let fn_name = bx.get_pgo_func_name_var(instance); + let hash = bx.const_u64(function_coverage_info.function_source_hash); + bx.mcdc_condbitmap_update(fn_name, hash, cond_loc, cond_bitmap, bool_value); + } + CoverageKind::UpdateTestVector { bitmap_idx } => { + drop(coverage_map); + let cond_bitmap = coverage_context + .try_get_mcdc_condition_bitmap(&instance) + .expect("mcdc cond bitmap must be set before being updated"); + let bitmap_bytes = bx.tcx().coverage_ids_info(instance.def).mcdc_bitmap_bytes; + assert!(bitmap_idx < bitmap_bytes, "bitmap index of the decision out of range"); + assert!( + bitmap_bytes <= function_coverage_info.mcdc_bitmap_bytes, + "bitmap length disagreement: query says {bitmap_bytes} but function info only has {}", + function_coverage_info.mcdc_bitmap_bytes + ); + + let fn_name = bx.get_pgo_func_name_var(instance); + let hash = bx.const_u64(function_coverage_info.function_source_hash); + let bitmap_bytes = bx.const_u32(bitmap_bytes); + let bitmap_index = bx.const_u32(bitmap_idx); + bx.mcdc_tvbitmap_update(fn_name, hash, bitmap_bytes, bitmap_index, cond_bitmap); + bx.mcdc_condbitmap_reset(cond_bitmap); + } } } } +fn ensure_mcdc_parameters<'ll, 'tcx>( + bx: &mut Builder<'_, 'll, 'tcx>, + instance: Instance<'tcx>, + function_coverage_info: &FunctionCoverageInfo, +) { + if bx + .coverage_context() + .is_some_and(|cx| !cx.mcdc_condition_bitmap_map.borrow().contains_key(&instance)) + { + let fn_name = bx.get_pgo_func_name_var(instance); + let hash = bx.const_u64(function_coverage_info.function_source_hash); + let bitmap_bytes = bx.const_u32(function_coverage_info.mcdc_bitmap_bytes); + let cond_bitmap = bx.mcdc_parameters(fn_name, hash, bitmap_bytes); + bx.coverage_context() + .unwrap() + .mcdc_condition_bitmap_map + .borrow_mut() + .insert(instance, cond_bitmap); + } +} + /// 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 diff --git a/compiler/rustc_codegen_ssa/src/traits/builder.rs b/compiler/rustc_codegen_ssa/src/traits/builder.rs index dcd930f84a09c..842526a6b759e 100644 --- a/compiler/rustc_codegen_ssa/src/traits/builder.rs +++ b/compiler/rustc_codegen_ssa/src/traits/builder.rs @@ -387,7 +387,7 @@ pub trait BuilderMethods<'a, 'tcx>: fn_name: Self::Value, hash: Self::Value, bitmap_bytes: Self::Value, - ); + ) -> Self::Value; fn mcdc_condbitmap_update( &mut self, diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index efcbf303617b6..45ec22dba68aa 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -112,7 +112,9 @@ pub enum CoverageKind { /// /// If this statement does not survive MIR optimizations, any mappings that /// refer to this counter can have those references simplified to zero. - CounterIncrement { id: CounterId }, + CounterIncrement { + id: CounterId, + }, /// Marks the point in MIR control-flow represented by a coverage expression. /// @@ -122,7 +124,18 @@ pub enum CoverageKind { /// (This is only inserted for expression IDs that are directly used by /// mappings. Intermediate expressions with no direct mappings are /// retained/zeroed based on whether they are transitively used.) - ExpressionUsed { id: ExpressionId }, + ExpressionUsed { + id: ExpressionId, + }, + + UpdateCondBitmap { + id: ConditionId, + value: bool, + }, + + UpdateTestVector { + bitmap_idx: u32, + }, } impl Debug for CoverageKind { @@ -133,6 +146,10 @@ impl Debug for CoverageKind { BlockMarker { id } => write!(fmt, "BlockMarker({:?})", id.index()), CounterIncrement { id } => write!(fmt, "CounterIncrement({:?})", id.index()), ExpressionUsed { id } => write!(fmt, "ExpressionUsed({:?})", id.index()), + UpdateCondBitmap { id, value } => { + write!(fmt, "UpdateCondBitmap({:?}, {value})", id.index()) + } + UpdateTestVector { bitmap_idx } => write!(fmt, "UpdateTestVector({:?})", bitmap_idx), } } } @@ -236,7 +253,7 @@ pub struct Mapping { pub struct FunctionCoverageInfo { pub function_source_hash: u64, pub num_counters: usize, - + pub mcdc_bitmap_bytes: u32, pub expressions: IndexVec, pub mappings: Vec, } @@ -250,6 +267,7 @@ pub struct BranchInfo { /// data structures without having to scan the entire body first. pub num_block_markers: usize, pub branch_spans: Vec, + pub mcdc_bitmap_bytes_num: u32, pub decision_spans: Vec, } @@ -291,5 +309,5 @@ pub struct DecisionInfo { #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub struct DecisionSpan { pub span: Span, - pub conditions_num: u16, + pub mcdc_params: DecisionInfo, } diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index 0f62212743086..c6d7cfd325ea8 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -361,4 +361,8 @@ pub struct CoverageIdsInfo { /// InstrumentCoverage MIR pass, if the highest-numbered counter increments /// were removed by MIR optimizations. pub max_counter_id: mir::coverage::CounterId, + + /// Coverage codegen for mcdc needs to know the size of the global bitmap so that it can + /// set the `bytemap-bytes` argument of the `llvm.instrprof.mcdc.tvbitmap.update` intrinsic. + pub mcdc_bitmap_bytes: u32, } diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index 3b43b1e792136..4078694256204 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -4,7 +4,7 @@ use std::collections::VecDeque; use rustc_data_structures::fx::FxHashMap; use rustc_middle::mir::coverage::{ - BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageKind, DecisionSpan, + BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageKind, DecisionInfo, DecisionSpan, }; use rustc_middle::mir::{self, BasicBlock, UnOp}; use rustc_middle::thir::{ExprId, ExprKind, LogicalOp, Thir}; @@ -106,11 +106,23 @@ impl BranchInfoBuilder { assert!(branch_spans.is_empty()); return None; } + let decision_spans = mcdc_state.map(|state| state.decisions).unwrap_or_default(); + let mcdc_bitmap_bytes_num = decision_spans + .last() + .map(|decision| { + decision + .mcdc_params + .bitmap_idx + .saturating_add((1 << decision.mcdc_params.conditions_num).max(8)) + / 8 + }) + .unwrap_or_default(); Some(Box::new(mir::coverage::BranchInfo { num_block_markers, branch_spans, - decision_spans: mcdc_state.map(|state| state.decisions).unwrap_or_default(), + mcdc_bitmap_bytes_num, + decision_spans, })) } } @@ -120,7 +132,6 @@ struct MCDCState { decision_stack: VecDeque, next_condition_id: usize, decisions: Vec, - conditions_counter: u16, } impl MCDCState { @@ -129,7 +140,6 @@ impl MCDCState { decision_stack: VecDeque::new(), next_condition_id: 0, decisions: vec![], - conditions_counter: 0, }) } @@ -181,12 +191,11 @@ impl MCDCState { fn pop_condition_info(&mut self) -> Option { let condition = self.decision_stack.pop_back(); if condition.is_some() { - self.conditions_counter += 1; if self.decision_stack.is_empty() && let Some(decision) = self.decisions.last_mut() { - decision.conditions_num = self.conditions_counter; - self.conditions_counter = 0; + decision.mcdc_params.conditions_num = (self.next_condition_id - 1) as u16; + self.next_condition_id = 0; } } condition @@ -224,7 +233,7 @@ impl Builder<'_, '_> { .and_then(MCDCState::pop_condition_info) .unwrap_or_default(); - let mut inject_branch_marker = |block: BasicBlock| { + let mut inject_branch_marker = |block: BasicBlock, value: bool| { let id = branch_info.next_block_marker_id(); let marker_statement = mir::Statement { @@ -233,11 +242,22 @@ impl Builder<'_, '_> { }; self.cfg.push(block, marker_statement); + if condition_info.condition_id != ConditionId::NONE { + let condbitmap_update_statement = mir::Statement { + source_info, + kind: mir::StatementKind::Coverage(CoverageKind::UpdateCondBitmap { + id: condition_info.condition_id, + value, + }), + }; + self.cfg.push(block, condbitmap_update_statement); + } + id }; - let true_marker = inject_branch_marker(then_block); - let false_marker = inject_branch_marker(else_block); + let true_marker = inject_branch_marker(then_block, true); + let false_marker = inject_branch_marker(else_block, false); branch_info.branch_spans.push(BranchSpan { span: source_info.span, @@ -251,11 +271,39 @@ impl Builder<'_, '_> { if let Some(mcdc_state) = self.coverage_branch_info.as_mut().and_then(BranchInfoBuilder::get_mcdc_state_mut) { - mcdc_state + let bitmap_idx = mcdc_state .decisions - .push(DecisionSpan { span: self.thir[expr_id].span, conditions_num: 0 }); + .last() + .map(|decision| { + decision + .mcdc_params + .bitmap_idx + .saturating_add((1 << decision.mcdc_params.conditions_num).max(8)) + }) + .unwrap_or_default(); + mcdc_state.decisions.push(DecisionSpan { + span: self.thir[expr_id].span, + mcdc_params: DecisionInfo { conditions_num: 0, bitmap_idx }, + }); + } + } + + pub(crate) fn visit_coverage_decision_end(&mut self, join_block: BasicBlock) { + if let Some((span, bitmap_idx)) = self + .coverage_branch_info + .as_mut() + .and_then(BranchInfoBuilder::get_mcdc_state_mut) + .and_then(|state| state.decisions.last_mut()) + .map(|end_decision| (end_decision.span, end_decision.mcdc_params.bitmap_idx)) + { + let statement = mir::Statement { + source_info: self.source_info(span), + kind: mir::StatementKind::Coverage(CoverageKind::UpdateTestVector { bitmap_idx }), + }; + self.cfg.push(join_block, statement); } } + pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp) { if let Some(mcdc_state) = self.coverage_branch_info.as_mut().and_then(BranchInfoBuilder::get_mcdc_state_mut) diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index a0e77b56ead88..b5f64550e6fde 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -114,6 +114,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // The `then` and `else` arms have been lowered into their respective // blocks, so make both of them meet up in a new block. let join_block = this.cfg.start_new_block(); + this.visit_coverage_decision_end(join_block); this.cfg.goto(then_blk, source_info, join_block); this.cfg.goto(else_blk, source_info, join_block); join_block.unit() @@ -186,6 +187,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); let rhs = unpack!(this.expr_into_dest(destination, continuation, rhs)); let target = this.cfg.start_new_block(); + this.visit_coverage_decision_end(target); this.cfg.goto(rhs, source_info, target); this.cfg.goto(short_circuit, source_info, target); target.unit() diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index e1228188f1e9a..6c8ead504b869 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -7,8 +7,6 @@ mod spans; #[cfg(test)] mod tests; -use std::collections::VecDeque; - use self::counters::{CounterIncrementSite, CoverageCounters}; use self::graph::{BasicCoverageBlock, CoverageGraph}; use self::spans::{BcbMapping, BcbMappingKind, CoverageSpans}; @@ -79,6 +77,15 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: return; }; + let mcdc_bitmap_bytes = coverage_spans + .decisions() + .last() + .map(|decision| { + (decision.mcdc_params.bitmap_idx + (1 << decision.mcdc_params.conditions_num).max(8)) + / 8 + }) + .unwrap_or_default(); + //////////////////////////////////////////////////// // Create an optimized mix of `Counter`s and `Expression`s for the `CoverageGraph`. Ensure // every coverage span has a `Counter` or `Expression` assigned to its `BasicCoverageBlock` @@ -105,6 +112,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo { function_source_hash: hir_info.function_source_hash, num_counters: coverage_counters.num_counters(), + mcdc_bitmap_bytes, expressions: coverage_counters.into_expressions(), mappings, })); @@ -140,29 +148,12 @@ fn create_mappings<'tcx>( let mut mappings = Vec::new(); - let mut ignored_conditions = VecDeque::new(); - let mut next_decision_bitmap_idx = 0; - let mut decision_begin_condition: usize = 1; for decision in coverage_spans.decisions() { - let conditions_num = decision.conditions_num as usize; - if conditions_num > 6 { - for cond_offset in 0..conditions_num { - ignored_conditions - .push_back(ConditionId::from(decision_begin_condition + cond_offset as usize)); - } - todo!("emit a warning"); - } else { - let kind = MappingKind::Decision(DecisionInfo { - bitmap_idx: next_decision_bitmap_idx, - conditions_num: decision.conditions_num, - }); - next_decision_bitmap_idx += (1 << conditions_num).max(8); - decision_begin_condition += conditions_num; - if let Some(code_region) = - make_code_region(source_map, file_name, decision.span, body_span) - { - mappings.push(Mapping { kind, code_region }); - } + let kind = MappingKind::Decision(decision.mcdc_params); + + if let Some(code_region) = make_code_region(source_map, file_name, decision.span, body_span) + { + mappings.push(Mapping { kind, code_region }); } } @@ -170,14 +161,7 @@ fn create_mappings<'tcx>( |&BcbMapping { kind: bcb_mapping_kind, span }| { let kind = match bcb_mapping_kind { BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(bcb)), - BcbMappingKind::Branch { true_bcb, false_bcb, mut mcdc_params } => { - if ignored_conditions - .front() - .is_some_and(|cond| *cond == mcdc_params.condition_id) - { - ignored_conditions.pop_front(); - mcdc_params = ConditionInfo::default(); - } + BcbMappingKind::Branch { true_bcb, false_bcb, mcdc_params } => { MappingKind::Branch { true_term: term_for_bcb(true_bcb), false_term: term_for_bcb(false_bcb), diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index 65715253647a8..0673430614d4b 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -61,7 +61,13 @@ fn coverage_ids_info<'tcx>( .max() .unwrap_or(CounterId::ZERO); - CoverageIdsInfo { max_counter_id } + let mcdc_bitmap_bytes = mir_body + .coverage_branch_info + .as_deref() + .map(|info| info.mcdc_bitmap_bytes_num) + .unwrap_or_default(); + + CoverageIdsInfo { max_counter_id, mcdc_bitmap_bytes } } fn all_coverage_in_mir_body<'a, 'tcx>( diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index a96fa4c90cf96..5cc9225361ab2 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -223,7 +223,11 @@ fn filtered_statement_span(statement: &Statement<'_>) -> Option { | StatementKind::AscribeUserType(_, _) => Some(statement.source_info.span), // Block markers are used for branch coverage, so ignore them here. - StatementKind::Coverage(CoverageKind::BlockMarker { .. }) => None, + StatementKind::Coverage( + CoverageKind::BlockMarker { .. } + | CoverageKind::UpdateCondBitmap { .. } + | CoverageKind::UpdateTestVector { .. }, + ) => None, // These coverage statements should not exist prior to coverage instrumentation. StatementKind::Coverage( From 0410a96697569f3418b30e03729ec9dc6a0036fd Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 2 Apr 2024 14:16:45 +0800 Subject: [PATCH 05/29] coverage. fix error on computing conditions num of decisions --- compiler/rustc_mir_build/src/build/coverageinfo.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index 4078694256204..b3f73752556f4 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -194,7 +194,7 @@ impl MCDCState { if self.decision_stack.is_empty() && let Some(decision) = self.decisions.last_mut() { - decision.mcdc_params.conditions_num = (self.next_condition_id - 1) as u16; + decision.mcdc_params.conditions_num = self.next_condition_id as u16; self.next_condition_id = 0; } } From 24344d368d94787829a307a2bce65811789ca08e Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 2 Apr 2024 14:58:06 +0800 Subject: [PATCH 06/29] coverage. Add warning for decisions with too many conditions --- compiler/rustc_mir_build/messages.ftl | 2 ++ compiler/rustc_mir_build/src/errors.rs | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index 1de691f32a70c..2f0fe278ad08d 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -406,3 +406,5 @@ mir_build_unused_unsafe_enclosing_block_label = because it's nested under this ` mir_build_variant_defined_here = not covered mir_build_wrap_suggestion = consider wrapping the function body in an unsafe block + +mir_build_exceeds_mcdc_condition_num_limit = Conditions number of the decision ({$conditions_num}) exceeds limit ({$max_conditions_num}). MCDC analysis will not count this expression. \ No newline at end of file diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 26f10fdd333cb..f7fadc0985b14 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -818,6 +818,15 @@ pub struct NontrivialStructuralMatch<'tcx> { pub non_sm_ty: Ty<'tcx>, } +#[derive(Diagnostic)] +#[diag(mir_build_exceeds_mcdc_condition_num_limit)] +pub(crate) struct MCDCExceedsConditionNumLimit { + #[primary_span] + pub span: Span, + pub conditions_num: u16, + pub max_conditions_num: u16, +} + #[derive(Diagnostic)] #[diag(mir_build_pattern_not_covered, code = E0005)] pub(crate) struct PatternNotCovered<'s, 'tcx> { From 305b94c9fd8ad8a675ec55ca7b15fe3ba4ca0de0 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 2 Apr 2024 17:13:10 +0800 Subject: [PATCH 07/29] coverage. Add BlockMarker for decisions and inject mcdc statements --- .../src/coverageinfo/ffi.rs | 45 ++++--- .../src/coverageinfo/mod.rs | 24 ++-- compiler/rustc_middle/src/mir/coverage.rs | 61 +++++---- .../rustc_mir_build/src/build/coverageinfo.rs | 123 +++++++++--------- .../rustc_mir_transform/src/coverage/mod.rs | 81 ++++++++---- .../rustc_mir_transform/src/coverage/query.rs | 6 +- .../rustc_mir_transform/src/coverage/spans.rs | 24 ++-- .../src/coverage/spans/from_mir.rs | 42 ++++-- 8 files changed, 242 insertions(+), 164 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index 62b8342f27958..50efd0ed45800 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -224,19 +224,27 @@ impl CounterMappingRegion { end_line, end_col, ), - MappingKind::Branch { true_term, false_term, mcdc_params: condition_info } => { - Self::branch_region( - Counter::from_term(true_term), - Counter::from_term(false_term), - condition_info, - local_file_id, - start_line, - start_col, - end_line, - end_col, - ) - } - MappingKind::Decision(decision_info) => Self::decision_region( + MappingKind::Branch { true_term, false_term } => Self::branch_region( + Counter::from_term(true_term), + Counter::from_term(false_term), + None, + local_file_id, + start_line, + start_col, + end_line, + end_col, + ), + MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => Self::branch_region( + Counter::from_term(true_term), + Counter::from_term(false_term), + Some(mcdc_params), + local_file_id, + start_line, + start_col, + end_line, + end_col, + ), + MappingKind::MCDCDecision(decision_info) => Self::decision_region( decision_info, local_file_id, start_line, @@ -272,14 +280,17 @@ impl CounterMappingRegion { pub(crate) fn branch_region( counter: Counter, false_counter: Counter, - condition_info: ConditionInfo, + condition_info: Option, file_id: u32, start_line: u32, start_col: u32, end_line: u32, end_col: u32, ) -> Self { - let mcdc_params = mcdc::Parameters::Branch(condition_info.into()); + let mcdc_params = condition_info + .map(mcdc::BranchParameters::from) + .map(mcdc::Parameters::Branch) + .unwrap_or(mcdc::Parameters::None); let kind = match mcdc_params { mcdc::Parameters::None => RegionKind::BranchRegion, mcdc::Parameters::Branch(_) => RegionKind::MCDCBranchRegion, @@ -307,10 +318,12 @@ impl CounterMappingRegion { end_line: u32, end_col: u32, ) -> Self { + let mcdc_params = mcdc::Parameters::Decision(decision_info.into()); + Self { counter: Counter::ZERO, false_counter: Counter::ZERO, - mcdc_params: mcdc::Parameters::Decision(decision_info.into()), + mcdc_params, file_id, expanded_file_id: 0, start_line, diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 323107e27ebd1..74a0123791651 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -98,12 +98,8 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { return; }; - match *kind { - // Ensure mcdc parameters are properly initialized for these operations. - CoverageKind::UpdateCondBitmap { .. } | CoverageKind::UpdateTestVector { .. } => { - ensure_mcdc_parameters(bx, instance, function_coverage_info) - } - _ => {} + if function_coverage_info.mcdc_bitmap_bytes > 0 { + ensure_mcdc_parameters(bx, instance, function_coverage_info); } let Some(coverage_context) = bx.coverage_context() else { return }; @@ -147,23 +143,27 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { CoverageKind::ExpressionUsed { id } => { func_coverage.mark_expression_id_seen(id); } - - CoverageKind::UpdateCondBitmap { id, value } => { + CoverageKind::CondBitmapUpdate { id, value, .. } => { drop(coverage_map); + assert_ne!( + id.as_u32(), + 0, + "ConditionId of evaluated conditions should never be zero" + ); let cond_bitmap = coverage_context .try_get_mcdc_condition_bitmap(&instance) - .expect("mcdc cond bitmap must be set before being updated"); + .expect("mcdc cond bitmap should have been allocated for updating"); let cond_loc = bx.const_i32(id.as_u32() as i32 - 1); let bool_value = bx.const_bool(value); let fn_name = bx.get_pgo_func_name_var(instance); let hash = bx.const_u64(function_coverage_info.function_source_hash); bx.mcdc_condbitmap_update(fn_name, hash, cond_loc, cond_bitmap, bool_value); } - CoverageKind::UpdateTestVector { bitmap_idx } => { + CoverageKind::TestVectorBitmapUpdate { bitmap_idx } => { drop(coverage_map); let cond_bitmap = coverage_context - .try_get_mcdc_condition_bitmap(&instance) - .expect("mcdc cond bitmap must be set before being updated"); + .try_get_mcdc_condition_bitmap(&instance) + .expect("mcdc cond bitmap should have been allocated for merging into the global bitmap"); let bitmap_bytes = bx.tcx().coverage_ids_info(instance.def).mcdc_bitmap_bytes; assert!(bitmap_idx < bitmap_bytes, "bitmap index of the decision out of range"); assert!( diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 45ec22dba68aa..a8b7fe8123505 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -112,9 +112,7 @@ pub enum CoverageKind { /// /// If this statement does not survive MIR optimizations, any mappings that /// refer to this counter can have those references simplified to zero. - CounterIncrement { - id: CounterId, - }, + CounterIncrement { id: CounterId }, /// Marks the point in MIR control-flow represented by a coverage expression. /// @@ -124,18 +122,23 @@ pub enum CoverageKind { /// (This is only inserted for expression IDs that are directly used by /// mappings. Intermediate expressions with no direct mappings are /// retained/zeroed based on whether they are transitively used.) - ExpressionUsed { - id: ExpressionId, - }, - - UpdateCondBitmap { - id: ConditionId, - value: bool, - }, - - UpdateTestVector { - bitmap_idx: u32, - }, + ExpressionUsed { id: ExpressionId }, + + /// Marks the point in MIR control flow represented by a evaluated condition. + /// + /// This is eventually lowered to `llvm.instrprof.mcdc.condbitmap.update` in LLVM IR. + /// + /// If this statement does not survive MIR optimizations, the condition would never be + /// taken as evaluated. + CondBitmapUpdate { id: ConditionId, value: bool }, + + /// Marks the point in MIR control flow represented by a evaluated decision. + /// + /// This is eventually lowered to `llvm.instrprof.mcdc.tvbitmap.update` in LLVM IR. + /// + /// If this statement does not survive MIR optimizations, the decision would never be + /// taken as evaluated. + TestVectorBitmapUpdate { bitmap_idx: u32 }, } impl Debug for CoverageKind { @@ -146,10 +149,12 @@ impl Debug for CoverageKind { BlockMarker { id } => write!(fmt, "BlockMarker({:?})", id.index()), CounterIncrement { id } => write!(fmt, "CounterIncrement({:?})", id.index()), ExpressionUsed { id } => write!(fmt, "ExpressionUsed({:?})", id.index()), - UpdateCondBitmap { id, value } => { - write!(fmt, "UpdateCondBitmap({:?}, {value})", id.index()) + CondBitmapUpdate { id, value } => { + write!(fmt, "CondBitmapUpdate({:?}, {:?})", id.index(), value) + } + TestVectorBitmapUpdate { bitmap_idx } => { + write!(fmt, "TestVectorUpdate({:?})", bitmap_idx) } - UpdateTestVector { bitmap_idx } => write!(fmt, "UpdateTestVector({:?})", bitmap_idx), } } } @@ -205,9 +210,11 @@ pub enum MappingKind { /// Associates a normal region of code with a counter/expression/zero. Code(CovTerm), /// Associates a branch region with separate counters for true and false. - Branch { true_term: CovTerm, false_term: CovTerm, mcdc_params: ConditionInfo }, + Branch { true_term: CovTerm, false_term: CovTerm }, + /// Associates a branch region with separate counters for true and false. + MCDCBranch { true_term: CovTerm, false_term: CovTerm, mcdc_params: ConditionInfo }, /// Associates a decision region with a bitmap and number of conditions. - Decision(DecisionInfo), + MCDCDecision(DecisionInfo), } impl MappingKind { @@ -219,7 +226,8 @@ impl MappingKind { match *self { Self::Code(term) => one(term), Self::Branch { true_term, false_term, .. } => two(true_term, false_term), - Self::Decision(_) => zero(), + Self::MCDCBranch { true_term, false_term, .. } => two(true_term, false_term), + Self::MCDCDecision(_) => zero(), } } @@ -228,12 +236,15 @@ impl MappingKind { pub fn map_terms(&self, map_fn: impl Fn(CovTerm) -> CovTerm) -> Self { match *self { Self::Code(term) => Self::Code(map_fn(term)), - Self::Branch { true_term, false_term, mcdc_params } => Self::Branch { + Self::Branch { true_term, false_term } => { + Self::Branch { true_term: map_fn(true_term), false_term: map_fn(false_term) } + } + Self::MCDCBranch { true_term, false_term, mcdc_params } => Self::MCDCBranch { true_term: map_fn(true_term), false_term: map_fn(false_term), mcdc_params, }, - Self::Decision(param) => Self::Decision(param), + Self::MCDCDecision(param) => Self::MCDCDecision(param), } } } @@ -267,7 +278,6 @@ pub struct BranchInfo { /// data structures without having to scan the entire body first. pub num_block_markers: usize, pub branch_spans: Vec, - pub mcdc_bitmap_bytes_num: u32, pub decision_spans: Vec, } @@ -309,5 +319,6 @@ pub struct DecisionInfo { #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub struct DecisionSpan { pub span: Span, - pub mcdc_params: DecisionInfo, + pub conditions_num: u16, + pub join_marker: BlockMarkerId, } diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index b3f73752556f4..dfc532389159b 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -4,7 +4,7 @@ use std::collections::VecDeque; use rustc_data_structures::fx::FxHashMap; use rustc_middle::mir::coverage::{ - BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageKind, DecisionInfo, DecisionSpan, + BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageKind, DecisionSpan, }; use rustc_middle::mir::{self, BasicBlock, UnOp}; use rustc_middle::thir::{ExprId, ExprKind, LogicalOp, Thir}; @@ -12,6 +12,7 @@ use rustc_middle::ty::TyCtxt; use rustc_span::def_id::LocalDefId; use crate::build::Builder; +use crate::errors::MCDCExceedsConditionNumLimit; pub(crate) struct BranchInfoBuilder { /// Maps condition expressions to their enclosing `!`, for better instrumentation. @@ -107,26 +108,20 @@ impl BranchInfoBuilder { return None; } let decision_spans = mcdc_state.map(|state| state.decisions).unwrap_or_default(); - let mcdc_bitmap_bytes_num = decision_spans - .last() - .map(|decision| { - decision - .mcdc_params - .bitmap_idx - .saturating_add((1 << decision.mcdc_params.conditions_num).max(8)) - / 8 - }) - .unwrap_or_default(); Some(Box::new(mir::coverage::BranchInfo { num_block_markers, branch_spans, - mcdc_bitmap_bytes_num, decision_spans, })) } } +/// The MCDC bitmap scales exponentially (2^n) based on the number of conditions seen, +/// So llvm sets a maximum value prevents the bitmap footprint from growing too large without the user's knowledge. +/// This limit may be relaxed if the [upstream change](https://github.com/llvm/llvm-project/pull/82448) is merged. +const MAX_CONDITIONS_NUM_IN_DECISION: u16 = 6; + struct MCDCState { /// To construct condition evaluation tree. decision_stack: VecDeque, @@ -187,19 +182,6 @@ impl MCDCState { self.decision_stack.push_back(rhs); self.decision_stack.push_back(lhs); } - - fn pop_condition_info(&mut self) -> Option { - let condition = self.decision_stack.pop_back(); - if condition.is_some() { - if self.decision_stack.is_empty() - && let Some(decision) = self.decisions.last_mut() - { - decision.mcdc_params.conditions_num = self.next_condition_id as u16; - self.next_condition_id = 0; - } - } - condition - } } impl Builder<'_, '_> { @@ -230,10 +212,10 @@ impl Builder<'_, '_> { let condition_info = branch_info .mcdc_state .as_mut() - .and_then(MCDCState::pop_condition_info) + .and_then(|state| state.decision_stack.pop_back()) .unwrap_or_default(); - let mut inject_branch_marker = |block: BasicBlock, value: bool| { + let mut inject_branch_marker = |block: BasicBlock| { let id = branch_info.next_block_marker_id(); let marker_statement = mir::Statement { @@ -242,22 +224,11 @@ impl Builder<'_, '_> { }; self.cfg.push(block, marker_statement); - if condition_info.condition_id != ConditionId::NONE { - let condbitmap_update_statement = mir::Statement { - source_info, - kind: mir::StatementKind::Coverage(CoverageKind::UpdateCondBitmap { - id: condition_info.condition_id, - value, - }), - }; - self.cfg.push(block, condbitmap_update_statement); - } - id }; - let true_marker = inject_branch_marker(then_block, true); - let false_marker = inject_branch_marker(else_block, false); + let true_marker = inject_branch_marker(then_block); + let false_marker = inject_branch_marker(else_block); branch_info.branch_spans.push(BranchSpan { span: source_info.span, @@ -268,39 +239,65 @@ impl Builder<'_, '_> { } pub(crate) fn visit_coverage_decision(&mut self, expr_id: ExprId) { - if let Some(mcdc_state) = - self.coverage_branch_info.as_mut().and_then(BranchInfoBuilder::get_mcdc_state_mut) - { - let bitmap_idx = mcdc_state - .decisions - .last() - .map(|decision| { - decision - .mcdc_params - .bitmap_idx - .saturating_add((1 << decision.mcdc_params.conditions_num).max(8)) - }) - .unwrap_or_default(); + let Some(branch_info) = self.coverage_branch_info.as_mut() else { return }; + if branch_info.mcdc_state.is_some() { + let join_marker = branch_info.next_block_marker_id(); + let mcdc_state = branch_info.mcdc_state.as_mut().unwrap(); + assert!( + mcdc_state.decision_stack.is_empty() && mcdc_state.next_condition_id == 0, + "There is a unfinished decision" + ); mcdc_state.decisions.push(DecisionSpan { span: self.thir[expr_id].span, - mcdc_params: DecisionInfo { conditions_num: 0, bitmap_idx }, + conditions_num: 0, + join_marker, }); } } pub(crate) fn visit_coverage_decision_end(&mut self, join_block: BasicBlock) { - if let Some((span, bitmap_idx)) = self + if let Some((mcdc_state, branches)) = self .coverage_branch_info .as_mut() - .and_then(BranchInfoBuilder::get_mcdc_state_mut) - .and_then(|state| state.decisions.last_mut()) - .map(|end_decision| (end_decision.span, end_decision.mcdc_params.bitmap_idx)) + .and_then(|builder| builder.mcdc_state.as_mut().zip(Some(&mut builder.branch_spans))) { - let statement = mir::Statement { - source_info: self.source_info(span), - kind: mir::StatementKind::Coverage(CoverageKind::UpdateTestVector { bitmap_idx }), - }; - self.cfg.push(join_block, statement); + assert!( + mcdc_state.decision_stack.is_empty(), + "All condition should have been checked before the decision ends" + ); + let Some(decision) = mcdc_state.decisions.last_mut() else { return }; + + decision.conditions_num = mcdc_state.next_condition_id as u16; + mcdc_state.next_condition_id = 0; + + // Do not generate mcdc mappings and statements for decisions with too many conditions. + match decision.conditions_num { + 0 => { + unreachable!("Decision with no conditions is not allowed"); + } + 1..=MAX_CONDITIONS_NUM_IN_DECISION => { + let span = decision.span; + let id = decision.join_marker; + + let statement = mir::Statement { + source_info: self.source_info(span), + kind: mir::StatementKind::Coverage(CoverageKind::BlockMarker { id }), + }; + self.cfg.push(join_block, statement); + } + _ => { + for branch in branches.iter_mut().rev().take(decision.conditions_num as usize) { + branch.condition_info = Default::default(); + } + + self.tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit { + span: decision.span, + conditions_num: decision.conditions_num, + max_conditions_num: MAX_CONDITIONS_NUM_IN_DECISION, + }); + mcdc_state.decisions.pop(); + } + } } } diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 6c8ead504b869..19eab5980c45d 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -77,16 +77,6 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: return; }; - let mcdc_bitmap_bytes = coverage_spans - .decisions() - .last() - .map(|decision| { - (decision.mcdc_params.bitmap_idx + (1 << decision.mcdc_params.conditions_num).max(8)) - / 8 - }) - .unwrap_or_default(); - - //////////////////////////////////////////////////// // Create an optimized mix of `Counter`s and `Expression`s for the `CoverageGraph`. Ensure // every coverage span has a `Counter` or `Expression` assigned to its `BasicCoverageBlock` // and all `Expression` dependencies (operands) are also generated, for any other @@ -109,10 +99,12 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: &coverage_counters, ); + inject_mcdc_statements(mir_body, &basic_coverage_blocks, &coverage_spans); + mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo { function_source_hash: hir_info.function_source_hash, num_counters: coverage_counters.num_counters(), - mcdc_bitmap_bytes, + mcdc_bitmap_bytes: coverage_spans.test_vector_bitmap_bytes(), expressions: coverage_counters.into_expressions(), mappings, })); @@ -148,26 +140,27 @@ fn create_mappings<'tcx>( let mut mappings = Vec::new(); - for decision in coverage_spans.decisions() { - let kind = MappingKind::Decision(decision.mcdc_params); - - if let Some(code_region) = make_code_region(source_map, file_name, decision.span, body_span) - { - mappings.push(Mapping { kind, code_region }); - } - } - mappings.extend(coverage_spans.all_bcb_mappings().filter_map( |&BcbMapping { kind: bcb_mapping_kind, span }| { let kind = match bcb_mapping_kind { BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(bcb)), BcbMappingKind::Branch { true_bcb, false_bcb, mcdc_params } => { - MappingKind::Branch { - true_term: term_for_bcb(true_bcb), - false_term: term_for_bcb(false_bcb), - mcdc_params, + if mcdc_params.condition_id == ConditionId::NONE { + MappingKind::Branch { + true_term: term_for_bcb(true_bcb), + false_term: term_for_bcb(false_bcb), + } + } else { + MappingKind::MCDCBranch { + true_term: term_for_bcb(true_bcb), + false_term: term_for_bcb(false_bcb), + mcdc_params, + } } } + BcbMappingKind::Decision { bitmap_idx, conditions_num, .. } => { + MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, conditions_num }) + } }; let code_region = make_code_region(source_map, file_name, span, body_span)?; Some(Mapping { kind, code_region }) @@ -229,6 +222,46 @@ fn inject_coverage_statements<'tcx>( } } +fn inject_mcdc_statements<'tcx>( + mir_body: &mut mir::Body<'tcx>, + basic_coverage_blocks: &CoverageGraph, + coverage_spans: &CoverageSpans, +) { + if coverage_spans.test_vector_bitmap_bytes() == 0 { + return; + } + for mapping in coverage_spans.all_bcb_mappings() { + match mapping.kind { + BcbMappingKind::Branch { true_bcb, false_bcb, mcdc_params } => { + if mcdc_params.condition_id == ConditionId::NONE { + continue; + } + let true_bb = basic_coverage_blocks[true_bcb].leader_bb(); + inject_statement( + mir_body, + CoverageKind::CondBitmapUpdate { id: mcdc_params.condition_id, value: true }, + true_bb, + ); + let false_bb = basic_coverage_blocks[false_bcb].leader_bb(); + inject_statement( + mir_body, + CoverageKind::CondBitmapUpdate { id: mcdc_params.condition_id, value: false }, + false_bb, + ); + } + BcbMappingKind::Decision { join_bcb, bitmap_idx, .. } => { + let join_bb = basic_coverage_blocks[join_bcb].leader_bb(); + inject_statement( + mir_body, + CoverageKind::TestVectorBitmapUpdate { bitmap_idx: bitmap_idx }, + join_bb, + ); + } + _ => {} + } + } +} + /// Given two basic blocks that have a control-flow edge between them, creates /// and returns a new block that sits between those blocks. fn inject_edge_counter_basic_block( diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index 0673430614d4b..8bb2b4d17d687 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -64,7 +64,11 @@ fn coverage_ids_info<'tcx>( let mcdc_bitmap_bytes = mir_body .coverage_branch_info .as_deref() - .map(|info| info.mcdc_bitmap_bytes_num) + .map(|info| { + info.decision_spans + .iter() + .fold(0, |acc, decision| acc + (1_u32 << decision.conditions_num).div_ceil(8)) + }) .unwrap_or_default(); CoverageIdsInfo { max_counter_id, mcdc_bitmap_bytes } diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index aef5e324a84e3..0bdbe7ed4537a 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -1,7 +1,7 @@ use rustc_data_structures::graph::WithNumNodes; use rustc_index::bit_set::BitSet; use rustc_middle::mir; -use rustc_middle::mir::coverage::{ConditionInfo, DecisionSpan}; +use rustc_middle::mir::coverage::ConditionInfo; use rustc_span::{BytePos, Span}; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph, START_BCB}; @@ -20,6 +20,8 @@ pub(super) enum BcbMappingKind { false_bcb: BasicCoverageBlock, mcdc_params: ConditionInfo, }, + /// Associates a decision with its join BCB. + Decision { join_bcb: BasicCoverageBlock, bitmap_idx: u32, conditions_num: u16 }, } #[derive(Debug)] @@ -31,7 +33,7 @@ pub(super) struct BcbMapping { pub(super) struct CoverageSpans { bcb_has_mappings: BitSet, mappings: Vec, - decisions: Vec, + test_vector_bitmap_bytes: u32, } impl CoverageSpans { @@ -43,8 +45,8 @@ impl CoverageSpans { self.mappings.iter() } - pub(super) fn decisions(&self) -> impl Iterator { - self.decisions.iter() + pub(super) fn test_vector_bitmap_bytes(&self) -> u32 { + self.test_vector_bitmap_bytes } } @@ -95,6 +97,7 @@ pub(super) fn generate_coverage_spans( let mut insert = |bcb| { bcb_has_mappings.insert(bcb); }; + let mut test_vector_bitmap_bytes = 0; for &BcbMapping { kind, span: _ } in &mappings { match kind { BcbMappingKind::Code(bcb) => insert(bcb), @@ -102,14 +105,17 @@ pub(super) fn generate_coverage_spans( insert(true_bcb); insert(false_bcb); } + BcbMappingKind::Decision { bitmap_idx, conditions_num, .. } => { + // `bcb_has_mappings` is used for inject coverage counters + // but they are not needed for decision BCBs. + // While the length of test vector bitmap should be calculated here. + test_vector_bitmap_bytes = test_vector_bitmap_bytes + .max(bitmap_idx + (1_u32 << conditions_num as u32).div_ceil(8)); + } } } - Some(CoverageSpans { - bcb_has_mappings, - mappings, - decisions: from_mir::extract_decision_spans(mir_body).unwrap_or_default(), - }) + Some(CoverageSpans { bcb_has_mappings, mappings, test_vector_bitmap_bytes }) } #[derive(Debug)] diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 5cc9225361ab2..3cc6fd60c2577 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -223,15 +223,14 @@ fn filtered_statement_span(statement: &Statement<'_>) -> Option { | StatementKind::AscribeUserType(_, _) => Some(statement.source_info.span), // Block markers are used for branch coverage, so ignore them here. - StatementKind::Coverage( - CoverageKind::BlockMarker { .. } - | CoverageKind::UpdateCondBitmap { .. } - | CoverageKind::UpdateTestVector { .. }, - ) => None, + StatementKind::Coverage(CoverageKind::BlockMarker { .. }) => None, // These coverage statements should not exist prior to coverage instrumentation. StatementKind::Coverage( - CoverageKind::CounterIncrement { .. } | CoverageKind::ExpressionUsed { .. }, + CoverageKind::CounterIncrement { .. } + | CoverageKind::ExpressionUsed { .. } + | CoverageKind::CondBitmapUpdate { .. } + | CoverageKind::TestVectorBitmapUpdate { .. }, ) => bug!( "Unexpected coverage statement found during coverage instrumentation: {statement:?}" ), @@ -388,10 +387,8 @@ pub(super) fn extract_branch_mappings( } } - branch_info - .branch_spans - .iter() - .filter_map(|&BranchSpan { span: raw_span, condition_info, true_marker, false_marker }| { + let condition_filter_map = + |&BranchSpan { span: raw_span, condition_info, true_marker, false_marker }| { // For now, ignore any branch span that was introduced by // expansion. This makes things like assert macros less noisy. if !raw_span.ctxt().outer_expn_data().is_root() { @@ -409,10 +406,27 @@ pub(super) fn extract_branch_mappings( kind: BcbMappingKind::Branch { true_bcb, false_bcb, mcdc_params: condition_info }, span, }) + }; + + let mut next_bitmap_idx = 0; + + let decision_filter_map = |&DecisionSpan { span: raw_span, conditions_num, join_marker }| { + let (span, _) = unexpand_into_body_span_with_visible_macro(raw_span, body_span)?; + + let join_bcb = basic_coverage_blocks.bcb_from_bb(block_markers[join_marker]?)?; + let bitmap_idx = next_bitmap_idx; + next_bitmap_idx += (1_u32 << conditions_num).div_ceil(8); + + Some(BcbMapping { + kind: BcbMappingKind::Decision { join_bcb, bitmap_idx, conditions_num }, + span, }) - .collect::>() -} + }; -pub(super) fn extract_decision_spans(mir_body: &mir::Body<'_>) -> Option> { - mir_body.coverage_branch_info.as_deref().map(|info| info.decision_spans.clone()) + branch_info + .branch_spans + .iter() + .filter_map(condition_filter_map) + .chain(branch_info.decision_spans.iter().filter_map(decision_filter_map)) + .collect::>() } From 75608afc6b83edb7e6da773946dcedd84dc4e347 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 2 Apr 2024 17:31:29 +0800 Subject: [PATCH 08/29] coverage. Add comment for some functions --- .../rustc_mir_build/src/build/coverageinfo.rs | 42 ++++++++++++++++++- .../rustc_mir_transform/src/coverage/mod.rs | 2 + 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index dfc532389159b..e8a1a30d658e0 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -138,6 +138,46 @@ impl MCDCState { }) } + /// At first we assign ConditionIds for each sub expression. + /// If the sub expression is composite, re-assign its ConditionId to its LHS and generate a new ConditionId for its RHS. + /// + /// Example: "x = (A && B) || (C && D) || (D && F)" + /// + /// Visit Depth1: + /// (A && B) || (C && D) || (D && F) + /// ^-------LHS--------^ ^-RHS--^ + /// ID=1 ID=2 + /// + /// Visit LHS-Depth2: + /// (A && B) || (C && D) + /// ^-LHS--^ ^-RHS--^ + /// ID=1 ID=3 + /// + /// Visit LHS-Depth3: + /// (A && B) + /// LHS RHS + /// ID=1 ID=4 + /// + /// Visit RHS-Depth3: + /// (C && D) + /// LHS RHS + /// ID=3 ID=5 + /// + /// Visit RHS-Depth2: (D && F) + /// LHS RHS + /// ID=2 ID=6 + /// + /// Visit Depth1: + /// (A && B) || (C && D) || (D && F) + /// ID=1 ID=4 ID=3 ID=5 ID=2 ID=6 + /// + /// A node ID of '0' always means MC/DC isn't being tracked. + /// + /// If a "next" node ID is '0', it means it's the end of the test vector. + /// + /// As the compiler tracks expression in pre-order, we can ensure that condition info of parents are always properly assigned when their children are visited. + /// - If the op is AND, the "false_next" of LHS and RHS should be the parent's "false_next". While "true_next" of the LHS is the RHS, the "true next" of RHS is the parent's "true_next". + /// - If the op is OR, the "true_next" of LHS and RHS should be the parent's "true_next". While "false_next" of the LHS is the RHS, the "false next" of RHS is the parent's "false_next". fn record_conditions(&mut self, op: LogicalOp) { let parent_condition = self.decision_stack.pop_back().unwrap_or_default(); let lhs_id = if parent_condition.condition_id == ConditionId::NONE { @@ -270,7 +310,6 @@ impl Builder<'_, '_> { decision.conditions_num = mcdc_state.next_condition_id as u16; mcdc_state.next_condition_id = 0; - // Do not generate mcdc mappings and statements for decisions with too many conditions. match decision.conditions_num { 0 => { unreachable!("Decision with no conditions is not allowed"); @@ -286,6 +325,7 @@ impl Builder<'_, '_> { self.cfg.push(join_block, statement); } _ => { + // Do not generate mcdc mappings and statements for decisions with too many conditions. for branch in branches.iter_mut().rev().take(decision.conditions_num as usize) { branch.condition_info = Default::default(); } diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 19eab5980c45d..a8d8015d320ff 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -222,6 +222,8 @@ fn inject_coverage_statements<'tcx>( } } +/// For each conditions inject statements to update condition bitmap after it has been evaluated. +/// For each decision inject statements to update test vector bitmap after it has been evaluated. fn inject_mcdc_statements<'tcx>( mir_body: &mut mir::Body<'tcx>, basic_coverage_blocks: &CoverageGraph, From d8f3ddebfc2b3068178791a92614ea9696320e7c Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 2 Apr 2024 19:07:53 +0800 Subject: [PATCH 09/29] coverage. Do not check raw logical expressions temporarily --- .../rustc_mir_build/src/build/coverageinfo.rs | 61 ++++++++----------- .../rustc_mir_build/src/build/expr/into.rs | 5 +- compiler/rustc_mir_build/src/errors.rs | 4 +- 3 files changed, 27 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index e8a1a30d658e0..35ec154512e10 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -20,7 +20,7 @@ pub(crate) struct BranchInfoBuilder { num_block_markers: usize, branch_spans: Vec, - + decision_spans: Vec, mcdc_state: Option, } @@ -43,6 +43,7 @@ impl BranchInfoBuilder { nots: FxHashMap::default(), num_block_markers: 0, branch_spans: vec![], + decision_spans: vec![], mcdc_state: MCDCState::new_if_enabled(tcx), }) } else { @@ -101,13 +102,12 @@ impl BranchInfoBuilder { } pub(crate) fn into_done(self) -> Option> { - let Self { nots: _, num_block_markers, branch_spans, mcdc_state } = self; + let Self { nots: _, num_block_markers, branch_spans, decision_spans, .. } = self; if num_block_markers == 0 { assert!(branch_spans.is_empty()); return None; } - let decision_spans = mcdc_state.map(|state| state.decisions).unwrap_or_default(); Some(Box::new(mir::coverage::BranchInfo { num_block_markers, @@ -120,22 +120,19 @@ impl BranchInfoBuilder { /// The MCDC bitmap scales exponentially (2^n) based on the number of conditions seen, /// So llvm sets a maximum value prevents the bitmap footprint from growing too large without the user's knowledge. /// This limit may be relaxed if the [upstream change](https://github.com/llvm/llvm-project/pull/82448) is merged. -const MAX_CONDITIONS_NUM_IN_DECISION: u16 = 6; +const MAX_CONDITIONS_NUM_IN_DECISION: usize = 6; struct MCDCState { /// To construct condition evaluation tree. decision_stack: VecDeque, next_condition_id: usize, - decisions: Vec, } impl MCDCState { fn new_if_enabled(tcx: TyCtxt<'_>) -> Option { - tcx.sess.instrument_coverage_mcdc().then(|| Self { - decision_stack: VecDeque::new(), - next_condition_id: 0, - decisions: vec![], - }) + tcx.sess + .instrument_coverage_mcdc() + .then(|| Self { decision_stack: VecDeque::new(), next_condition_id: 0 }) } /// At first we assign ConditionIds for each sub expression. @@ -278,24 +275,7 @@ impl Builder<'_, '_> { }); } - pub(crate) fn visit_coverage_decision(&mut self, expr_id: ExprId) { - let Some(branch_info) = self.coverage_branch_info.as_mut() else { return }; - if branch_info.mcdc_state.is_some() { - let join_marker = branch_info.next_block_marker_id(); - let mcdc_state = branch_info.mcdc_state.as_mut().unwrap(); - assert!( - mcdc_state.decision_stack.is_empty() && mcdc_state.next_condition_id == 0, - "There is a unfinished decision" - ); - mcdc_state.decisions.push(DecisionSpan { - span: self.thir[expr_id].span, - conditions_num: 0, - join_marker, - }); - } - } - - pub(crate) fn visit_coverage_decision_end(&mut self, join_block: BasicBlock) { + pub(crate) fn visit_coverage_decision(&mut self, expr_id: ExprId, join_block: BasicBlock) { if let Some((mcdc_state, branches)) = self .coverage_branch_info .as_mut() @@ -305,18 +285,26 @@ impl Builder<'_, '_> { mcdc_state.decision_stack.is_empty(), "All condition should have been checked before the decision ends" ); - let Some(decision) = mcdc_state.decisions.last_mut() else { return }; - decision.conditions_num = mcdc_state.next_condition_id as u16; + let conditions_num = mcdc_state.next_condition_id; + mcdc_state.next_condition_id = 0; - match decision.conditions_num { + match conditions_num { 0 => { unreachable!("Decision with no conditions is not allowed"); } 1..=MAX_CONDITIONS_NUM_IN_DECISION => { - let span = decision.span; - let id = decision.join_marker; + let span = self.thir[expr_id].span; + let branch_info = + self.coverage_branch_info.as_mut().expect("updating to existed"); + let id = branch_info.next_block_marker_id(); + + branch_info.decision_spans.push(DecisionSpan { + span, + conditions_num: conditions_num as u16, + join_marker: id, + }); let statement = mir::Statement { source_info: self.source_info(span), @@ -326,16 +314,15 @@ impl Builder<'_, '_> { } _ => { // Do not generate mcdc mappings and statements for decisions with too many conditions. - for branch in branches.iter_mut().rev().take(decision.conditions_num as usize) { + for branch in branches.iter_mut().rev().take(conditions_num) { branch.condition_info = Default::default(); } self.tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit { - span: decision.span, - conditions_num: decision.conditions_num, + span: self.thir[expr_id].span, + conditions_num, max_conditions_num: MAX_CONDITIONS_NUM_IN_DECISION, }); - mcdc_state.decisions.pop(); } } } diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index b5f64550e6fde..82400597d1c69 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -61,7 +61,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let then_span = this.thir[then].span; let then_source_info = this.source_info(then_span); let condition_scope = this.local_scope(); - this.visit_coverage_decision(cond); let then_and_else_blocks = this.in_scope( (if_then_scope, then_source_info), @@ -114,7 +113,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // The `then` and `else` arms have been lowered into their respective // blocks, so make both of them meet up in a new block. let join_block = this.cfg.start_new_block(); - this.visit_coverage_decision_end(join_block); + this.visit_coverage_decision(cond, join_block); this.cfg.goto(then_blk, source_info, join_block); this.cfg.goto(else_blk, source_info, join_block); join_block.unit() @@ -151,7 +150,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let condition_scope = this.local_scope(); let source_info = this.source_info(expr.span); - this.visit_coverage_decision(expr_id); // We first evaluate the left-hand side of the predicate ... let (then_block, else_block) = this.in_if_then_scope(condition_scope, expr.span, |this| { @@ -187,7 +185,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); let rhs = unpack!(this.expr_into_dest(destination, continuation, rhs)); let target = this.cfg.start_new_block(); - this.visit_coverage_decision_end(target); this.cfg.goto(rhs, source_info, target); this.cfg.goto(short_circuit, source_info, target); target.unit() diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index f7fadc0985b14..9ddfb12bf7643 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -823,8 +823,8 @@ pub struct NontrivialStructuralMatch<'tcx> { pub(crate) struct MCDCExceedsConditionNumLimit { #[primary_span] pub span: Span, - pub conditions_num: u16, - pub max_conditions_num: u16, + pub conditions_num: usize, + pub max_conditions_num: usize, } #[derive(Diagnostic)] From 6cf7f5d11a4e98449e25c77d34030046ebabdad4 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Wed, 3 Apr 2024 09:54:30 +0800 Subject: [PATCH 10/29] coverage. Let coverage-dump support parsing mcdc info --- src/tools/coverage-dump/src/covfun.rs | 51 ++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 4 deletions(-) diff --git a/src/tools/coverage-dump/src/covfun.rs b/src/tools/coverage-dump/src/covfun.rs index 49e3a6ed58383..ba3a61137449e 100644 --- a/src/tools/coverage-dump/src/covfun.rs +++ b/src/tools/coverage-dump/src/covfun.rs @@ -70,10 +70,13 @@ pub(crate) fn dump_covfun_mappings( } // If the mapping is a branch region, print both of its arms // in resolved form (even if they aren't expressions). - MappingKind::Branch { r#true, r#false } => { + MappingKind::Branch { r#true, r#false } + | MappingKind::MCDCBranch { r#true, r#false, .. } => { println!(" true = {}", expression_resolver.format_term(r#true)); println!(" false = {}", expression_resolver.format_term(r#false)); } + + MappingKind::MCDCDecision { .. } => {} _ => (), } } @@ -159,11 +162,30 @@ impl<'a> Parser<'a> { match high { 0 => unreachable!("zero kind should have already been handled as a code mapping"), 2 => Ok(MappingKind::Skip), - 4 => { + 4 | 6 => { let r#true = self.read_simple_term()?; let r#false = self.read_simple_term()?; - Ok(MappingKind::Branch { r#true, r#false }) + if high == 6 { + let condition_id = self.read_uleb128_u32()?; + let true_next_id = self.read_uleb128_u32()?; + let false_next_id = self.read_uleb128_u32()?; + Ok(MappingKind::MCDCBranch { + r#true, + r#false, + condition_id, + true_next_id, + false_next_id, + }) + } else { + Ok(MappingKind::Branch { r#true, r#false }) + } } + 5 => { + let bitmap_idx = self.read_uleb128_u32()?; + let conditions_num = self.read_uleb128_u32()?; + Ok(MappingKind::MCDCDecision { bitmap_idx, conditions_num }) + } + _ => Err(anyhow!("unknown mapping kind: {raw_mapping_kind:#x}")), } } @@ -224,7 +246,28 @@ enum MappingKind { // Using raw identifiers here makes the dump output a little bit nicer // (via the derived Debug), at the expense of making this tool's source // code a little bit uglier. - Branch { r#true: CovTerm, r#false: CovTerm }, + Branch { + r#true: CovTerm, + r#false: CovTerm, + }, + MCDCBranch { + r#true: CovTerm, + r#false: CovTerm, + // These attributes are printed in Debug but not used directly. + #[allow(dead_code)] + condition_id: u32, + #[allow(dead_code)] + true_next_id: u32, + #[allow(dead_code)] + false_next_id: u32, + }, + MCDCDecision { + // These attributes are printed in Debug but not used directly. + #[allow(dead_code)] + bitmap_idx: u32, + #[allow(dead_code)] + conditions_num: u32, + }, } struct MappingRegion { From 5bdc58ad7887cf44fd59ec08613737de9026165d Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Wed, 3 Apr 2024 10:22:52 +0800 Subject: [PATCH 11/29] coverage. Add mcdc test for if --- tests/coverage/mcdc_if.cov-map | 170 ++++++++++++++++++++++++++ tests/coverage/mcdc_if.coverage | 208 ++++++++++++++++++++++++++++++++ tests/coverage/mcdc_if.rs | 87 +++++++++++++ 3 files changed, 465 insertions(+) create mode 100644 tests/coverage/mcdc_if.cov-map create mode 100644 tests/coverage/mcdc_if.coverage create mode 100644 tests/coverage/mcdc_if.rs diff --git a/tests/coverage/mcdc_if.cov-map b/tests/coverage/mcdc_if.cov-map new file mode 100644 index 0000000000000..b1aab5ac6c76f --- /dev/null +++ b/tests/coverage/mcdc_if.cov-map @@ -0,0 +1,170 @@ +Function name: mcdc_if::mcdc_check_a +Raw bytes (64): 0x[01, 01, 04, 01, 05, 09, 02, 0d, 0f, 09, 02, 08, 01, 0e, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 30, 0d, 09, 02, 00, 00, 00, 0d, 00, 0e, 0d, 00, 0f, 02, 06, 0f, 02, 0c, 02, 06, 0b, 03, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 4 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Counter(2), rhs = Expression(0, Sub) +- expression 2 operands: lhs = Counter(3), rhs = Expression(3, Add) +- expression 3 operands: lhs = Counter(2), rhs = Expression(0, Sub) +Number of file 0 mappings: 8 +- Code(Counter(0)) at (prev + 14, 1) to (start + 1, 9) +- MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 1, 8) to (start + 0, 14) +- MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 8) to (start + 0, 9) + true = c1 + false = (c0 - c1) +- Code(Counter(1)) at (prev + 0, 13) to (start + 0, 14) +- MCDCBranch { true: Counter(3), false: Counter(2), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 13) to (start + 0, 14) + true = c3 + false = c2 +- Code(Counter(3)) at (prev + 0, 15) to (start + 2, 6) +- Code(Expression(3, Add)) at (prev + 2, 12) to (start + 2, 6) + = (c2 + (c0 - c1)) +- Code(Expression(2, Add)) at (prev + 3, 1) to (start + 0, 2) + = (c3 + (c2 + (c0 - c1))) + +Function name: mcdc_if::mcdc_check_b +Raw bytes (64): 0x[01, 01, 04, 01, 05, 09, 02, 0d, 0f, 09, 02, 08, 01, 16, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 30, 0d, 09, 02, 00, 00, 00, 0d, 00, 0e, 0d, 00, 0f, 02, 06, 0f, 02, 0c, 02, 06, 0b, 03, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 4 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Counter(2), rhs = Expression(0, Sub) +- expression 2 operands: lhs = Counter(3), rhs = Expression(3, Add) +- expression 3 operands: lhs = Counter(2), rhs = Expression(0, Sub) +Number of file 0 mappings: 8 +- Code(Counter(0)) at (prev + 22, 1) to (start + 1, 9) +- MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 1, 8) to (start + 0, 14) +- MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 8) to (start + 0, 9) + true = c1 + false = (c0 - c1) +- Code(Counter(1)) at (prev + 0, 13) to (start + 0, 14) +- MCDCBranch { true: Counter(3), false: Counter(2), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 13) to (start + 0, 14) + true = c3 + false = c2 +- Code(Counter(3)) at (prev + 0, 15) to (start + 2, 6) +- Code(Expression(3, Add)) at (prev + 2, 12) to (start + 2, 6) + = (c2 + (c0 - c1)) +- Code(Expression(2, Add)) at (prev + 3, 1) to (start + 0, 2) + = (c3 + (c2 + (c0 - c1))) + +Function name: mcdc_if::mcdc_check_both +Raw bytes (64): 0x[01, 01, 04, 01, 05, 09, 02, 0d, 0f, 09, 02, 08, 01, 1e, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 30, 0d, 09, 02, 00, 00, 00, 0d, 00, 0e, 0d, 00, 0f, 02, 06, 0f, 02, 0c, 02, 06, 0b, 03, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 4 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Counter(2), rhs = Expression(0, Sub) +- expression 2 operands: lhs = Counter(3), rhs = Expression(3, Add) +- expression 3 operands: lhs = Counter(2), rhs = Expression(0, Sub) +Number of file 0 mappings: 8 +- Code(Counter(0)) at (prev + 30, 1) to (start + 1, 9) +- MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 1, 8) to (start + 0, 14) +- MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 8) to (start + 0, 9) + true = c1 + false = (c0 - c1) +- Code(Counter(1)) at (prev + 0, 13) to (start + 0, 14) +- MCDCBranch { true: Counter(3), false: Counter(2), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 13) to (start + 0, 14) + true = c3 + false = c2 +- Code(Counter(3)) at (prev + 0, 15) to (start + 2, 6) +- Code(Expression(3, Add)) at (prev + 2, 12) to (start + 2, 6) + = (c2 + (c0 - c1)) +- Code(Expression(2, Add)) at (prev + 3, 1) to (start + 0, 2) + = (c3 + (c2 + (c0 - c1))) + +Function name: mcdc_if::mcdc_check_neither +Raw bytes (64): 0x[01, 01, 04, 01, 05, 09, 02, 0d, 0f, 09, 02, 08, 01, 06, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 30, 0d, 09, 02, 00, 00, 00, 0d, 00, 0e, 0d, 00, 0f, 02, 06, 0f, 02, 0c, 02, 06, 0b, 03, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 4 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Counter(2), rhs = Expression(0, Sub) +- expression 2 operands: lhs = Counter(3), rhs = Expression(3, Add) +- expression 3 operands: lhs = Counter(2), rhs = Expression(0, Sub) +Number of file 0 mappings: 8 +- Code(Counter(0)) at (prev + 6, 1) to (start + 1, 9) +- MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 1, 8) to (start + 0, 14) +- MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 8) to (start + 0, 9) + true = c1 + false = (c0 - c1) +- Code(Counter(1)) at (prev + 0, 13) to (start + 0, 14) +- MCDCBranch { true: Counter(3), false: Counter(2), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 13) to (start + 0, 14) + true = c3 + false = c2 +- Code(Counter(3)) at (prev + 0, 15) to (start + 2, 6) +- Code(Expression(3, Add)) at (prev + 2, 12) to (start + 2, 6) + = (c2 + (c0 - c1)) +- Code(Expression(2, Add)) at (prev + 3, 1) to (start + 0, 2) + = (c3 + (c2 + (c0 - c1))) + +Function name: mcdc_if::mcdc_check_not_tree_decision +Raw bytes (87): 0x[01, 01, 08, 01, 05, 02, 09, 05, 09, 0d, 1e, 02, 09, 11, 1b, 0d, 1e, 02, 09, 0a, 01, 30, 01, 03, 0a, 28, 00, 03, 03, 08, 00, 15, 30, 05, 02, 01, 02, 03, 00, 09, 00, 0a, 02, 00, 0e, 00, 0f, 30, 09, 1e, 03, 02, 00, 00, 0e, 00, 0f, 0b, 00, 14, 00, 15, 30, 11, 0d, 02, 00, 00, 00, 14, 00, 15, 11, 00, 16, 02, 06, 1b, 02, 0c, 02, 06, 17, 03, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 8 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Expression(0, Sub), rhs = Counter(2) +- expression 2 operands: lhs = Counter(1), rhs = Counter(2) +- expression 3 operands: lhs = Counter(3), rhs = Expression(7, Sub) +- expression 4 operands: lhs = Expression(0, Sub), rhs = Counter(2) +- expression 5 operands: lhs = Counter(4), rhs = Expression(6, Add) +- expression 6 operands: lhs = Counter(3), rhs = Expression(7, Sub) +- expression 7 operands: lhs = Expression(0, Sub), rhs = Counter(2) +Number of file 0 mappings: 10 +- Code(Counter(0)) at (prev + 48, 1) to (start + 3, 10) +- MCDCDecision { bitmap_idx: 0, conditions_num: 3 } at (prev + 3, 8) to (start + 0, 21) +- MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 2, false_next_id: 3 } at (prev + 0, 9) to (start + 0, 10) + true = c1 + false = (c0 - c1) +- Code(Expression(0, Sub)) at (prev + 0, 14) to (start + 0, 15) + = (c0 - c1) +- MCDCBranch { true: Counter(2), false: Expression(7, Sub), condition_id: 3, true_next_id: 2, false_next_id: 0 } at (prev + 0, 14) to (start + 0, 15) + true = c2 + false = ((c0 - c1) - c2) +- Code(Expression(2, Add)) at (prev + 0, 20) to (start + 0, 21) + = (c1 + c2) +- MCDCBranch { true: Counter(4), false: Counter(3), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 20) to (start + 0, 21) + true = c4 + false = c3 +- Code(Counter(4)) at (prev + 0, 22) to (start + 2, 6) +- Code(Expression(6, Add)) at (prev + 2, 12) to (start + 2, 6) + = (c3 + ((c0 - c1) - c2)) +- Code(Expression(5, Add)) at (prev + 3, 1) to (start + 0, 2) + = (c4 + (c3 + ((c0 - c1) - c2))) + +Function name: mcdc_if::mcdc_check_tree_decision +Raw bytes (87): 0x[01, 01, 08, 01, 05, 05, 0d, 05, 0d, 0d, 11, 09, 02, 1b, 1f, 0d, 11, 09, 02, 0a, 01, 26, 01, 03, 09, 28, 00, 03, 03, 08, 00, 15, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0e, 00, 0f, 30, 0d, 0a, 02, 00, 03, 00, 0e, 00, 0f, 0a, 00, 13, 00, 14, 30, 11, 09, 03, 00, 00, 00, 13, 00, 14, 1b, 00, 16, 02, 06, 1f, 02, 0c, 02, 06, 17, 03, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 8 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Counter(1), rhs = Counter(3) +- expression 2 operands: lhs = Counter(1), rhs = Counter(3) +- expression 3 operands: lhs = Counter(3), rhs = Counter(4) +- expression 4 operands: lhs = Counter(2), rhs = Expression(0, Sub) +- expression 5 operands: lhs = Expression(6, Add), rhs = Expression(7, Add) +- expression 6 operands: lhs = Counter(3), rhs = Counter(4) +- expression 7 operands: lhs = Counter(2), rhs = Expression(0, Sub) +Number of file 0 mappings: 10 +- Code(Counter(0)) at (prev + 38, 1) to (start + 3, 9) +- MCDCDecision { bitmap_idx: 0, conditions_num: 3 } at (prev + 3, 8) to (start + 0, 21) +- MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 8) to (start + 0, 9) + true = c1 + false = (c0 - c1) +- Code(Counter(1)) at (prev + 0, 14) to (start + 0, 15) +- MCDCBranch { true: Counter(3), false: Expression(2, Sub), condition_id: 2, true_next_id: 0, false_next_id: 3 } at (prev + 0, 14) to (start + 0, 15) + true = c3 + false = (c1 - c3) +- Code(Expression(2, Sub)) at (prev + 0, 19) to (start + 0, 20) + = (c1 - c3) +- MCDCBranch { true: Counter(4), false: Counter(2), condition_id: 3, true_next_id: 0, false_next_id: 0 } at (prev + 0, 19) to (start + 0, 20) + true = c4 + false = c2 +- Code(Expression(6, Add)) at (prev + 0, 22) to (start + 2, 6) + = (c3 + c4) +- Code(Expression(7, Add)) at (prev + 2, 12) to (start + 2, 6) + = (c2 + (c0 - c1)) +- Code(Expression(5, Add)) at (prev + 3, 1) to (start + 0, 2) + = ((c3 + c4) + (c2 + (c0 - c1))) + diff --git a/tests/coverage/mcdc_if.coverage b/tests/coverage/mcdc_if.coverage new file mode 100644 index 0000000000000..ef4efa4a0d313 --- /dev/null +++ b/tests/coverage/mcdc_if.coverage @@ -0,0 +1,208 @@ + LL| |#![feature(coverage_attribute)] + LL| |//@ edition: 2021 + LL| |//@ compile-flags: -Zcoverage-options=mcdc + LL| |//@ llvm-cov-flags: --show-mcdc + LL| | + LL| 2|fn mcdc_check_neither(a: bool, b: bool) { + LL| 2| if a && b { + ^0 + ------------------ + |---> MC/DC Decision Region (7:8) to (7:14) + | + | Number of Conditions: 2 + | Condition C1 --> (7:8) + | Condition C2 --> (7:13) + | + | Executed MC/DC Test Vectors: + | + | C1, C2 Result + | 1 { F, - = F } + | + | C1-Pair: not covered + | C2-Pair: not covered + | MC/DC Coverage for Decision: 0.00% + | + ------------------ + LL| 0| say("a and b"); + LL| 2| } else { + LL| 2| say("not both"); + LL| 2| } + LL| 2|} + LL| | + LL| 2|fn mcdc_check_a(a: bool, b: bool) { + LL| 2| if a && b { + ^1 + ------------------ + |---> MC/DC Decision Region (15:8) to (15:14) + | + | Number of Conditions: 2 + | Condition C1 --> (15:8) + | Condition C2 --> (15:13) + | + | Executed MC/DC Test Vectors: + | + | C1, C2 Result + | 1 { F, - = F } + | 2 { T, T = T } + | + | C1-Pair: covered: (1,2) + | C2-Pair: not covered + | MC/DC Coverage for Decision: 50.00% + | + ------------------ + LL| 1| say("a and b"); + LL| 1| } else { + LL| 1| say("not both"); + LL| 1| } + LL| 2|} + LL| | + LL| 2|fn mcdc_check_b(a: bool, b: bool) { + LL| 2| if a && b { + ------------------ + |---> MC/DC Decision Region (23:8) to (23:14) + | + | Number of Conditions: 2 + | Condition C1 --> (23:8) + | Condition C2 --> (23:13) + | + | Executed MC/DC Test Vectors: + | + | C1, C2 Result + | 1 { T, F = F } + | 2 { T, T = T } + | + | C1-Pair: not covered + | C2-Pair: covered: (1,2) + | MC/DC Coverage for Decision: 50.00% + | + ------------------ + LL| 1| say("a and b"); + LL| 1| } else { + LL| 1| say("not both"); + LL| 1| } + LL| 2|} + LL| | + LL| 3|fn mcdc_check_both(a: bool, b: bool) { + LL| 3| if a && b { + ^2 + ------------------ + |---> MC/DC Decision Region (31:8) to (31:14) + | + | Number of Conditions: 2 + | Condition C1 --> (31:8) + | Condition C2 --> (31:13) + | + | Executed MC/DC Test Vectors: + | + | C1, C2 Result + | 1 { F, - = F } + | 2 { T, F = F } + | 3 { T, T = T } + | + | C1-Pair: covered: (1,3) + | C2-Pair: covered: (2,3) + | MC/DC Coverage for Decision: 100.00% + | + ------------------ + LL| 1| say("a and b"); + LL| 2| } else { + LL| 2| say("not both"); + LL| 2| } + LL| 3|} + LL| | + LL| 4|fn mcdc_check_tree_decision(a: bool, b: bool, c: bool) { + LL| 4| // This expression is intentionally written in a way + LL| 4| // where 100% branch coverage indicates 100% mcdc coverage. + LL| 4| if a && (b || c) { + ^3 ^2 + ------------------ + |---> MC/DC Decision Region (41:8) to (41:21) + | + | Number of Conditions: 3 + | Condition C1 --> (41:8) + | Condition C2 --> (41:14) + | Condition C3 --> (41:19) + | + | Executed MC/DC Test Vectors: + | + | C1, C2, C3 Result + | 1 { F, -, - = F } + | 2 { T, F, F = F } + | 3 { T, T, - = T } + | 4 { T, F, T = T } + | + | C1-Pair: covered: (1,3) + | C2-Pair: covered: (2,3) + | C3-Pair: covered: (2,4) + | MC/DC Coverage for Decision: 100.00% + | + ------------------ + LL| 2| say("pass"); + LL| 2| } else { + LL| 2| say("reject"); + LL| 2| } + LL| 4|} + LL| | + LL| 4|fn mcdc_check_not_tree_decision(a: bool, b: bool, c: bool) { + LL| 4| // Contradict to `mcdc_check_tree_decision`, + LL| 4| // 100% branch coverage of this expression does not mean indicates 100% mcdc coverage. + LL| 4| if (a || b) && c { + ^1 + ------------------ + |---> MC/DC Decision Region (51:8) to (51:21) + | + | Number of Conditions: 3 + | Condition C1 --> (51:9) + | Condition C2 --> (51:14) + | Condition C3 --> (51:20) + | + | Executed MC/DC Test Vectors: + | + | C1, C2, C3 Result + | 1 { T, -, F = F } + | 2 { T, -, T = T } + | 3 { F, T, T = T } + | + | C1-Pair: not covered + | C2-Pair: not covered + | C3-Pair: covered: (1,2) + | MC/DC Coverage for Decision: 33.33% + | + ------------------ + LL| 2| say("pass"); + LL| 2| } else { + LL| 2| say("reject"); + LL| 2| } + LL| 4|} + LL| | + LL| |#[coverage(off)] + LL| |fn main() { + LL| | mcdc_check_neither(false, false); + LL| | mcdc_check_neither(false, true); + LL| | + LL| | mcdc_check_a(true, true); + LL| | mcdc_check_a(false, true); + LL| | + LL| | mcdc_check_b(true, true); + LL| | mcdc_check_b(true, false); + LL| | + LL| | mcdc_check_both(false, true); + LL| | mcdc_check_both(true, true); + LL| | mcdc_check_both(true, false); + LL| | + LL| | mcdc_check_tree_decision(false, true, true); + LL| | mcdc_check_tree_decision(true, true, false); + LL| | mcdc_check_tree_decision(true, false, false); + LL| | mcdc_check_tree_decision(true, false, true); + LL| | + LL| | mcdc_check_not_tree_decision(false, true, true); + LL| | mcdc_check_not_tree_decision(true, true, false); + LL| | mcdc_check_not_tree_decision(true, false, false); + LL| | mcdc_check_not_tree_decision(true, false, true); + LL| |} + LL| | + LL| |#[coverage(off)] + LL| |fn say(message: &str) { + LL| | core::hint::black_box(message); + LL| |} + diff --git a/tests/coverage/mcdc_if.rs b/tests/coverage/mcdc_if.rs new file mode 100644 index 0000000000000..437ea913a6a9b --- /dev/null +++ b/tests/coverage/mcdc_if.rs @@ -0,0 +1,87 @@ +#![feature(coverage_attribute)] +//@ edition: 2021 +//@ compile-flags: -Zcoverage-options=mcdc +//@ llvm-cov-flags: --show-mcdc + +fn mcdc_check_neither(a: bool, b: bool) { + if a && b { + say("a and b"); + } else { + say("not both"); + } +} + +fn mcdc_check_a(a: bool, b: bool) { + if a && b { + say("a and b"); + } else { + say("not both"); + } +} + +fn mcdc_check_b(a: bool, b: bool) { + if a && b { + say("a and b"); + } else { + say("not both"); + } +} + +fn mcdc_check_both(a: bool, b: bool) { + if a && b { + say("a and b"); + } else { + say("not both"); + } +} + +fn mcdc_check_tree_decision(a: bool, b: bool, c: bool) { + // This expression is intentionally written in a way + // where 100% branch coverage indicates 100% mcdc coverage. + if a && (b || c) { + say("pass"); + } else { + say("reject"); + } +} + +fn mcdc_check_not_tree_decision(a: bool, b: bool, c: bool) { + // Contradict to `mcdc_check_tree_decision`, + // 100% branch coverage of this expression does not mean indicates 100% mcdc coverage. + if (a || b) && c { + say("pass"); + } else { + say("reject"); + } +} + +#[coverage(off)] +fn main() { + mcdc_check_neither(false, false); + mcdc_check_neither(false, true); + + mcdc_check_a(true, true); + mcdc_check_a(false, true); + + mcdc_check_b(true, true); + mcdc_check_b(true, false); + + mcdc_check_both(false, true); + mcdc_check_both(true, true); + mcdc_check_both(true, false); + + mcdc_check_tree_decision(false, true, true); + mcdc_check_tree_decision(true, true, false); + mcdc_check_tree_decision(true, false, false); + mcdc_check_tree_decision(true, false, true); + + mcdc_check_not_tree_decision(false, true, true); + mcdc_check_not_tree_decision(true, true, false); + mcdc_check_not_tree_decision(true, false, false); + mcdc_check_not_tree_decision(true, false, true); +} + +#[coverage(off)] +fn say(message: &str) { + core::hint::black_box(message); +} From da99de4db56ae151ebd18bfe420b666e0e25d9ad Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Wed, 3 Apr 2024 10:34:46 +0800 Subject: [PATCH 12/29] coverage. put mcdc warnings in alphabetical order --- compiler/rustc_mir_build/messages.ftl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index 2f0fe278ad08d..34440c60cf378 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -97,6 +97,8 @@ mir_build_deref_raw_pointer_requires_unsafe_unsafe_op_in_unsafe_fn_allowed = .note = raw pointers may be null, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior .label = dereference of raw pointer +mir_build_exceeds_mcdc_condition_num_limit = Conditions number of the decision ({$conditions_num}) exceeds limit ({$max_conditions_num}). MCDC analysis will not count this expression. + mir_build_extern_static_requires_unsafe = use of extern static is unsafe and requires unsafe block .note = extern statics are not controlled by the Rust type system: invalid data, aliasing violations or data races will cause undefined behavior @@ -406,5 +408,3 @@ mir_build_unused_unsafe_enclosing_block_label = because it's nested under this ` mir_build_variant_defined_here = not covered mir_build_wrap_suggestion = consider wrapping the function body in an unsafe block - -mir_build_exceeds_mcdc_condition_num_limit = Conditions number of the decision ({$conditions_num}) exceeds limit ({$max_conditions_num}). MCDC analysis will not count this expression. \ No newline at end of file From e34f966df02f7a78479cfe40c765aadf3a584131 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Wed, 3 Apr 2024 12:14:51 +0800 Subject: [PATCH 13/29] coverage. MC/DC analysis ignores decisions with only one condition --- .../rustc_mir_build/src/build/coverageinfo.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index 35ec154512e10..c2214e68005ad 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -178,7 +178,7 @@ impl MCDCState { fn record_conditions(&mut self, op: LogicalOp) { let parent_condition = self.decision_stack.pop_back().unwrap_or_default(); let lhs_id = if parent_condition.condition_id == ConditionId::NONE { - self.next_condition_id += 1; + self.next_condition_id = 1; ConditionId::from(self.next_condition_id) } else { parent_condition.condition_id @@ -249,7 +249,12 @@ impl Builder<'_, '_> { let condition_info = branch_info .mcdc_state .as_mut() - .and_then(|state| state.decision_stack.pop_back()) + .and_then(|state| { + // If mcdc is enabled but no condition recorded in the stack, the branch must be standalone. + // In this case mc/dc is equivalent to branch coverage. Because each checked decision takes at least 1 byte + // in global bitmap of the function, we'd better not to generate too many mc/dc statements if could. + state.decision_stack.pop_back() + }) .unwrap_or_default(); let mut inject_branch_marker = |block: BasicBlock| { @@ -283,7 +288,7 @@ impl Builder<'_, '_> { { assert!( mcdc_state.decision_stack.is_empty(), - "All condition should have been checked before the decision ends" + "All conditions should have been checked before the decision ends" ); let conditions_num = mcdc_state.next_condition_id; @@ -292,7 +297,9 @@ impl Builder<'_, '_> { match conditions_num { 0 => { - unreachable!("Decision with no conditions is not allowed"); + // By here means the decision has only one condition, mc/dc analysis could ignore it. + // since mc/dc is equivalent to branch coverage in this case. + return; } 1..=MAX_CONDITIONS_NUM_IN_DECISION => { let span = self.thir[expr_id].span; From 55279aae5ca2fe7547d803b872d67feb0acf322a Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Wed, 3 Apr 2024 18:48:00 +0800 Subject: [PATCH 14/29] coverage. Fix mcdc analysis cannot work for nesting if --- compiler/rustc_middle/src/mir/coverage.rs | 4 +- .../rustc_mir_build/src/build/coverageinfo.rs | 174 ++++++++++-------- .../rustc_mir_build/src/build/expr/into.rs | 1 - .../rustc_mir_build/src/build/matches/mod.rs | 4 +- .../rustc_mir_transform/src/coverage/mod.rs | 87 +++++---- .../rustc_mir_transform/src/coverage/spans.rs | 18 +- .../src/coverage/spans/from_mir.rs | 27 ++- 7 files changed, 175 insertions(+), 140 deletions(-) diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index a8b7fe8123505..14fcf7af38882 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -319,6 +319,6 @@ pub struct DecisionInfo { #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub struct DecisionSpan { pub span: Span, - pub conditions_num: u16, - pub join_marker: BlockMarkerId, + pub conditions_num: usize, + pub end_marker: Vec, } diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index c2214e68005ad..4265d9eeaebd3 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -10,6 +10,7 @@ use rustc_middle::mir::{self, BasicBlock, UnOp}; use rustc_middle::thir::{ExprId, ExprKind, LogicalOp, Thir}; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::LocalDefId; +use rustc_span::Span; use crate::build::Builder; use crate::errors::MCDCExceedsConditionNumLimit; @@ -91,8 +92,50 @@ impl BranchInfoBuilder { } } - fn get_mcdc_state_mut(&mut self) -> Option<&mut MCDCState> { - self.mcdc_state.as_mut() + fn record_conditions_operation(&mut self, logical_op: LogicalOp, span: Span) { + if let Some(mcdc_state) = self.mcdc_state.as_mut() { + mcdc_state.record_conditions(logical_op, span); + } + } + + fn fetch_condition_info( + &mut self, + tcx: TyCtxt<'_>, + true_marker: BlockMarkerId, + false_marker: BlockMarkerId, + ) -> ConditionInfo { + let Some(mcdc_state) = self.mcdc_state.as_mut() else { + return ConditionInfo::default(); + }; + let (mut condition_info, decision_result) = + mcdc_state.take_condition(true_marker, false_marker); + if let Some(decision) = decision_result { + match decision.conditions_num { + 0 => { + unreachable!("Decision with no condition is not expected"); + } + 1..=MAX_CONDITIONS_NUM_IN_DECISION => { + self.decision_spans.push(decision); + } + _ => { + // Do not generate mcdc mappings and statements for decisions with too many conditions. + for branch in + self.branch_spans.iter_mut().rev().take(decision.conditions_num - 1) + { + branch.condition_info = ConditionInfo::default(); + } + // ConditionInfo of this branch shall also be reset. + condition_info = ConditionInfo::default(); + + tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit { + span: decision.span, + conditions_num: decision.conditions_num, + max_conditions_num: MAX_CONDITIONS_NUM_IN_DECISION, + }); + } + } + } + condition_info } fn next_block_marker_id(&mut self) -> BlockMarkerId { @@ -125,14 +168,14 @@ const MAX_CONDITIONS_NUM_IN_DECISION: usize = 6; struct MCDCState { /// To construct condition evaluation tree. decision_stack: VecDeque, - next_condition_id: usize, + processing_decision: Option, } impl MCDCState { fn new_if_enabled(tcx: TyCtxt<'_>) -> Option { tcx.sess .instrument_coverage_mcdc() - .then(|| Self { decision_stack: VecDeque::new(), next_condition_id: 0 }) + .then(|| Self { decision_stack: VecDeque::new(), processing_decision: None }) } /// At first we assign ConditionIds for each sub expression. @@ -175,17 +218,29 @@ impl MCDCState { /// As the compiler tracks expression in pre-order, we can ensure that condition info of parents are always properly assigned when their children are visited. /// - If the op is AND, the "false_next" of LHS and RHS should be the parent's "false_next". While "true_next" of the LHS is the RHS, the "true next" of RHS is the parent's "true_next". /// - If the op is OR, the "true_next" of LHS and RHS should be the parent's "true_next". While "false_next" of the LHS is the RHS, the "false next" of RHS is the parent's "false_next". - fn record_conditions(&mut self, op: LogicalOp) { + fn record_conditions(&mut self, op: LogicalOp, span: Span) { + let decision = match self.processing_decision.as_mut() { + Some(decision) => { + decision.span = decision.span.to(span); + decision + } + None => self.processing_decision.insert(DecisionSpan { + span, + conditions_num: 0, + end_marker: vec![], + }), + }; + let parent_condition = self.decision_stack.pop_back().unwrap_or_default(); let lhs_id = if parent_condition.condition_id == ConditionId::NONE { - self.next_condition_id = 1; - ConditionId::from(self.next_condition_id) + decision.conditions_num += 1; + ConditionId::from(decision.conditions_num) } else { parent_condition.condition_id }; - self.next_condition_id += 1; - let rhs_condition_id = ConditionId::from(self.next_condition_id); + decision.conditions_num += 1; + let rhs_condition_id = ConditionId::from(decision.conditions_num); let (lhs, rhs) = match op { LogicalOp::And => { @@ -219,6 +274,31 @@ impl MCDCState { self.decision_stack.push_back(rhs); self.decision_stack.push_back(lhs); } + + fn take_condition( + &mut self, + true_marker: BlockMarkerId, + false_marker: BlockMarkerId, + ) -> (ConditionInfo, Option) { + let Some(condition_info) = self.decision_stack.pop_back() else { + return (ConditionInfo::default(), None); + }; + let Some(decision) = self.processing_decision.as_mut() else { + bug!("Processing decision should have been created before any conditions are taken"); + }; + if condition_info.true_next_id == ConditionId::NONE { + decision.end_marker.push(true_marker); + } + if condition_info.false_next_id == ConditionId::NONE { + decision.end_marker.push(false_marker); + } + + if self.decision_stack.is_empty() { + (condition_info, self.processing_decision.take()) + } else { + (condition_info, None) + } + } } impl Builder<'_, '_> { @@ -246,17 +326,6 @@ impl Builder<'_, '_> { // Now that we have `source_info`, we can upgrade to a &mut reference. let branch_info = self.coverage_branch_info.as_mut().expect("upgrading & to &mut"); - let condition_info = branch_info - .mcdc_state - .as_mut() - .and_then(|state| { - // If mcdc is enabled but no condition recorded in the stack, the branch must be standalone. - // In this case mc/dc is equivalent to branch coverage. Because each checked decision takes at least 1 byte - // in global bitmap of the function, we'd better not to generate too many mc/dc statements if could. - state.decision_stack.pop_back() - }) - .unwrap_or_default(); - let mut inject_branch_marker = |block: BasicBlock| { let id = branch_info.next_block_marker_id(); @@ -272,6 +341,8 @@ impl Builder<'_, '_> { let true_marker = inject_branch_marker(then_block); let false_marker = inject_branch_marker(else_block); + let condition_info = branch_info.fetch_condition_info(self.tcx, true_marker, false_marker); + branch_info.branch_spans.push(BranchSpan { span: source_info.span, condition_info, @@ -280,66 +351,9 @@ impl Builder<'_, '_> { }); } - pub(crate) fn visit_coverage_decision(&mut self, expr_id: ExprId, join_block: BasicBlock) { - if let Some((mcdc_state, branches)) = self - .coverage_branch_info - .as_mut() - .and_then(|builder| builder.mcdc_state.as_mut().zip(Some(&mut builder.branch_spans))) - { - assert!( - mcdc_state.decision_stack.is_empty(), - "All conditions should have been checked before the decision ends" - ); - - let conditions_num = mcdc_state.next_condition_id; - - mcdc_state.next_condition_id = 0; - - match conditions_num { - 0 => { - // By here means the decision has only one condition, mc/dc analysis could ignore it. - // since mc/dc is equivalent to branch coverage in this case. - return; - } - 1..=MAX_CONDITIONS_NUM_IN_DECISION => { - let span = self.thir[expr_id].span; - let branch_info = - self.coverage_branch_info.as_mut().expect("updating to existed"); - let id = branch_info.next_block_marker_id(); - - branch_info.decision_spans.push(DecisionSpan { - span, - conditions_num: conditions_num as u16, - join_marker: id, - }); - - let statement = mir::Statement { - source_info: self.source_info(span), - kind: mir::StatementKind::Coverage(CoverageKind::BlockMarker { id }), - }; - self.cfg.push(join_block, statement); - } - _ => { - // Do not generate mcdc mappings and statements for decisions with too many conditions. - for branch in branches.iter_mut().rev().take(conditions_num) { - branch.condition_info = Default::default(); - } - - self.tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit { - span: self.thir[expr_id].span, - conditions_num, - max_conditions_num: MAX_CONDITIONS_NUM_IN_DECISION, - }); - } - } - } - } - - pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp) { - if let Some(mcdc_state) = - self.coverage_branch_info.as_mut().and_then(BranchInfoBuilder::get_mcdc_state_mut) - { - mcdc_state.record_conditions(logical_op); + pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp, span: Span) { + if let Some(branch_info) = self.coverage_branch_info.as_mut() { + branch_info.record_conditions_operation(logical_op, span); } } } diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 82400597d1c69..1bf83c90ea782 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -113,7 +113,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // The `then` and `else` arms have been lowered into their respective // blocks, so make both of them meet up in a new block. let join_block = this.cfg.start_new_block(); - this.visit_coverage_decision(cond, join_block); this.cfg.goto(then_blk, source_info, join_block); this.cfg.goto(else_blk, source_info, join_block); join_block.unit() diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index ba67d9654c896..82c3107634bc1 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -77,13 +77,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { match expr.kind { ExprKind::LogicalOp { op: LogicalOp::And, lhs, rhs } => { - this.visit_coverage_branch_operation(LogicalOp::And); + this.visit_coverage_branch_operation(LogicalOp::And, expr_span); let lhs_then_block = unpack!(this.then_else_break_inner(block, lhs, args)); let rhs_then_block = unpack!(this.then_else_break_inner(lhs_then_block, rhs, args)); rhs_then_block.unit() } ExprKind::LogicalOp { op: LogicalOp::Or, lhs, rhs } => { - this.visit_coverage_branch_operation(LogicalOp::Or); + this.visit_coverage_branch_operation(LogicalOp::Or, expr_span); let local_scope = this.local_scope(); let (lhs_success_block, failure_block) = this.in_if_then_scope(local_scope, expr_span, |this| { diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index a8d8015d320ff..4ad5dc0232440 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -141,28 +141,31 @@ fn create_mappings<'tcx>( let mut mappings = Vec::new(); mappings.extend(coverage_spans.all_bcb_mappings().filter_map( - |&BcbMapping { kind: bcb_mapping_kind, span }| { + |BcbMapping { kind: bcb_mapping_kind, span }| { let kind = match bcb_mapping_kind { - BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(bcb)), - BcbMappingKind::Branch { true_bcb, false_bcb, mcdc_params } => { - if mcdc_params.condition_id == ConditionId::NONE { + BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(*bcb)), + BcbMappingKind::Branch { true_bcb, false_bcb, condition_info } => { + if condition_info.condition_id == ConditionId::NONE { MappingKind::Branch { - true_term: term_for_bcb(true_bcb), - false_term: term_for_bcb(false_bcb), + true_term: term_for_bcb(*true_bcb), + false_term: term_for_bcb(*false_bcb), } } else { MappingKind::MCDCBranch { - true_term: term_for_bcb(true_bcb), - false_term: term_for_bcb(false_bcb), - mcdc_params, + true_term: term_for_bcb(*true_bcb), + false_term: term_for_bcb(*false_bcb), + mcdc_params: *condition_info, } } } BcbMappingKind::Decision { bitmap_idx, conditions_num, .. } => { - MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, conditions_num }) + MappingKind::MCDCDecision(DecisionInfo { + bitmap_idx: *bitmap_idx, + conditions_num: *conditions_num, + }) } }; - let code_region = make_code_region(source_map, file_name, span, body_span)?; + let code_region = make_code_region(source_map, file_name, *span, body_span)?; Some(Mapping { kind, code_region }) }, )); @@ -232,36 +235,44 @@ fn inject_mcdc_statements<'tcx>( if coverage_spans.test_vector_bitmap_bytes() == 0 { return; } - for mapping in coverage_spans.all_bcb_mappings() { - match mapping.kind { - BcbMappingKind::Branch { true_bcb, false_bcb, mcdc_params } => { - if mcdc_params.condition_id == ConditionId::NONE { - continue; - } - let true_bb = basic_coverage_blocks[true_bcb].leader_bb(); - inject_statement( - mir_body, - CoverageKind::CondBitmapUpdate { id: mcdc_params.condition_id, value: true }, - true_bb, - ); - let false_bb = basic_coverage_blocks[false_bcb].leader_bb(); - inject_statement( - mir_body, - CoverageKind::CondBitmapUpdate { id: mcdc_params.condition_id, value: false }, - false_bb, - ); - } - BcbMappingKind::Decision { join_bcb, bitmap_idx, .. } => { - let join_bb = basic_coverage_blocks[join_bcb].leader_bb(); - inject_statement( - mir_body, - CoverageKind::TestVectorBitmapUpdate { bitmap_idx: bitmap_idx }, - join_bb, - ); + + // Inject test vector update first because inject statement always inject new statement at head. + for (end_bcbs, bitmap_idx) in + coverage_spans.all_bcb_mappings().filter_map(|mapping| match &mapping.kind { + BcbMappingKind::Decision { end_bcbs: end_bcb, bitmap_idx, .. } => { + Some((end_bcb, *bitmap_idx)) } - _ => {} + _ => None, + }) + { + for end in end_bcbs { + let end_bb = basic_coverage_blocks[*end].leader_bb(); + inject_statement(mir_body, CoverageKind::TestVectorBitmapUpdate { bitmap_idx }, end_bb); } } + + for (true_bcb, false_bcb, condition_id) in + coverage_spans.all_bcb_mappings().filter_map(|mapping| match mapping.kind { + BcbMappingKind::Branch { true_bcb, false_bcb, condition_info } => (condition_info + .condition_id + != ConditionId::NONE) + .then_some((true_bcb, false_bcb, condition_info.condition_id)), + _ => None, + }) + { + let true_bb = basic_coverage_blocks[true_bcb].leader_bb(); + inject_statement( + mir_body, + CoverageKind::CondBitmapUpdate { id: condition_id, value: true }, + true_bb, + ); + let false_bb = basic_coverage_blocks[false_bcb].leader_bb(); + inject_statement( + mir_body, + CoverageKind::CondBitmapUpdate { id: condition_id, value: false }, + false_bb, + ); + } } /// Given two basic blocks that have a control-flow edge between them, creates diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 0bdbe7ed4537a..5c8d884e1cb1d 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeSet; + use rustc_data_structures::graph::WithNumNodes; use rustc_index::bit_set::BitSet; use rustc_middle::mir; @@ -10,7 +12,7 @@ use crate::coverage::ExtractedHirInfo; mod from_mir; -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Debug)] pub(super) enum BcbMappingKind { /// Associates an ordinary executable code span with its corresponding BCB. Code(BasicCoverageBlock), @@ -18,10 +20,10 @@ pub(super) enum BcbMappingKind { Branch { true_bcb: BasicCoverageBlock, false_bcb: BasicCoverageBlock, - mcdc_params: ConditionInfo, + condition_info: ConditionInfo, }, /// Associates a decision with its join BCB. - Decision { join_bcb: BasicCoverageBlock, bitmap_idx: u32, conditions_num: u16 }, + Decision { end_bcbs: BTreeSet, bitmap_idx: u32, conditions_num: u16 }, } #[derive(Debug)] @@ -98,19 +100,19 @@ pub(super) fn generate_coverage_spans( bcb_has_mappings.insert(bcb); }; let mut test_vector_bitmap_bytes = 0; - for &BcbMapping { kind, span: _ } in &mappings { + for BcbMapping { kind, span: _ } in &mappings { match kind { - BcbMappingKind::Code(bcb) => insert(bcb), + BcbMappingKind::Code(bcb) => insert(*bcb), BcbMappingKind::Branch { true_bcb, false_bcb, .. } => { - insert(true_bcb); - insert(false_bcb); + insert(*true_bcb); + insert(*false_bcb); } BcbMappingKind::Decision { bitmap_idx, conditions_num, .. } => { // `bcb_has_mappings` is used for inject coverage counters // but they are not needed for decision BCBs. // While the length of test vector bitmap should be calculated here. test_vector_bitmap_bytes = test_vector_bitmap_bytes - .max(bitmap_idx + (1_u32 << conditions_num as u32).div_ceil(8)); + .max(bitmap_idx + (1_u32 << *conditions_num as u32).div_ceil(8)); } } } diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 3cc6fd60c2577..195d4ad6b08a9 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -387,6 +387,9 @@ pub(super) fn extract_branch_mappings( } } + let bcb_from_marker = + |marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?); + let condition_filter_map = |&BranchSpan { span: raw_span, condition_info, true_marker, false_marker }| { // For now, ignore any branch span that was introduced by @@ -396,29 +399,35 @@ pub(super) fn extract_branch_mappings( } let (span, _) = unexpand_into_body_span_with_visible_macro(raw_span, body_span)?; - let bcb_from_marker = - |marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?); - let true_bcb = bcb_from_marker(true_marker)?; let false_bcb = bcb_from_marker(false_marker)?; Some(BcbMapping { - kind: BcbMappingKind::Branch { true_bcb, false_bcb, mcdc_params: condition_info }, + kind: BcbMappingKind::Branch { true_bcb, false_bcb, condition_info }, span, }) }; let mut next_bitmap_idx = 0; - let decision_filter_map = |&DecisionSpan { span: raw_span, conditions_num, join_marker }| { - let (span, _) = unexpand_into_body_span_with_visible_macro(raw_span, body_span)?; + let decision_filter_map = |decision: &DecisionSpan| { + let (span, _) = unexpand_into_body_span_with_visible_macro(decision.span, body_span)?; + + let end_bcbs = decision + .end_marker + .iter() + .map(|&marker| bcb_from_marker(marker)) + .collect::>()?; - let join_bcb = basic_coverage_blocks.bcb_from_bb(block_markers[join_marker]?)?; let bitmap_idx = next_bitmap_idx; - next_bitmap_idx += (1_u32 << conditions_num).div_ceil(8); + next_bitmap_idx += (1_u32 << decision.conditions_num).div_ceil(8); Some(BcbMapping { - kind: BcbMappingKind::Decision { join_bcb, bitmap_idx, conditions_num }, + kind: BcbMappingKind::Decision { + end_bcbs, + bitmap_idx, + conditions_num: decision.conditions_num as u16, + }, span, }) }; From 482d32e6d27c0293ea9798dffdce0e4a13ca2a75 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Wed, 3 Apr 2024 19:47:11 +0800 Subject: [PATCH 15/29] coverage. Add mcdc test for nested if --- tests/coverage/mcdc_if.cov-map | 48 +++++++++++++++++++++++++++++ tests/coverage/mcdc_if.coverage | 53 +++++++++++++++++++++++++++++++++ tests/coverage/mcdc_if.rs | 15 ++++++++++ 3 files changed, 116 insertions(+) diff --git a/tests/coverage/mcdc_if.cov-map b/tests/coverage/mcdc_if.cov-map index b1aab5ac6c76f..8ca6982a1967e 100644 --- a/tests/coverage/mcdc_if.cov-map +++ b/tests/coverage/mcdc_if.cov-map @@ -168,3 +168,51 @@ Number of file 0 mappings: 10 - Code(Expression(5, Add)) at (prev + 3, 1) to (start + 0, 2) = ((c3 + c4) + (c2 + (c0 - c1))) +Function name: mcdc_if::mcdc_nested_if +Raw bytes (124): 0x[01, 01, 0d, 01, 05, 02, 09, 05, 09, 1b, 15, 05, 09, 1b, 15, 05, 09, 11, 15, 02, 09, 2b, 32, 0d, 2f, 11, 15, 02, 09, 0e, 01, 3a, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 00, 02, 00, 08, 00, 09, 02, 00, 0d, 00, 0e, 30, 09, 32, 02, 00, 00, 00, 0d, 00, 0e, 1b, 01, 09, 01, 0d, 28, 01, 02, 01, 0c, 00, 12, 30, 16, 15, 01, 02, 00, 00, 0c, 00, 0d, 16, 00, 11, 00, 12, 30, 0d, 11, 02, 00, 00, 00, 11, 00, 12, 0d, 00, 13, 02, 0a, 2f, 02, 0a, 00, 0b, 32, 01, 0c, 02, 06, 27, 03, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 13 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Expression(0, Sub), rhs = Counter(2) +- expression 2 operands: lhs = Counter(1), rhs = Counter(2) +- expression 3 operands: lhs = Expression(6, Add), rhs = Counter(5) +- expression 4 operands: lhs = Counter(1), rhs = Counter(2) +- expression 5 operands: lhs = Expression(6, Add), rhs = Counter(5) +- expression 6 operands: lhs = Counter(1), rhs = Counter(2) +- expression 7 operands: lhs = Counter(4), rhs = Counter(5) +- expression 8 operands: lhs = Expression(0, Sub), rhs = Counter(2) +- expression 9 operands: lhs = Expression(10, Add), rhs = Expression(12, Sub) +- expression 10 operands: lhs = Counter(3), rhs = Expression(11, Add) +- expression 11 operands: lhs = Counter(4), rhs = Counter(5) +- expression 12 operands: lhs = Expression(0, Sub), rhs = Counter(2) +Number of file 0 mappings: 14 +- Code(Counter(0)) at (prev + 58, 1) to (start + 1, 9) +- MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 1, 8) to (start + 0, 14) +- MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 0, false_next_id: 2 } at (prev + 0, 8) to (start + 0, 9) + true = c1 + false = (c0 - c1) +- Code(Expression(0, Sub)) at (prev + 0, 13) to (start + 0, 14) + = (c0 - c1) +- MCDCBranch { true: Counter(2), false: Expression(12, Sub), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 13) to (start + 0, 14) + true = c2 + false = ((c0 - c1) - c2) +- Code(Expression(6, Add)) at (prev + 1, 9) to (start + 1, 13) + = (c1 + c2) +- MCDCDecision { bitmap_idx: 1, conditions_num: 2 } at (prev + 1, 12) to (start + 0, 18) +- MCDCBranch { true: Expression(5, Sub), false: Counter(5), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 12) to (start + 0, 13) + true = ((c1 + c2) - c5) + false = c5 +- Code(Expression(5, Sub)) at (prev + 0, 17) to (start + 0, 18) + = ((c1 + c2) - c5) +- MCDCBranch { true: Counter(3), false: Counter(4), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 17) to (start + 0, 18) + true = c3 + false = c4 +- Code(Counter(3)) at (prev + 0, 19) to (start + 2, 10) +- Code(Expression(11, Add)) at (prev + 2, 10) to (start + 0, 11) + = (c4 + c5) +- Code(Expression(12, Sub)) at (prev + 1, 12) to (start + 2, 6) + = ((c0 - c1) - c2) +- Code(Expression(9, Add)) at (prev + 3, 1) to (start + 0, 2) + = ((c3 + (c4 + c5)) + ((c0 - c1) - c2)) + diff --git a/tests/coverage/mcdc_if.coverage b/tests/coverage/mcdc_if.coverage index ef4efa4a0d313..81c6460591138 100644 --- a/tests/coverage/mcdc_if.coverage +++ b/tests/coverage/mcdc_if.coverage @@ -175,6 +175,55 @@ LL| 2| } LL| 4|} LL| | + LL| 3|fn mcdc_nested_if(a: bool, b: bool, c: bool) { + LL| 3| if a || b { + ^0 + ------------------ + |---> MC/DC Decision Region (59:8) to (59:14) + | + | Number of Conditions: 2 + | Condition C1 --> (59:8) + | Condition C2 --> (59:13) + | + | Executed MC/DC Test Vectors: + | + | C1, C2 Result + | 1 { T, - = T } + | + | C1-Pair: not covered + | C2-Pair: not covered + | MC/DC Coverage for Decision: 0.00% + | + ------------------ + LL| 3| say("a or b"); + LL| 3| if b && c { + ^2 + ------------------ + |---> MC/DC Decision Region (61:12) to (61:18) + | + | Number of Conditions: 2 + | Condition C1 --> (61:12) + | Condition C2 --> (61:17) + | + | Executed MC/DC Test Vectors: + | + | C1, C2 Result + | 1 { F, - = F } + | 2 { T, F = F } + | 3 { T, T = T } + | + | C1-Pair: covered: (1,3) + | C2-Pair: covered: (2,3) + | MC/DC Coverage for Decision: 100.00% + | + ------------------ + LL| 1| say("b and c"); + LL| 2| } + LL| 0| } else { + LL| 0| say("neither a nor b"); + LL| 0| } + LL| 3|} + LL| | LL| |#[coverage(off)] LL| |fn main() { LL| | mcdc_check_neither(false, false); @@ -199,6 +248,10 @@ LL| | mcdc_check_not_tree_decision(true, true, false); LL| | mcdc_check_not_tree_decision(true, false, false); LL| | mcdc_check_not_tree_decision(true, false, true); + LL| | + LL| | mcdc_nested_if(true, false, true); + LL| | mcdc_nested_if(true, true, true); + LL| | mcdc_nested_if(true, true, false); LL| |} LL| | LL| |#[coverage(off)] diff --git a/tests/coverage/mcdc_if.rs b/tests/coverage/mcdc_if.rs index 437ea913a6a9b..97cb4b5ef5f7f 100644 --- a/tests/coverage/mcdc_if.rs +++ b/tests/coverage/mcdc_if.rs @@ -55,6 +55,17 @@ fn mcdc_check_not_tree_decision(a: bool, b: bool, c: bool) { } } +fn mcdc_nested_if(a: bool, b: bool, c: bool) { + if a || b { + say("a or b"); + if b && c { + say("b and c"); + } + } else { + say("neither a nor b"); + } +} + #[coverage(off)] fn main() { mcdc_check_neither(false, false); @@ -79,6 +90,10 @@ fn main() { mcdc_check_not_tree_decision(true, true, false); mcdc_check_not_tree_decision(true, false, false); mcdc_check_not_tree_decision(true, false, true); + + mcdc_nested_if(true, false, true); + mcdc_nested_if(true, true, true); + mcdc_nested_if(true, true, false); } #[coverage(off)] From d72e711ca03d4e6a6dbc65eddef8c159ee2e20fe Mon Sep 17 00:00:00 2001 From: Deltalice Date: Thu, 4 Apr 2024 13:37:33 +0800 Subject: [PATCH 16/29] coverage. Change assign statements cl does not support on c++17 --- .../llvm-wrapper/CoverageMappingWrapper.cpp | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp index 4d721d22d9665..08e860ded1d83 100644 --- a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp @@ -94,23 +94,26 @@ struct LLVMRustMCDCParameters { static coverage::CounterMappingRegion::MCDCParameters fromRust(LLVMRustMCDCParameters Params) { + auto parameter = coverage::CounterMappingRegion::MCDCParameters{}; switch (Params.Tag) { case LLVMRustMCDCParametersTag::None: - return coverage::CounterMappingRegion::MCDCParameters{}; + return parameter; case LLVMRustMCDCParametersTag::Decision: - return coverage::CounterMappingRegion::MCDCParameters{ - .BitmapIdx = - static_cast(Params.Payload.DecisionParameters.BitmapIdx), - .NumConditions = static_cast( - Params.Payload.DecisionParameters.NumConditions)}; + parameter.BitmapIdx = + static_cast(Params.Payload.DecisionParameters.BitmapIdx), + parameter.NumConditions = + static_cast(Params.Payload.DecisionParameters.NumConditions); + return parameter; case LLVMRustMCDCParametersTag::Branch: - return coverage::CounterMappingRegion::MCDCParameters{ - .ID = static_cast( - Params.Payload.BranchParameters.ConditionID), - .FalseID = static_cast( + parameter.ID = static_cast( + Params.Payload.BranchParameters.ConditionID), + parameter.FalseID = + static_cast( Params.Payload.BranchParameters.ConditionIDs[0]), - .TrueID = static_cast( - Params.Payload.BranchParameters.ConditionIDs[1])}; + parameter.TrueID = + static_cast( + Params.Payload.BranchParameters.ConditionIDs[1]); + return parameter; } report_fatal_error("Bad LLVMRustMCDCParametersTag!"); } From eaa4f73fe821e6713cb5a3a05742658198f3c6d8 Mon Sep 17 00:00:00 2001 From: Deltalice Date: Thu, 4 Apr 2024 13:38:06 +0800 Subject: [PATCH 17/29] coverage. Add unimplemented mcdc functions for gcc codegen --- compiler/rustc_codegen_gcc/src/builder.rs | 37 ++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_gcc/src/builder.rs b/compiler/rustc_codegen_gcc/src/builder.rs index 43cc46cfe682f..adf8f986ddd9b 100644 --- a/compiler/rustc_codegen_gcc/src/builder.rs +++ b/compiler/rustc_codegen_gcc/src/builder.rs @@ -25,7 +25,7 @@ use rustc_middle::ty::layout::{ FnAbiError, FnAbiOfHelpers, FnAbiRequest, HasParamEnv, HasTyCtxt, LayoutError, LayoutOfHelpers, TyAndLayout, }; -use rustc_middle::ty::{ParamEnv, Ty, TyCtxt, Instance}; +use rustc_middle::ty::{Instance, ParamEnv, Ty, TyCtxt}; use rustc_span::def_id::DefId; use rustc_span::Span; use rustc_target::abi::{ @@ -1736,6 +1736,41 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> { ) { unimplemented!(); } + + fn mcdc_parameters( + &mut self, + _fn_name: Self::Value, + _hash: Self::Value, + _bitmap_bytes: Self::Value, + ) -> Self::Value { + unimplemented!(); + } + + fn mcdc_tvbitmap_update( + &mut self, + _fn_name: Self::Value, + _hash: Self::Value, + _bitmap_bytes: Self::Value, + _bitmap_index: Self::Value, + _mcdc_temp: Self::Value, + ) { + unimplemented!(); + } + + fn mcdc_condbitmap_update( + &mut self, + _fn_name: Self::Value, + _hash: Self::Value, + _cond_loc: Self::Value, + _mcdc_temp: Self::Value, + _bool_value: Self::Value, + ) { + unimplemented!(); + } + + fn mcdc_condbitmap_reset(&mut self, _mcdc_temp: Self::Value) { + unimplemented!(); + } } impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> { From 4713614f3a87395efaee7dfa330639ffd357f76b Mon Sep 17 00:00:00 2001 From: Deltalice Date: Thu, 4 Apr 2024 13:39:36 +0800 Subject: [PATCH 18/29] coverage. Allow multi options passed to coverage-options --- compiler/rustc_session/src/options.rs | 29 +++++++++------------------ 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index e3eaaf3eb455c..c791532a16e1c 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -943,32 +943,21 @@ mod parse { pub(crate) fn parse_coverage_options(slot: &mut CoverageOptions, v: Option<&str>) -> bool { let Some(v) = v else { return true }; - let set_branch_option = |slot: &mut CoverageOptions, option: &str| { + for option in v.split(',') { match option { - "no-branch" => slot.branch = false, - "branch" => slot.branch = true, + "no-branch" => { + slot.branch = false; + } + "branch" => { + slot.branch = true; + } "mcdc" => { - slot.mcdc = true; - slot.branch = true + slot.branch = true; + slot.mcdc = true } _ => return false, } - true - }; - - // Once an option is parsed we removed it from the array so that conflicting options such as "branch,no-branch" could be detected. - let mut parsers_set: [Option<&dyn Fn(&mut CoverageOptions, &str) -> bool>; 1] = - [Some(&set_branch_option)]; - - for option in v.split(',') { - if !parsers_set - .iter_mut() - .any(|p| p.is_some_and(|parser| parser(slot, option)).then(|| p.take()).is_some()) - { - return false; - } } - true } From 9c357e6eb6b0ff8d33fbe1437764a2a70bc3d287 Mon Sep 17 00:00:00 2001 From: Deltalice Date: Thu, 4 Apr 2024 13:40:17 +0800 Subject: [PATCH 19/29] coverage. Replace line numbers in mcdc reports with LL --- src/tools/compiletest/src/runtest.rs | 42 +++++++++++++++++++--- tests/coverage/mcdc_if.coverage | 52 ++++++++++++++-------------- 2 files changed, 63 insertions(+), 31 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 689fdc5dfebc0..e6f67c243a40a 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -277,11 +277,19 @@ impl<'test> TestCx<'test> { Ui | MirOpt => false, mode => panic!("unimplemented for mode {:?}", mode), }; - if test_should_run { self.run_if_enabled() } else { WillExecute::No } + if test_should_run { + self.run_if_enabled() + } else { + WillExecute::No + } } fn run_if_enabled(&self) -> WillExecute { - if self.config.run_enabled() { WillExecute::Yes } else { WillExecute::Disabled } + if self.config.run_enabled() { + WillExecute::Yes + } else { + WillExecute::Disabled + } } fn should_run_successfully(&self, pm: Option) -> bool { @@ -752,6 +760,18 @@ impl<'test> TestCx<'test> { Lazy::new(|| Regex::new(r"(?m:^)(?(?: \|)+ Branch \()[0-9]+:").unwrap()); let coverage = BRANCH_LINE_NUMBER_RE.replace_all(&coverage, "${prefix}LL:"); + // ` |---> MC/DC Decision Region (1:` => ` |---> MC/DC Decision Region (LL:` + static MCDC_DECISION_LINE_NUMBER_RE: Lazy = Lazy::new(|| { + Regex::new(r"(?m:^)(?(?: \|)+---> MC/DC Decision Region \()[0-9]+:").unwrap() + }); + let coverage = MCDC_DECISION_LINE_NUMBER_RE.replace_all(&coverage, "${prefix}LL:"); + + // ` | Condition C1 --> (1:` => ` | Condition C1 --> (LL:` + static MCDC_CONDITION_LINE_NUMBER_RE: Lazy = Lazy::new(|| { + Regex::new(r"(?m:^)(?(?: \|)+ Condition C\d+ --> \()[0-9]+:").unwrap() + }); + let coverage = MCDC_CONDITION_LINE_NUMBER_RE.replace_all(&coverage, "${prefix}LL:"); + coverage.into_owned() } @@ -2722,7 +2742,11 @@ impl<'test> TestCx<'test> { /// The revision, ignored for incremental compilation since it wants all revisions in /// the same directory. fn safe_revision(&self) -> Option<&str> { - if self.config.mode == Incremental { None } else { self.revision } + if self.config.mode == Incremental { + None + } else { + self.revision + } } /// Gets the absolute path to the directory where all output for the given @@ -2945,7 +2969,11 @@ impl<'test> TestCx<'test> { fn charset() -> &'static str { // FreeBSD 10.1 defaults to GDB 6.1.1 which doesn't support "auto" charset - if cfg!(target_os = "freebsd") { "ISO-8859-1" } else { "UTF-8" } + if cfg!(target_os = "freebsd") { + "ISO-8859-1" + } else { + "UTF-8" + } } fn run_rustdoc_test(&self) { @@ -4747,7 +4775,11 @@ impl<'test> TestCx<'test> { for output_file in files { println!("Actual {} saved to {}", kind, output_file.display()); } - if self.config.bless { 0 } else { 1 } + if self.config.bless { + 0 + } else { + 1 + } } fn check_and_prune_duplicate_outputs( diff --git a/tests/coverage/mcdc_if.coverage b/tests/coverage/mcdc_if.coverage index 81c6460591138..bb07aff4414e0 100644 --- a/tests/coverage/mcdc_if.coverage +++ b/tests/coverage/mcdc_if.coverage @@ -7,11 +7,11 @@ LL| 2| if a && b { ^0 ------------------ - |---> MC/DC Decision Region (7:8) to (7:14) + |---> MC/DC Decision Region (LL:8) to (7:14) | | Number of Conditions: 2 - | Condition C1 --> (7:8) - | Condition C2 --> (7:13) + | Condition C1 --> (LL:8) + | Condition C2 --> (LL:13) | | Executed MC/DC Test Vectors: | @@ -33,11 +33,11 @@ LL| 2| if a && b { ^1 ------------------ - |---> MC/DC Decision Region (15:8) to (15:14) + |---> MC/DC Decision Region (LL:8) to (15:14) | | Number of Conditions: 2 - | Condition C1 --> (15:8) - | Condition C2 --> (15:13) + | Condition C1 --> (LL:8) + | Condition C2 --> (LL:13) | | Executed MC/DC Test Vectors: | @@ -59,11 +59,11 @@ LL| 2|fn mcdc_check_b(a: bool, b: bool) { LL| 2| if a && b { ------------------ - |---> MC/DC Decision Region (23:8) to (23:14) + |---> MC/DC Decision Region (LL:8) to (23:14) | | Number of Conditions: 2 - | Condition C1 --> (23:8) - | Condition C2 --> (23:13) + | Condition C1 --> (LL:8) + | Condition C2 --> (LL:13) | | Executed MC/DC Test Vectors: | @@ -86,11 +86,11 @@ LL| 3| if a && b { ^2 ------------------ - |---> MC/DC Decision Region (31:8) to (31:14) + |---> MC/DC Decision Region (LL:8) to (31:14) | | Number of Conditions: 2 - | Condition C1 --> (31:8) - | Condition C2 --> (31:13) + | Condition C1 --> (LL:8) + | Condition C2 --> (LL:13) | | Executed MC/DC Test Vectors: | @@ -116,12 +116,12 @@ LL| 4| if a && (b || c) { ^3 ^2 ------------------ - |---> MC/DC Decision Region (41:8) to (41:21) + |---> MC/DC Decision Region (LL:8) to (41:21) | | Number of Conditions: 3 - | Condition C1 --> (41:8) - | Condition C2 --> (41:14) - | Condition C3 --> (41:19) + | Condition C1 --> (LL:8) + | Condition C2 --> (LL:14) + | Condition C3 --> (LL:19) | | Executed MC/DC Test Vectors: | @@ -149,12 +149,12 @@ LL| 4| if (a || b) && c { ^1 ------------------ - |---> MC/DC Decision Region (51:8) to (51:21) + |---> MC/DC Decision Region (LL:8) to (51:21) | | Number of Conditions: 3 - | Condition C1 --> (51:9) - | Condition C2 --> (51:14) - | Condition C3 --> (51:20) + | Condition C1 --> (LL:9) + | Condition C2 --> (LL:14) + | Condition C3 --> (LL:20) | | Executed MC/DC Test Vectors: | @@ -179,11 +179,11 @@ LL| 3| if a || b { ^0 ------------------ - |---> MC/DC Decision Region (59:8) to (59:14) + |---> MC/DC Decision Region (LL:8) to (59:14) | | Number of Conditions: 2 - | Condition C1 --> (59:8) - | Condition C2 --> (59:13) + | Condition C1 --> (LL:8) + | Condition C2 --> (LL:13) | | Executed MC/DC Test Vectors: | @@ -199,11 +199,11 @@ LL| 3| if b && c { ^2 ------------------ - |---> MC/DC Decision Region (61:12) to (61:18) + |---> MC/DC Decision Region (LL:12) to (61:18) | | Number of Conditions: 2 - | Condition C1 --> (61:12) - | Condition C2 --> (61:17) + | Condition C1 --> (LL:12) + | Condition C2 --> (LL:17) | | Executed MC/DC Test Vectors: | From eeb9fd43171dbc09c413b08929a953e5fef6b908 Mon Sep 17 00:00:00 2001 From: Deltalice Date: Thu, 4 Apr 2024 13:40:57 +0800 Subject: [PATCH 20/29] coverage. Improve code style --- src/tools/compiletest/src/runtest.rs | 30 ++++-------------------- src/tools/coverage-dump/src/covfun.rs | 33 +++++++++++++-------------- 2 files changed, 21 insertions(+), 42 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index e6f67c243a40a..9f9e336ccd4f6 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -277,19 +277,11 @@ impl<'test> TestCx<'test> { Ui | MirOpt => false, mode => panic!("unimplemented for mode {:?}", mode), }; - if test_should_run { - self.run_if_enabled() - } else { - WillExecute::No - } + if test_should_run { self.run_if_enabled() } else { WillExecute::No } } fn run_if_enabled(&self) -> WillExecute { - if self.config.run_enabled() { - WillExecute::Yes - } else { - WillExecute::Disabled - } + if self.config.run_enabled() { WillExecute::Yes } else { WillExecute::Disabled } } fn should_run_successfully(&self, pm: Option) -> bool { @@ -2742,11 +2734,7 @@ impl<'test> TestCx<'test> { /// The revision, ignored for incremental compilation since it wants all revisions in /// the same directory. fn safe_revision(&self) -> Option<&str> { - if self.config.mode == Incremental { - None - } else { - self.revision - } + if self.config.mode == Incremental { None } else { self.revision } } /// Gets the absolute path to the directory where all output for the given @@ -2969,11 +2957,7 @@ impl<'test> TestCx<'test> { fn charset() -> &'static str { // FreeBSD 10.1 defaults to GDB 6.1.1 which doesn't support "auto" charset - if cfg!(target_os = "freebsd") { - "ISO-8859-1" - } else { - "UTF-8" - } + if cfg!(target_os = "freebsd") { "ISO-8859-1" } else { "UTF-8" } } fn run_rustdoc_test(&self) { @@ -4775,11 +4759,7 @@ impl<'test> TestCx<'test> { for output_file in files { println!("Actual {} saved to {}", kind, output_file.display()); } - if self.config.bless { - 0 - } else { - 1 - } + if self.config.bless { 0 } else { 1 } } fn check_and_prune_duplicate_outputs( diff --git a/src/tools/coverage-dump/src/covfun.rs b/src/tools/coverage-dump/src/covfun.rs index ba3a61137449e..b308c8de14fe3 100644 --- a/src/tools/coverage-dump/src/covfun.rs +++ b/src/tools/coverage-dump/src/covfun.rs @@ -75,8 +75,6 @@ pub(crate) fn dump_covfun_mappings( println!(" true = {}", expression_resolver.format_term(r#true)); println!(" false = {}", expression_resolver.format_term(r#false)); } - - MappingKind::MCDCDecision { .. } => {} _ => (), } } @@ -162,29 +160,30 @@ impl<'a> Parser<'a> { match high { 0 => unreachable!("zero kind should have already been handled as a code mapping"), 2 => Ok(MappingKind::Skip), - 4 | 6 => { + 4 => { let r#true = self.read_simple_term()?; let r#false = self.read_simple_term()?; - if high == 6 { - let condition_id = self.read_uleb128_u32()?; - let true_next_id = self.read_uleb128_u32()?; - let false_next_id = self.read_uleb128_u32()?; - Ok(MappingKind::MCDCBranch { - r#true, - r#false, - condition_id, - true_next_id, - false_next_id, - }) - } else { - Ok(MappingKind::Branch { r#true, r#false }) - } + Ok(MappingKind::Branch { r#true, r#false }) } 5 => { let bitmap_idx = self.read_uleb128_u32()?; let conditions_num = self.read_uleb128_u32()?; Ok(MappingKind::MCDCDecision { bitmap_idx, conditions_num }) } + 6 => { + let r#true = self.read_simple_term()?; + let r#false = self.read_simple_term()?; + let condition_id = self.read_uleb128_u32()?; + let true_next_id = self.read_uleb128_u32()?; + let false_next_id = self.read_uleb128_u32()?; + Ok(MappingKind::MCDCBranch { + r#true, + r#false, + condition_id, + true_next_id, + false_next_id, + }) + } _ => Err(anyhow!("unknown mapping kind: {raw_mapping_kind:#x}")), } From c8bf12495f209a58fa5c3ddbc29cb11607f61425 Mon Sep 17 00:00:00 2001 From: Deltalice Date: Thu, 4 Apr 2024 14:11:49 +0800 Subject: [PATCH 21/29] coverage. Fix missing LL line number replacement --- src/tools/compiletest/src/runtest.rs | 7 ++++--- tests/coverage/mcdc_if.coverage | 16 ++++++++-------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 9f9e336ccd4f6..0e244b9369747 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -754,13 +754,14 @@ impl<'test> TestCx<'test> { // ` |---> MC/DC Decision Region (1:` => ` |---> MC/DC Decision Region (LL:` static MCDC_DECISION_LINE_NUMBER_RE: Lazy = Lazy::new(|| { - Regex::new(r"(?m:^)(?(?: \|)+---> MC/DC Decision Region \()[0-9]+:").unwrap() + Regex::new(r"(?m:^)(?(?: \|)+---> MC/DC Decision Region \()[0-9]+:(?[0-9]+\) to \()[0-9]+:").unwrap() }); - let coverage = MCDC_DECISION_LINE_NUMBER_RE.replace_all(&coverage, "${prefix}LL:"); + let coverage = + MCDC_DECISION_LINE_NUMBER_RE.replace_all(&coverage, "${prefix}LL:${middle}LL:"); // ` | Condition C1 --> (1:` => ` | Condition C1 --> (LL:` static MCDC_CONDITION_LINE_NUMBER_RE: Lazy = Lazy::new(|| { - Regex::new(r"(?m:^)(?(?: \|)+ Condition C\d+ --> \()[0-9]+:").unwrap() + Regex::new(r"(?m:^)(?(?: \|)+ Condition C[0-9]+ --> \()[0-9]+:").unwrap() }); let coverage = MCDC_CONDITION_LINE_NUMBER_RE.replace_all(&coverage, "${prefix}LL:"); diff --git a/tests/coverage/mcdc_if.coverage b/tests/coverage/mcdc_if.coverage index bb07aff4414e0..14d7f6c670d50 100644 --- a/tests/coverage/mcdc_if.coverage +++ b/tests/coverage/mcdc_if.coverage @@ -7,7 +7,7 @@ LL| 2| if a && b { ^0 ------------------ - |---> MC/DC Decision Region (LL:8) to (7:14) + |---> MC/DC Decision Region (LL:8) to (LL:14) | | Number of Conditions: 2 | Condition C1 --> (LL:8) @@ -33,7 +33,7 @@ LL| 2| if a && b { ^1 ------------------ - |---> MC/DC Decision Region (LL:8) to (15:14) + |---> MC/DC Decision Region (LL:8) to (LL:14) | | Number of Conditions: 2 | Condition C1 --> (LL:8) @@ -59,7 +59,7 @@ LL| 2|fn mcdc_check_b(a: bool, b: bool) { LL| 2| if a && b { ------------------ - |---> MC/DC Decision Region (LL:8) to (23:14) + |---> MC/DC Decision Region (LL:8) to (LL:14) | | Number of Conditions: 2 | Condition C1 --> (LL:8) @@ -86,7 +86,7 @@ LL| 3| if a && b { ^2 ------------------ - |---> MC/DC Decision Region (LL:8) to (31:14) + |---> MC/DC Decision Region (LL:8) to (LL:14) | | Number of Conditions: 2 | Condition C1 --> (LL:8) @@ -116,7 +116,7 @@ LL| 4| if a && (b || c) { ^3 ^2 ------------------ - |---> MC/DC Decision Region (LL:8) to (41:21) + |---> MC/DC Decision Region (LL:8) to (LL:21) | | Number of Conditions: 3 | Condition C1 --> (LL:8) @@ -149,7 +149,7 @@ LL| 4| if (a || b) && c { ^1 ------------------ - |---> MC/DC Decision Region (LL:8) to (51:21) + |---> MC/DC Decision Region (LL:8) to (LL:21) | | Number of Conditions: 3 | Condition C1 --> (LL:9) @@ -179,7 +179,7 @@ LL| 3| if a || b { ^0 ------------------ - |---> MC/DC Decision Region (LL:8) to (59:14) + |---> MC/DC Decision Region (LL:8) to (LL:14) | | Number of Conditions: 2 | Condition C1 --> (LL:8) @@ -199,7 +199,7 @@ LL| 3| if b && c { ^2 ------------------ - |---> MC/DC Decision Region (LL:12) to (61:18) + |---> MC/DC Decision Region (LL:12) to (LL:18) | | Number of Conditions: 2 | Condition C1 --> (LL:12) From 29e0881ed38d4c2f52a654a0206e9f665c83c200 Mon Sep 17 00:00:00 2001 From: Deltalice Date: Thu, 4 Apr 2024 15:08:36 +0800 Subject: [PATCH 22/29] coverage. Adopt a nicer dereference style for BcbMappingKind --- .../rustc_mir_transform/src/coverage/mod.rs | 19 ++++++++----------- .../rustc_mir_transform/src/coverage/spans.rs | 10 +++++----- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 4ad5dc0232440..dfd5c000fd021 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -142,27 +142,24 @@ fn create_mappings<'tcx>( mappings.extend(coverage_spans.all_bcb_mappings().filter_map( |BcbMapping { kind: bcb_mapping_kind, span }| { - let kind = match bcb_mapping_kind { - BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(*bcb)), + let kind = match *bcb_mapping_kind { + BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(bcb)), BcbMappingKind::Branch { true_bcb, false_bcb, condition_info } => { if condition_info.condition_id == ConditionId::NONE { MappingKind::Branch { - true_term: term_for_bcb(*true_bcb), - false_term: term_for_bcb(*false_bcb), + true_term: term_for_bcb(true_bcb), + false_term: term_for_bcb(false_bcb), } } else { MappingKind::MCDCBranch { - true_term: term_for_bcb(*true_bcb), - false_term: term_for_bcb(*false_bcb), - mcdc_params: *condition_info, + true_term: term_for_bcb(true_bcb), + false_term: term_for_bcb(false_bcb), + mcdc_params: condition_info, } } } BcbMappingKind::Decision { bitmap_idx, conditions_num, .. } => { - MappingKind::MCDCDecision(DecisionInfo { - bitmap_idx: *bitmap_idx, - conditions_num: *conditions_num, - }) + MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, conditions_num }) } }; let code_region = make_code_region(source_map, file_name, *span, body_span)?; diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 5c8d884e1cb1d..020a3fd9129e4 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -101,18 +101,18 @@ pub(super) fn generate_coverage_spans( }; let mut test_vector_bitmap_bytes = 0; for BcbMapping { kind, span: _ } in &mappings { - match kind { - BcbMappingKind::Code(bcb) => insert(*bcb), + match *kind { + BcbMappingKind::Code(bcb) => insert(bcb), BcbMappingKind::Branch { true_bcb, false_bcb, .. } => { - insert(*true_bcb); - insert(*false_bcb); + insert(true_bcb); + insert(false_bcb); } BcbMappingKind::Decision { bitmap_idx, conditions_num, .. } => { // `bcb_has_mappings` is used for inject coverage counters // but they are not needed for decision BCBs. // While the length of test vector bitmap should be calculated here. test_vector_bitmap_bytes = test_vector_bitmap_bytes - .max(bitmap_idx + (1_u32 << *conditions_num as u32).div_ceil(8)); + .max(bitmap_idx + (1_u32 << conditions_num as u32).div_ceil(8)); } } } From 391ad511e526c441bf76b734edc2ecbb4c8c9ac3 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Sun, 7 Apr 2024 14:46:08 +0800 Subject: [PATCH 23/29] coverage. Adopt nicer code style --- .../src/coverageinfo/ffi.rs | 53 +++++++++++++------ .../src/coverageinfo/mod.rs | 26 ++++----- .../llvm-wrapper/CoverageMappingWrapper.cpp | 37 ++++++++++--- compiler/rustc_middle/src/mir/coverage.rs | 2 +- .../rustc_mir_transform/src/coverage/mod.rs | 6 +-- compiler/rustc_session/src/options.rs | 7 ++- .../src/compiler-flags/coverage-options.md | 2 +- 7 files changed, 88 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index 50efd0ed45800..5e64d52c42e65 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -115,7 +115,7 @@ pub mod mcdc { /// Must match the layout of `LLVMRustMCDCDecisionParameters`. #[repr(C)] - #[derive(Clone, Copy, Debug)] + #[derive(Clone, Copy, Debug, Default)] pub struct DecisionParameters { bitmap_idx: u32, conditions_num: u16, @@ -126,19 +126,47 @@ pub mod mcdc { /// Must match the layout of `LLVMRustMCDCBranchParameters`. #[repr(C)] - #[derive(Clone, Copy, Debug)] + #[derive(Clone, Copy, Debug, Default)] pub struct BranchParameters { condition_id: LLVMConditionId, condition_ids: [LLVMConditionId; 2], } - /// Same layout with `LLVMRustMCDCParameters` #[repr(C, u8)] - #[derive(Clone, Copy, Debug)] - pub enum Parameters { + pub enum ParameterTag { None, - Decision(DecisionParameters), - Branch(BranchParameters), + Decision, + Branch, + } + /// Same layout with `LLVMRustMCDCParameters` + #[repr(C)] + #[derive(Clone, Copy, Debug)] + pub struct Parameters { + tag: ParameterTag, + decision_params: DecisionParameters, + branch_params: BranchParameters, + } + + impl Parameters { + pub fn none() -> Self { + Self { + tag: ParameterTag::None, + decision_params: Default::default(), + branch_params: Default::default(), + } + } + pub fn decision(decision_params: DecisionParameters) -> Self { + Self { tag: ParameterTag::Decision, decision_params, branch_params: Default::default() } + } + pub fn branch(branch_params: BranchParameters) -> Self { + Self { tag: ParameterTag::Branch, decision_params: Default, branch_params } + } + } + + impl Default for Parameters { + fn default() -> Self { + Self::none() + } } impl From for BranchParameters { @@ -287,14 +315,9 @@ impl CounterMappingRegion { end_line: u32, end_col: u32, ) -> Self { - let mcdc_params = condition_info - .map(mcdc::BranchParameters::from) - .map(mcdc::Parameters::Branch) - .unwrap_or(mcdc::Parameters::None); - let kind = match mcdc_params { - mcdc::Parameters::None => RegionKind::BranchRegion, - mcdc::Parameters::Branch(_) => RegionKind::MCDCBranchRegion, - _ => unreachable!("invalid mcdc params for branch"), + let (kind, mcdc_params) = match condition_info { + Some(info) => (RegionKind::MCDCBranchRegion, mcdc::Parameters::branch(info.into())), + None => (RegionKind::BranchRegion, Default::default()), }; Self { counter, diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 74a0123791651..4426fa3930f2e 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -188,20 +188,20 @@ fn ensure_mcdc_parameters<'ll, 'tcx>( instance: Instance<'tcx>, function_coverage_info: &FunctionCoverageInfo, ) { - if bx - .coverage_context() - .is_some_and(|cx| !cx.mcdc_condition_bitmap_map.borrow().contains_key(&instance)) - { - let fn_name = bx.get_pgo_func_name_var(instance); - let hash = bx.const_u64(function_coverage_info.function_source_hash); - let bitmap_bytes = bx.const_u32(function_coverage_info.mcdc_bitmap_bytes); - let cond_bitmap = bx.mcdc_parameters(fn_name, hash, bitmap_bytes); - bx.coverage_context() - .unwrap() - .mcdc_condition_bitmap_map - .borrow_mut() - .insert(instance, cond_bitmap); + let Some(cx) = bx.coverage_context() else { return }; + if cx.mcdc_condition_bitmap_map.borrow().contains_key(&instance) { + return; } + + let fn_name = bx.get_pgo_func_name_var(instance); + let hash = bx.const_u64(function_coverage_info.function_source_hash); + let bitmap_bytes = bx.const_u32(function_coverage_info.mcdc_bitmap_bytes); + let cond_bitmap = bx.mcdc_parameters(fn_name, hash, bitmap_bytes); + bx.coverage_context() + .expect("already checked above") + .mcdc_condition_bitmap_map + .borrow_mut() + .insert(instance, cond_bitmap); } /// Calls llvm::createPGOFuncNameVar() with the given function instance's diff --git a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp index 08e860ded1d83..da22ceb2d50c2 100644 --- a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp @@ -82,16 +82,18 @@ struct LLVMRustMCDCBranchParameters { int16_t ConditionIDs[2]; }; -union LLVMRustMCDCParametersPayload { - LLVMRustMCDCDecisionParameters DecisionParameters; - LLVMRustMCDCBranchParameters BranchParameters; -}; - struct LLVMRustMCDCParameters { LLVMRustMCDCParametersTag Tag; - LLVMRustMCDCParametersPayload Payload; + LLVMRustMCDCDecisionParameters DecisionParameters; + LLVMRustMCDCBranchParameters BranchParameters; }; +// LLVM representations for `MCDCParameters` evolved from LLVM 18 to 19. +// Look at representations in 18 +// https://github.com/rust-lang/llvm-project/blob/66a2881a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L253-L263 +// and representations in 19 +// https://github.com/llvm/llvm-project/blob/843cc474faefad1d639f4c44c1cf3ad7dbda76c8/llvm/include/llvm/ProfileData/Coverage/MCDCTypes.h +#if LLVM_VERSION_GE(18, 0) && LLVM_VERSION_LT(19, 0) static coverage::CounterMappingRegion::MCDCParameters fromRust(LLVMRustMCDCParameters Params) { auto parameter = coverage::CounterMappingRegion::MCDCParameters{}; @@ -117,6 +119,27 @@ fromRust(LLVMRustMCDCParameters Params) { } report_fatal_error("Bad LLVMRustMCDCParametersTag!"); } +#elif LLVM_VERSION_GE(19, 0) +static coverage::mcdc::MCDCParameters fromRust(LLVMRustMCDCParameters Params) { + switch (Params.Tag) { + case LLVMRustMCDCParametersTag::None: + return coverage::mcdc::MCDCParameters; + case LLVMRustMCDCParametersTag::Decision: + return coverage::mcdc::DecisionParameters( + Params.Payload.DecisionParameters.BitmapIdx, + Params.Payload.DecisionParameters.NumConditions); + case LLVMRustMCDCParametersTag::Branch: + return coverage::mcdc::BranchParameters( + static_cast( + Params.Payload.BranchParameters.ConditionID), + {static_cast( + Params.Payload.BranchParameters.ConditionIDs[0]), + static_cast( + Params.Payload.BranchParameters.ConditionIDs[1])}); + } + report_fatal_error("Bad LLVMRustMCDCParametersTag!"); +} +#endif // FFI equivalent of struct `llvm::coverage::CounterMappingRegion` // https://github.com/rust-lang/llvm-project/blob/ea6fa9c2/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L211-L304 @@ -191,7 +214,7 @@ extern "C" void LLVMRustCoverageWriteMappingToBuffer( RustMappingRegions, NumMappingRegions)) { MappingRegions.emplace_back( fromRust(Region.Count), fromRust(Region.FalseCount), -#if LLVM_VERSION_GE(18, 0) && LLVM_VERSION_LT(19, 0) +#if LLVM_VERSION_GE(18, 0) fromRust(Region.MCDCParameters), #endif Region.FileID, Region.ExpandedFileID, // File IDs, then region info. diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 14fcf7af38882..821708ea0fcf3 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -225,7 +225,7 @@ impl MappingKind { let two = |a, b| Some(a).into_iter().chain(Some(b)); match *self { Self::Code(term) => one(term), - Self::Branch { true_term, false_term, .. } => two(true_term, false_term), + Self::Branch { true_term, false_term } => two(true_term, false_term), Self::MCDCBranch { true_term, false_term, .. } => two(true_term, false_term), Self::MCDCDecision(_) => zero(), } diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index dfd5c000fd021..3fd0d7c5c3f8a 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -233,12 +233,10 @@ fn inject_mcdc_statements<'tcx>( return; } - // Inject test vector update first because inject statement always inject new statement at head. + // Inject test vector update first because `inject_statement` always insert new statement at head. for (end_bcbs, bitmap_idx) in coverage_spans.all_bcb_mappings().filter_map(|mapping| match &mapping.kind { - BcbMappingKind::Decision { end_bcbs: end_bcb, bitmap_idx, .. } => { - Some((end_bcb, *bitmap_idx)) - } + BcbMappingKind::Decision { end_bcbs, bitmap_idx, .. } => Some((end_bcbs, *bitmap_idx)), _ => None, }) { diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index c791532a16e1c..fff5ac05ef0b2 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -947,13 +947,12 @@ mod parse { match option { "no-branch" => { slot.branch = false; + slot.mcdc = false; } - "branch" => { - slot.branch = true; - } + "branch" => slot.branch = true, "mcdc" => { slot.branch = true; - slot.mcdc = true + slot.mcdc = true; } _ => return false, } diff --git a/src/doc/unstable-book/src/compiler-flags/coverage-options.md b/src/doc/unstable-book/src/compiler-flags/coverage-options.md index 7972e73119bfe..5e192d9aca13e 100644 --- a/src/doc/unstable-book/src/compiler-flags/coverage-options.md +++ b/src/doc/unstable-book/src/compiler-flags/coverage-options.md @@ -5,4 +5,4 @@ This option controls details of the coverage instrumentation performed by Multiple options can be passed, separated by commas. Valid options are: -- `no-branch`, `branch` or `mcdc`: `branch` enables branch coverage instrumentation and `mcdc` further enables modified condition/decision coverage instrumentation. `no-branch` disables branch coverage instrumentation, which is same as do not pass `branch` or `mcdc` +- `no-branch`, `branch` or `mcdc`: `branch` enables branch coverage instrumentation and `mcdc` further enables modified condition/decision coverage instrumentation. `no-branch` disables branch coverage instrumentation as well as mcdc instrumentation, which is same as do not pass `branch` or `mcdc`. From 3024f22114d6dcf072b11859b378d27888b94667 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Sun, 7 Apr 2024 16:11:36 +0800 Subject: [PATCH 24/29] coverage. Flatten ffi for MCDCParameter --- .../src/coverageinfo/ffi.rs | 29 +++++++--------- .../src/coverageinfo/mod.rs | 2 +- .../llvm-wrapper/CoverageMappingWrapper.cpp | 33 ++++++++++--------- 3 files changed, 30 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index 5e64d52c42e65..aa561f32c0e69 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -132,11 +132,12 @@ pub mod mcdc { condition_ids: [LLVMConditionId; 2], } - #[repr(C, u8)] + #[repr(C)] + #[derive(Clone, Copy, Debug)] pub enum ParameterTag { - None, - Decision, - Branch, + None = 0, + Decision = 1, + Branch = 2, } /// Same layout with `LLVMRustMCDCParameters` #[repr(C)] @@ -159,13 +160,7 @@ pub mod mcdc { Self { tag: ParameterTag::Decision, decision_params, branch_params: Default::default() } } pub fn branch(branch_params: BranchParameters) -> Self { - Self { tag: ParameterTag::Branch, decision_params: Default, branch_params } - } - } - - impl Default for Parameters { - fn default() -> Self { - Self::none() + Self { tag: ParameterTag::Branch, decision_params: Default::default(), branch_params } } } @@ -294,7 +289,7 @@ impl CounterMappingRegion { Self { counter, false_counter: Counter::ZERO, - mcdc_params: mcdc::Parameters::None, + mcdc_params: mcdc::Parameters::none(), file_id, expanded_file_id: 0, start_line, @@ -317,7 +312,7 @@ impl CounterMappingRegion { ) -> Self { let (kind, mcdc_params) = match condition_info { Some(info) => (RegionKind::MCDCBranchRegion, mcdc::Parameters::branch(info.into())), - None => (RegionKind::BranchRegion, Default::default()), + None => (RegionKind::BranchRegion, mcdc::Parameters::none()), }; Self { counter, @@ -341,7 +336,7 @@ impl CounterMappingRegion { end_line: u32, end_col: u32, ) -> Self { - let mcdc_params = mcdc::Parameters::Decision(decision_info.into()); + let mcdc_params = mcdc::Parameters::decision(decision_info.into()); Self { counter: Counter::ZERO, @@ -371,7 +366,7 @@ impl CounterMappingRegion { Self { counter: Counter::ZERO, false_counter: Counter::ZERO, - mcdc_params: mcdc::Parameters::None, + mcdc_params: mcdc::Parameters::none(), file_id, expanded_file_id, start_line, @@ -395,7 +390,7 @@ impl CounterMappingRegion { Self { counter: Counter::ZERO, false_counter: Counter::ZERO, - mcdc_params: mcdc::Parameters::None, + mcdc_params: mcdc::Parameters::none(), file_id, expanded_file_id: 0, start_line, @@ -420,7 +415,7 @@ impl CounterMappingRegion { Self { counter, false_counter: Counter::ZERO, - mcdc_params: mcdc::Parameters::None, + mcdc_params: mcdc::Parameters::none(), file_id, expanded_file_id: 0, start_line, diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 4426fa3930f2e..d9d940a84ae4e 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -192,7 +192,7 @@ fn ensure_mcdc_parameters<'ll, 'tcx>( if cx.mcdc_condition_bitmap_map.borrow().contains_key(&instance) { return; } - + let fn_name = bx.get_pgo_func_name_var(instance); let hash = bx.const_u64(function_coverage_info.function_source_hash); let bitmap_bytes = bx.const_u32(function_coverage_info.mcdc_bitmap_bytes); diff --git a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp index da22ceb2d50c2..41a42700b782b 100644 --- a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp @@ -66,7 +66,7 @@ fromRust(LLVMRustCounterMappingRegionKind Kind) { report_fatal_error("Bad LLVMRustCounterMappingRegionKind!"); } -enum class LLVMRustMCDCParametersTag : uint8_t { +enum LLVMRustMCDCParametersTag { None = 0, Decision = 1, Branch = 2, @@ -102,40 +102,40 @@ fromRust(LLVMRustMCDCParameters Params) { return parameter; case LLVMRustMCDCParametersTag::Decision: parameter.BitmapIdx = - static_cast(Params.Payload.DecisionParameters.BitmapIdx), + static_cast(Params.DecisionParameters.BitmapIdx), parameter.NumConditions = - static_cast(Params.Payload.DecisionParameters.NumConditions); + static_cast(Params.DecisionParameters.NumConditions); return parameter; case LLVMRustMCDCParametersTag::Branch: parameter.ID = static_cast( - Params.Payload.BranchParameters.ConditionID), + Params.BranchParameters.ConditionID), parameter.FalseID = static_cast( - Params.Payload.BranchParameters.ConditionIDs[0]), + Params.BranchParameters.ConditionIDs[0]), parameter.TrueID = static_cast( - Params.Payload.BranchParameters.ConditionIDs[1]); + Params.BranchParameters.ConditionIDs[1]); return parameter; } report_fatal_error("Bad LLVMRustMCDCParametersTag!"); } #elif LLVM_VERSION_GE(19, 0) -static coverage::mcdc::MCDCParameters fromRust(LLVMRustMCDCParameters Params) { +static coverage::mcdc::Parameters fromRust(LLVMRustMCDCParameters Params) { switch (Params.Tag) { case LLVMRustMCDCParametersTag::None: - return coverage::mcdc::MCDCParameters; + return std::monostate(); case LLVMRustMCDCParametersTag::Decision: return coverage::mcdc::DecisionParameters( - Params.Payload.DecisionParameters.BitmapIdx, - Params.Payload.DecisionParameters.NumConditions); + Params.DecisionParameters.BitmapIdx, + Params.DecisionParameters.NumConditions); case LLVMRustMCDCParametersTag::Branch: return coverage::mcdc::BranchParameters( static_cast( - Params.Payload.BranchParameters.ConditionID), - {static_cast( - Params.Payload.BranchParameters.ConditionIDs[0]), - static_cast( - Params.Payload.BranchParameters.ConditionIDs[1])}); + Params.BranchParameters.ConditionID), + {static_cast( + Params.BranchParameters.ConditionIDs[0]), + static_cast( + Params.BranchParameters.ConditionIDs[1])}); } report_fatal_error("Bad LLVMRustMCDCParametersTag!"); } @@ -214,7 +214,8 @@ extern "C" void LLVMRustCoverageWriteMappingToBuffer( RustMappingRegions, NumMappingRegions)) { MappingRegions.emplace_back( fromRust(Region.Count), fromRust(Region.FalseCount), -#if LLVM_VERSION_GE(18, 0) +#if LLVM_VERSION_GE(18, 0) && LLVM_VERSION_LT(19, 0) + // LLVM 19 may move this argument to last. fromRust(Region.MCDCParameters), #endif Region.FileID, Region.ExpandedFileID, // File IDs, then region info. From 2c054b0855fef848b08a4c3973d275710a2b43e7 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Sun, 7 Apr 2024 19:15:35 +0800 Subject: [PATCH 25/29] coverage. Split structures for mcdc branches and normal branches --- .../src/coverageinfo/ffi.rs | 56 ++++++++---- compiler/rustc_middle/src/mir/coverage.rs | 17 +++- compiler/rustc_middle/src/mir/pretty.rs | 42 +++++---- .../rustc_mir_build/src/build/coverageinfo.rs | 88 ++++++++++++------- .../rustc_mir_transform/src/coverage/mod.rs | 34 ++++--- .../rustc_mir_transform/src/coverage/query.rs | 2 +- .../rustc_mir_transform/src/coverage/spans.rs | 13 +-- .../src/coverage/spans/from_mir.rs | 39 +++++--- 8 files changed, 185 insertions(+), 106 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index aa561f32c0e69..12a846a49ec16 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -250,23 +250,24 @@ impl CounterMappingRegion { MappingKind::Branch { true_term, false_term } => Self::branch_region( Counter::from_term(true_term), Counter::from_term(false_term), - None, - local_file_id, - start_line, - start_col, - end_line, - end_col, - ), - MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => Self::branch_region( - Counter::from_term(true_term), - Counter::from_term(false_term), - Some(mcdc_params), local_file_id, start_line, start_col, end_line, end_col, ), + MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => { + Self::mcdc_branch_region( + Counter::from_term(true_term), + Counter::from_term(false_term), + mcdc_params, + local_file_id, + start_line, + start_col, + end_line, + end_col, + ) + } MappingKind::MCDCDecision(decision_info) => Self::decision_region( decision_info, local_file_id, @@ -303,28 +304,47 @@ impl CounterMappingRegion { pub(crate) fn branch_region( counter: Counter, false_counter: Counter, - condition_info: Option, file_id: u32, start_line: u32, start_col: u32, end_line: u32, end_col: u32, ) -> Self { - let (kind, mcdc_params) = match condition_info { - Some(info) => (RegionKind::MCDCBranchRegion, mcdc::Parameters::branch(info.into())), - None => (RegionKind::BranchRegion, mcdc::Parameters::none()), - }; Self { counter, false_counter, - mcdc_params, + mcdc_params: mcdc::Parameters::none(), + file_id, + expanded_file_id: 0, + start_line, + start_col, + end_line, + end_col, + kind: RegionKind::BranchRegion, + } + } + + pub(crate) fn mcdc_branch_region( + counter: Counter, + false_counter: Counter, + condition_info: ConditionInfo, + file_id: u32, + start_line: u32, + start_col: u32, + end_line: u32, + end_col: u32, + ) -> Self { + Self { + counter, + false_counter, + mcdc_params: mcdc::Parameters::branch(condition_info.into()), file_id, expanded_file_id: 0, start_line, start_col, end_line, end_col, - kind, + kind: RegionKind::MCDCBranchRegion, } } diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 821708ea0fcf3..d900aac0dc03b 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -278,14 +278,14 @@ pub struct BranchInfo { /// data structures without having to scan the entire body first. pub num_block_markers: usize, pub branch_spans: Vec, - pub decision_spans: Vec, + pub mcdc_branch_spans: Vec, + pub mcdc_decision_spans: Vec, } #[derive(Clone, Debug)] #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub struct BranchSpan { pub span: Span, - pub condition_info: ConditionInfo, pub true_marker: BlockMarkerId, pub false_marker: BlockMarkerId, } @@ -308,6 +308,15 @@ impl Default for ConditionInfo { } } +#[derive(Clone, Debug)] +#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] +pub struct MCDCBranchSpan { + pub span: Span, + pub condition_info: ConditionInfo, + pub true_marker: BlockMarkerId, + pub false_marker: BlockMarkerId, +} + #[derive(Copy, Clone, Debug)] #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub struct DecisionInfo { @@ -317,8 +326,8 @@ pub struct DecisionInfo { #[derive(Clone, Debug)] #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] -pub struct DecisionSpan { +pub struct MCDCDecisionSpan { pub span: Span, pub conditions_num: usize, - pub end_marker: Vec, + pub end_markers: Vec, } diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index faa70606f186b..c18b1d350e8a7 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -475,23 +475,35 @@ fn write_coverage_branch_info( branch_info: &coverage::BranchInfo, w: &mut dyn io::Write, ) -> io::Result<()> { - let coverage::BranchInfo { branch_spans, .. } = branch_info; + let coverage::BranchInfo { branch_spans, mcdc_branch_spans, mcdc_decision_spans, .. } = + branch_info; - for coverage::BranchSpan { span, true_marker, false_marker, condition_info } in branch_spans { - if condition_info.condition_id == coverage::ConditionId::NONE { - writeln!( - w, - "{INDENT}coverage branch {{ true: {true_marker:?}, false: {false_marker:?} }} => {span:?}", - )?; - } else { - let id = condition_info.condition_id; - writeln!( - w, - "{INDENT}coverage branch {{ condition_id: {id:?}, true: {true_marker:?}, false: {false_marker:?} }} => {span:?}", - )?; - } + for coverage::BranchSpan { span, true_marker, false_marker } in branch_spans { + writeln!( + w, + "{INDENT}coverage branch {{ true: {true_marker:?}, false: {false_marker:?} }} => {span:?}", + )?; } - if !branch_spans.is_empty() { + + for coverage::MCDCBranchSpan { span, condition_info, true_marker, false_marker } in + mcdc_branch_spans + { + writeln!( + w, + "{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_marker:?}, false: {false_marker:?} }} => {span:?}", + condition_info.condition_id + )?; + } + + for coverage::MCDCDecisionSpan { span, conditions_num, end_markers } in mcdc_decision_spans { + writeln!( + w, + "{INDENT}coverage mcdc decision {{ conditions_num: {conditions_num:?}, end: {end_markers:?} }} => {span:?}" + )?; + } + + if !branch_spans.is_empty() || !mcdc_branch_spans.is_empty() || !mcdc_decision_spans.is_empty() + { writeln!(w)?; } diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index 4265d9eeaebd3..7b0dcc603ea81 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -4,7 +4,8 @@ use std::collections::VecDeque; use rustc_data_structures::fx::FxHashMap; use rustc_middle::mir::coverage::{ - BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageKind, DecisionSpan, + BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageKind, MCDCBranchSpan, + MCDCDecisionSpan, }; use rustc_middle::mir::{self, BasicBlock, UnOp}; use rustc_middle::thir::{ExprId, ExprKind, LogicalOp, Thir}; @@ -21,7 +22,8 @@ pub(crate) struct BranchInfoBuilder { num_block_markers: usize, branch_spans: Vec, - decision_spans: Vec, + mcdc_branch_spans: Vec, + mcdc_decision_spans: Vec, mcdc_state: Option, } @@ -44,7 +46,8 @@ impl BranchInfoBuilder { nots: FxHashMap::default(), num_block_markers: 0, branch_spans: vec![], - decision_spans: vec![], + mcdc_branch_spans: vec![], + mcdc_decision_spans: vec![], mcdc_state: MCDCState::new_if_enabled(tcx), }) } else { @@ -103,10 +106,8 @@ impl BranchInfoBuilder { tcx: TyCtxt<'_>, true_marker: BlockMarkerId, false_marker: BlockMarkerId, - ) -> ConditionInfo { - let Some(mcdc_state) = self.mcdc_state.as_mut() else { - return ConditionInfo::default(); - }; + ) -> Option { + let mcdc_state = self.mcdc_state.as_mut()?; let (mut condition_info, decision_result) = mcdc_state.take_condition(true_marker, false_marker); if let Some(decision) = decision_result { @@ -115,17 +116,22 @@ impl BranchInfoBuilder { unreachable!("Decision with no condition is not expected"); } 1..=MAX_CONDITIONS_NUM_IN_DECISION => { - self.decision_spans.push(decision); + self.mcdc_decision_spans.push(decision); } _ => { // Do not generate mcdc mappings and statements for decisions with too many conditions. - for branch in - self.branch_spans.iter_mut().rev().take(decision.conditions_num - 1) - { - branch.condition_info = ConditionInfo::default(); - } + let rebase_idx = self.mcdc_branch_spans.len() - decision.conditions_num + 1; + let to_normal_branches = self.mcdc_branch_spans.split_off(rebase_idx); + self.branch_spans.extend(to_normal_branches.into_iter().map( + |MCDCBranchSpan { span, true_marker, false_marker, .. }| BranchSpan { + span, + true_marker, + false_marker, + }, + )); + // ConditionInfo of this branch shall also be reset. - condition_info = ConditionInfo::default(); + condition_info = None; tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit { span: decision.span, @@ -145,7 +151,14 @@ impl BranchInfoBuilder { } pub(crate) fn into_done(self) -> Option> { - let Self { nots: _, num_block_markers, branch_spans, decision_spans, .. } = self; + let Self { + nots: _, + num_block_markers, + branch_spans, + mcdc_branch_spans, + mcdc_decision_spans, + .. + } = self; if num_block_markers == 0 { assert!(branch_spans.is_empty()); @@ -155,7 +168,8 @@ impl BranchInfoBuilder { Some(Box::new(mir::coverage::BranchInfo { num_block_markers, branch_spans, - decision_spans, + mcdc_branch_spans, + mcdc_decision_spans, })) } } @@ -168,7 +182,7 @@ const MAX_CONDITIONS_NUM_IN_DECISION: usize = 6; struct MCDCState { /// To construct condition evaluation tree. decision_stack: VecDeque, - processing_decision: Option, + processing_decision: Option, } impl MCDCState { @@ -224,10 +238,10 @@ impl MCDCState { decision.span = decision.span.to(span); decision } - None => self.processing_decision.insert(DecisionSpan { + None => self.processing_decision.insert(MCDCDecisionSpan { span, conditions_num: 0, - end_marker: vec![], + end_markers: vec![], }), }; @@ -279,24 +293,24 @@ impl MCDCState { &mut self, true_marker: BlockMarkerId, false_marker: BlockMarkerId, - ) -> (ConditionInfo, Option) { + ) -> (Option, Option) { let Some(condition_info) = self.decision_stack.pop_back() else { - return (ConditionInfo::default(), None); + return (None, None); }; let Some(decision) = self.processing_decision.as_mut() else { bug!("Processing decision should have been created before any conditions are taken"); }; if condition_info.true_next_id == ConditionId::NONE { - decision.end_marker.push(true_marker); + decision.end_markers.push(true_marker); } if condition_info.false_next_id == ConditionId::NONE { - decision.end_marker.push(false_marker); + decision.end_markers.push(false_marker); } if self.decision_stack.is_empty() { - (condition_info, self.processing_decision.take()) + (Some(condition_info), self.processing_decision.take()) } else { - (condition_info, None) + (Some(condition_info), None) } } } @@ -341,14 +355,22 @@ impl Builder<'_, '_> { let true_marker = inject_branch_marker(then_block); let false_marker = inject_branch_marker(else_block); - let condition_info = branch_info.fetch_condition_info(self.tcx, true_marker, false_marker); - - branch_info.branch_spans.push(BranchSpan { - span: source_info.span, - condition_info, - true_marker, - false_marker, - }); + if let Some(condition_info) = + branch_info.fetch_condition_info(self.tcx, true_marker, false_marker) + { + branch_info.mcdc_branch_spans.push(MCDCBranchSpan { + span: source_info.span, + condition_info, + true_marker, + false_marker, + }); + } else { + branch_info.branch_spans.push(BranchSpan { + span: source_info.span, + true_marker, + false_marker, + }); + } } pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp, span: Span) { diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 3fd0d7c5c3f8a..8736e7308957b 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -144,21 +144,18 @@ fn create_mappings<'tcx>( |BcbMapping { kind: bcb_mapping_kind, span }| { let kind = match *bcb_mapping_kind { BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(bcb)), - BcbMappingKind::Branch { true_bcb, false_bcb, condition_info } => { - if condition_info.condition_id == ConditionId::NONE { - MappingKind::Branch { - true_term: term_for_bcb(true_bcb), - false_term: term_for_bcb(false_bcb), - } - } else { - MappingKind::MCDCBranch { - true_term: term_for_bcb(true_bcb), - false_term: term_for_bcb(false_bcb), - mcdc_params: condition_info, - } + BcbMappingKind::Branch { true_bcb, false_bcb } => MappingKind::Branch { + true_term: term_for_bcb(true_bcb), + false_term: term_for_bcb(false_bcb), + }, + BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info } => { + MappingKind::MCDCBranch { + true_term: term_for_bcb(true_bcb), + false_term: term_for_bcb(false_bcb), + mcdc_params: condition_info, } } - BcbMappingKind::Decision { bitmap_idx, conditions_num, .. } => { + BcbMappingKind::MCDCDecision { bitmap_idx, conditions_num, .. } => { MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, conditions_num }) } }; @@ -236,7 +233,9 @@ fn inject_mcdc_statements<'tcx>( // Inject test vector update first because `inject_statement` always insert new statement at head. for (end_bcbs, bitmap_idx) in coverage_spans.all_bcb_mappings().filter_map(|mapping| match &mapping.kind { - BcbMappingKind::Decision { end_bcbs, bitmap_idx, .. } => Some((end_bcbs, *bitmap_idx)), + BcbMappingKind::MCDCDecision { end_bcbs, bitmap_idx, .. } => { + Some((end_bcbs, *bitmap_idx)) + } _ => None, }) { @@ -248,10 +247,9 @@ fn inject_mcdc_statements<'tcx>( for (true_bcb, false_bcb, condition_id) in coverage_spans.all_bcb_mappings().filter_map(|mapping| match mapping.kind { - BcbMappingKind::Branch { true_bcb, false_bcb, condition_info } => (condition_info - .condition_id - != ConditionId::NONE) - .then_some((true_bcb, false_bcb, condition_info.condition_id)), + BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info } => { + Some((true_bcb, false_bcb, condition_info.condition_id)) + } _ => None, }) { diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index 8bb2b4d17d687..f77ee63d02c20 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -65,7 +65,7 @@ fn coverage_ids_info<'tcx>( .coverage_branch_info .as_deref() .map(|info| { - info.decision_spans + info.mcdc_decision_spans .iter() .fold(0, |acc, decision| acc + (1_u32 << decision.conditions_num).div_ceil(8)) }) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 020a3fd9129e4..a96afed82f105 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -17,13 +17,15 @@ pub(super) enum BcbMappingKind { /// Associates an ordinary executable code span with its corresponding BCB. Code(BasicCoverageBlock), /// Associates a branch span with BCBs for its true and false arms. - Branch { + Branch { true_bcb: BasicCoverageBlock, false_bcb: BasicCoverageBlock }, + /// Associates a mcdc branch span with condition info besides fields for normal branch. + MCDCBranch { true_bcb: BasicCoverageBlock, false_bcb: BasicCoverageBlock, condition_info: ConditionInfo, }, - /// Associates a decision with its join BCB. - Decision { end_bcbs: BTreeSet, bitmap_idx: u32, conditions_num: u16 }, + /// Associates a mcdc decision with its join BCB. + MCDCDecision { end_bcbs: BTreeSet, bitmap_idx: u32, conditions_num: u16 }, } #[derive(Debug)] @@ -103,11 +105,12 @@ pub(super) fn generate_coverage_spans( for BcbMapping { kind, span: _ } in &mappings { match *kind { BcbMappingKind::Code(bcb) => insert(bcb), - BcbMappingKind::Branch { true_bcb, false_bcb, .. } => { + BcbMappingKind::Branch { true_bcb, false_bcb } + | BcbMappingKind::MCDCBranch { true_bcb, false_bcb, .. } => { insert(true_bcb); insert(false_bcb); } - BcbMappingKind::Decision { bitmap_idx, conditions_num, .. } => { + BcbMappingKind::MCDCDecision { bitmap_idx, conditions_num, .. } => { // `bcb_has_mappings` is used for inject coverage counters // but they are not needed for decision BCBs. // While the length of test vector bitmap should be calculated here. diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 195d4ad6b08a9..b9919a2ae884e 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -1,7 +1,9 @@ use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashSet; use rustc_index::IndexVec; -use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind, DecisionSpan}; +use rustc_middle::mir::coverage::{ + BlockMarkerId, BranchSpan, CoverageKind, MCDCBranchSpan, MCDCDecisionSpan, +}; use rustc_middle::mir::{ self, AggregateKind, BasicBlock, FakeReadCause, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, @@ -390,8 +392,8 @@ pub(super) fn extract_branch_mappings( let bcb_from_marker = |marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?); - let condition_filter_map = - |&BranchSpan { span: raw_span, condition_info, true_marker, false_marker }| { + let check_branch_bcb = + |raw_span: Span, true_marker: BlockMarkerId, false_marker: BlockMarkerId| { // For now, ignore any branch span that was introduced by // expansion. This makes things like assert macros less noisy. if !raw_span.ctxt().outer_expn_data().is_root() { @@ -401,20 +403,32 @@ pub(super) fn extract_branch_mappings( let true_bcb = bcb_from_marker(true_marker)?; let false_bcb = bcb_from_marker(false_marker)?; + Some((span, true_bcb, false_bcb)) + }; + + let branch_filter_map = |&BranchSpan { span: raw_span, true_marker, false_marker }| { + check_branch_bcb(raw_span, true_marker, false_marker).map(|(span, true_bcb, false_bcb)| { + BcbMapping { kind: BcbMappingKind::Branch { true_bcb, false_bcb }, span } + }) + }; - Some(BcbMapping { - kind: BcbMappingKind::Branch { true_bcb, false_bcb, condition_info }, - span, - }) + let mcdc_branch_filter_map = + |&MCDCBranchSpan { span: raw_span, true_marker, false_marker, condition_info }| { + check_branch_bcb(raw_span, true_marker, false_marker).map( + |(span, true_bcb, false_bcb)| BcbMapping { + kind: BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info }, + span, + }, + ) }; let mut next_bitmap_idx = 0; - let decision_filter_map = |decision: &DecisionSpan| { + let decision_filter_map = |decision: &MCDCDecisionSpan| { let (span, _) = unexpand_into_body_span_with_visible_macro(decision.span, body_span)?; let end_bcbs = decision - .end_marker + .end_markers .iter() .map(|&marker| bcb_from_marker(marker)) .collect::>()?; @@ -423,7 +437,7 @@ pub(super) fn extract_branch_mappings( next_bitmap_idx += (1_u32 << decision.conditions_num).div_ceil(8); Some(BcbMapping { - kind: BcbMappingKind::Decision { + kind: BcbMappingKind::MCDCDecision { end_bcbs, bitmap_idx, conditions_num: decision.conditions_num as u16, @@ -435,7 +449,8 @@ pub(super) fn extract_branch_mappings( branch_info .branch_spans .iter() - .filter_map(condition_filter_map) - .chain(branch_info.decision_spans.iter().filter_map(decision_filter_map)) + .filter_map(branch_filter_map) + .chain(branch_info.mcdc_branch_spans.iter().filter_map(mcdc_branch_filter_map)) + .chain(branch_info.mcdc_decision_spans.iter().filter_map(decision_filter_map)) .collect::>() } From 58f62e8225406d7380ff2eba3b808e3f0b7253ed Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Mon, 8 Apr 2024 10:59:32 +0800 Subject: [PATCH 26/29] coverage. Allow to be compiled on llvm-17 --- compiler/rustc_codegen_llvm/src/builder.rs | 8 +++++++ .../llvm-wrapper/CoverageMappingWrapper.cpp | 7 ++++++ .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 24 +++++++++++++++---- 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 6394637f3b231..b06a181e955e6 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -1246,6 +1246,8 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { ) -> &'ll Value { debug!("mcdc_parameters() with args ({:?}, {:?}, {:?})", fn_name, hash, bitmap_bytes); + assert!(llvm_util::get_version() >= (18, 0, 0), "MCDC intrinsics require LLVM 18 or later"); + let llfn = unsafe { llvm::LLVMRustGetInstrProfMCDCParametersIntrinsic(self.cx().llmod) }; let llty = self.cx.type_func( &[self.cx.type_ptr(), self.cx.type_i64(), self.cx.type_i32()], @@ -1290,6 +1292,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { "mcdc_tvbitmap_update() with args ({:?}, {:?}, {:?}, {:?}, {:?})", fn_name, hash, bitmap_bytes, bitmap_index, mcdc_temp ); + assert!(llvm_util::get_version() >= (18, 0, 0), "MCDC intrinsics require LLVM 18 or later"); let llfn = unsafe { llvm::LLVMRustGetInstrProfMCDCTVBitmapUpdateIntrinsic(self.cx().llmod) }; @@ -1326,6 +1329,11 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { mcdc_temp: Self::Value, bool_value: Self::Value, ) { + debug!( + "mcdc_condbitmap_update() with args ({:?}, {:?}, {:?}, {:?}, {:?})", + fn_name, hash, cond_loc, mcdc_temp, bool_value + ); + assert!(llvm_util::get_version() >= (18, 0, 0), "MCDC intrinsics require LLVM 18 or later"); let llfn = unsafe { llvm::LLVMRustGetInstrProfMCDCCondBitmapIntrinsic(self.cx().llmod) }; let llty = self.cx.type_func( &[ diff --git a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp index 41a42700b782b..e842f47f48c23 100644 --- a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp @@ -58,10 +58,17 @@ fromRust(LLVMRustCounterMappingRegionKind Kind) { return coverage::CounterMappingRegion::GapRegion; case LLVMRustCounterMappingRegionKind::BranchRegion: return coverage::CounterMappingRegion::BranchRegion; +#if LLVM_VERSION_GE(18, 0) case LLVMRustCounterMappingRegionKind::MCDCDecisionRegion: return coverage::CounterMappingRegion::MCDCDecisionRegion; case LLVMRustCounterMappingRegionKind::MCDCBranchRegion: return coverage::CounterMappingRegion::MCDCBranchRegion; +#else + case LLVMRustCounterMappingRegionKind::MCDCDecisionRegion: + break; + case LLVMRustCounterMappingRegionKind::MCDCBranchRegion: + break; +#endif } report_fatal_error("Bad LLVMRustCounterMappingRegionKind!"); } diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 11f9661eec754..b912a1ff244ad 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -23,6 +23,7 @@ #include "llvm/Bitcode/BitcodeWriter.h" #include "llvm/Support/Signals.h" +#include #include // for raw `write` in the bad-alloc handler @@ -1529,18 +1530,33 @@ extern "C" LLVMValueRef LLVMRustGetInstrProfIncrementIntrinsic(LLVMModuleRef M) } extern "C" LLVMValueRef LLVMRustGetInstrProfMCDCParametersIntrinsic(LLVMModuleRef M) { + assert(LLVM_VERSION_GE(18, 0)); +#if LLVM_VERSION_GE(18, 0) return wrap(llvm::Intrinsic::getDeclaration(unwrap(M), (llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_mcdc_parameters)); +#else // Just make the wrapper can be compiled + return LLVMRustGetInstrProfIncrementIntrinsic(M); +#endif } extern "C" LLVMValueRef LLVMRustGetInstrProfMCDCTVBitmapUpdateIntrinsic(LLVMModuleRef M) { - return wrap(llvm::Intrinsic::getDeclaration(unwrap(M), - (llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_mcdc_tvbitmap_update)); + assert(LLVM_VERSION_GE(18, 0)); +#if LLVM_VERSION_GE(18, 0) + return wrap(llvm::Intrinsic::getDeclaration( + unwrap(M), llvm::Intrinsic::instrprof_mcdc_tvbitmap_update)); +#else // Just make the wrapper can be compiled + return LLVMRustGetInstrProfIncrementIntrinsic(M); +#endif } extern "C" LLVMValueRef LLVMRustGetInstrProfMCDCCondBitmapIntrinsic(LLVMModuleRef M) { - return wrap(llvm::Intrinsic::getDeclaration(unwrap(M), - (llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_mcdc_condbitmap_update)); + assert(LLVM_VERSION_GE(18, 0)); +#if LLVM_VERSION_GE(18, 0) + return wrap(llvm::Intrinsic::getDeclaration( + unwrap(M), llvm::Intrinsic::instrprof_mcdc_condbitmap_update)); +#else // Just make the wrapper can be compiled + return LLVMRustGetInstrProfIncrementIntrinsic(M); +#endif } extern "C" LLVMValueRef LLVMRustBuildMemCpy(LLVMBuilderRef B, From 2788afeca4c3bccda30f5a0d959fba3996ac0d6d Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Mon, 8 Apr 2024 13:43:36 +0800 Subject: [PATCH 27/29] coverage. Gate mcdc tests by @min-llvm-version --- tests/coverage/mcdc_if.cov-map | 28 ++++++++++++++-------------- tests/coverage/mcdc_if.coverage | 1 + tests/coverage/mcdc_if.rs | 1 + 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/tests/coverage/mcdc_if.cov-map b/tests/coverage/mcdc_if.cov-map index 8ca6982a1967e..35a265684d2f6 100644 --- a/tests/coverage/mcdc_if.cov-map +++ b/tests/coverage/mcdc_if.cov-map @@ -1,5 +1,5 @@ Function name: mcdc_if::mcdc_check_a -Raw bytes (64): 0x[01, 01, 04, 01, 05, 09, 02, 0d, 0f, 09, 02, 08, 01, 0e, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 30, 0d, 09, 02, 00, 00, 00, 0d, 00, 0e, 0d, 00, 0f, 02, 06, 0f, 02, 0c, 02, 06, 0b, 03, 01, 00, 02] +Raw bytes (64): 0x[01, 01, 04, 01, 05, 09, 02, 0d, 0f, 09, 02, 08, 01, 0f, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 30, 0d, 09, 02, 00, 00, 00, 0d, 00, 0e, 0d, 00, 0f, 02, 06, 0f, 02, 0c, 02, 06, 0b, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 4 @@ -8,7 +8,7 @@ Number of expressions: 4 - expression 2 operands: lhs = Counter(3), rhs = Expression(3, Add) - expression 3 operands: lhs = Counter(2), rhs = Expression(0, Sub) Number of file 0 mappings: 8 -- Code(Counter(0)) at (prev + 14, 1) to (start + 1, 9) +- Code(Counter(0)) at (prev + 15, 1) to (start + 1, 9) - MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 1, 8) to (start + 0, 14) - MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 8) to (start + 0, 9) true = c1 @@ -24,7 +24,7 @@ Number of file 0 mappings: 8 = (c3 + (c2 + (c0 - c1))) Function name: mcdc_if::mcdc_check_b -Raw bytes (64): 0x[01, 01, 04, 01, 05, 09, 02, 0d, 0f, 09, 02, 08, 01, 16, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 30, 0d, 09, 02, 00, 00, 00, 0d, 00, 0e, 0d, 00, 0f, 02, 06, 0f, 02, 0c, 02, 06, 0b, 03, 01, 00, 02] +Raw bytes (64): 0x[01, 01, 04, 01, 05, 09, 02, 0d, 0f, 09, 02, 08, 01, 17, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 30, 0d, 09, 02, 00, 00, 00, 0d, 00, 0e, 0d, 00, 0f, 02, 06, 0f, 02, 0c, 02, 06, 0b, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 4 @@ -33,7 +33,7 @@ Number of expressions: 4 - expression 2 operands: lhs = Counter(3), rhs = Expression(3, Add) - expression 3 operands: lhs = Counter(2), rhs = Expression(0, Sub) Number of file 0 mappings: 8 -- Code(Counter(0)) at (prev + 22, 1) to (start + 1, 9) +- Code(Counter(0)) at (prev + 23, 1) to (start + 1, 9) - MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 1, 8) to (start + 0, 14) - MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 8) to (start + 0, 9) true = c1 @@ -49,7 +49,7 @@ Number of file 0 mappings: 8 = (c3 + (c2 + (c0 - c1))) Function name: mcdc_if::mcdc_check_both -Raw bytes (64): 0x[01, 01, 04, 01, 05, 09, 02, 0d, 0f, 09, 02, 08, 01, 1e, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 30, 0d, 09, 02, 00, 00, 00, 0d, 00, 0e, 0d, 00, 0f, 02, 06, 0f, 02, 0c, 02, 06, 0b, 03, 01, 00, 02] +Raw bytes (64): 0x[01, 01, 04, 01, 05, 09, 02, 0d, 0f, 09, 02, 08, 01, 1f, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 30, 0d, 09, 02, 00, 00, 00, 0d, 00, 0e, 0d, 00, 0f, 02, 06, 0f, 02, 0c, 02, 06, 0b, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 4 @@ -58,7 +58,7 @@ Number of expressions: 4 - expression 2 operands: lhs = Counter(3), rhs = Expression(3, Add) - expression 3 operands: lhs = Counter(2), rhs = Expression(0, Sub) Number of file 0 mappings: 8 -- Code(Counter(0)) at (prev + 30, 1) to (start + 1, 9) +- Code(Counter(0)) at (prev + 31, 1) to (start + 1, 9) - MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 1, 8) to (start + 0, 14) - MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 8) to (start + 0, 9) true = c1 @@ -74,7 +74,7 @@ Number of file 0 mappings: 8 = (c3 + (c2 + (c0 - c1))) Function name: mcdc_if::mcdc_check_neither -Raw bytes (64): 0x[01, 01, 04, 01, 05, 09, 02, 0d, 0f, 09, 02, 08, 01, 06, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 30, 0d, 09, 02, 00, 00, 00, 0d, 00, 0e, 0d, 00, 0f, 02, 06, 0f, 02, 0c, 02, 06, 0b, 03, 01, 00, 02] +Raw bytes (64): 0x[01, 01, 04, 01, 05, 09, 02, 0d, 0f, 09, 02, 08, 01, 07, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0d, 00, 0e, 30, 0d, 09, 02, 00, 00, 00, 0d, 00, 0e, 0d, 00, 0f, 02, 06, 0f, 02, 0c, 02, 06, 0b, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 4 @@ -83,7 +83,7 @@ Number of expressions: 4 - expression 2 operands: lhs = Counter(3), rhs = Expression(3, Add) - expression 3 operands: lhs = Counter(2), rhs = Expression(0, Sub) Number of file 0 mappings: 8 -- Code(Counter(0)) at (prev + 6, 1) to (start + 1, 9) +- Code(Counter(0)) at (prev + 7, 1) to (start + 1, 9) - MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 1, 8) to (start + 0, 14) - MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 8) to (start + 0, 9) true = c1 @@ -99,7 +99,7 @@ Number of file 0 mappings: 8 = (c3 + (c2 + (c0 - c1))) Function name: mcdc_if::mcdc_check_not_tree_decision -Raw bytes (87): 0x[01, 01, 08, 01, 05, 02, 09, 05, 09, 0d, 1e, 02, 09, 11, 1b, 0d, 1e, 02, 09, 0a, 01, 30, 01, 03, 0a, 28, 00, 03, 03, 08, 00, 15, 30, 05, 02, 01, 02, 03, 00, 09, 00, 0a, 02, 00, 0e, 00, 0f, 30, 09, 1e, 03, 02, 00, 00, 0e, 00, 0f, 0b, 00, 14, 00, 15, 30, 11, 0d, 02, 00, 00, 00, 14, 00, 15, 11, 00, 16, 02, 06, 1b, 02, 0c, 02, 06, 17, 03, 01, 00, 02] +Raw bytes (87): 0x[01, 01, 08, 01, 05, 02, 09, 05, 09, 0d, 1e, 02, 09, 11, 1b, 0d, 1e, 02, 09, 0a, 01, 31, 01, 03, 0a, 28, 00, 03, 03, 08, 00, 15, 30, 05, 02, 01, 02, 03, 00, 09, 00, 0a, 02, 00, 0e, 00, 0f, 30, 09, 1e, 03, 02, 00, 00, 0e, 00, 0f, 0b, 00, 14, 00, 15, 30, 11, 0d, 02, 00, 00, 00, 14, 00, 15, 11, 00, 16, 02, 06, 1b, 02, 0c, 02, 06, 17, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 8 @@ -112,7 +112,7 @@ Number of expressions: 8 - expression 6 operands: lhs = Counter(3), rhs = Expression(7, Sub) - expression 7 operands: lhs = Expression(0, Sub), rhs = Counter(2) Number of file 0 mappings: 10 -- Code(Counter(0)) at (prev + 48, 1) to (start + 3, 10) +- Code(Counter(0)) at (prev + 49, 1) to (start + 3, 10) - MCDCDecision { bitmap_idx: 0, conditions_num: 3 } at (prev + 3, 8) to (start + 0, 21) - MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 2, false_next_id: 3 } at (prev + 0, 9) to (start + 0, 10) true = c1 @@ -134,7 +134,7 @@ Number of file 0 mappings: 10 = (c4 + (c3 + ((c0 - c1) - c2))) Function name: mcdc_if::mcdc_check_tree_decision -Raw bytes (87): 0x[01, 01, 08, 01, 05, 05, 0d, 05, 0d, 0d, 11, 09, 02, 1b, 1f, 0d, 11, 09, 02, 0a, 01, 26, 01, 03, 09, 28, 00, 03, 03, 08, 00, 15, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0e, 00, 0f, 30, 0d, 0a, 02, 00, 03, 00, 0e, 00, 0f, 0a, 00, 13, 00, 14, 30, 11, 09, 03, 00, 00, 00, 13, 00, 14, 1b, 00, 16, 02, 06, 1f, 02, 0c, 02, 06, 17, 03, 01, 00, 02] +Raw bytes (87): 0x[01, 01, 08, 01, 05, 05, 0d, 05, 0d, 0d, 11, 09, 02, 1b, 1f, 0d, 11, 09, 02, 0a, 01, 27, 01, 03, 09, 28, 00, 03, 03, 08, 00, 15, 30, 05, 02, 01, 02, 00, 00, 08, 00, 09, 05, 00, 0e, 00, 0f, 30, 0d, 0a, 02, 00, 03, 00, 0e, 00, 0f, 0a, 00, 13, 00, 14, 30, 11, 09, 03, 00, 00, 00, 13, 00, 14, 1b, 00, 16, 02, 06, 1f, 02, 0c, 02, 06, 17, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 8 @@ -147,7 +147,7 @@ Number of expressions: 8 - expression 6 operands: lhs = Counter(3), rhs = Counter(4) - expression 7 operands: lhs = Counter(2), rhs = Expression(0, Sub) Number of file 0 mappings: 10 -- Code(Counter(0)) at (prev + 38, 1) to (start + 3, 9) +- Code(Counter(0)) at (prev + 39, 1) to (start + 3, 9) - MCDCDecision { bitmap_idx: 0, conditions_num: 3 } at (prev + 3, 8) to (start + 0, 21) - MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 8) to (start + 0, 9) true = c1 @@ -169,7 +169,7 @@ Number of file 0 mappings: 10 = ((c3 + c4) + (c2 + (c0 - c1))) Function name: mcdc_if::mcdc_nested_if -Raw bytes (124): 0x[01, 01, 0d, 01, 05, 02, 09, 05, 09, 1b, 15, 05, 09, 1b, 15, 05, 09, 11, 15, 02, 09, 2b, 32, 0d, 2f, 11, 15, 02, 09, 0e, 01, 3a, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 00, 02, 00, 08, 00, 09, 02, 00, 0d, 00, 0e, 30, 09, 32, 02, 00, 00, 00, 0d, 00, 0e, 1b, 01, 09, 01, 0d, 28, 01, 02, 01, 0c, 00, 12, 30, 16, 15, 01, 02, 00, 00, 0c, 00, 0d, 16, 00, 11, 00, 12, 30, 0d, 11, 02, 00, 00, 00, 11, 00, 12, 0d, 00, 13, 02, 0a, 2f, 02, 0a, 00, 0b, 32, 01, 0c, 02, 06, 27, 03, 01, 00, 02] +Raw bytes (124): 0x[01, 01, 0d, 01, 05, 02, 09, 05, 09, 1b, 15, 05, 09, 1b, 15, 05, 09, 11, 15, 02, 09, 2b, 32, 0d, 2f, 11, 15, 02, 09, 0e, 01, 3b, 01, 01, 09, 28, 00, 02, 01, 08, 00, 0e, 30, 05, 02, 01, 00, 02, 00, 08, 00, 09, 02, 00, 0d, 00, 0e, 30, 09, 32, 02, 00, 00, 00, 0d, 00, 0e, 1b, 01, 09, 01, 0d, 28, 01, 02, 01, 0c, 00, 12, 30, 16, 15, 01, 02, 00, 00, 0c, 00, 0d, 16, 00, 11, 00, 12, 30, 0d, 11, 02, 00, 00, 00, 11, 00, 12, 0d, 00, 13, 02, 0a, 2f, 02, 0a, 00, 0b, 32, 01, 0c, 02, 06, 27, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 13 @@ -187,7 +187,7 @@ Number of expressions: 13 - expression 11 operands: lhs = Counter(4), rhs = Counter(5) - expression 12 operands: lhs = Expression(0, Sub), rhs = Counter(2) Number of file 0 mappings: 14 -- Code(Counter(0)) at (prev + 58, 1) to (start + 1, 9) +- Code(Counter(0)) at (prev + 59, 1) to (start + 1, 9) - MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 1, 8) to (start + 0, 14) - MCDCBranch { true: Counter(1), false: Expression(0, Sub), condition_id: 1, true_next_id: 0, false_next_id: 2 } at (prev + 0, 8) to (start + 0, 9) true = c1 diff --git a/tests/coverage/mcdc_if.coverage b/tests/coverage/mcdc_if.coverage index 14d7f6c670d50..c2ed311a5bc2c 100644 --- a/tests/coverage/mcdc_if.coverage +++ b/tests/coverage/mcdc_if.coverage @@ -1,5 +1,6 @@ LL| |#![feature(coverage_attribute)] LL| |//@ edition: 2021 + LL| |//@ min-llvm-version: 18 LL| |//@ compile-flags: -Zcoverage-options=mcdc LL| |//@ llvm-cov-flags: --show-mcdc LL| | diff --git a/tests/coverage/mcdc_if.rs b/tests/coverage/mcdc_if.rs index 97cb4b5ef5f7f..a85843721c6ce 100644 --- a/tests/coverage/mcdc_if.rs +++ b/tests/coverage/mcdc_if.rs @@ -1,5 +1,6 @@ #![feature(coverage_attribute)] //@ edition: 2021 +//@ min-llvm-version: 18 //@ compile-flags: -Zcoverage-options=mcdc //@ llvm-cov-flags: --show-mcdc From 4723dd0969f1b7e9d805cc6a7b1317296ddd9dff Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Mon, 8 Apr 2024 14:51:58 +0800 Subject: [PATCH 28/29] coverage. Change doc comments of MCDCState::record_conditions to idiomatic because it only describes the implementation and has no doc test --- .../rustc_mir_build/src/build/coverageinfo.rs | 80 +++++++++---------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index 7b0dcc603ea81..57f809ef7cf8a 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -192,46 +192,46 @@ impl MCDCState { .then(|| Self { decision_stack: VecDeque::new(), processing_decision: None }) } - /// At first we assign ConditionIds for each sub expression. - /// If the sub expression is composite, re-assign its ConditionId to its LHS and generate a new ConditionId for its RHS. - /// - /// Example: "x = (A && B) || (C && D) || (D && F)" - /// - /// Visit Depth1: - /// (A && B) || (C && D) || (D && F) - /// ^-------LHS--------^ ^-RHS--^ - /// ID=1 ID=2 - /// - /// Visit LHS-Depth2: - /// (A && B) || (C && D) - /// ^-LHS--^ ^-RHS--^ - /// ID=1 ID=3 - /// - /// Visit LHS-Depth3: - /// (A && B) - /// LHS RHS - /// ID=1 ID=4 - /// - /// Visit RHS-Depth3: - /// (C && D) - /// LHS RHS - /// ID=3 ID=5 - /// - /// Visit RHS-Depth2: (D && F) - /// LHS RHS - /// ID=2 ID=6 - /// - /// Visit Depth1: - /// (A && B) || (C && D) || (D && F) - /// ID=1 ID=4 ID=3 ID=5 ID=2 ID=6 - /// - /// A node ID of '0' always means MC/DC isn't being tracked. - /// - /// If a "next" node ID is '0', it means it's the end of the test vector. - /// - /// As the compiler tracks expression in pre-order, we can ensure that condition info of parents are always properly assigned when their children are visited. - /// - If the op is AND, the "false_next" of LHS and RHS should be the parent's "false_next". While "true_next" of the LHS is the RHS, the "true next" of RHS is the parent's "true_next". - /// - If the op is OR, the "true_next" of LHS and RHS should be the parent's "true_next". While "false_next" of the LHS is the RHS, the "false next" of RHS is the parent's "false_next". + // At first we assign ConditionIds for each sub expression. + // If the sub expression is composite, re-assign its ConditionId to its LHS and generate a new ConditionId for its RHS. + // + // Example: "x = (A && B) || (C && D) || (D && F)" + // + // Visit Depth1: + // (A && B) || (C && D) || (D && F) + // ^-------LHS--------^ ^-RHS--^ + // ID=1 ID=2 + // + // Visit LHS-Depth2: + // (A && B) || (C && D) + // ^-LHS--^ ^-RHS--^ + // ID=1 ID=3 + // + // Visit LHS-Depth3: + // (A && B) + // LHS RHS + // ID=1 ID=4 + // + // Visit RHS-Depth3: + // (C && D) + // LHS RHS + // ID=3 ID=5 + // + // Visit RHS-Depth2: (D && F) + // LHS RHS + // ID=2 ID=6 + // + // Visit Depth1: + // (A && B) || (C && D) || (D && F) + // ID=1 ID=4 ID=3 ID=5 ID=2 ID=6 + // + // A node ID of '0' always means MC/DC isn't being tracked. + // + // If a "next" node ID is '0', it means it's the end of the test vector. + // + // As the compiler tracks expression in pre-order, we can ensure that condition info of parents are always properly assigned when their children are visited. + // - If the op is AND, the "false_next" of LHS and RHS should be the parent's "false_next". While "true_next" of the LHS is the RHS, the "true next" of RHS is the parent's "true_next". + // - If the op is OR, the "true_next" of LHS and RHS should be the parent's "true_next". While "false_next" of the LHS is the RHS, the "false next" of RHS is the parent's "false_next". fn record_conditions(&mut self, op: LogicalOp, span: Span) { let decision = match self.processing_decision.as_mut() { Some(decision) => { From 8b74525ba284d1c3593de68d24ad53e4d588d0d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dorian=20P=C3=A9ron?= Date: Mon, 8 Apr 2024 13:15:06 +0000 Subject: [PATCH 29/29] coverage. Add test cases for while-loops instrumentation --- tests/coverage/mcdc_while.cov-map | 129 +++++++++++++++++++++++++++++ tests/coverage/mcdc_while.coverage | 113 +++++++++++++++++++++++++ tests/coverage/mcdc_while.rs | 44 ++++++++++ 3 files changed, 286 insertions(+) create mode 100644 tests/coverage/mcdc_while.cov-map create mode 100644 tests/coverage/mcdc_while.coverage create mode 100644 tests/coverage/mcdc_while.rs diff --git a/tests/coverage/mcdc_while.cov-map b/tests/coverage/mcdc_while.cov-map new file mode 100644 index 0000000000000..e5e6c792f5604 --- /dev/null +++ b/tests/coverage/mcdc_while.cov-map @@ -0,0 +1,129 @@ +Function name: mcdc_while::mcdc_while_100_percent +Raw bytes (103): 0x[01, 01, 10, 01, 2f, 05, 09, 03, 0d, 0d, 05, 3a, 3e, 0d, 05, 03, 0d, 37, 09, 3a, 3e, 0d, 05, 03, 0d, 05, 09, 37, 09, 3a, 3e, 0d, 05, 03, 0d, 0a, 01, 1a, 01, 01, 12, 28, 00, 03, 03, 0b, 00, 36, 03, 00, 0c, 00, 17, 30, 0d, 3e, 01, 03, 02, 00, 0c, 00, 17, 0d, 00, 1b, 00, 26, 30, 05, 3a, 03, 00, 02, 00, 1b, 00, 26, 37, 00, 2b, 00, 36, 30, 09, 32, 02, 00, 00, 00, 2b, 00, 36, 2f, 01, 09, 00, 0f, 32, 02, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 16 +- expression 0 operands: lhs = Counter(0), rhs = Expression(11, Add) +- expression 1 operands: lhs = Counter(1), rhs = Counter(2) +- expression 2 operands: lhs = Expression(0, Add), rhs = Counter(3) +- expression 3 operands: lhs = Counter(3), rhs = Counter(1) +- expression 4 operands: lhs = Expression(14, Sub), rhs = Expression(15, Sub) +- expression 5 operands: lhs = Counter(3), rhs = Counter(1) +- expression 6 operands: lhs = Expression(0, Add), rhs = Counter(3) +- expression 7 operands: lhs = Expression(13, Add), rhs = Counter(2) +- expression 8 operands: lhs = Expression(14, Sub), rhs = Expression(15, Sub) +- expression 9 operands: lhs = Counter(3), rhs = Counter(1) +- expression 10 operands: lhs = Expression(0, Add), rhs = Counter(3) +- expression 11 operands: lhs = Counter(1), rhs = Counter(2) +- expression 12 operands: lhs = Expression(13, Add), rhs = Counter(2) +- expression 13 operands: lhs = Expression(14, Sub), rhs = Expression(15, Sub) +- expression 14 operands: lhs = Counter(3), rhs = Counter(1) +- expression 15 operands: lhs = Expression(0, Add), rhs = Counter(3) +Number of file 0 mappings: 10 +- Code(Counter(0)) at (prev + 26, 1) to (start + 1, 18) +- MCDCDecision { bitmap_idx: 0, conditions_num: 3 } at (prev + 3, 11) to (start + 0, 54) +- Code(Expression(0, Add)) at (prev + 0, 12) to (start + 0, 23) + = (c0 + (c1 + c2)) +- MCDCBranch { true: Counter(3), false: Expression(15, Sub), condition_id: 1, true_next_id: 3, false_next_id: 2 } at (prev + 0, 12) to (start + 0, 23) + true = c3 + false = ((c0 + (c1 + c2)) - c3) +- Code(Counter(3)) at (prev + 0, 27) to (start + 0, 38) +- MCDCBranch { true: Counter(1), false: Expression(14, Sub), condition_id: 3, true_next_id: 0, false_next_id: 2 } at (prev + 0, 27) to (start + 0, 38) + true = c1 + false = (c3 - c1) +- Code(Expression(13, Add)) at (prev + 0, 43) to (start + 0, 54) + = ((c3 - c1) + ((c0 + (c1 + c2)) - c3)) +- MCDCBranch { true: Counter(2), false: Expression(12, Sub), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 43) to (start + 0, 54) + true = c2 + false = (((c3 - c1) + ((c0 + (c1 + c2)) - c3)) - c2) +- Code(Expression(11, Add)) at (prev + 1, 9) to (start + 0, 15) + = (c1 + c2) +- Code(Expression(12, Sub)) at (prev + 2, 1) to (start + 0, 2) + = (((c3 - c1) + ((c0 + (c1 + c2)) - c3)) - c2) + +Function name: mcdc_while::mcdc_while_33_percent +Raw bytes (103): 0x[01, 01, 10, 01, 2f, 05, 09, 03, 0d, 0d, 05, 3a, 3e, 0d, 05, 03, 0d, 37, 09, 3a, 3e, 0d, 05, 03, 0d, 05, 09, 37, 09, 3a, 3e, 0d, 05, 03, 0d, 0a, 01, 07, 01, 02, 12, 28, 00, 03, 04, 0b, 00, 36, 03, 00, 0c, 00, 17, 30, 0d, 3e, 01, 03, 02, 00, 0c, 00, 17, 0d, 00, 1b, 00, 26, 30, 05, 3a, 03, 00, 02, 00, 1b, 00, 26, 37, 00, 2b, 00, 36, 30, 09, 32, 02, 00, 00, 00, 2b, 00, 36, 2f, 01, 09, 00, 0f, 32, 02, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 16 +- expression 0 operands: lhs = Counter(0), rhs = Expression(11, Add) +- expression 1 operands: lhs = Counter(1), rhs = Counter(2) +- expression 2 operands: lhs = Expression(0, Add), rhs = Counter(3) +- expression 3 operands: lhs = Counter(3), rhs = Counter(1) +- expression 4 operands: lhs = Expression(14, Sub), rhs = Expression(15, Sub) +- expression 5 operands: lhs = Counter(3), rhs = Counter(1) +- expression 6 operands: lhs = Expression(0, Add), rhs = Counter(3) +- expression 7 operands: lhs = Expression(13, Add), rhs = Counter(2) +- expression 8 operands: lhs = Expression(14, Sub), rhs = Expression(15, Sub) +- expression 9 operands: lhs = Counter(3), rhs = Counter(1) +- expression 10 operands: lhs = Expression(0, Add), rhs = Counter(3) +- expression 11 operands: lhs = Counter(1), rhs = Counter(2) +- expression 12 operands: lhs = Expression(13, Add), rhs = Counter(2) +- expression 13 operands: lhs = Expression(14, Sub), rhs = Expression(15, Sub) +- expression 14 operands: lhs = Counter(3), rhs = Counter(1) +- expression 15 operands: lhs = Expression(0, Add), rhs = Counter(3) +Number of file 0 mappings: 10 +- Code(Counter(0)) at (prev + 7, 1) to (start + 2, 18) +- MCDCDecision { bitmap_idx: 0, conditions_num: 3 } at (prev + 4, 11) to (start + 0, 54) +- Code(Expression(0, Add)) at (prev + 0, 12) to (start + 0, 23) + = (c0 + (c1 + c2)) +- MCDCBranch { true: Counter(3), false: Expression(15, Sub), condition_id: 1, true_next_id: 3, false_next_id: 2 } at (prev + 0, 12) to (start + 0, 23) + true = c3 + false = ((c0 + (c1 + c2)) - c3) +- Code(Counter(3)) at (prev + 0, 27) to (start + 0, 38) +- MCDCBranch { true: Counter(1), false: Expression(14, Sub), condition_id: 3, true_next_id: 0, false_next_id: 2 } at (prev + 0, 27) to (start + 0, 38) + true = c1 + false = (c3 - c1) +- Code(Expression(13, Add)) at (prev + 0, 43) to (start + 0, 54) + = ((c3 - c1) + ((c0 + (c1 + c2)) - c3)) +- MCDCBranch { true: Counter(2), false: Expression(12, Sub), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 43) to (start + 0, 54) + true = c2 + false = (((c3 - c1) + ((c0 + (c1 + c2)) - c3)) - c2) +- Code(Expression(11, Add)) at (prev + 1, 9) to (start + 0, 15) + = (c1 + c2) +- Code(Expression(12, Sub)) at (prev + 2, 1) to (start + 0, 2) + = (((c3 - c1) + ((c0 + (c1 + c2)) - c3)) - c2) + +Function name: mcdc_while::mcdc_while_66_percent +Raw bytes (103): 0x[01, 01, 10, 01, 2f, 05, 09, 03, 0d, 0d, 05, 3a, 3e, 0d, 05, 03, 0d, 37, 09, 3a, 3e, 0d, 05, 03, 0d, 05, 09, 37, 09, 3a, 3e, 0d, 05, 03, 0d, 0a, 01, 10, 01, 03, 12, 28, 00, 03, 05, 0b, 00, 36, 03, 00, 0c, 00, 17, 30, 0d, 3e, 01, 03, 02, 00, 0c, 00, 17, 0d, 00, 1b, 00, 26, 30, 05, 3a, 03, 00, 02, 00, 1b, 00, 26, 37, 00, 2b, 00, 36, 30, 09, 32, 02, 00, 00, 00, 2b, 00, 36, 2f, 01, 09, 00, 0f, 32, 02, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 16 +- expression 0 operands: lhs = Counter(0), rhs = Expression(11, Add) +- expression 1 operands: lhs = Counter(1), rhs = Counter(2) +- expression 2 operands: lhs = Expression(0, Add), rhs = Counter(3) +- expression 3 operands: lhs = Counter(3), rhs = Counter(1) +- expression 4 operands: lhs = Expression(14, Sub), rhs = Expression(15, Sub) +- expression 5 operands: lhs = Counter(3), rhs = Counter(1) +- expression 6 operands: lhs = Expression(0, Add), rhs = Counter(3) +- expression 7 operands: lhs = Expression(13, Add), rhs = Counter(2) +- expression 8 operands: lhs = Expression(14, Sub), rhs = Expression(15, Sub) +- expression 9 operands: lhs = Counter(3), rhs = Counter(1) +- expression 10 operands: lhs = Expression(0, Add), rhs = Counter(3) +- expression 11 operands: lhs = Counter(1), rhs = Counter(2) +- expression 12 operands: lhs = Expression(13, Add), rhs = Counter(2) +- expression 13 operands: lhs = Expression(14, Sub), rhs = Expression(15, Sub) +- expression 14 operands: lhs = Counter(3), rhs = Counter(1) +- expression 15 operands: lhs = Expression(0, Add), rhs = Counter(3) +Number of file 0 mappings: 10 +- Code(Counter(0)) at (prev + 16, 1) to (start + 3, 18) +- MCDCDecision { bitmap_idx: 0, conditions_num: 3 } at (prev + 5, 11) to (start + 0, 54) +- Code(Expression(0, Add)) at (prev + 0, 12) to (start + 0, 23) + = (c0 + (c1 + c2)) +- MCDCBranch { true: Counter(3), false: Expression(15, Sub), condition_id: 1, true_next_id: 3, false_next_id: 2 } at (prev + 0, 12) to (start + 0, 23) + true = c3 + false = ((c0 + (c1 + c2)) - c3) +- Code(Counter(3)) at (prev + 0, 27) to (start + 0, 38) +- MCDCBranch { true: Counter(1), false: Expression(14, Sub), condition_id: 3, true_next_id: 0, false_next_id: 2 } at (prev + 0, 27) to (start + 0, 38) + true = c1 + false = (c3 - c1) +- Code(Expression(13, Add)) at (prev + 0, 43) to (start + 0, 54) + = ((c3 - c1) + ((c0 + (c1 + c2)) - c3)) +- MCDCBranch { true: Counter(2), false: Expression(12, Sub), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 43) to (start + 0, 54) + true = c2 + false = (((c3 - c1) + ((c0 + (c1 + c2)) - c3)) - c2) +- Code(Expression(11, Add)) at (prev + 1, 9) to (start + 0, 15) + = (c1 + c2) +- Code(Expression(12, Sub)) at (prev + 2, 1) to (start + 0, 2) + = (((c3 - c1) + ((c0 + (c1 + c2)) - c3)) - c2) + diff --git a/tests/coverage/mcdc_while.coverage b/tests/coverage/mcdc_while.coverage new file mode 100644 index 0000000000000..f431c6be78c04 --- /dev/null +++ b/tests/coverage/mcdc_while.coverage @@ -0,0 +1,113 @@ + LL| |#![feature(coverage_attribute)] + LL| |//@ edition: 2021 + LL| |//@ min-llvm-version: 18 + LL| |//@ compile-flags: -Zcoverage-options=mcdc + LL| |//@ llvm-cov-flags: --show-mcdc + LL| | + LL| 1|fn mcdc_while_33_percent() { + LL| 1| let values = [(false, true, true), (true, false, true), (true, false, false)]; + LL| 1| let mut i = 0; + LL| | + LL| 3| while (values[i].0 && values[i].1) || values[i].2 { + ^2 + ------------------ + |---> MC/DC Decision Region (LL:11) to (LL:54) + | + | Number of Conditions: 3 + | Condition C1 --> (LL:12) + | Condition C2 --> (LL:27) + | Condition C3 --> (LL:43) + | + | Executed MC/DC Test Vectors: + | + | C1, C2, C3 Result + | 1 { T, F, F = F } + | 2 { F, -, T = T } + | 3 { T, F, T = T } + | + | C1-Pair: not covered + | C2-Pair: not covered + | C3-Pair: covered: (1,3) + | MC/DC Coverage for Decision: 33.33% + | + ------------------ + LL| 2| i += 1 + LL| | } + LL| 1|} + LL| | + LL| 1|fn mcdc_while_66_percent() { + LL| 1| let values = + LL| 1| [(false, true, true), (true, false, true), (true, true, true), (false, false, false)]; + LL| 1| let mut i = 0; + LL| | + LL| 4| while (values[i].0 && values[i].1) || values[i].2 { + ^2 ^3 + ------------------ + |---> MC/DC Decision Region (LL:11) to (LL:54) + | + | Number of Conditions: 3 + | Condition C1 --> (LL:12) + | Condition C2 --> (LL:27) + | Condition C3 --> (LL:43) + | + | Executed MC/DC Test Vectors: + | + | C1, C2, C3 Result + | 1 { F, -, F = F } + | 2 { F, -, T = T } + | 3 { T, F, T = T } + | 4 { T, T, - = T } + | + | C1-Pair: covered: (1,4) + | C2-Pair: not covered + | C3-Pair: covered: (1,2) + | MC/DC Coverage for Decision: 66.67% + | + ------------------ + LL| 3| i += 1 + LL| | } + LL| 1|} + LL| | + LL| 2|fn mcdc_while_100_percent(values: &[(bool, bool, bool)]) { + LL| 2| let mut i = 0; + LL| | + LL| 4| while (values[i].0 && values[i].1) || values[i].2 { + ^2 ^3 + ------------------ + |---> MC/DC Decision Region (LL:11) to (LL:54) + | + | Number of Conditions: 3 + | Condition C1 --> (LL:12) + | Condition C2 --> (LL:27) + | Condition C3 --> (LL:43) + | + | Executed MC/DC Test Vectors: + | + | C1, C2, C3 Result + | 1 { F, -, F = F } + | 2 { T, F, F = F } + | 3 { F, -, T = T } + | 4 { T, T, - = T } + | + | C1-Pair: covered: (1,4) + | C2-Pair: covered: (2,4) + | C3-Pair: covered: (1,3) + | MC/DC Coverage for Decision: 100.00% + | + ------------------ + LL| 2| i += 1 + LL| | } + LL| 2|} + LL| | + LL| |#[coverage(off)] + LL| |fn main() { + LL| | mcdc_while_33_percent(); + LL| | + LL| | mcdc_while_66_percent(); + LL| | + LL| | let values_1 = [(true, true, true), (true, false, false)]; + LL| | let values_2 = [(false, true, true), (false, false, false)]; + LL| | mcdc_while_100_percent(&values_1); + LL| | mcdc_while_100_percent(&values_2); + LL| |} + diff --git a/tests/coverage/mcdc_while.rs b/tests/coverage/mcdc_while.rs new file mode 100644 index 0000000000000..0009a2569d786 --- /dev/null +++ b/tests/coverage/mcdc_while.rs @@ -0,0 +1,44 @@ +#![feature(coverage_attribute)] +//@ edition: 2021 +//@ min-llvm-version: 18 +//@ compile-flags: -Zcoverage-options=mcdc +//@ llvm-cov-flags: --show-mcdc + +fn mcdc_while_33_percent() { + let values = [(false, true, true), (true, false, true), (true, false, false)]; + let mut i = 0; + + while (values[i].0 && values[i].1) || values[i].2 { + i += 1 + } +} + +fn mcdc_while_66_percent() { + let values = + [(false, true, true), (true, false, true), (true, true, true), (false, false, false)]; + let mut i = 0; + + while (values[i].0 && values[i].1) || values[i].2 { + i += 1 + } +} + +fn mcdc_while_100_percent(values: &[(bool, bool, bool)]) { + let mut i = 0; + + while (values[i].0 && values[i].1) || values[i].2 { + i += 1 + } +} + +#[coverage(off)] +fn main() { + mcdc_while_33_percent(); + + mcdc_while_66_percent(); + + let values_1 = [(true, true, true), (true, false, false)]; + let values_2 = [(false, true, true), (false, false, false)]; + mcdc_while_100_percent(&values_1); + mcdc_while_100_percent(&values_2); +}