Skip to content

Commit 4294f93

Browse files
authored
Unrolled build for rust-lang#138731
Rollup merge of rust-lang#138731 - Zalathar:llvm-expansion, r=jieyouxu coverage: Add LLVM plumbing for expansion regions This is currently unused, but paves the way for future work on expansion regions without having to worry about the FFI parts. The span conversion refactoring is only loosely related, but I've included it here because it would conflict with the main changes in `fill_region_tables`, and is pretty straightforward on its own.
2 parents 78948ac + 2e36990 commit 4294f93

File tree

6 files changed

+97
-34
lines changed

6 files changed

+97
-34
lines changed

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

+17-2
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ pub(crate) struct CoverageSpan {
146146
#[derive(Clone, Debug, Default)]
147147
pub(crate) struct Regions {
148148
pub(crate) code_regions: Vec<CodeRegion>,
149+
pub(crate) expansion_regions: Vec<ExpansionRegion>,
149150
pub(crate) branch_regions: Vec<BranchRegion>,
150151
pub(crate) mcdc_branch_regions: Vec<MCDCBranchRegion>,
151152
pub(crate) mcdc_decision_regions: Vec<MCDCDecisionRegion>,
@@ -154,10 +155,16 @@ pub(crate) struct Regions {
154155
impl Regions {
155156
/// Returns true if none of this structure's tables contain any regions.
156157
pub(crate) fn has_no_regions(&self) -> bool {
157-
let Self { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } =
158-
self;
158+
let Self {
159+
code_regions,
160+
expansion_regions,
161+
branch_regions,
162+
mcdc_branch_regions,
163+
mcdc_decision_regions,
164+
} = self;
159165

160166
code_regions.is_empty()
167+
&& expansion_regions.is_empty()
161168
&& branch_regions.is_empty()
162169
&& mcdc_branch_regions.is_empty()
163170
&& mcdc_decision_regions.is_empty()
@@ -172,6 +179,14 @@ pub(crate) struct CodeRegion {
172179
pub(crate) counter: Counter,
173180
}
174181

182+
/// Must match the layout of `LLVMRustCoverageExpansionRegion`.
183+
#[derive(Clone, Debug)]
184+
#[repr(C)]
185+
pub(crate) struct ExpansionRegion {
186+
pub(crate) cov_span: CoverageSpan,
187+
pub(crate) expanded_file_id: u32,
188+
}
189+
175190
/// Must match the layout of `LLVMRustCoverageBranchRegion`.
176191
#[derive(Clone, Debug)]
177192
#[repr(C)]

compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs

+14-2
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,18 @@ pub(crate) fn write_function_mappings_to_buffer(
6363
expressions: &[ffi::CounterExpression],
6464
regions: &ffi::Regions,
6565
) -> Vec<u8> {
66-
let ffi::Regions { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } =
67-
regions;
66+
let ffi::Regions {
67+
code_regions,
68+
expansion_regions,
69+
branch_regions,
70+
mcdc_branch_regions,
71+
mcdc_decision_regions,
72+
} = regions;
73+
74+
// SAFETY:
75+
// - All types are FFI-compatible and have matching representations in Rust/C++.
76+
// - For pointer/length pairs, the pointer and length come from the same vector or slice.
77+
// - C++ code does not retain any pointers after the call returns.
6878
llvm::build_byte_buffer(|buffer| unsafe {
6979
llvm::LLVMRustCoverageWriteFunctionMappingsToBuffer(
7080
virtual_file_mapping.as_ptr(),
@@ -73,6 +83,8 @@ pub(crate) fn write_function_mappings_to_buffer(
7383
expressions.len(),
7484
code_regions.as_ptr(),
7585
code_regions.len(),
86+
expansion_regions.as_ptr(),
87+
expansion_regions.len(),
7688
branch_regions.as_ptr(),
7789
branch_regions.len(),
7890
mcdc_branch_regions.as_ptr(),

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs

+17-16
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,22 @@ fn fill_region_tables<'tcx>(
120120
// Associate that global file ID with a local file ID for this function.
121121
let local_file_id = covfun.virtual_file_mapping.local_id_for_global(global_file_id);
122122

123-
let ffi::Regions { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } =
124-
&mut covfun.regions;
125-
126-
let make_cov_span =
127-
|span: Span| spans::make_coverage_span(local_file_id, source_map, &source_file, span);
123+
// In rare cases, _all_ of a function's spans are discarded, and coverage
124+
// codegen needs to handle that gracefully to avoid #133606.
125+
// It's hard for tests to trigger this organically, so instead we set
126+
// `-Zcoverage-options=discard-all-spans-in-codegen` to force it to occur.
128127
let discard_all = tcx.sess.coverage_discard_all_spans_in_codegen();
128+
let make_coords = |span: Span| {
129+
if discard_all { None } else { spans::make_coords(source_map, &source_file, span) }
130+
};
131+
132+
let ffi::Regions {
133+
code_regions,
134+
expansion_regions: _, // FIXME(Zalathar): Fill out support for expansion regions
135+
branch_regions,
136+
mcdc_branch_regions,
137+
mcdc_decision_regions,
138+
} = &mut covfun.regions;
129139

130140
// For each counter/region pair in this function+file, convert it to a
131141
// form suitable for FFI.
@@ -140,17 +150,8 @@ fn fill_region_tables<'tcx>(
140150
ffi::Counter::from_term(term)
141151
};
142152

143-
// Convert the `Span` into coordinates that we can pass to LLVM, or
144-
// discard the span if conversion fails. In rare, cases _all_ of a
145-
// function's spans are discarded, and the rest of coverage codegen
146-
// needs to handle that gracefully to avoid a repeat of #133606.
147-
// We don't have a good test case for triggering that organically, so
148-
// instead we set `-Zcoverage-options=discard-all-spans-in-codegen`
149-
// to force it to occur.
150-
let Some(cov_span) = make_cov_span(span) else { continue };
151-
if discard_all {
152-
continue;
153-
}
153+
let Some(coords) = make_coords(span) else { continue };
154+
let cov_span = coords.make_coverage_span(local_file_id);
154155

155156
match *kind {
156157
MappingKind::Code { bcb } => {

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs

+31-14
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,40 @@ use tracing::debug;
55
use crate::coverageinfo::ffi;
66
use crate::coverageinfo::mapgen::LocalFileId;
77

8+
/// Line and byte-column coordinates of a source code span within some file.
9+
/// The file itself must be tracked separately.
10+
#[derive(Clone, Copy, Debug)]
11+
pub(crate) struct Coords {
12+
/// 1-based starting line of the source code span.
13+
pub(crate) start_line: u32,
14+
/// 1-based starting column (in bytes) of the source code span.
15+
pub(crate) start_col: u32,
16+
/// 1-based ending line of the source code span.
17+
pub(crate) end_line: u32,
18+
/// 1-based ending column (in bytes) of the source code span. High bit must be unset.
19+
pub(crate) end_col: u32,
20+
}
21+
22+
impl Coords {
23+
/// Attaches a local file ID to these coordinates to produce an `ffi::CoverageSpan`.
24+
pub(crate) fn make_coverage_span(&self, local_file_id: LocalFileId) -> ffi::CoverageSpan {
25+
let &Self { start_line, start_col, end_line, end_col } = self;
26+
let file_id = local_file_id.as_u32();
27+
ffi::CoverageSpan { file_id, start_line, start_col, end_line, end_col }
28+
}
29+
}
30+
831
/// Converts the span into its start line and column, and end line and column.
932
///
1033
/// Line numbers and column numbers are 1-based. Unlike most column numbers emitted by
1134
/// the compiler, these column numbers are denoted in **bytes**, because that's what
1235
/// LLVM's `llvm-cov` tool expects to see in coverage maps.
1336
///
14-
/// Returns `None` if the conversion failed for some reason. This shouldn't happen,
37+
/// Returns `None` if the conversion failed for some reason. This should be uncommon,
1538
/// but it's hard to rule out entirely (especially in the presence of complex macros
1639
/// or other expansions), and if it does happen then skipping a span or function is
1740
/// better than an ICE or `llvm-cov` failure that the user might have no way to avoid.
18-
pub(crate) fn make_coverage_span(
19-
file_id: LocalFileId,
20-
source_map: &SourceMap,
21-
file: &SourceFile,
22-
span: Span,
23-
) -> Option<ffi::CoverageSpan> {
41+
pub(crate) fn make_coords(source_map: &SourceMap, file: &SourceFile, span: Span) -> Option<Coords> {
2442
let span = ensure_non_empty_span(source_map, span)?;
2543

2644
let lo = span.lo();
@@ -44,8 +62,7 @@ pub(crate) fn make_coverage_span(
4462
start_line = source_map.doctest_offset_line(&file.name, start_line);
4563
end_line = source_map.doctest_offset_line(&file.name, end_line);
4664

47-
check_coverage_span(ffi::CoverageSpan {
48-
file_id: file_id.as_u32(),
65+
check_coords(Coords {
4966
start_line: start_line as u32,
5067
start_col: start_col as u32,
5168
end_line: end_line as u32,
@@ -80,8 +97,8 @@ fn ensure_non_empty_span(source_map: &SourceMap, span: Span) -> Option<Span> {
8097
/// it will immediately exit with a fatal error. To prevent that from happening,
8198
/// discard regions that are improperly ordered, or might be interpreted in a
8299
/// way that makes them improperly ordered.
83-
fn check_coverage_span(cov_span: ffi::CoverageSpan) -> Option<ffi::CoverageSpan> {
84-
let ffi::CoverageSpan { file_id: _, start_line, start_col, end_line, end_col } = cov_span;
100+
fn check_coords(coords: Coords) -> Option<Coords> {
101+
let Coords { start_line, start_col, end_line, end_col } = coords;
85102

86103
// Line/column coordinates are supposed to be 1-based. If we ever emit
87104
// coordinates of 0, `llvm-cov` might misinterpret them.
@@ -94,17 +111,17 @@ fn check_coverage_span(cov_span: ffi::CoverageSpan) -> Option<ffi::CoverageSpan>
94111
let is_ordered = (start_line, start_col) <= (end_line, end_col);
95112

96113
if all_nonzero && end_col_has_high_bit_unset && is_ordered {
97-
Some(cov_span)
114+
Some(coords)
98115
} else {
99116
debug!(
100-
?cov_span,
117+
?coords,
101118
?all_nonzero,
102119
?end_col_has_high_bit_unset,
103120
?is_ordered,
104121
"Skipping source region that would be misinterpreted or rejected by LLVM"
105122
);
106123
// If this happens in a debug build, ICE to make it easier to notice.
107-
debug_assert!(false, "Improper source region: {cov_span:?}");
124+
debug_assert!(false, "Improper source region: {coords:?}");
108125
None
109126
}
110127
}

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

+2
Original file line numberDiff line numberDiff line change
@@ -2019,6 +2019,8 @@ unsafe extern "C" {
20192019
NumExpressions: size_t,
20202020
CodeRegions: *const crate::coverageinfo::ffi::CodeRegion,
20212021
NumCodeRegions: size_t,
2022+
ExpansionRegions: *const crate::coverageinfo::ffi::ExpansionRegion,
2023+
NumExpansionRegions: size_t,
20222024
BranchRegions: *const crate::coverageinfo::ffi::BranchRegion,
20232025
NumBranchRegions: size_t,
20242026
MCDCBranchRegions: *const crate::coverageinfo::ffi::MCDCBranchRegion,

compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp

+16
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,13 @@ struct LLVMRustCoverageCodeRegion {
7777
LLVMRustCounter Count;
7878
};
7979

80+
// Must match the layout of
81+
// `rustc_codegen_llvm::coverageinfo::ffi::ExpansionRegion`.
82+
struct LLVMRustCoverageExpansionRegion {
83+
LLVMRustCoverageSpan Span;
84+
uint32_t ExpandedFileID;
85+
};
86+
8087
// Must match the layout of
8188
// `rustc_codegen_llvm::coverageinfo::ffi::BranchRegion`.
8289
struct LLVMRustCoverageBranchRegion {
@@ -151,6 +158,8 @@ extern "C" void LLVMRustCoverageWriteFunctionMappingsToBuffer(
151158
const unsigned *VirtualFileMappingIDs, size_t NumVirtualFileMappingIDs,
152159
const LLVMRustCounterExpression *RustExpressions, size_t NumExpressions,
153160
const LLVMRustCoverageCodeRegion *CodeRegions, size_t NumCodeRegions,
161+
const LLVMRustCoverageExpansionRegion *ExpansionRegions,
162+
size_t NumExpansionRegions,
154163
const LLVMRustCoverageBranchRegion *BranchRegions, size_t NumBranchRegions,
155164
const LLVMRustCoverageMCDCBranchRegion *MCDCBranchRegions,
156165
size_t NumMCDCBranchRegions,
@@ -179,6 +188,13 @@ extern "C" void LLVMRustCoverageWriteFunctionMappingsToBuffer(
179188
Region.Span.ColumnStart, Region.Span.LineEnd, Region.Span.ColumnEnd));
180189
}
181190

191+
// Expansion regions:
192+
for (const auto &Region : ArrayRef(ExpansionRegions, NumExpansionRegions)) {
193+
MappingRegions.push_back(coverage::CounterMappingRegion::makeExpansion(
194+
Region.Span.FileID, Region.ExpandedFileID, Region.Span.LineStart,
195+
Region.Span.ColumnStart, Region.Span.LineEnd, Region.Span.ColumnEnd));
196+
}
197+
182198
// Branch regions:
183199
for (const auto &Region : ArrayRef(BranchRegions, NumBranchRegions)) {
184200
MappingRegions.push_back(coverage::CounterMappingRegion::makeBranchRegion(

0 commit comments

Comments
 (0)