Skip to content

Commit e40568f

Browse files
authored
Unrolled build for rust-lang#123409
Rollup merge of rust-lang#123409 - ZhuUx:master, r=oli-obk Implement Modified Condition/Decision Coverage This is an implementation based on llvm backend support (>= 18) by `@evodius96` and branch coverage support by `@Zalathar.` ### Major changes: * Add -Zcoverage-options=mcdc as switch. Now coverage options accept either `no-branch`, `branch`, or `mcdc`. `mcdc` also enables `branch` because it is essential to work. * Add coverage mapping for MCDCBranch and MCDCDecision. Note that MCDCParameter evolves from llvm 18 to llvm 19. The mapping in rust side mainly references to 19 and is casted to 18 types in llvm wrapper. * Add wrapper for mcdc instrinc functions from llvm. And inject associated statements to mir. * Add BcbMappingKind::Decision, I'm not sure is it proper but can't find a better way temporarily. * Let coverage-dump support parsing MCDCBranch and MCDCDecision from llvm ir. * Add simple tests to check whether mcdc works. * Same as clang, currently rustc does not generate instrument for decision with more than 6 condtions or only 1 condition due to considerations of resource. ### Implementation Details 1. To get information about conditions and decisions, `MCDCState` in `BranchInfoBuilder` is used during hir lowering to mir. For expressions with logical op we call `Builder::visit_coverage_branch_operation` to record its sub conditions, generate condition ids for them and save their spans (to construct the span of whole decision). This process mainly references to the implementation in clang and is described in comments over `MCDCState::record_conditions`. Also true marks and false marks introduced by branch coverage are used to detect where the decision evaluation ends: the next id of the condition == 0. 2. Once the `MCDCState::decision_stack` popped all recorded conditions, we can ensure that the decision is checked over and push it into `decision_spans`. We do not manually insert decision span to avoid complexity from then_else_break in nested if scopes. 3. When constructing CoverageSpans, add condition info to BcbMappingKind::Branch and decision info to BcbMappingKind::Decision. If the branch mapping has non-zero condition id it will be transformed to MCDCBranch mapping and insert `CondBitmapUpdate` statements to its evaluated blocks. While decision bcb mapping will insert `TestVectorBitmapUpdate` in all its end blocks. ### Usage ```bash echo "[build]\nprofiler=true" >> config.toml ./x build --stage 1 ./x test tests/coverage/mcdc_if.rs ``` to build the compiler and run tests. ```shell export PATH=path/to/llvm-build:$PATH rustup toolchain link mcdc build/host/stage1 cargo +mcdc rustc --bin foo -- -Cinstrument-coverage -Zcoverage-options=mcdc cd target/debug LLVM_PROFILE_FILE="foo.profraw" ./foo llvm-profdata merge -sparse foo.profraw -o foo.profdata llvm-cov show ./foo -instr-profile=foo.profdata --show-mcdc ``` to check "foo" code. ### Problems to solve For now decision mapping will insert statements to its all end blocks, which may be optimized by inserting a final block of the decision. To do this we must also trace the evaluated value at each end of the decision and join them separately. This implementation is not heavily tested so there should be some unrevealed issues. We are going to check our rust products in the next. Please let me know if you had any suggestions or comments.
2 parents a61b14d + 402dc38 commit e40568f

File tree

29 files changed

+1642
-59
lines changed

29 files changed

+1642
-59
lines changed

compiler/rustc_codegen_llvm/src/builder.rs

+125-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use rustc_data_structures::small_c_str::SmallCStr;
1717
use rustc_hir::def_id::DefId;
1818
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
1919
use rustc_middle::ty::layout::{
20-
FnAbiError, FnAbiOfHelpers, FnAbiRequest, LayoutError, LayoutOfHelpers, TyAndLayout,
20+
FnAbiError, FnAbiOfHelpers, FnAbiRequest, HasTyCtxt, LayoutError, LayoutOfHelpers, TyAndLayout,
2121
};
2222
use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
2323
use rustc_sanitizers::{cfi, kcfi};
@@ -1702,4 +1702,128 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
17021702
};
17031703
kcfi_bundle
17041704
}
1705+
1706+
pub(crate) fn mcdc_parameters(
1707+
&mut self,
1708+
fn_name: &'ll Value,
1709+
hash: &'ll Value,
1710+
bitmap_bytes: &'ll Value,
1711+
) -> &'ll Value {
1712+
debug!("mcdc_parameters() with args ({:?}, {:?}, {:?})", fn_name, hash, bitmap_bytes);
1713+
1714+
assert!(llvm_util::get_version() >= (18, 0, 0), "MCDC intrinsics require LLVM 18 or later");
1715+
1716+
let llfn = unsafe { llvm::LLVMRustGetInstrProfMCDCParametersIntrinsic(self.cx().llmod) };
1717+
let llty = self.cx.type_func(
1718+
&[self.cx.type_ptr(), self.cx.type_i64(), self.cx.type_i32()],
1719+
self.cx.type_void(),
1720+
);
1721+
let args = &[fn_name, hash, bitmap_bytes];
1722+
let args = self.check_call("call", llty, llfn, args);
1723+
1724+
unsafe {
1725+
let _ = llvm::LLVMRustBuildCall(
1726+
self.llbuilder,
1727+
llty,
1728+
llfn,
1729+
args.as_ptr() as *const &llvm::Value,
1730+
args.len() as c_uint,
1731+
[].as_ptr(),
1732+
0 as c_uint,
1733+
);
1734+
// Create condition bitmap named `mcdc.addr`.
1735+
let mut bx = Builder::with_cx(self.cx);
1736+
bx.position_at_start(llvm::LLVMGetFirstBasicBlock(self.llfn()));
1737+
let cond_bitmap = {
1738+
let alloca =
1739+
llvm::LLVMBuildAlloca(bx.llbuilder, bx.cx.type_i32(), c"mcdc.addr".as_ptr());
1740+
llvm::LLVMSetAlignment(alloca, 4);
1741+
alloca
1742+
};
1743+
bx.store(self.const_i32(0), cond_bitmap, self.tcx().data_layout.i32_align.abi);
1744+
cond_bitmap
1745+
}
1746+
}
1747+
1748+
pub(crate) fn mcdc_tvbitmap_update(
1749+
&mut self,
1750+
fn_name: &'ll Value,
1751+
hash: &'ll Value,
1752+
bitmap_bytes: &'ll Value,
1753+
bitmap_index: &'ll Value,
1754+
mcdc_temp: &'ll Value,
1755+
) {
1756+
debug!(
1757+
"mcdc_tvbitmap_update() with args ({:?}, {:?}, {:?}, {:?}, {:?})",
1758+
fn_name, hash, bitmap_bytes, bitmap_index, mcdc_temp
1759+
);
1760+
assert!(llvm_util::get_version() >= (18, 0, 0), "MCDC intrinsics require LLVM 18 or later");
1761+
1762+
let llfn =
1763+
unsafe { llvm::LLVMRustGetInstrProfMCDCTVBitmapUpdateIntrinsic(self.cx().llmod) };
1764+
let llty = self.cx.type_func(
1765+
&[
1766+
self.cx.type_ptr(),
1767+
self.cx.type_i64(),
1768+
self.cx.type_i32(),
1769+
self.cx.type_i32(),
1770+
self.cx.type_ptr(),
1771+
],
1772+
self.cx.type_void(),
1773+
);
1774+
let args = &[fn_name, hash, bitmap_bytes, bitmap_index, mcdc_temp];
1775+
let args = self.check_call("call", llty, llfn, args);
1776+
unsafe {
1777+
let _ = llvm::LLVMRustBuildCall(
1778+
self.llbuilder,
1779+
llty,
1780+
llfn,
1781+
args.as_ptr() as *const &llvm::Value,
1782+
args.len() as c_uint,
1783+
[].as_ptr(),
1784+
0 as c_uint,
1785+
);
1786+
}
1787+
let i32_align = self.tcx().data_layout.i32_align.abi;
1788+
self.store(self.const_i32(0), mcdc_temp, i32_align);
1789+
}
1790+
1791+
pub(crate) fn mcdc_condbitmap_update(
1792+
&mut self,
1793+
fn_name: &'ll Value,
1794+
hash: &'ll Value,
1795+
cond_loc: &'ll Value,
1796+
mcdc_temp: &'ll Value,
1797+
bool_value: &'ll Value,
1798+
) {
1799+
debug!(
1800+
"mcdc_condbitmap_update() with args ({:?}, {:?}, {:?}, {:?}, {:?})",
1801+
fn_name, hash, cond_loc, mcdc_temp, bool_value
1802+
);
1803+
assert!(llvm_util::get_version() >= (18, 0, 0), "MCDC intrinsics require LLVM 18 or later");
1804+
let llfn = unsafe { llvm::LLVMRustGetInstrProfMCDCCondBitmapIntrinsic(self.cx().llmod) };
1805+
let llty = self.cx.type_func(
1806+
&[
1807+
self.cx.type_ptr(),
1808+
self.cx.type_i64(),
1809+
self.cx.type_i32(),
1810+
self.cx.type_ptr(),
1811+
self.cx.type_i1(),
1812+
],
1813+
self.cx.type_void(),
1814+
);
1815+
let args = &[fn_name, hash, cond_loc, mcdc_temp, bool_value];
1816+
self.check_call("call", llty, llfn, args);
1817+
unsafe {
1818+
let _ = llvm::LLVMRustBuildCall(
1819+
self.llbuilder,
1820+
llty,
1821+
llfn,
1822+
args.as_ptr() as *const &llvm::Value,
1823+
args.len() as c_uint,
1824+
[].as_ptr(),
1825+
0 as c_uint,
1826+
);
1827+
}
1828+
}
17051829
}

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

+157-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
use rustc_middle::mir::coverage::{CodeRegion, CounterId, CovTerm, ExpressionId, MappingKind};
1+
use rustc_middle::mir::coverage::{
2+
CodeRegion, ConditionInfo, CounterId, CovTerm, DecisionInfo, ExpressionId, MappingKind,
3+
};
24

35
/// Must match the layout of `LLVMRustCounterKind`.
46
#[derive(Copy, Clone, Debug)]
@@ -99,6 +101,86 @@ pub enum RegionKind {
99101
/// associated with two counters, each representing the number of times the
100102
/// expression evaluates to true or false.
101103
BranchRegion = 4,
104+
105+
/// A DecisionRegion represents a top-level boolean expression and is
106+
/// associated with a variable length bitmap index and condition number.
107+
MCDCDecisionRegion = 5,
108+
109+
/// A Branch Region can be extended to include IDs to facilitate MC/DC.
110+
MCDCBranchRegion = 6,
111+
}
112+
113+
pub mod mcdc {
114+
use rustc_middle::mir::coverage::{ConditionInfo, DecisionInfo};
115+
116+
/// Must match the layout of `LLVMRustMCDCDecisionParameters`.
117+
#[repr(C)]
118+
#[derive(Clone, Copy, Debug, Default)]
119+
pub struct DecisionParameters {
120+
bitmap_idx: u32,
121+
conditions_num: u16,
122+
}
123+
124+
// ConditionId in llvm is `unsigned int` at 18 while `int16_t` at [19](https://github.com/llvm/llvm-project/pull/81257)
125+
type LLVMConditionId = i16;
126+
127+
/// Must match the layout of `LLVMRustMCDCBranchParameters`.
128+
#[repr(C)]
129+
#[derive(Clone, Copy, Debug, Default)]
130+
pub struct BranchParameters {
131+
condition_id: LLVMConditionId,
132+
condition_ids: [LLVMConditionId; 2],
133+
}
134+
135+
#[repr(C)]
136+
#[derive(Clone, Copy, Debug)]
137+
pub enum ParameterTag {
138+
None = 0,
139+
Decision = 1,
140+
Branch = 2,
141+
}
142+
/// Same layout with `LLVMRustMCDCParameters`
143+
#[repr(C)]
144+
#[derive(Clone, Copy, Debug)]
145+
pub struct Parameters {
146+
tag: ParameterTag,
147+
decision_params: DecisionParameters,
148+
branch_params: BranchParameters,
149+
}
150+
151+
impl Parameters {
152+
pub fn none() -> Self {
153+
Self {
154+
tag: ParameterTag::None,
155+
decision_params: Default::default(),
156+
branch_params: Default::default(),
157+
}
158+
}
159+
pub fn decision(decision_params: DecisionParameters) -> Self {
160+
Self { tag: ParameterTag::Decision, decision_params, branch_params: Default::default() }
161+
}
162+
pub fn branch(branch_params: BranchParameters) -> Self {
163+
Self { tag: ParameterTag::Branch, decision_params: Default::default(), branch_params }
164+
}
165+
}
166+
167+
impl From<ConditionInfo> for BranchParameters {
168+
fn from(value: ConditionInfo) -> Self {
169+
Self {
170+
condition_id: value.condition_id.as_u32() as LLVMConditionId,
171+
condition_ids: [
172+
value.false_next_id.as_u32() as LLVMConditionId,
173+
value.true_next_id.as_u32() as LLVMConditionId,
174+
],
175+
}
176+
}
177+
}
178+
179+
impl From<DecisionInfo> for DecisionParameters {
180+
fn from(value: DecisionInfo) -> Self {
181+
Self { bitmap_idx: value.bitmap_idx, conditions_num: value.conditions_num }
182+
}
183+
}
102184
}
103185

104186
/// This struct provides LLVM's representation of a "CoverageMappingRegion", encoded into the
@@ -122,6 +204,7 @@ pub struct CounterMappingRegion {
122204
/// for the false branch of the region.
123205
false_counter: Counter,
124206

207+
mcdc_params: mcdc::Parameters,
125208
/// An indirect reference to the source filename. In the LLVM Coverage Mapping Format, the
126209
/// file_id is an index into a function-specific `virtual_file_mapping` array of indexes
127210
/// that, in turn, are used to look up the filename for this region.
@@ -173,6 +256,26 @@ impl CounterMappingRegion {
173256
end_line,
174257
end_col,
175258
),
259+
MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => {
260+
Self::mcdc_branch_region(
261+
Counter::from_term(true_term),
262+
Counter::from_term(false_term),
263+
mcdc_params,
264+
local_file_id,
265+
start_line,
266+
start_col,
267+
end_line,
268+
end_col,
269+
)
270+
}
271+
MappingKind::MCDCDecision(decision_info) => Self::decision_region(
272+
decision_info,
273+
local_file_id,
274+
start_line,
275+
start_col,
276+
end_line,
277+
end_col,
278+
),
176279
}
177280
}
178281

@@ -187,6 +290,7 @@ impl CounterMappingRegion {
187290
Self {
188291
counter,
189292
false_counter: Counter::ZERO,
293+
mcdc_params: mcdc::Parameters::none(),
190294
file_id,
191295
expanded_file_id: 0,
192296
start_line,
@@ -209,6 +313,7 @@ impl CounterMappingRegion {
209313
Self {
210314
counter,
211315
false_counter,
316+
mcdc_params: mcdc::Parameters::none(),
212317
file_id,
213318
expanded_file_id: 0,
214319
start_line,
@@ -219,6 +324,54 @@ impl CounterMappingRegion {
219324
}
220325
}
221326

327+
pub(crate) fn mcdc_branch_region(
328+
counter: Counter,
329+
false_counter: Counter,
330+
condition_info: ConditionInfo,
331+
file_id: u32,
332+
start_line: u32,
333+
start_col: u32,
334+
end_line: u32,
335+
end_col: u32,
336+
) -> Self {
337+
Self {
338+
counter,
339+
false_counter,
340+
mcdc_params: mcdc::Parameters::branch(condition_info.into()),
341+
file_id,
342+
expanded_file_id: 0,
343+
start_line,
344+
start_col,
345+
end_line,
346+
end_col,
347+
kind: RegionKind::MCDCBranchRegion,
348+
}
349+
}
350+
351+
pub(crate) fn decision_region(
352+
decision_info: DecisionInfo,
353+
file_id: u32,
354+
start_line: u32,
355+
start_col: u32,
356+
end_line: u32,
357+
end_col: u32,
358+
) -> Self {
359+
let mcdc_params = mcdc::Parameters::decision(decision_info.into());
360+
361+
Self {
362+
counter: Counter::ZERO,
363+
false_counter: Counter::ZERO,
364+
mcdc_params,
365+
file_id,
366+
expanded_file_id: 0,
367+
start_line,
368+
start_col,
369+
end_line,
370+
end_col,
371+
kind: RegionKind::MCDCDecisionRegion,
372+
}
373+
}
374+
222375
// This function might be used in the future; the LLVM API is still evolving, as is coverage
223376
// support.
224377
#[allow(dead_code)]
@@ -233,6 +386,7 @@ impl CounterMappingRegion {
233386
Self {
234387
counter: Counter::ZERO,
235388
false_counter: Counter::ZERO,
389+
mcdc_params: mcdc::Parameters::none(),
236390
file_id,
237391
expanded_file_id,
238392
start_line,
@@ -256,6 +410,7 @@ impl CounterMappingRegion {
256410
Self {
257411
counter: Counter::ZERO,
258412
false_counter: Counter::ZERO,
413+
mcdc_params: mcdc::Parameters::none(),
259414
file_id,
260415
expanded_file_id: 0,
261416
start_line,
@@ -280,6 +435,7 @@ impl CounterMappingRegion {
280435
Self {
281436
counter,
282437
false_counter: Counter::ZERO,
438+
mcdc_params: mcdc::Parameters::none(),
283439
file_id,
284440
expanded_file_id: 0,
285441
start_line,

0 commit comments

Comments
 (0)