diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 1a32958d3627b..e3b23f499a922 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -1238,6 +1238,122 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { } } + fn instrprof_mcdc_parameters( + &mut self, + fn_name: Self::Value, + hash: Self::Value, + num_bitmap_bytes: Self::Value, + ) { + debug!( + "instrprof_mcdc_parameters() with args ({:?}, {:?}, {:?})", + fn_name, hash, num_bitmap_bytes + ); + + let llfn = unsafe { llvm::LLVMRustGetInstrProfMCDCParametersIntrinsic(self.cx().llmod) }; + let llty = self.cx.type_func( + &[self.cx.type_ptr(), self.cx.type_i64(), self.cx.type_i32()], + self.cx.type_void(), + ); + let args = &[fn_name, hash, num_bitmap_bytes]; + let args = self.check_call("call", llty, llfn, args); + + unsafe { + let _ = llvm::LLVMRustBuildCall( + self.llbuilder, + llty, + llfn, + args.as_ptr() as *const &llvm::Value, + args.len() as c_uint, + [].as_ptr(), + 0 as c_uint, + ); + } + } + + fn instrprof_mcdc_condbitmap_update( + &mut self, + fn_name: Self::Value, + hash: Self::Value, + cond_id: Self::Value, + cond_bitmap_addr: Self::Value, + cond_result: Self::Value, + ) { + debug!( + "instrprof_mcdc_condbitmap_update() with args ({:?}, {:?}, {:?}, {:?}, {:?})", + fn_name, hash, cond_id, cond_bitmap_addr, cond_result + ); + + let llfn = + unsafe { llvm::LLVMRustGetInstrProfMCDCCondBitmapUpdateIntrinsic(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_id, cond_bitmap_addr, cond_result]; + let args = self.check_call("call", llty, llfn, args); + + unsafe { + let _ = llvm::LLVMRustBuildCall( + self.llbuilder, + llty, + llfn, + args.as_ptr(), + args.len() as c_uint, + [].as_ptr(), + 0 as c_uint, + ); + } + } + + fn instrprof_mcdc_tvbitmap_update( + &mut self, + fn_name: Self::Value, + hash: Self::Value, + needed_bytes: Self::Value, + bitmap_idx: Self::Value, + cond_bitmap_addr: Self::Value, + ) { + debug!( + "instrprof_mcdc_tvbitmap_update() with args ({:?}, {:?}, {:?}, {:?}, {:?})", + fn_name, hash, needed_bytes, bitmap_idx, cond_bitmap_addr + ); + + 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, needed_bytes, bitmap_idx, cond_bitmap_addr]; + let args = self.check_call("call", llty, llfn, args); + + unsafe { + let _ = llvm::LLVMRustBuildCall( + self.llbuilder, + llty, + llfn, + args.as_ptr(), + args.len() as c_uint, + [].as_ptr(), + 0 as c_uint, + ); + } + } + fn call( &mut self, llty: &'ll Type, diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index 2af28146a51ea..0e1d6af42820f 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -99,6 +99,36 @@ 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. + #[allow(dead_code)] + MCDCBranchRegion = 6, +} + +/// This struct provides LLVM's representation of "MCDCParameters" that may be defined for a +/// Coverage Mapping Region. +/// +/// Correspond to struct `llvm::coverage::CounterMappingRegion::MCDCParameters` +/// +/// Must match The layout of `LLVMRustMCDCParameters` +#[derive(Copy, Clone, Debug, Default)] +#[repr(C)] +pub struct MCDCParameters { + /// Byte Index of Bitmap Coverage Object for a Decision Region. + bitmap_idx: u32, + + /// Number of Conditions used for a Decision Region. + num_conditions: u32, + + /// IDs used to represent a branch region and other branch regions + /// evaluated based on True and False branches. + id: u32, + true_id: u32, + false_id: u32, } /// This struct provides LLVM's representation of a "CoverageMappingRegion", encoded into the @@ -146,6 +176,8 @@ pub struct CounterMappingRegion { end_col: u32, kind: RegionKind, + + mcdc_params: MCDCParameters, } impl CounterMappingRegion { @@ -172,6 +204,33 @@ impl CounterMappingRegion { start_col, end_line, end_col, + None, + ), + MappingKind::MCDCBranch { true_term, false_term, id, true_id, false_id } => { + Self::branch_region( + Counter::from_term(true_term), + Counter::from_term(false_term), + local_file_id, + start_line, + start_col, + end_line, + end_col, + Some(MCDCParameters { + id, + true_id, + false_id, + .. Default::default() + }), + ) + } + MappingKind::MCDCDecision { bitmap_idx, num_conditions } => Self::decision_region( + bitmap_idx, + num_conditions, + local_file_id, + start_line, + start_col, + end_line, + end_col, ), } } @@ -194,9 +253,11 @@ impl CounterMappingRegion { end_line, end_col, kind: RegionKind::CodeRegion, + mcdc_params: Default::default(), } } + /// - `mcdc_params` should be None when MCDC is disabled. pub(crate) fn branch_region( counter: Counter, false_counter: Counter, @@ -205,7 +266,13 @@ impl CounterMappingRegion { start_col: u32, end_line: u32, end_col: u32, + mcdc_params: Option, ) -> Self { + let (kind, mcdc_params) = match mcdc_params { + None => (RegionKind::BranchRegion, Default::default()), + Some(params) => (RegionKind::MCDCBranchRegion, params), + }; + Self { counter, false_counter, @@ -215,7 +282,31 @@ impl CounterMappingRegion { start_col, end_line, end_col, - kind: RegionKind::BranchRegion, + kind, + mcdc_params, + } + } + + pub(crate) fn decision_region( + bitmap_idx: u32, + num_conditions: u32, + file_id: u32, + start_line: u32, + start_col: u32, + end_line: u32, + end_col: u32, + ) -> Self { + Self { + counter: Counter::ZERO, + false_counter: Counter::ZERO, + file_id, + expanded_file_id: 0, + start_line, + start_col, + end_line, + end_col, + kind: RegionKind::MCDCDecisionRegion, + mcdc_params: MCDCParameters { bitmap_idx, num_conditions, ..Default::default() }, } } @@ -240,6 +331,7 @@ impl CounterMappingRegion { end_line, end_col, kind: RegionKind::ExpansionRegion, + mcdc_params: Default::default(), } } @@ -263,6 +355,7 @@ impl CounterMappingRegion { end_line, end_col, kind: RegionKind::SkippedRegion, + mcdc_params: Default::default(), } } @@ -287,6 +380,7 @@ impl CounterMappingRegion { end_line, end_col: (1_u32 << 31) | end_col, kind: RegionKind::GapRegion, + mcdc_params: Default::default(), } } } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 278db21b0a1f2..730a65e476488 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -31,10 +31,10 @@ use rustc_span::Symbol; pub fn finalize(cx: &CodegenCx<'_, '_>) { let tcx = cx.tcx; - // Ensure the installed version of LLVM supports Coverage Map Version 6 - // (encoded as a zero-based value: 5), which was introduced with LLVM 13. + // Ensure the installed version of LLVM supports Coverage Map Version 7 + // (encoded as a zero-based value: 6), which was introduced with LLVM 13. let version = coverageinfo::mapping_version(); - assert_eq!(version, 5, "The `CoverageMappingVersion` exposed by `llvm-wrapper` is out of sync"); + assert_eq!(version, 6, "The `CoverageMappingVersion` exposed by `llvm-wrapper` is out of sync"); debug!("Generating coverage map for CodegenUnit: `{}`", cx.codegen_unit.name()); @@ -276,6 +276,7 @@ fn encode_mappings_for_function( /// Construct coverage map header and the array of function records, and combine them into the /// coverage map. Save the coverage map data into the LLVM IR as a static global using a /// specific, well-known section and name. +/// https://llvm.org/docs/CoverageMappingFormat.html#llvm-ir-representation fn generate_coverage_map<'ll>( cx: &CodegenCx<'ll, '_>, version: u32, diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 68c1770c1d4a1..75765e53461b7 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -32,6 +32,10 @@ pub struct CrateCoverageContext<'ll, 'tcx> { pub(crate) function_coverage_map: RefCell, FunctionCoverageCollector<'tcx>>>, pub(crate) pgo_func_name_var_map: RefCell, &'ll llvm::Value>>, + + /// When MCDC is enabled, holds references to the stack-allocated function-wise + /// condition bitmaps. + pub(crate) mcdc_condbitmap_map: RefCell, &'ll llvm::Value>>, } impl<'ll, 'tcx> CrateCoverageContext<'ll, 'tcx> { @@ -39,6 +43,7 @@ impl<'ll, 'tcx> CrateCoverageContext<'ll, 'tcx> { Self { function_coverage_map: Default::default(), pgo_func_name_var_map: Default::default(), + mcdc_condbitmap_map: Default::default(), } } @@ -99,7 +104,12 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { .or_insert_with(|| FunctionCoverageCollector::new(instance, function_coverage_info)); match *kind { - CoverageKind::SpanMarker | CoverageKind::BlockMarker { .. } => unreachable!( + CoverageKind::SpanMarker + | CoverageKind::BlockMarker { .. } + | CoverageKind::MCDCDecisionEntryMarker { .. } + | CoverageKind::MCDCDecisionOutputMarker { .. } + | CoverageKind::MCDCConditionEntryMarker { .. } + | CoverageKind::MCDCConditionOutputMarker { .. } => unreachable!( "marker statement {kind:?} should have been removed by CleanupPostBorrowck" ), CoverageKind::CounterIncrement { id } => { @@ -133,6 +143,111 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { CoverageKind::ExpressionUsed { id } => { func_coverage.mark_expression_id_seen(id); } + CoverageKind::MCDCBitmapRequire { needed_bytes } => { + // We need to explicitly drop the `RefMut` before calling into `instrprof_mcdc_parameters`, + // as that needs an exclusive borrow. + drop(coverage_map); + + let fn_name = bx.get_pgo_func_name_var(instance); + let hash = bx.const_u64(function_coverage_info.function_source_hash); + let num_bitmap_bytes = bx.const_u32(needed_bytes); + debug!( + "codegen intrinsic instrprof.mcdc.parameters(fn_name={:?}, hash={:?}, bitmap_bytes={:?})", + fn_name, hash, num_bitmap_bytes, + ); + // Call the intrinsic to ask LLVM to allocate a global variable for the + // test vector bitmaps in the function body. + bx.instrprof_mcdc_parameters(fn_name, hash, num_bitmap_bytes); + + // Allocates an integer in the function stackframe that will be + // used for the condition bitmaps of the decisions. + let cond_bitmap_addr = bx.alloca( + bx.type_uint_from_ty(rustc_middle::ty::UintTy::U32), + Align::from_bytes(4).expect("4 bytes alignment failed"), + ); + + // Acquire a new handle to coverage_context. + let coverage_context = bx.coverage_context().expect("Presence checked"); + coverage_context + .mcdc_condbitmap_map + .borrow_mut() + .insert(instance, cond_bitmap_addr); + } + CoverageKind::MCDCCondBitmapReset => { + // Drop unused coverage_map, which prevents us to use bx. + drop(coverage_map); + + let cond_bitmap_addr = match coverage_context + .mcdc_condbitmap_map + .borrow() + .get(&instance) + { + Some(addr) => *addr, + None => bug!( + "Condition bitmap reset instruction was encountered without a condition bitmap being declared." + ), + }; + + bx.store( + bx.const_i32(0), + cond_bitmap_addr, + Align::from_bytes(4).expect("4 bytes alignement failed"), + ); + } + CoverageKind::MCDCCondBitmapUpdate { condition_id, bool_value } => { + // Drop unused coverage_map, which prevents us to use bx. + drop(coverage_map); + + let fn_name = bx.get_pgo_func_name_var(instance); + let hash = bx.const_u64(function_coverage_info.function_source_hash); + let cond_id = bx.const_u32(condition_id); + let cond_bitmap_addr = match coverage_context + .mcdc_condbitmap_map + .borrow() + .get(&instance) + { + Some(addr) => *addr, + None => bug!( + "Condition bitmap reset instruction was encountered without a condition bitmap being declared." + ), + }; + let cond_result = bx.const_bool(bool_value); + + bx.instrprof_mcdc_condbitmap_update( + fn_name, + hash, + cond_id, + cond_bitmap_addr, + cond_result, + ) + } + CoverageKind::MCDCTestVectorBitmapUpdate { needed_bytes, decision_index } => { + // Drop unused coverage_map, which prevents us to use bx. + drop(coverage_map); + + let fn_name = bx.get_pgo_func_name_var(instance); + let hash = bx.const_u64(function_coverage_info.function_source_hash); + let needed_bytes = bx.const_u32(needed_bytes); + let bitmap_idx = bx.const_u32(decision_index); + let cond_bitmap_addr = match coverage_context + .mcdc_condbitmap_map + .borrow() + .get(&instance) + { + Some(addr) => *addr, + None => bug!( + "Condition bitmap reset instruction was encountered without a condition bitmap being declared." + ), + }; + + bx.instrprof_mcdc_tvbitmap_update( + fn_name, + hash, + needed_bytes, + bitmap_idx, + cond_bitmap_addr, + ) + } } } } @@ -278,3 +393,13 @@ pub(crate) fn covfun_section_name(cx: &CodegenCx<'_, '_>) -> String { }) .expect("Rust Coverage function record section name failed UTF-8 conversion") } + +/// Returns the section name for the MC/DC Bitmap section passed to the linker. +/// FIXME: Remove allow deadcode +#[allow(dead_code)] +pub(crate) fn prf_bits_section_name(cx: &CodegenCx<'_, '_>) -> String { + llvm::build_string(|s| unsafe { + llvm::LLVMRustCoverageWriteBitmapSectionNameToString(cx.llmod, s); + }) + .expect("Rust Coverage function record section name failed UTF-8 conversion") +} diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 284bc74d5c434..7f55339dd6c08 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -1631,6 +1631,9 @@ extern "C" { // Miscellaneous instructions pub fn LLVMRustGetInstrProfIncrementIntrinsic(M: &Module) -> &Value; + pub fn LLVMRustGetInstrProfMCDCParametersIntrinsic(M: &Module) -> &Value; + pub fn LLVMRustGetInstrProfMCDCCondBitmapUpdateIntrinsic(M: &Module) -> &Value; + pub fn LLVMRustGetInstrProfMCDCTVBitmapUpdateIntrinsic(M: &Module) -> &Value; pub fn LLVMRustBuildCall<'a>( B: &Builder<'a>, Ty: &'a Type, @@ -1788,6 +1791,9 @@ extern "C" { #[allow(improper_ctypes)] pub fn LLVMRustCoverageWriteFuncSectionNameToString(M: &Module, Str: &RustString); + #[allow(improper_ctypes)] + pub fn LLVMRustCoverageWriteBitmapSectionNameToString(M: &Module, Str: &RustString); + #[allow(improper_ctypes)] pub fn LLVMRustCoverageWriteMappingVarNameToString(Str: &RustString); diff --git a/compiler/rustc_codegen_ssa/src/traits/builder.rs b/compiler/rustc_codegen_ssa/src/traits/builder.rs index 6c8dcc5b69001..5333346fe629e 100644 --- a/compiler/rustc_codegen_ssa/src/traits/builder.rs +++ b/compiler/rustc_codegen_ssa/src/traits/builder.rs @@ -382,6 +382,31 @@ pub trait BuilderMethods<'a, 'tcx>: index: Self::Value, ); + fn instrprof_mcdc_parameters( + &mut self, + fn_name: Self::Value, + hash: Self::Value, + num_bitmap_bytes: Self::Value + ); + + fn instrprof_mcdc_condbitmap_update( + &mut self, + fn_name: Self::Value, + hash: Self::Value, + cond_id: Self::Value, + cond_bitmap_addr: Self::Value, + cond_result: Self::Value, + ); + + fn instrprof_mcdc_tvbitmap_update( + &mut self, + fn_name: Self::Value, + hash: Self::Value, + needed_bytes: Self::Value, + bitmap_idx: Self::Value, + cond_bitmap_addr: Self::Value, + ); + fn call( &mut self, llty: Self::Type, diff --git a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp index 60789b07e54ea..61ce73e14a75c 100644 --- a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp @@ -43,6 +43,8 @@ enum class LLVMRustCounterMappingRegionKind { SkippedRegion = 2, GapRegion = 3, BranchRegion = 4, + MCDCDecisionRegion = 5, + MCDCBranchRegion = 6, }; static coverage::CounterMappingRegion::RegionKind @@ -58,10 +60,38 @@ fromRust(LLVMRustCounterMappingRegionKind Kind) { return coverage::CounterMappingRegion::GapRegion; case LLVMRustCounterMappingRegionKind::BranchRegion: return coverage::CounterMappingRegion::BranchRegion; + case LLVMRustCounterMappingRegionKind::MCDCDecisionRegion: + return coverage::CounterMappingRegion::MCDCDecisionRegion; + case LLVMRustCounterMappingRegionKind::MCDCBranchRegion: + return coverage::CounterMappingRegion::MCDCBranchRegion; } report_fatal_error("Bad LLVMRustCounterMappingRegionKind!"); } +// FFI equivalent of struct `llvm::coverage::CounterMappingRegion::MCDCParameters` +// https://github.com/rust-lang/llvm-project/blob/66a2881a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L253-L263 +struct LLVMRustMCDCParameters { + /// Byte Index of Bitmap Coverage Object for a Decision Region. + uint32_t BitmapIdx = 0; + + /// Number of Conditions used for a Decision Region. + uint32_t NumConditions = 0; + + /// IDs used to represent a branch region and other branch regions + /// evaluated based on True and False branches. + uint32_t ID = 0; + uint32_t TrueID = 0; + uint32_t FalseID = 0; +}; + +static coverage::CounterMappingRegion::MCDCParameters +fromRust(LLVMRustMCDCParameters MCDCParams) { + return coverage::CounterMappingRegion::MCDCParameters{ + MCDCParams.BitmapIdx, MCDCParams.NumConditions, + MCDCParams.ID, MCDCParams.TrueID, MCDCParams.FalseID + }; +} + // 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 { @@ -74,6 +104,7 @@ struct LLVMRustCounterMappingRegion { uint32_t LineEnd; uint32_t ColumnEnd; LLVMRustCounterMappingRegionKind Kind; + LLVMRustMCDCParameters MCDCParams; }; // FFI equivalent of enum `llvm::coverage::CounterExpression::ExprKind` @@ -140,7 +171,7 @@ extern "C" void LLVMRustCoverageWriteMappingToBuffer( MappingRegions.emplace_back( fromRust(Region.Count), fromRust(Region.FalseCount), #if LLVM_VERSION_GE(18, 0) && LLVM_VERSION_LT(19, 0) - coverage::CounterMappingRegion::MCDCParameters{}, + fromRust(Region.MCDCParams), #endif Region.FileID, Region.ExpandedFileID, Region.LineStart, Region.ColumnStart, Region.LineEnd, Region.ColumnEnd, @@ -198,6 +229,11 @@ extern "C" void LLVMRustCoverageWriteFuncSectionNameToString(LLVMModuleRef M, WriteSectionNameToString(M, IPSK_covfun, Str); } +extern "C" void LLVMRustCoverageWriteBitmapSectionNameToString(LLVMModuleRef M, + RustStringRef Str) { + WriteSectionNameToString(M, IPSK_bitmap, Str); +} + extern "C" void LLVMRustCoverageWriteMappingVarNameToString(RustStringRef Str) { auto name = getCoverageMappingVarName(); auto OS = RawRustStringOstream(Str); @@ -205,5 +241,5 @@ extern "C" void LLVMRustCoverageWriteMappingVarNameToString(RustStringRef Str) { } extern "C" uint32_t LLVMRustCoverageMappingVersion() { - return coverage::CovMapVersion::Version6; + return coverage::CovMapVersion::Version7; } diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 8ec1f5a99e7ca..25d54bd466753 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1528,6 +1528,21 @@ extern "C" LLVMValueRef LLVMRustGetInstrProfIncrementIntrinsic(LLVMModuleRef M) (llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_increment)); } +extern "C" LLVMValueRef LLVMRustGetInstrProfMCDCParametersIntrinsic(LLVMModuleRef M) { + return wrap(llvm::Intrinsic::getDeclaration(unwrap(M), + (llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_mcdc_parameters)); +} + +extern "C" LLVMValueRef LLVMRustGetInstrProfMCDCCondBitmapUpdateIntrinsic(LLVMModuleRef M) { + return wrap(llvm::Intrinsic::getDeclaration(unwrap(M), + (llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_mcdc_condbitmap_update)); +} + +extern "C" LLVMValueRef LLVMRustGetInstrProfMCDCTVBitmapUpdateIntrinsic(LLVMModuleRef M) { + return wrap(llvm::Intrinsic::getDeclaration(unwrap(M), + (llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_mcdc_tvbitmap_update)); +} + extern "C" LLVMValueRef LLVMRustBuildMemCpy(LLVMBuilderRef B, LLVMValueRef Dst, unsigned DstAlign, LLVMValueRef Src, unsigned SrcAlign, diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 588aa1f40d79c..1ce7372d94a20 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -15,6 +15,37 @@ rustc_index::newtype_index! { pub struct BlockMarkerId {} } +// DecisionMarkerId and ConditionMarkerId are used between THIR and MIR representations. +// DecisionId and ConditionId are used between MIR representation and LLVM-IR. + +rustc_index::newtype_index! { + #[derive(HashStable)] + #[encodable] + #[debug_format = "DecisionMarkerId({})"] + pub struct DecisionMarkerId {} +} + +rustc_index::newtype_index! { + #[derive(HashStable)] + #[encodable] + #[debug_format = "ConditionMarkerId({})"] + pub struct ConditionMarkerId {} +} + +rustc_index::newtype_index! { + #[derive(HashStable)] + #[encodable] + #[debug_format = "DecisionId({})"] + pub struct DecisionId {} +} + +rustc_index::newtype_index! { + #[derive(HashStable)] + #[encodable] + #[debug_format = "ConditionId({})"] + pub struct ConditionId {} +} + rustc_index::newtype_index! { /// ID of a coverage counter. Values ascend from 0. /// @@ -97,6 +128,36 @@ pub enum CoverageKind { /// Should be erased before codegen (at some point after `InstrumentCoverage`). BlockMarker { id: BlockMarkerId }, + /// Marks the first condition of a decision (boolean expression). All + /// conditions in the same decision will reference this id. + /// + /// Has no effect during codegen. + MCDCDecisionEntryMarker { decm_id: DecisionMarkerId }, + + /// Marks one of the basic blocks following the decision referenced with `id`. + /// the outcome bool designates which branch of the decision it is: + /// `true` for the then block, `false` for the else block. + /// + /// Has no effect during codegen. + MCDCDecisionOutputMarker { decm_id: DecisionMarkerId, outcome: bool }, + + /// Marks a basic block where a condition evaluation occurs + /// The block may end with a SwitchInt where the 2 successors BBs have a + /// MCDCConditionOutcomeMarker statement with a matching ID. + /// + /// Has no effect during codegen. + MCDCConditionEntryMarker { decm_id: DecisionMarkerId, condm_id: ConditionMarkerId }, + + /// Marks a basic block that is branched from a condition evaluation. + /// The block may have a predecessor with a matching ID. + /// + /// Has no effect during codegen. + MCDCConditionOutputMarker { + decm_id: DecisionMarkerId, + condm_id: ConditionMarkerId, + outcome: bool, + }, + /// Marks the point in MIR control flow represented by a coverage counter. /// /// This is eventually lowered to `llvm.instrprof.increment` in LLVM IR. @@ -114,6 +175,22 @@ pub enum CoverageKind { /// mappings. Intermediate expressions with no direct mappings are /// retained/zeroed based on whether they are transitively used.) ExpressionUsed { id: ExpressionId }, + + /// Declares the number of bytes needed to store the test-vector bitmaps of + /// all the decisions in the function body. + /// + /// In LLVM backend, this is done by inserting a call to the + /// `instrprof.mcdc.parameters` intrinsic. + MCDCBitmapRequire { needed_bytes: u32 }, + + /// Marks a point where the condition bitmap should be set to 0. + MCDCCondBitmapReset, + + /// Marks a point where a bit of the condition bitmap should be set. + MCDCCondBitmapUpdate { condition_id: u32, bool_value: bool }, + + /// Marks a point where a bit of the global Test Vector bitmap should be set to one. + MCDCTestVectorBitmapUpdate { needed_bytes: u32, decision_index: u32 }, } impl Debug for CoverageKind { @@ -122,8 +199,38 @@ impl Debug for CoverageKind { match self { SpanMarker => write!(fmt, "SpanMarker"), BlockMarker { id } => write!(fmt, "BlockMarker({:?})", id.index()), + MCDCDecisionEntryMarker { decm_id: id } => { + write!(fmt, "MCDCDecisionEntryMarker({:?})", id.index()) + } + MCDCDecisionOutputMarker { decm_id: id, outcome } => { + write!(fmt, "MCDCDecisionOutputMarker({:?}, {})", id.index(), outcome) + } + MCDCConditionEntryMarker { decm_id: decision_id, condm_id: id } => { + write!(fmt, "MCDCConditionEntryMarker({:?}, {:?})", decision_id.index(), id.index()) + } + MCDCConditionOutputMarker { decm_id: decision_marker_id, condm_id: id, outcome } => { + write!( + fmt, + "MCDCConditionOutcomeMarker({:?}, {:?}, {})", + decision_marker_id.index(), + id.index(), + outcome + ) + } CounterIncrement { id } => write!(fmt, "CounterIncrement({:?})", id.index()), ExpressionUsed { id } => write!(fmt, "ExpressionUsed({:?})", id.index()), + MCDCBitmapRequire { needed_bytes } => { + write!(fmt, "MCDCBitmapRequire({needed_bytes} bytes)") + } + MCDCCondBitmapReset => { + write!(fmt, "MCDCCondBitmapReset()") + } + MCDCCondBitmapUpdate { condition_id, bool_value } => { + write!(fmt, "MCDCCondBitmapUpdate({condition_id}, {bool_value})") + } + MCDCTestVectorBitmapUpdate { needed_bytes, decision_index } => { + write!(fmt, "MCDCTVBitmapUpdate({needed_bytes} bytes, {decision_index})") + } } } } @@ -180,16 +287,23 @@ pub enum MappingKind { Code(CovTerm), /// Associates a branch region with separate counters for true and false. Branch { true_term: CovTerm, false_term: CovTerm }, + /// FIXME(dprn): doc + MCDCBranch { true_term: CovTerm, false_term: CovTerm, id: u32, true_id: u32, false_id: u32 }, + /// Associates an MCDC Decision with + MCDCDecision { bitmap_idx: u32, num_conditions: u32 }, } 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 = || std::iter::empty().chain(None).chain(None); + let one = |a| std::iter::empty().chain(Some(a)).chain(None); + let two = |a, b| std::iter::empty().chain(Some(a)).chain(Some(b)); match *self { Self::Code(term) => one(term), Self::Branch { true_term, false_term } => two(true_term, false_term), + Self::MCDCBranch { .. } => zero(), + Self::MCDCDecision { .. } => zero(), } } @@ -201,6 +315,7 @@ impl MappingKind { Self::Branch { true_term, false_term } => { Self::Branch { true_term: map_fn(true_term), false_term: map_fn(false_term) } } + Self::MCDCBranch { .. } | Self::MCDCDecision { .. } => self.clone(), } } } @@ -234,12 +349,29 @@ pub struct BranchInfo { /// data structures without having to scan the entire body first. pub num_block_markers: usize, pub branch_spans: Vec, + + /// Associate a span for every decision in the function body. + /// Empty if MCDC coverage is disabled. + pub decision_spans: IndexVec, +} + +#[derive(Clone, Debug)] +#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] +pub struct DecisionSpan { + /// Source code region associated to the decision. + pub span: Span, + /// Number of conditions in the decision. + pub num_conditions: u32, } #[derive(Clone, Debug)] #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub struct BranchSpan { + /// Source code region associated to the branch. pub span: Span, + /// ID of Decision structure the branch is part of. Only used in + /// the MCDC coverage. + pub decision_id: DecisionMarkerId, pub true_marker: BlockMarkerId, pub false_marker: BlockMarkerId, } diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 41df2e3b5875a..30d31de2667e5 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -475,14 +475,33 @@ 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, decision_spans, .. } = branch_info; - for coverage::BranchSpan { span, true_marker, false_marker } in branch_spans { + // Write MCDC stuff as well if present + for (id, coverage::DecisionSpan { span, num_conditions }) in decision_spans.iter_enumerated() { writeln!( w, - "{INDENT}coverage branch {{ true: {true_marker:?}, false: {false_marker:?} }} => {span:?}", + "{INDENT}coverage MCDC decision {{ id: {id:?}, num_conditions: {num_conditions:?} }} => {span:?}", )?; } + + if !decision_spans.is_empty() { + writeln!(w)?; + for coverage::BranchSpan { span, true_marker, false_marker, decision_id } in branch_spans { + writeln!( + w, + "{INDENT}coverage branch {{ decision: {decision_id:?}, true: {true_marker:?}, false: {false_marker:?} }} => {span:?}", + )?; + } + } else { + for coverage::BranchSpan { span, true_marker, false_marker, .. } in branch_spans { + writeln!( + w, + "{INDENT}coverage branch {{ true: {true_marker:?}, false: {false_marker:?} }} => {span:?}", + )?; + } + } + if !branch_spans.is_empty() { writeln!(w)?; } diff --git a/compiler/rustc_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs index a62379def534e..6ba4ecfd2b560 100644 --- a/compiler/rustc_middle/src/ty/structural_impls.rs +++ b/compiler/rustc_middle/src/ty/structural_impls.rs @@ -407,6 +407,10 @@ TrivialTypeTraversalImpls! { ::rustc_hir::MatchSource, ::rustc_target::asm::InlineAsmRegOrRegClass, crate::mir::coverage::BlockMarkerId, + crate::mir::coverage::DecisionMarkerId, + crate::mir::coverage::ConditionMarkerId, + crate::mir::coverage::DecisionId, + crate::mir::coverage::ConditionId, crate::mir::coverage::CounterId, crate::mir::coverage::ExpressionId, crate::mir::Local, diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index 1de691f32a70c..f332abf54208c 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -200,6 +200,8 @@ mir_build_lower_range_bound_must_be_less_than_or_equal_to_upper = mir_build_lower_range_bound_must_be_less_than_upper = lower range bound must be less than upper +mir_build_mcdc_nested_decision = Unsupported MCDC Decision: Nested decision. Expression will not be corevered. + mir_build_more_information = for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html mir_build_moved = value is moved into `{$name}` here diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index ab0043906b19f..5da880edcd93e 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -2,13 +2,66 @@ use std::assert_matches::assert_matches; use std::collections::hash_map::Entry; use rustc_data_structures::fx::FxHashMap; -use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind}; +use rustc_index::IndexVec; +use rustc_middle::mir::coverage::{ + BlockMarkerId, BranchSpan, ConditionMarkerId, CoverageKind, DecisionMarkerId, DecisionSpan, +}; use rustc_middle::mir::{self, BasicBlock, UnOp}; use rustc_middle::thir::{ExprId, ExprKind, Thir}; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::LocalDefId; +use rustc_span::Span; use crate::build::Builder; +use crate::errors::MCDCNestedDecision; + +struct MCDCInfoBuilder { + /// ID of the current decision. + /// Do not use directly. Use the function instead, as it will hide + /// the decision in the scope of nested decisions. + current_decision_id: Option, + /// Track the nesting level of decision to avoid MCDC instrumentation of + /// nested decisions. + nested_decision_level: u32, + /// Vector for storing all the decisions with their span + decision_spans: IndexVec, + + next_condition_id: u32, +} + +impl MCDCInfoBuilder { + /// Increase the nested decision level and return true if the + /// decision can be instrumented (not in a nested condition). + pub fn enter_decision(&mut self, span: Span) -> bool { + self.nested_decision_level += 1; + let can_mcdc = !self.in_nested_condition(); + + if can_mcdc { + self.current_decision_id = Some(self.decision_spans.push(span)); + } + + can_mcdc + } + + pub fn exit_decision(&mut self) { + self.nested_decision_level -= 1; + } + + /// Return true if the current decision is located inside another decision. + pub fn in_nested_condition(&self) -> bool { + self.nested_decision_level > 1 + } + + pub fn current_decision_id(&self) -> Option { + if self.in_nested_condition() { None } else { self.current_decision_id } + } + + pub fn next_condition_id(&mut self) -> u32 { + let res = self.next_condition_id; + self.next_condition_id += 1; + res + } +} pub(crate) struct BranchInfoBuilder { /// Maps condition expressions to their enclosing `!`, for better instrumentation. @@ -16,6 +69,9 @@ pub(crate) struct BranchInfoBuilder { num_block_markers: usize, branch_spans: Vec, + + // MCDC decision stuff + mcdc_info: Option, } #[derive(Clone, Copy)] @@ -32,8 +88,25 @@ impl BranchInfoBuilder { /// Creates a new branch info builder, but only if branch coverage instrumentation /// 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![] }) + if (tcx.sess.instrument_coverage_branch() || tcx.sess.instrument_coverage_mcdc()) + && tcx.is_eligible_for_coverage(def_id) + { + let mcdc_info = if tcx.sess.instrument_coverage_mcdc() { + Some(MCDCInfoBuilder { + current_decision_id: None, + nested_decision_level: 0, + decision_spans: IndexVec::new(), + next_condition_id: 0, + }) + } else { + None + }; + Some(Self { + nots: FxHashMap::default(), + num_block_markers: 0, + branch_spans: vec![], + mcdc_info, + }) } else { None } @@ -86,14 +159,32 @@ 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_info, .. } = self; if num_block_markers == 0 { assert!(branch_spans.is_empty()); return None; } - Some(Box::new(mir::coverage::BranchInfo { num_block_markers, branch_spans })) + let mut decision_spans = IndexVec::from_iter( + mcdc_info + .map_or(IndexVec::new(), |mcdc_info| mcdc_info.decision_spans) + .into_iter() + .map(|span| DecisionSpan { span, num_conditions: 0 }), + ); + + // Count the number of conditions linked to each decision. + if !decision_spans.is_empty() { + for branch_span in branch_spans.iter() { + decision_spans[branch_span.decision_id].num_conditions += 1; + } + } + + Some(Box::new(mir::coverage::BranchInfo { + num_block_markers, + branch_spans, + decision_spans, + })) } } @@ -139,8 +230,134 @@ impl Builder<'_, '_> { branch_info.branch_spans.push(BranchSpan { span: source_info.span, + // FIXME(dprn): Handle case when MCDC is disabled better than just putting 0. + decision_id: branch_info + .mcdc_info + .as_ref() + .and_then(|mcdc_info| mcdc_info.current_decision_id()) + .unwrap_or(DecisionMarkerId::from_u32(0)), true_marker, false_marker, }); } + + /// If MCDC coverage is enabled, inject a decision entry marker in the given decision. + /// return true + pub(crate) fn begin_mcdc_decision_coverage(&mut self, expr_id: ExprId, block: BasicBlock) { + // Get the MCDCInfoBuilder object, which existence implies that MCDC is enabled. + let Some(BranchInfoBuilder { mcdc_info: Some(mcdc_info), .. }) = + self.coverage_branch_info.as_mut() + else { + return; + }; + + let span = self.thir[expr_id].span; + + // enter_decision returns false if it detects nested decisions. + if !mcdc_info.enter_decision(span) { + // FIXME(dprn): WARNING for nested decision does not seem to work properly + debug!("MCDC: Unsupported nested decision"); + self.tcx.dcx().emit_warn(MCDCNestedDecision { span }); + return; + } + + let decision_id = mcdc_info.current_decision_id().expect("Should have returned."); + + // Inject a decision marker + let source_info = self.source_info(span); + let marker_statement = mir::Statement { + source_info, + kind: mir::StatementKind::Coverage(CoverageKind::MCDCDecisionEntryMarker { + decm_id: decision_id, + }), + }; + self.cfg.push(block, marker_statement); + } + + /// If MCDC is enabled, and function is instrumented, + pub(crate) fn end_mcdc_decision_coverage(&mut self) { + // Get the MCDCInfoBuilder object, which existence implies that MCDC is enabled. + let Some(BranchInfoBuilder { mcdc_info: Some(mcdc_info), .. }) = + self.coverage_branch_info.as_mut() + else { + return; + }; + + // Exit decision now so we can drop &mut to branch info + mcdc_info.exit_decision(); + } + + /// If MCDC is enabled and the current decision is being instrumented, + /// inject an `MCDCDecisionOutputMarker` to the given basic block. + /// `outcome` should be true for the then block and false for the else block. + pub(crate) fn mcdc_decision_outcome_block(&mut self, bb: BasicBlock, outcome: bool) { + // Get the MCDCInfoBuilder object, which existence implies that MCDC is enabled. + let Some(BranchInfoBuilder { mcdc_info: Some(mcdc_info), .. }) = + self.coverage_branch_info.as_mut() + else { + return; + }; + + let Some(decision_id) = mcdc_info.current_decision_id() else { + // Decision is not instrumented + return; + }; + + let span = mcdc_info.decision_spans[decision_id]; + let source_info = self.source_info(span); + let marker_statement = mir::Statement { + source_info, + kind: mir::StatementKind::Coverage(CoverageKind::MCDCDecisionOutputMarker { + decm_id: decision_id, + outcome, + }), + }; + + // Insert statements at the beginning of the following basic block + self.cfg.block_data_mut(bb).statements.insert(0, marker_statement); + } + + /// Add markers on the condition's basic blocks to ease the later MCDC instrumentation. + pub(crate) fn visit_mcdc_condition( + &mut self, + condition_expr: ExprId, + condition_block: BasicBlock, + then_block: BasicBlock, + else_block: BasicBlock, + ) { + // Get the MCDCInfoBuilder object, which existence implies that MCDC is enabled. + let Some(BranchInfoBuilder { mcdc_info: Some(mcdc_info), .. }) = + self.coverage_branch_info.as_mut() + else { + return; + }; + + let Some(decm_id) = mcdc_info.current_decision_id() else { + // If current_decision_id() is None, the decision is not instrumented. + return; + }; + + let condm_id = ConditionMarkerId::from_u32(mcdc_info.next_condition_id()); + let span = self.thir[condition_expr].span; + let source_info = self.source_info(span); + + let mut inject_statement = |bb, cov_kind| { + let statement = + mir::Statement { source_info, kind: mir::StatementKind::Coverage(cov_kind) }; + self.cfg.basic_blocks[bb].statements.insert(0, statement); + }; + + inject_statement( + condition_block, + CoverageKind::MCDCConditionEntryMarker { decm_id, condm_id }, + ); + inject_statement( + then_block, + CoverageKind::MCDCConditionOutputMarker { decm_id, condm_id, outcome: true }, + ); + inject_statement( + else_block, + CoverageKind::MCDCConditionOutputMarker { decm_id, condm_id, outcome: false }, + ); + } } diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index b4eeeccc127b4..b7b00cdf903bb 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -76,6 +76,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.source_info(then_span) }; + // If MCDC is enabled AND we are not in a nested decision, + // Mark the decision's root, true and false outcomes. + this.begin_mcdc_decision_coverage(expr_id, block); + // Lower the condition, and have it branch into `then` and `else` blocks. let (then_block, else_block) = this.in_if_then_scope(condition_scope, then_span, |this| { @@ -87,10 +91,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { true, // Declare `let` bindings normally )); + // If MCDC is enabled, inject an outcome marker. + this.mcdc_decision_outcome_block(then_blk, true); + // Lower the `then` arm into its block. this.expr_into_dest(destination, then_blk, then) }); + // If MCDC is enabled and decision was instrumented, exit the + // decision scope and inject an MCDC decision output marker + // in the then and else blocks. + this.mcdc_decision_outcome_block(else_block, false); + this.end_mcdc_decision_coverage(); + // Pack `(then_block, else_block)` into `BlockAnd`. then_block.and(else_block) }, diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index a6b513ce7d065..d89c52731469a 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -58,10 +58,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { variable_source_info: SourceInfo, declare_let_bindings: bool, ) -> BlockAnd<()> { + self.then_else_break_inner( block, expr_id, - ThenElseArgs { temp_scope_override, variable_source_info, declare_let_bindings }, + ThenElseArgs { + temp_scope_override, + variable_source_info, + declare_let_bindings, + }, ) } @@ -159,7 +164,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Record branch coverage info for this condition. // (Does nothing if branch coverage is not enabled.) - this.visit_coverage_branch_condition(expr_id, then_block, else_block); + this.visit_coverage_branch_condition( + expr_id, + then_block, + else_block, + ); + // Record MCDC coverage info + // (Does nothing if mcdc coverage is not enabled.) + this.visit_mcdc_condition( + expr_id, + block, + then_block, + else_block, + ); let source_info = this.source_info(expr_span); this.cfg.terminate(block, source_info, term); diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 848da56f98168..a31caa14243c0 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -850,6 +850,13 @@ pub(crate) struct PatternNotCovered<'s, 'tcx> { pub misc_suggestion: Option, } +#[derive(Diagnostic)] +#[diag(mir_build_mcdc_nested_decision)] +pub(crate) struct MCDCNestedDecision { + #[primary_span] + pub span: Span, +} + #[derive(Subdiagnostic)] #[note(mir_build_inform_irrefutable)] #[note(mir_build_more_information)] diff --git a/compiler/rustc_mir_transform/messages.ftl b/compiler/rustc_mir_transform/messages.ftl index b8dbdf18db3ea..1af73d1314258 100644 --- a/compiler/rustc_mir_transform/messages.ftl +++ b/compiler/rustc_mir_transform/messages.ftl @@ -34,6 +34,8 @@ mir_transform_mutation_layout_constrained_borrow_label = borrow of layout constr mir_transform_mutation_layout_constrained_borrow_note = references to fields of layout constrained fields lose the constraints. Coupled with interior mutability, the field can be changed to invalid values mir_transform_mutation_layout_constrained_label = mutation of layout constrained field mir_transform_mutation_layout_constrained_note = mutating layout constrained fields cannot statically be checked for valid values +mir_transform_mcdc_too_many_conditions = Unsupported MCDC Decision: Number of conditions ({$num_conditions}) exceeds max ({$max_conditions}). Expression will be skipped. + mir_transform_operation_will_panic = this operation will panic at runtime mir_transform_requires_unsafe = {$details} is unsafe and requires unsafe {$op_in_unsafe_fn_allowed -> diff --git a/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs b/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs index da82f8de78147..f3e4b912da2e1 100644 --- a/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs +++ b/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs @@ -5,7 +5,7 @@ //! - [`AscribeUserType`] //! - [`FakeRead`] //! - [`Assign`] statements with a [`Fake`] borrow -//! - [`Coverage`] statements of kind [`BlockMarker`] or [`SpanMarker`] +//! - [`Coverage`] statements of kind [`BlockMarker`], [`SpanMarker`], [`MCDCBlockMarker`], or [`MCDCDecisionMarker`] //! //! [`AscribeUserType`]: rustc_middle::mir::StatementKind::AscribeUserType //! [`Assign`]: rustc_middle::mir::StatementKind::Assign @@ -15,6 +15,8 @@ //! [`Coverage`]: rustc_middle::mir::StatementKind::Coverage //! [`BlockMarker`]: rustc_middle::mir::coverage::CoverageKind::BlockMarker //! [`SpanMarker`]: rustc_middle::mir::coverage::CoverageKind::SpanMarker +//! [`MCDCBlockMarker`]: rustc_middle::mir::coverage::CoverageKind::MCDCBlockMarker +//! [`MCDCDecisionMarker`]: rustc_middle::mir::coverage::CoverageKind::MCDCDecisionMarker use crate::MirPass; use rustc_middle::mir::coverage::CoverageKind; @@ -33,7 +35,12 @@ impl<'tcx> MirPass<'tcx> for CleanupPostBorrowck { | StatementKind::Coverage( // These kinds of coverage statements are markers inserted during // MIR building, and are not needed after InstrumentCoverage. - CoverageKind::BlockMarker { .. } | CoverageKind::SpanMarker { .. }, + CoverageKind::BlockMarker { .. } + | CoverageKind::SpanMarker { .. } + | CoverageKind::MCDCDecisionEntryMarker { .. } + | CoverageKind::MCDCDecisionOutputMarker { .. } + | CoverageKind::MCDCConditionEntryMarker { .. } + | CoverageKind::MCDCConditionOutputMarker { .. }, ) | StatementKind::FakeRead(..) => statement.make_nop(), _ => (), diff --git a/compiler/rustc_mir_transform/src/coverage/mcdc.rs b/compiler/rustc_mir_transform/src/coverage/mcdc.rs new file mode 100644 index 0000000000000..3585b7cb715a8 --- /dev/null +++ b/compiler/rustc_mir_transform/src/coverage/mcdc.rs @@ -0,0 +1,323 @@ +use rustc_data_structures::fx::FxHashMap; +use rustc_index::{bit_set::BitSet, IndexVec}; +use rustc_middle::{ + mir::{ + self, + coverage::{self, ConditionId, CoverageKind, DecisionId, DecisionMarkerId}, + BasicBlock, StatementKind, + }, + ty::TyCtxt, +}; + +use crate::coverage::{graph::CoverageGraph, inject_statement}; +use crate::errors::MCDCTooManyConditions; + +const MAX_COND_DECISION: u32 = 6; + +#[allow(dead_code)] + +struct Decisions { + data: IndexVec, + needed_bytes: u32, +} + +impl Decisions { + /// Use MIR markers and THIR extracted data to create the data we need for + /// MCDC instrumentation. + /// + /// Note: MCDC Instrumentation might skip some decisions that contains too + /// may conditions. + pub fn extract_decisions<'tcx>( + tcx: TyCtxt<'tcx>, + branch_info: &coverage::BranchInfo, + mir_body: &mir::Body<'_>, + ) -> Self { + let mut decisions_builder: IndexVec = IndexVec::new(); + let mut decm_id_to_dec_id: FxHashMap = Default::default(); + let mut ignored_decisions: BitSet = BitSet::new_empty( + branch_info.decision_spans.last_index().map(|i| i.as_usize()).unwrap_or(0) + 1, + ); + let mut needed_bytes: u32 = 0; + + // Start by gathering all the decisions. + for (bb, data) in mir_body.basic_blocks.iter_enumerated() { + for statement in &data.statements { + match &statement.kind { + StatementKind::Coverage(CoverageKind::MCDCDecisionEntryMarker { decm_id }) => { + assert!( + !decm_id_to_dec_id.contains_key(decm_id) + && !ignored_decisions.contains(*decm_id), + "Duplicated decision marker id {:?}.", + decm_id + ); + + // Skip uninstrumentable conditions and flag them + // as ignored for the rest of the process. + let dec_span = &branch_info.decision_spans[*decm_id]; + if dec_span.num_conditions > MAX_COND_DECISION { + tcx.dcx().emit_warn(MCDCTooManyConditions { + span: dec_span.span, + num_conditions: dec_span.num_conditions, + max_conditions: MAX_COND_DECISION, + }); + ignored_decisions.insert(*decm_id); + } else { + decm_id_to_dec_id.insert( + *decm_id, + decisions_builder.push(DecisionDataBuilder::new(bb, needed_bytes)), + ); + needed_bytes += 1 << dec_span.num_conditions; + } + } + _ => (), + } + } + } + + for (bb, data) in mir_body.basic_blocks.iter_enumerated() { + for statement in &data.statements { + let StatementKind::Coverage(cov_kind) = &statement.kind else { + continue; + }; + use CoverageKind::*; + match cov_kind { + // Handled above + // MCDCDecisionEntryMarker { decm_id } => { + // } + MCDCDecisionOutputMarker { decm_id, outcome } => { + if let Some(decision_id) = decm_id_to_dec_id.get(decm_id) { + if *outcome { + decisions_builder[*decision_id].set_then_bb(bb); + } else { + decisions_builder[*decision_id].set_else_bb(bb); + } + } else if !ignored_decisions.contains(*decm_id) { + // If id is not in the mapping vector nor in the ignored IDs bitset, + // It means we have not encountered the corresponding DecisionEntryMarker. + bug!( + "Decision output marker {:?} coming before its decision entry marker.", + decm_id + ); + } + } + MCDCConditionEntryMarker { decm_id, condm_id } => { + if let Some(decision_id) = decm_id_to_dec_id.get(decm_id) { + debug!("TODO MCDCConditionEntryMarker"); + let dec_data = &mut decisions_builder[*decision_id]; + dec_data.conditions.push(bb); + } else if !ignored_decisions.contains(*decm_id) { + // If id is not in the mapping vector nor in the ignored IDs bitset, + // It means we have not encountered the corresponding DecisionEntryMarker. + bug!( + "Condition marker {:?} references unknown decision entry marker.", + condm_id + ); + } + } + MCDCConditionOutputMarker { decm_id, condm_id, outcome: _ } => { + if let Some(_decision_id) = decm_id_to_dec_id.get(decm_id) { + debug!("TODO MCDCConditionOutcomeMarker"); + } else if !ignored_decisions.contains(*decm_id) { + // If id is not in the mapping vector nor in the ignored IDs bitset, + // It means we have not encountered the corresponding DecisionEntryMarker. + bug!( + "Condition marker {:?} references unknown decision entry marker.", + condm_id + ); + } + } + _ => (), // Ignore other marker kinds. + } + } + } + + Self { + data: IndexVec::from_iter(decisions_builder.into_iter().map(|b| b.into_done())), + needed_bytes, + } + } +} + +// FIXME(dprn): Remove allow dead code +#[allow(unused)] +struct DecisionData { + entry_bb: BasicBlock, + + /// Decision's offset in the global TV update table. + bitmap_idx: u32, + + conditions: IndexVec, + then_bb: BasicBlock, + else_bb: BasicBlock, +} + +// FIXME(dprn): Remove allow dead code +#[allow(dead_code)] +impl DecisionData { + pub fn new( + entry_bb: BasicBlock, + bitmap_idx: u32, + then_bb: BasicBlock, + else_bb: BasicBlock, + ) -> Self { + Self { entry_bb, bitmap_idx, conditions: IndexVec::new(), then_bb, else_bb } + } +} + +struct DecisionDataBuilder { + entry_bb: BasicBlock, + + /// Decision's offset in the global TV update table. + bitmap_idx: u32, + + conditions: IndexVec, + then_bb: Option, + else_bb: Option, +} + +// FIXME(dprn): Remove allow dead code +#[allow(dead_code)] +impl DecisionDataBuilder { + pub fn new(entry_bb: BasicBlock, bitmap_idx: u32) -> Self { + Self { + entry_bb, + bitmap_idx, + conditions: Default::default(), + then_bb: Default::default(), + else_bb: Default::default(), + } + } + + pub fn set_then_bb(&mut self, then_bb: BasicBlock) -> &mut Self { + self.then_bb = Some(then_bb); + self + } + + pub fn set_else_bb(&mut self, else_bb: BasicBlock) -> &mut Self { + self.else_bb = Some(else_bb); + self + } + + pub fn into_done(self) -> DecisionData { + assert!(!self.conditions.is_empty(), "Empty condition vector"); + + DecisionData { + entry_bb: self.entry_bb, + bitmap_idx: self.bitmap_idx, + conditions: self.conditions, + then_bb: self.then_bb.expect("Missing then_bb"), + else_bb: self.else_bb.expect("Missing else_bb"), + } + } +} + +/// If MCDC coverage is enabled, add MCDC instrumentation to the function. +/// +/// Assume a decision to be the following: +/// +/// if (A && B) || C { then(); } else { otherwise(); } +/// +/// The corresponding BDD (Binary Decision Diagram) will look like so: +/// +/// ``` +/// │ +/// ┌▼┐ +/// │A│ +/// ┌──┴─┴──┐ +/// │t f│ +/// │ │ +/// ┌▼┐ ┌▼┐ +/// │B├─────►C├───┐ +/// └┬┘ f └┬┘ f│ +/// │t │t │ +/// ┌──▼───┐ │ ┌─▼─────────┐ +/// │then()◄───┘ │otherwise()│ +/// └──────┘ └───────────┘ +/// ``` +/// +/// The coverage graph is similar, up to unwinding mechanics. The goal is to +/// instrument each edge of the BDD to update two bitmaps. +/// +/// The first bitmap (CBM) is updated upon the evaluation of each contidion. +/// It tracks the state of a condition at a given instant. +/// is given an index, such that when the decision is taken, CBM represents the +/// state of all conditions that were evaluated (1 for true, 0 for +/// false/skipped). +/// +/// The second bitmap (TVBM) is the test vector bitmap. It tracks all the test +/// vectors that were executed during the program's life. It is updated before +/// branching to `then()` or `otherwise()` by using CBM as an integer n and +/// setting the nth integer of TVBM to 1. +/// Basically, we do `TVBM |= 1 << CBM`. +/// +/// Note: This technique is very sub-optimal, as it implies that TVBM to have a +/// size of 2^n bits, (n being the number of conditions in the decision) to +/// account for every combination, even though only a subset of theses +/// combinations are actually achievable because of logical operators +/// short-circuit semantics. +/// An improvement to this instrumentation is being implemented in Clang and +/// shall be ported to Rustc in the future: +/// https://discourse.llvm.org/t/rfc-coverage-new-algorithm-and-file-format-for-mc-dc/76798 +/// +/// In the meantime, to follow the original implementation of Clang, we use this +/// suboptimal technique, while limiting the size of the decisions we can +/// instrument to 6 conditions. +/// +/// Will do the following things : +/// 1. Add an instruction in the first basic block for the codegen to call +/// the `instrprof.mcdc.parameters` instrinsic. +/// 2. Before each decision, add an instruction to reset CBM. +/// 3. Add an isntruction to update CBM upon condition evaluation. +/// 4. Add an instruction to update TVBM with the CBM value before jumping +/// to the `then` or `else` block. +/// 5. Build mappings for the reporting tools to get the results and transpose +/// it to the source code. +pub fn instrument_function_mcdc<'tcx>( + tcx: TyCtxt<'tcx>, + mir_body: &mut mir::Body<'tcx>, + coverage_graph: &CoverageGraph, +) { + let _ = coverage_graph; + if !tcx.sess.instrument_coverage_mcdc() { + return; + } + debug!("Called instrument_function_mcdc()"); + + let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else { + return; + }; + let _decision_data = Decisions::extract_decisions(tcx, branch_info, mir_body); + + let mut needed_bytes = 0; + let mut bitmap_indexes = vec![]; + + for dec_span in branch_info.decision_spans.iter() { + // Skip uninstrumentable conditions. + if dec_span.num_conditions > MAX_COND_DECISION { + tcx.dcx().emit_warn(MCDCTooManyConditions { + span: dec_span.span, + num_conditions: dec_span.num_conditions, + max_conditions: MAX_COND_DECISION, + }); + continue; + } + bitmap_indexes.push(needed_bytes); + needed_bytes += 1 << dec_span.num_conditions; + } + + if needed_bytes == 0 { + // No decision to instrument + return; + } + + // In the first BB, specify that we need to allocate bitmaps. + inject_statement(mir_body, CoverageKind::MCDCBitmapRequire { needed_bytes }, mir::START_BLOCK); + + // For each decision: + // - Find its 'root' basic block + // - Insert a 'reset CDM' instruction + // - for each branch, find the root condition, give it an index and + // call a condbitmapUpdate on it + // - Get the Output markers, and insert goto blocks before to put a + // tvbitmapupdate on it. +} diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index d382d2c03c24a..739feeec71ed2 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -3,6 +3,7 @@ pub mod query; mod counters; mod graph; mod spans; +mod mcdc; #[cfg(test)] mod tests; @@ -100,6 +101,10 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: &coverage_counters, ); + //////////////////////////////////////////////////// + // Add the MCDC instrumentation. This is a no-op if MCDC coverage is disabled. + mcdc::instrument_function_mcdc(tcx, mir_body, &basic_coverage_blocks); + mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo { function_source_hash: hir_info.function_source_hash, num_counters: coverage_counters.num_counters(), @@ -145,6 +150,9 @@ fn create_mappings<'tcx>( true_term: term_for_bcb(true_bcb), false_term: term_for_bcb(false_bcb), }, + BcbMappingKind::MCDCDecision { bitmap_idx, num_conditions } => { + MappingKind::MCDCDecision { bitmap_idx, num_conditions } + } }; let code_region = make_code_region(source_map, file_name, span, body_span)?; Some(Mapping { kind, code_region }) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 672de1fbe60fd..24f3f0efd2f0b 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -9,12 +9,21 @@ use crate::coverage::ExtractedHirInfo; mod from_mir; +// FIXME(dprn): Remove allow dead code +#[allow(dead_code)] #[derive(Clone, Copy, 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 }, + Branch { + true_bcb: BasicCoverageBlock, + false_bcb: BasicCoverageBlock, + }, + MCDCDecision { + bitmap_idx: u32, + num_conditions: u32, + }, } #[derive(Debug)] @@ -92,6 +101,7 @@ pub(super) fn generate_coverage_spans( insert(true_bcb); insert(false_bcb); } + BcbMappingKind::MCDCDecision { .. } => (), } } diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index adb0c9f1929d9..ab19fd4264751 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -222,12 +222,24 @@ fn filtered_statement_span(statement: &Statement<'_>) -> Option { | StatementKind::PlaceMention(..) | StatementKind::AscribeUserType(_, _) => Some(statement.source_info.span), - // Block markers are used for branch coverage, so ignore them here. - StatementKind::Coverage(CoverageKind::BlockMarker { .. }) => None, + StatementKind::Coverage( + // Block markers are used for branch coverage, so ignore them here. + CoverageKind::BlockMarker {..} + // Ignore MCDC markers as well + | CoverageKind::MCDCDecisionEntryMarker{ .. } + | CoverageKind::MCDCDecisionOutputMarker { .. } + | CoverageKind::MCDCConditionEntryMarker { .. } + | CoverageKind::MCDCConditionOutputMarker { .. } + ) => None, // These coverage statements should not exist prior to coverage instrumentation. StatementKind::Coverage( - CoverageKind::CounterIncrement { .. } | CoverageKind::ExpressionUsed { .. }, + CoverageKind::CounterIncrement { .. } + | CoverageKind::ExpressionUsed { .. } + | CoverageKind::MCDCBitmapRequire { .. } + | CoverageKind::MCDCCondBitmapReset + | CoverageKind::MCDCCondBitmapUpdate { .. } + | CoverageKind::MCDCTestVectorBitmapUpdate { .. } ) => bug!( "Unexpected coverage statement found during coverage instrumentation: {statement:?}" ), @@ -378,8 +390,8 @@ pub(super) fn extract_branch_mappings( // Fill out the mapping from block marker IDs to their enclosing blocks. for (bb, data) in mir_body.basic_blocks.iter_enumerated() { for statement in &data.statements { - if let StatementKind::Coverage(CoverageKind::BlockMarker { id }) = statement.kind { - block_markers[id] = Some(bb); + if let StatementKind::Coverage(CoverageKind::BlockMarker { id }) = &statement.kind { + block_markers[*id] = Some(bb); } } } @@ -387,7 +399,7 @@ pub(super) fn extract_branch_mappings( branch_info .branch_spans .iter() - .filter_map(|&BranchSpan { span: raw_span, true_marker, false_marker }| { + .filter_map(|&BranchSpan { span: raw_span, decision_id: _, true_marker, false_marker }| { // For now, ignore any branch span that was introduced by // expansion. This makes things like assert macros less noisy. if !raw_span.ctxt().outer_expn_data().is_root() { diff --git a/compiler/rustc_mir_transform/src/errors.rs b/compiler/rustc_mir_transform/src/errors.rs index 9297bc51fad99..5ef711ce64421 100644 --- a/compiler/rustc_mir_transform/src/errors.rs +++ b/compiler/rustc_mir_transform/src/errors.rs @@ -32,6 +32,15 @@ pub(crate) enum ConstMutate { }, } +#[derive(Diagnostic)] +#[diag(mir_transform_mcdc_too_many_conditions)] +pub(crate) struct MCDCTooManyConditions { + #[primary_span] + pub span: Span, + pub num_conditions: u32, + pub max_conditions: u32, +} + #[derive(Diagnostic)] #[diag(mir_transform_unaligned_packed_ref, code = E0793)] #[note] diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index d51fcf693ed38..0b0a36791155c 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 6204f86838548..b6d60379fdf38 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 = "`branch`, `no-branch`, `mcdc`, `no-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"; @@ -950,6 +950,7 @@ mod parse { }; let slot = match option { "branch" => &mut slot.branch, + "mcdc" => &mut slot.mcdc, _ => return false, }; *slot = enabled; diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 55fff4421ae46..53740a8a6b1ea 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -352,6 +352,10 @@ impl Session { self.instrument_coverage() && self.opts.unstable_opts.coverage_options.branch } + pub fn instrument_coverage_mcdc(&self) -> bool { + self.instrument_coverage() && self.opts.unstable_opts.coverage_options.mcdc + } + pub fn is_sanitizer_cfi_enabled(&self) -> bool { self.opts.unstable_opts.sanitizer.contains(SanitizerSet::CFI) } diff --git a/src/doc/rustc/src/instrument-coverage.md b/src/doc/rustc/src/instrument-coverage.md index 185a3ba5dbd41..2a289bd4242dd 100644 --- a/src/doc/rustc/src/instrument-coverage.md +++ b/src/doc/rustc/src/instrument-coverage.md @@ -353,6 +353,8 @@ instrumentation. Pass one or more of the following values, separated by commas. - `branch` or `no-branch` - Enables or disables branch coverage instrumentation. +- `mcdc` or `no-mcdc` + - Enables or disables MCDC coverage instrumentation. ## Other references