From 68f86381ee9a5af00a6071773d54c00f032fe385 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Fri, 19 Apr 2024 10:43:53 +0800 Subject: [PATCH 1/6] coverage. Add coverage-options=mcdc as gate for MC/DC instrument --- compiler/rustc_interface/src/tests.rs | 2 +- compiler/rustc_mir_build/messages.ftl | 2 ++ compiler/rustc_mir_build/src/errors.rs | 9 ++++++++ compiler/rustc_session/src/config.rs | 2 ++ compiler/rustc_session/src/options.rs | 22 ++++++++++--------- 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 +++++ 10 files changed, 40 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index e563728c893..49b0d80ec29 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -758,7 +758,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_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index 1de691f32a7..34440c60cf3 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 diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 26f10fdd333..9ddfb12bf76 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: usize, + pub max_conditions_num: usize, +} + #[derive(Diagnostic)] #[diag(mir_build_pattern_not_covered, code = E0005)] pub(crate) struct PatternNotCovered<'s, 'tcx> { diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 2a7b5650fc2..c5168f503f9 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 6a2975263cf..1a0b02205d4 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"; @@ -946,17 +946,19 @@ mod parse { 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, + match option { + "no-branch" => { + slot.branch = false; + slot.mcdc = false; + } + "branch" => slot.branch = true, + "mcdc" => { + slot.branch = true; + slot.mcdc = true; + } _ => return false, - }; - *slot = enabled; + } } - true } diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index b63c119eee0..2bc14b43234 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 185a3ba5dbd..bbd81e7437c 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 450573cc6c7..5e192d9aca1 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 as well as mcdc 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 ca82dc5bdb9..f6e5421f878 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 a62e0554f76..50c01ed29b5 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 cf6b6cb2b4d6e1d4b1d7ec9daa2debf075bd7ed6 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Fri, 19 Apr 2024 10:53:04 +0800 Subject: [PATCH 2/6] coverage. Generate Mappings of decisions and conditions for MC/DC --- .../src/coverageinfo/ffi.rs | 158 ++++++++++- .../llvm-wrapper/CoverageMappingWrapper.cpp | 94 ++++++- compiler/rustc_middle/src/mir/coverage.rs | 104 ++++++- compiler/rustc_middle/src/mir/pretty.rs | 24 +- .../rustc_middle/src/ty/structural_impls.rs | 1 + .../rustc_mir_build/src/build/coverageinfo.rs | 255 +++++++++++++++++- .../rustc_mir_build/src/build/matches/mod.rs | 2 + .../rustc_mir_transform/src/coverage/mod.rs | 79 +++++- .../rustc_mir_transform/src/coverage/spans.rs | 34 ++- .../src/coverage/spans/from_mir.rs | 69 ++++- 10 files changed, 779 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index 2af28146a51..12a846a49ec 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,86 @@ 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, Default)] + 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, Default)] + pub struct BranchParameters { + condition_id: LLVMConditionId, + condition_ids: [LLVMConditionId; 2], + } + + #[repr(C)] + #[derive(Clone, Copy, Debug)] + pub enum ParameterTag { + None = 0, + Decision = 1, + Branch = 2, + } + /// 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::default(), branch_params } + } + } + + 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 +204,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. @@ -173,6 +256,26 @@ impl CounterMappingRegion { 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, + start_line, + start_col, + end_line, + end_col, + ), } } @@ -187,6 +290,7 @@ impl CounterMappingRegion { Self { counter, false_counter: Counter::ZERO, + mcdc_params: mcdc::Parameters::none(), file_id, expanded_file_id: 0, start_line, @@ -209,6 +313,7 @@ impl CounterMappingRegion { Self { counter, false_counter, + mcdc_params: mcdc::Parameters::none(), file_id, expanded_file_id: 0, start_line, @@ -219,6 +324,54 @@ impl CounterMappingRegion { } } + 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: RegionKind::MCDCBranchRegion, + } + } + + pub(crate) fn decision_region( + decision_info: DecisionInfo, + file_id: u32, + start_line: u32, + start_col: u32, + 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, + file_id, + expanded_file_id: 0, + start_line, + start_col, + end_line, + end_col, + kind: RegionKind::MCDCDecisionRegion, + } + } + // This function might be used in the future; the LLVM API is still evolving, as is coverage // support. #[allow(dead_code)] @@ -233,6 +386,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 +410,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 +435,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 0998b463a88..e842f47f48c 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,102 @@ 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!"); } +enum LLVMRustMCDCParametersTag { + None = 0, + Decision = 1, + Branch = 2, +}; + +struct LLVMRustMCDCDecisionParameters { + uint32_t BitmapIdx; + uint16_t NumConditions; +}; + +struct LLVMRustMCDCBranchParameters { + int16_t ConditionID; + int16_t ConditionIDs[2]; +}; + +struct LLVMRustMCDCParameters { + LLVMRustMCDCParametersTag Tag; + 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{}; + switch (Params.Tag) { + case LLVMRustMCDCParametersTag::None: + return parameter; + case LLVMRustMCDCParametersTag::Decision: + parameter.BitmapIdx = + static_cast(Params.DecisionParameters.BitmapIdx), + parameter.NumConditions = + static_cast(Params.DecisionParameters.NumConditions); + return parameter; + case LLVMRustMCDCParametersTag::Branch: + parameter.ID = static_cast( + Params.BranchParameters.ConditionID), + parameter.FalseID = + static_cast( + Params.BranchParameters.ConditionIDs[0]), + parameter.TrueID = + static_cast( + Params.BranchParameters.ConditionIDs[1]); + return parameter; + } + report_fatal_error("Bad LLVMRustMCDCParametersTag!"); +} +#elif LLVM_VERSION_GE(19, 0) +static coverage::mcdc::Parameters fromRust(LLVMRustMCDCParameters Params) { + switch (Params.Tag) { + case LLVMRustMCDCParametersTag::None: + return std::monostate(); + case LLVMRustMCDCParametersTag::Decision: + return coverage::mcdc::DecisionParameters( + Params.DecisionParameters.BitmapIdx, + Params.DecisionParameters.NumConditions); + case LLVMRustMCDCParametersTag::Branch: + return coverage::mcdc::BranchParameters( + static_cast( + Params.BranchParameters.ConditionID), + {static_cast( + Params.BranchParameters.ConditionIDs[0]), + static_cast( + Params.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 struct LLVMRustCounterMappingRegion { LLVMRustCounter Count; LLVMRustCounter FalseCount; + LLVMRustMCDCParameters MCDCParameters; uint32_t FileID; uint32_t ExpandedFileID; uint32_t LineStart; @@ -135,7 +222,8 @@ 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{}, + // LLVM 19 may move this argument to last. + 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 582a1806688..b1d0c815ae0 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -51,6 +51,25 @@ rustc_index::newtype_index! { pub struct ExpressionId {} } +rustc_index::newtype_index! { + /// ID of a mcdc condition. Used by llvm to check mcdc coverage. + /// + /// Note for future: the max limit of 0xFFFF is probably too loose. Actually llvm does not + /// support decisions with too many conditions (7 and more at LLVM 18 while may be hundreds at 19) + /// and represents it with `int16_t`. This max value may be changed once we could + /// figure out an accurate limit. + #[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. /// @@ -106,6 +125,22 @@ pub enum CoverageKind { /// mappings. Intermediate expressions with no direct mappings are /// retained/zeroed based on whether they are transitively used.) 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 { @@ -116,6 +151,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()), + CondBitmapUpdate { id, value } => { + write!(fmt, "CondBitmapUpdate({:?}, {:?})", id.index(), value) + } + TestVectorBitmapUpdate { bitmap_idx } => { + write!(fmt, "TestVectorUpdate({:?})", bitmap_idx) + } } } } @@ -172,16 +213,23 @@ pub enum MappingKind { Code(CovTerm), /// Associates a branch region with separate counters for true and false. 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. + MCDCDecision(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::MCDCBranch { true_term, false_term, .. } => two(true_term, false_term), + Self::MCDCDecision(_) => zero(), } } @@ -193,6 +241,12 @@ impl MappingKind { 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::MCDCDecision(param) => Self::MCDCDecision(param), } } } @@ -212,7 +266,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, } @@ -226,6 +280,8 @@ pub struct BranchInfo { /// data structures without having to scan the entire body first. pub num_block_markers: usize, pub branch_spans: Vec, + pub mcdc_branch_spans: Vec, + pub mcdc_decision_spans: Vec, } #[derive(Clone, Debug)] @@ -235,3 +291,45 @@ pub struct BranchSpan { 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(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 { + pub bitmap_idx: u32, + pub conditions_num: u16, +} + +#[derive(Clone, Debug)] +#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] +pub struct MCDCDecisionSpan { + pub span: Span, + pub conditions_num: usize, + pub end_markers: Vec, +} diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 15bd5c08965..a3cdcec9ec0 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -475,7 +475,8 @@ 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 } in branch_spans { writeln!( @@ -483,7 +484,26 @@ fn write_coverage_branch_info( "{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_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs index 90c68e7ddfc..14a77d4b37e 100644 --- a/compiler/rustc_middle/src/ty/structural_impls.rs +++ b/compiler/rustc_middle/src/ty/structural_impls.rs @@ -427,6 +427,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 ab0043906b1..57f809ef7cf 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -1,14 +1,20 @@ 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, MCDCBranchSpan, + MCDCDecisionSpan, +}; 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; +use rustc_span::Span; use crate::build::Builder; +use crate::errors::MCDCExceedsConditionNumLimit; pub(crate) struct BranchInfoBuilder { /// Maps condition expressions to their enclosing `!`, for better instrumentation. @@ -16,6 +22,9 @@ pub(crate) struct BranchInfoBuilder { num_block_markers: usize, branch_spans: Vec, + mcdc_branch_spans: Vec, + mcdc_decision_spans: Vec, + mcdc_state: Option, } #[derive(Clone, Copy)] @@ -33,7 +42,14 @@ 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_branch_spans: vec![], + mcdc_decision_spans: vec![], + mcdc_state: MCDCState::new_if_enabled(tcx), + }) } else { None } @@ -79,6 +95,55 @@ impl BranchInfoBuilder { } } + 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, + ) -> 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 { + match decision.conditions_num { + 0 => { + unreachable!("Decision with no condition is not expected"); + } + 1..=MAX_CONDITIONS_NUM_IN_DECISION => { + self.mcdc_decision_spans.push(decision); + } + _ => { + // Do not generate mcdc mappings and statements for decisions with too many conditions. + 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 = None; + + 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 { let id = BlockMarkerId::from_usize(self.num_block_markers); self.num_block_markers += 1; @@ -86,14 +151,167 @@ 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_branch_spans, + mcdc_decision_spans, + .. + } = 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, + mcdc_branch_spans, + mcdc_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: usize = 6; + +struct MCDCState { + /// To construct condition evaluation tree. + decision_stack: VecDeque, + processing_decision: Option, +} + +impl MCDCState { + fn new_if_enabled(tcx: TyCtxt<'_>) -> Option { + tcx.sess + .instrument_coverage_mcdc() + .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". + 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(MCDCDecisionSpan { + span, + conditions_num: 0, + end_markers: vec![], + }), + }; + + let parent_condition = self.decision_stack.pop_back().unwrap_or_default(); + let lhs_id = if parent_condition.condition_id == ConditionId::NONE { + decision.conditions_num += 1; + ConditionId::from(decision.conditions_num) + } else { + parent_condition.condition_id + }; + + decision.conditions_num += 1; + let rhs_condition_id = ConditionId::from(decision.conditions_num); + + 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 take_condition( + &mut self, + true_marker: BlockMarkerId, + false_marker: BlockMarkerId, + ) -> (Option, Option) { + let Some(condition_info) = self.decision_stack.pop_back() else { + 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_markers.push(true_marker); + } + if condition_info.false_next_id == ConditionId::NONE { + decision.end_markers.push(false_marker); + } + + if self.decision_stack.is_empty() { + (Some(condition_info), self.processing_decision.take()) + } else { + (Some(condition_info), None) + } } } @@ -137,10 +355,27 @@ impl Builder<'_, '_> { 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, - 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) { + 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/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 9730473c428..f46dceeeedf 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, 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, 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 d382d2c03c2..9e8648b0f93 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -100,9 +100,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: coverage_spans.test_vector_bitmap_bytes(), expressions: coverage_counters.into_expressions(), mappings, })); @@ -136,20 +139,33 @@ fn create_mappings<'tcx>( .as_term() }; - coverage_spans - .all_bcb_mappings() - .filter_map(|&BcbMapping { kind: bcb_mapping_kind, span }| { - let kind = match bcb_mapping_kind { + let mut mappings = Vec::new(); + + 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::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::MCDCDecision { bitmap_idx, conditions_num, .. } => { + MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, 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 }) - }) - .collect::>() + }, + )); + + mappings } /// For each BCB node or BCB edge that has an associated coverage counter, @@ -204,6 +220,55 @@ 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, + coverage_spans: &CoverageSpans, +) { + if coverage_spans.test_vector_bitmap_bytes() == 0 { + return; + } + + // 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::MCDCDecision { end_bcbs, bitmap_idx, .. } => { + Some((end_bcbs, *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::MCDCBranch { true_bcb, false_bcb, condition_info } => { + 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 /// 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/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 03ede886688..a4cd8a38c66 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -1,7 +1,9 @@ use rustc_data_structures::graph::DirectedGraph; use rustc_index::bit_set::BitSet; use rustc_middle::mir; +use rustc_middle::mir::coverage::ConditionInfo; use rustc_span::{BytePos, Span}; +use std::collections::BTreeSet; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph, START_BCB}; use crate::coverage::spans::from_mir::SpanFromMir; @@ -9,12 +11,20 @@ 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), /// Associates a branch span with BCBs for its true and false arms. 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 mcdc decision with its join BCB. + MCDCDecision { end_bcbs: BTreeSet, bitmap_idx: u32, conditions_num: u16 }, } #[derive(Debug)] @@ -26,6 +36,7 @@ pub(super) struct BcbMapping { pub(super) struct CoverageSpans { bcb_has_mappings: BitSet, mappings: Vec, + test_vector_bitmap_bytes: u32, } impl CoverageSpans { @@ -36,6 +47,10 @@ impl CoverageSpans { pub(super) fn all_bcb_mappings(&self) -> impl Iterator { self.mappings.iter() } + + pub(super) fn test_vector_bitmap_bytes(&self) -> u32 { + self.test_vector_bitmap_bytes + } } /// Extracts coverage-relevant spans from MIR, and associates them with @@ -85,17 +100,26 @@ pub(super) fn generate_coverage_spans( let mut insert = |bcb| { bcb_has_mappings.insert(bcb); }; - for &BcbMapping { kind, span: _ } in &mappings { - match kind { + let mut test_vector_bitmap_bytes = 0; + 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::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. + 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 }) + 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 adb0c9f1929..b9919a2ae88 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}; +use rustc_middle::mir::coverage::{ + BlockMarkerId, BranchSpan, CoverageKind, MCDCBranchSpan, MCDCDecisionSpan, +}; use rustc_middle::mir::{ self, AggregateKind, BasicBlock, FakeReadCause, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, @@ -227,7 +229,10 @@ fn filtered_statement_span(statement: &Statement<'_>) -> Option { // 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:?}" ), @@ -384,10 +389,11 @@ pub(super) fn extract_branch_mappings( } } - branch_info - .branch_spans - .iter() - .filter_map(|&BranchSpan { span: raw_span, true_marker, false_marker }| { + let bcb_from_marker = + |marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[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() { @@ -395,13 +401,56 @@ 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((span, true_bcb, false_bcb)) + }; - Some(BcbMapping { kind: BcbMappingKind::Branch { true_bcb, false_bcb }, span }) + 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 } }) + }; + + 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: &MCDCDecisionSpan| { + let (span, _) = unexpand_into_body_span_with_visible_macro(decision.span, body_span)?; + + let end_bcbs = decision + .end_markers + .iter() + .map(|&marker| bcb_from_marker(marker)) + .collect::>()?; + + let bitmap_idx = next_bitmap_idx; + next_bitmap_idx += (1_u32 << decision.conditions_num).div_ceil(8); + + Some(BcbMapping { + kind: BcbMappingKind::MCDCDecision { + end_bcbs, + bitmap_idx, + conditions_num: decision.conditions_num as u16, + }, + span, + }) + }; + + branch_info + .branch_spans + .iter() + .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 439dbfa1ecb1b45db0acc728094d6e8ed3dc3080 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Fri, 19 Apr 2024 10:58:16 +0800 Subject: [PATCH 3/6] coverage. Lowering MC/DC statements to llvm-ir --- compiler/rustc_codegen_llvm/src/builder.rs | 126 +++++++++++++++++- .../src/coverageinfo/mod.rs | 70 +++++++++- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 4 + .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 27 ++++ compiler/rustc_middle/src/mir/query.rs | 4 + .../rustc_mir_transform/src/coverage/query.rs | 12 +- 6 files changed, 240 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 160f361b9b5..4c2bdb2f5ec 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_sanitizers::{cfi, kcfi}; @@ -1702,4 +1702,128 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> { }; kcfi_bundle } + + pub(crate) fn mcdc_parameters( + &mut self, + fn_name: &'ll Value, + hash: &'ll Value, + bitmap_bytes: &'ll Value, + ) -> &'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()], + 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, + ); + // 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 + } + } + + pub(crate) fn mcdc_tvbitmap_update( + &mut self, + fn_name: &'ll Value, + hash: &'ll Value, + bitmap_bytes: &'ll Value, + bitmap_index: &'ll Value, + mcdc_temp: &'ll Value, + ) { + debug!( + "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) }; + 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, + ); + } + let i32_align = self.tcx().data_layout.i32_align.abi; + self.store(self.const_i32(0), mcdc_temp, i32_align); + } + + pub(crate) fn mcdc_condbitmap_update( + &mut self, + fn_name: &'ll Value, + hash: &'ll Value, + cond_loc: &'ll Value, + mcdc_temp: &'ll Value, + bool_value: &'ll 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( + &[ + self.cx.type_ptr(), + self.cx.type_i64(), + self.cx.type_i32(), + self.cx.type_ptr(), + self.cx.type_i1(), + ], + self.cx.type_void(), + ); + let args = &[fn_name, hash, cond_loc, mcdc_temp, bool_value]; + 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, + ); + } + } } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 140566e8da9..085ce15d81f 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,10 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { return; }; + 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 }; let mut coverage_map = coverage_context.function_coverage_map.borrow_mut(); let func_coverage = coverage_map @@ -131,10 +143,66 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { CoverageKind::ExpressionUsed { id } => { func_coverage.mark_expression_id_seen(id); } + 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 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::TestVectorBitmapUpdate { bitmap_idx } => { + drop(coverage_map); + let cond_bitmap = coverage_context + .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!( + 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); + } } } } +fn ensure_mcdc_parameters<'ll, 'tcx>( + bx: &mut Builder<'_, 'll, 'tcx>, + instance: Instance<'tcx>, + function_coverage_info: &FunctionCoverageInfo, +) { + 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 /// 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_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 83158f6f1d5..a10dc61967e 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_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 6e11fd629e4..2a9de499622 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1546,6 +1546,33 @@ extern "C" LLVMValueRef LLVMRustGetInstrProfIncrementIntrinsic(LLVMModuleRef M) unwrap(M), llvm::Intrinsic::instrprof_increment)); } +extern "C" LLVMValueRef LLVMRustGetInstrProfMCDCParametersIntrinsic(LLVMModuleRef M) { +#if LLVM_VERSION_GE(18, 0) + return wrap(llvm::Intrinsic::getDeclaration( + unwrap(M), llvm::Intrinsic::instrprof_mcdc_parameters)); +#else + report_fatal_error("LLVM 18.0 is required for mcdc intrinsic functions"); +#endif +} + +extern "C" LLVMValueRef LLVMRustGetInstrProfMCDCTVBitmapUpdateIntrinsic(LLVMModuleRef M) { +#if LLVM_VERSION_GE(18, 0) + return wrap(llvm::Intrinsic::getDeclaration( + unwrap(M), llvm::Intrinsic::instrprof_mcdc_tvbitmap_update)); +#else + report_fatal_error("LLVM 18.0 is required for mcdc intrinsic functions"); +#endif +} + +extern "C" LLVMValueRef LLVMRustGetInstrProfMCDCCondBitmapIntrinsic(LLVMModuleRef M) { +#if LLVM_VERSION_GE(18, 0) + return wrap(llvm::Intrinsic::getDeclaration( + unwrap(M), llvm::Intrinsic::instrprof_mcdc_condbitmap_update)); +#else + report_fatal_error("LLVM 18.0 is required for mcdc intrinsic functions"); +#endif +} + extern "C" LLVMValueRef LLVMRustBuildMemCpy(LLVMBuilderRef B, LLVMValueRef Dst, unsigned DstAlign, LLVMValueRef Src, unsigned SrcAlign, diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index 105f30f1db8..e3f58729fbd 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_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index 65715253647..f77ee63d02c 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -61,7 +61,17 @@ 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_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 } } fn all_coverage_in_mir_body<'a, 'tcx>( From 402dc38d99767e7e73896587be42ade438148151 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Fri, 19 Apr 2024 10:58:42 +0800 Subject: [PATCH 4/6] coverage. Add basic tests for MC/DC --- tests/coverage/mcdc_if.cov-map | 218 ++++++++++++++++++++++++++ tests/coverage/mcdc_if.coverage | 262 ++++++++++++++++++++++++++++++++ tests/coverage/mcdc_if.rs | 103 +++++++++++++ 3 files changed, 583 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 00000000000..35a265684d2 --- /dev/null +++ b/tests/coverage/mcdc_if.cov-map @@ -0,0 +1,218 @@ +Function name: mcdc_if::mcdc_check_a +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 +- 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 + 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 + 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, 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 +- 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 + 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 + 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, 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 +- 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 + 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 + 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, 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 +- 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 + 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 + 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, 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 +- 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 + 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 + 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, 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 +- 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 + 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 + 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))) + +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, 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 +- 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 + 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 + 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 new file mode 100644 index 00000000000..c2ed311a5bc --- /dev/null +++ b/tests/coverage/mcdc_if.coverage @@ -0,0 +1,262 @@ + 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| 2|fn mcdc_check_neither(a: bool, b: bool) { + LL| 2| if a && b { + ^0 + ------------------ + |---> MC/DC Decision Region (LL:8) to (LL:14) + | + | Number of Conditions: 2 + | Condition C1 --> (LL:8) + | Condition C2 --> (LL: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 (LL:8) to (LL:14) + | + | Number of Conditions: 2 + | Condition C1 --> (LL:8) + | Condition C2 --> (LL: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 (LL:8) to (LL:14) + | + | Number of Conditions: 2 + | Condition C1 --> (LL:8) + | Condition C2 --> (LL: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 (LL:8) to (LL:14) + | + | Number of Conditions: 2 + | Condition C1 --> (LL:8) + | Condition C2 --> (LL: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 (LL:8) to (LL:21) + | + | Number of Conditions: 3 + | Condition C1 --> (LL:8) + | Condition C2 --> (LL:14) + | Condition C3 --> (LL: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 (LL:8) to (LL:21) + | + | Number of Conditions: 3 + | Condition C1 --> (LL:9) + | Condition C2 --> (LL:14) + | Condition C3 --> (LL: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| 3|fn mcdc_nested_if(a: bool, b: bool, c: bool) { + LL| 3| if a || b { + ^0 + ------------------ + |---> MC/DC Decision Region (LL:8) to (LL:14) + | + | Number of Conditions: 2 + | Condition C1 --> (LL:8) + | Condition C2 --> (LL: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 (LL:12) to (LL:18) + | + | Number of Conditions: 2 + | Condition C1 --> (LL:12) + | Condition C2 --> (LL: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); + 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| | 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)] + 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 00000000000..a85843721c6 --- /dev/null +++ b/tests/coverage/mcdc_if.rs @@ -0,0 +1,103 @@ +#![feature(coverage_attribute)] +//@ edition: 2021 +//@ min-llvm-version: 18 +//@ 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"); + } +} + +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); + 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); + + mcdc_nested_if(true, false, true); + mcdc_nested_if(true, true, true); + mcdc_nested_if(true, true, false); +} + +#[coverage(off)] +fn say(message: &str) { + core::hint::black_box(message); +} From 5daf58ffc1d46a63d33652cc953a3a801b8d1265 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 17 Apr 2024 21:30:41 -0400 Subject: [PATCH 5/6] Fix capturing duplicated lifetimes via parent --- .../rustc_hir_analysis/src/check/check.rs | 37 ++++++++++++++---- .../precise-capturing/capture-parent-arg.rs | 38 +++++++++++++++++++ .../capture-parent-arg.stderr | 28 ++++++++++++++ 3 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 tests/ui/impl-trait/precise-capturing/capture-parent-arg.rs create mode 100644 tests/ui/impl-trait/precise-capturing/capture-parent-arg.stderr diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 3881e240ced..36553591de8 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -492,6 +492,7 @@ fn check_opaque_precise_captures<'tcx>(tcx: TyCtxt<'tcx>, opaque_def_id: LocalDe }; let mut expected_captures = UnordSet::default(); + let mut shadowed_captures = UnordSet::default(); let mut seen_params = UnordMap::default(); let mut prev_non_lifetime_param = None; for arg in precise_capturing_args { @@ -530,6 +531,21 @@ fn check_opaque_precise_captures<'tcx>(tcx: TyCtxt<'tcx>, opaque_def_id: LocalDe match tcx.named_bound_var(hir_id) { Some(ResolvedArg::EarlyBound(def_id)) => { expected_captures.insert(def_id); + + // Make sure we allow capturing these lifetimes through `Self` and + // `T::Assoc` projection syntax, too. These will occur when we only + // see lifetimes are captured after hir-lowering -- this aligns with + // the cases that were stabilized with the `impl_trait_projection` + // feature -- see . + if let DefKind::LifetimeParam = tcx.def_kind(def_id) + && let ty::ReEarlyParam(ty::EarlyParamRegion { def_id, .. }) + | ty::ReLateParam(ty::LateParamRegion { + bound_region: ty::BoundRegionKind::BrNamed(def_id, _), + .. + }) = *tcx.map_opaque_lifetime_to_parent_lifetime(def_id.expect_local()) + { + shadowed_captures.insert(def_id); + } } _ => { tcx.dcx().span_delayed_bug( @@ -555,23 +571,30 @@ fn check_opaque_precise_captures<'tcx>(tcx: TyCtxt<'tcx>, opaque_def_id: LocalDe ); continue; } + // If a param is shadowed by a early-bound (duplicated) lifetime, then + // it may or may not be captured as invariant, depending on if it shows + // up through `Self` or `T::Assoc` syntax. + if shadowed_captures.contains(¶m.def_id) { + continue; + } match param.kind { ty::GenericParamDefKind::Lifetime => { // Check if the lifetime param was captured but isn't named in the precise captures list. if variances[param.index as usize] == ty::Invariant { - let param_span = - if let ty::ReEarlyParam(ty::EarlyParamRegion { def_id, .. }) + let param_span = if let DefKind::OpaqueTy = + tcx.def_kind(tcx.parent(param.def_id)) + && let ty::ReEarlyParam(ty::EarlyParamRegion { def_id, .. }) | ty::ReLateParam(ty::LateParamRegion { bound_region: ty::BoundRegionKind::BrNamed(def_id, _), .. }) = *tcx .map_opaque_lifetime_to_parent_lifetime(param.def_id.expect_local()) - { - Some(tcx.def_span(def_id)) - } else { - None - }; + { + Some(tcx.def_span(def_id)) + } else { + None + }; // FIXME(precise_capturing): Structured suggestion for this would be useful tcx.dcx().emit_err(errors::LifetimeNotCaptured { use_span: tcx.def_span(param.def_id), diff --git a/tests/ui/impl-trait/precise-capturing/capture-parent-arg.rs b/tests/ui/impl-trait/precise-capturing/capture-parent-arg.rs new file mode 100644 index 00000000000..f880bb038d5 --- /dev/null +++ b/tests/ui/impl-trait/precise-capturing/capture-parent-arg.rs @@ -0,0 +1,38 @@ +#![feature(precise_capturing)] +//~^ WARN the feature `precise_capturing` is incomplete + +trait Tr { + type Assoc; +} + +struct W<'a>(&'a ()); + +impl Tr for W<'_> { + type Assoc = (); +} + +// The normal way of capturing `'a`... +impl<'a> W<'a> { + fn good1() -> impl use<'a> Into< as Tr>::Assoc> {} +} + +// This ensures that we don't error when we capture the *parent* copy of `'a`, +// since the opaque captures that rather than the duplicated `'a` lifetime +// synthesized from mentioning `'a` directly in the bounds. +impl<'a> W<'a> { + fn good2() -> impl use<'a> Into<::Assoc> {} +} + +// The normal way of capturing `'a`... but not mentioned in the bounds. +impl<'a> W<'a> { + fn bad1() -> impl use<> Into< as Tr>::Assoc> {} + //~^ ERROR `impl Trait` captures lifetime parameter, but it is not mentioned in `use<...>` precise captures list +} + +// But also make sure that we error here... +impl<'a> W<'a> { +//~^ ERROR `impl Trait` captures lifetime parameter, but it is not mentioned in `use<...>` precise captures list + fn bad2() -> impl use<> Into<::Assoc> {} +} + +fn main() {} diff --git a/tests/ui/impl-trait/precise-capturing/capture-parent-arg.stderr b/tests/ui/impl-trait/precise-capturing/capture-parent-arg.stderr new file mode 100644 index 00000000000..85790d57163 --- /dev/null +++ b/tests/ui/impl-trait/precise-capturing/capture-parent-arg.stderr @@ -0,0 +1,28 @@ +warning: the feature `precise_capturing` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/capture-parent-arg.rs:1:12 + | +LL | #![feature(precise_capturing)] + | ^^^^^^^^^^^^^^^^^ + | + = note: see issue #123432 for more information + = note: `#[warn(incomplete_features)]` on by default + +error: `impl Trait` captures lifetime parameter, but it is not mentioned in `use<...>` precise captures list + --> $DIR/capture-parent-arg.rs:28:37 + | +LL | impl<'a> W<'a> { + | -- this lifetime parameter is captured +LL | fn bad1() -> impl use<> Into< as Tr>::Assoc> {} + | -------------------^^---------------- lifetime captured due to being mentioned in the bounds of the `impl Trait` + +error: `impl Trait` captures lifetime parameter, but it is not mentioned in `use<...>` precise captures list + --> $DIR/capture-parent-arg.rs:33:6 + | +LL | impl<'a> W<'a> { + | ^^ +LL | +LL | fn bad2() -> impl use<> Into<::Assoc> {} + | ------------------------------------ lifetime captured due to being mentioned in the bounds of the `impl Trait` + +error: aborting due to 2 previous errors; 1 warning emitted + From 282488c80871191718c1ba6d04dcb53e83ee6de5 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Thu, 18 Apr 2024 17:17:47 -0400 Subject: [PATCH 6/6] Match hyphen in multi-revision comment matchers Currently, the matcher `//[rev-foo,rev-bar]~` does not get selected by the regex. Change the matcher to also match strings that contain a `-`.h --- src/tools/compiletest/src/errors.rs | 5 ++++- src/tools/compiletest/src/errors/tests.rs | 12 ++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 src/tools/compiletest/src/errors/tests.rs diff --git a/src/tools/compiletest/src/errors.rs b/src/tools/compiletest/src/errors.rs index 140c3aa0a64..3231f9fd3a4 100644 --- a/src/tools/compiletest/src/errors.rs +++ b/src/tools/compiletest/src/errors.rs @@ -118,7 +118,7 @@ fn parse_expected( // //[rev1]~ // //[rev1,rev2]~^^ static RE: Lazy = - Lazy::new(|| Regex::new(r"//(?:\[(?P[\w,]+)])?~(?P\||\^*)").unwrap()); + Lazy::new(|| Regex::new(r"//(?:\[(?P[\w\-,]+)])?~(?P\||\^*)").unwrap()); let captures = RE.captures(line)?; @@ -178,3 +178,6 @@ fn parse_expected( ); Some((which, Error { line_num, kind, msg })) } + +#[cfg(test)] +mod tests; diff --git a/src/tools/compiletest/src/errors/tests.rs b/src/tools/compiletest/src/errors/tests.rs new file mode 100644 index 00000000000..dfb001d8e39 --- /dev/null +++ b/src/tools/compiletest/src/errors/tests.rs @@ -0,0 +1,12 @@ +use super::*; + +#[test] +fn test_parse_expected_matching() { + // Ensure that we correctly extract expected revisions + let d1 = "//[rev1,rev2]~^ ERROR foo"; + let d2 = "//[rev1,rev2-foo]~^ ERROR foo"; + assert!(parse_expected(None, 1, d1, Some("rev1")).is_some()); + assert!(parse_expected(None, 1, d1, Some("rev2")).is_some()); + assert!(parse_expected(None, 1, d2, Some("rev1")).is_some()); + assert!(parse_expected(None, 1, d2, Some("rev2-foo")).is_some()); +}