Skip to content

Commit

Permalink
Auto merge of rust-lang#121026 - Zalathar:version, r=oli-obk
Browse files Browse the repository at this point in the history
coverage: Correctly report and check LLVM's coverage mapping version

I was puzzled by the fact that the LLVM 18 update (rust-lang#120055) didn't need to modify this version check, despite the fact that LLVM 18 uses a newer version of the coverage mapping format.

This turned out to be because we were inappropriately hard-coding a specific version (`Version6`) in the C++ wrapper, instead of using `CovMapVersion::CurrentVersion` to reflect the version actually used by LLVM on our behalf.

This PR fixes that, and also changes the Rust-side version check to accept the new coverage mapping version used by LLVM 18, since the necessary compatibility work has already been done.

---

### Quick history of `LLVMRustCoverageMappingVersion`:

- Originally it returned LLVM's `coverage::CovMapVersion::CurrentVersion`, as intended. The Rust-side code would verify it, and also embed it as the actual coverage version number in the output binary.
- At some point it was changed to a hard-coded value, to work around a (now-irrelevant) compatibility issue. This was incorrect (but mostly benign), because the override should have been performed on the Rust side instead, after verifying LLVM's value.
- Later contributors dutifully updated the hard-coded value, because they didn't have enough context to identify the problem.
- With this PR, it once again returns LLVM's current coverage version number, and the Rust-side code checks it against an expected range. We don't override the result, but we do indicate where that override should occur if it ever becomes necessary.
  • Loading branch information
bors committed Apr 4, 2024
2 parents 4c6c629 + 8289dad commit ca7d34e
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 10 deletions.
29 changes: 20 additions & 9 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ use rustc_span::Symbol;

/// Generates and exports the Coverage Map.
///
/// Rust Coverage Map generation supports LLVM Coverage Mapping Format version
/// 6 (zero-based encoded as 5), as defined at
/// [LLVM Code Coverage Mapping Format](https://github.com/rust-lang/llvm-project/blob/rustc/13.0-2021-09-30/llvm/docs/CoverageMappingFormat.rst#llvm-code-coverage-mapping-format).
/// Rust Coverage Map generation supports LLVM Coverage Mapping Format versions
/// 6 and 7 (encoded as 5 and 6 respectively), as described at
/// [LLVM Code Coverage Mapping Format](https://github.com/rust-lang/llvm-project/blob/rustc/18.0-2024-02-13/llvm/docs/CoverageMappingFormat.rst).
/// These versions are supported by the LLVM coverage tools (`llvm-profdata` and `llvm-cov`)
/// bundled with Rust's fork of LLVM.
/// distributed in the `llvm-tools-preview` rustup component.
///
/// Consequently, Rust's bundled version of Clang also generates Coverage Maps compliant with
/// the same version. Clang's implementation of Coverage Map generation was referenced when
Expand All @@ -31,10 +31,21 @@ 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.
let version = coverageinfo::mapping_version();
assert_eq!(version, 5, "The `CoverageMappingVersion` exposed by `llvm-wrapper` is out of sync");
// Ensure that LLVM is using a version of the coverage mapping format that
// agrees with our Rust-side code. Expected versions (encoded as n-1) are:
// - `CovMapVersion::Version6` (5) used by LLVM 13-17
// - `CovMapVersion::Version7` (6) used by LLVM 18
let covmap_version = {
let llvm_covmap_version = coverageinfo::mapping_version();
let expected_versions = 5..=6;
assert!(
expected_versions.contains(&llvm_covmap_version),
"Coverage mapping version exposed by `llvm-wrapper` is out of sync; \
expected {expected_versions:?} but was {llvm_covmap_version}"
);
// This is the version number that we will embed in the covmap section:
llvm_covmap_version
};

debug!("Generating coverage map for CodegenUnit: `{}`", cx.codegen_unit.name());

Expand Down Expand Up @@ -74,7 +85,7 @@ pub fn finalize(cx: &CodegenCx<'_, '_>) {

// Generate the coverage map header, which contains the filenames used by
// this CGU's coverage mappings, and store it in a well-known global.
let cov_data_val = generate_coverage_map(cx, version, filenames_size, filenames_val);
let cov_data_val = generate_coverage_map(cx, covmap_version, filenames_size, filenames_val);
coverageinfo::save_cov_data_to_mod(cx, cov_data_val);

let mut unused_function_names = Vec::new();
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,5 +205,8 @@ extern "C" void LLVMRustCoverageWriteMappingVarNameToString(RustStringRef Str) {
}

extern "C" uint32_t LLVMRustCoverageMappingVersion() {
return coverage::CovMapVersion::Version6;
// This should always be `CurrentVersion`, because that's the version LLVM
// will use when encoding the data we give it. If for some reason we ever
// want to override the version number we _emit_, do it on the Rust side.
return coverage::CovMapVersion::CurrentVersion;
}

0 comments on commit ca7d34e

Please sign in to comment.