Support LLVM 23 raw profile format (version 11) - #82
Conversation
LLVM 23 bumped INSTR_PROF_RAW_VERSION to 11 and changed two structures, so v11 .profraw files are misparsed. The visible symptom is a `Nom(Satisfy)` parser failure preceded by "consistency check for reading counts failed". Three changes are needed, and the third is easy to miss: 1. The raw header gains three uint64 fields before NamesSize: NumUniformCounters, PaddingBytesAfterUniformCounters, UniformCountersDelta. Without reading them, every later field is off by 24 bytes -- notably counters_delta, which read_raw_counts uses for offset arithmetic, which is what trips the consistency check. 2. The per-function ProfileData record gains UniformCounterPtr (after CounterPtr) and OffloadDeviceWaveSize (a uint16 after NumValueSites[]). 3. The struct tail padding MOVED. In v9/v10 ProfileData ends on a 4-byte field at offset 60 so C pads to 64 -- which is what the existing `take(4)` in parse_bytes compensates for (answering its "TODO WHAT AM I MISSING HERE?": it is struct tail padding). In v11 OffloadDeviceWaveSize pushes NumBitmapBytes to offset 68, the struct ends at 72 which is already 8-aligned, so there is no tail padding and the 2 padding bytes move INSIDE the struct instead. Taking 4 there as well over-consumes and desynchronises every record after the first. Verified end to end against a real v11 profraw from rustc 1.99.0-nightly (LLVM 23.1.0): the header and both ProfileData records now decode correctly, and `cargo tarpaulin --engine llvm` reports 100.00% coverage, 1/1 lines on a one-function crate -- matching what the ptrace engine independently reports. Refs xd009642#81
|
Cool, I might not have time to look at this today but the way the test vectors are done for the integration tests is I grab them from the llvm repo, they should be in the tests, tools, llvm profdata folder. I then add in all the profdata and profraw files and some proftext and see if anything fails. It also needs a feature for that llvm version added as well. So if you have time to do that before I review it'll be helpful |
|
@Nexlab-One I'm gonna guess you didn't meant to close this PR and it was an issue with github's commit message handling - so I've reopened this 😅 |
|
|
||
| // Raw profile version 11 (LLVM 23) inserts three uint64 fields here, before | ||
| // NamesSize. Without reading them every subsequent field is off by 24 bytes. | ||
| let (bytes, num_uniform_counters, padding_bytes_after_uniform_counters, uniform_counters_delta) = |
There was a problem hiding this comment.
You don't parse the non uniform counters section. So in the instances where this could be non-zero the name section parsing will instead start parsing from the non uniform parsing section.
I think this and updating the integration tests with the test vectors from the latest LLVM, adding the feature for the version and making sure the "latest llvm version" check matches this LLVM version are all that are needed 👍
Addresses the review feedback on xd009642#82. The header change alone was not enough. compiler-rt writes the body as data, PaddingBytesBeforeCounters, counters, PaddingBytesAfterCounters, bitmap, PaddingBytesAfterBitmapBytes, uniform counters, PaddingBytesAfterUniformCounters, names (the IOVec list in lprofWriteDataImpl). parse_bytes stopped at the end of PaddingBytesAfterCounters and went straight to names, so with a non-empty uniform counter section the names parser starts reading inside the counter payload. In practice it does not produce garbage names, it panics: the first thing it reads there is a length prefix, and an arbitrary 8 bytes of counter data is a very large length. range end index 197877615 out of range for slice of length 90 The same gap applied to the bitmap section, which sits immediately before the uniform counters and was also never skipped, so both are handled together. Both are zero-sized in the common case, which is why this went unnoticed. The bitmap is only populated for MC/DC instrumentation and NumUniformCounters is 0 unless the uniform counter section is emitted, so the arithmetic is a no-op on an ordinary profile. Because both sections are normally empty, a profile captured from a normal build cannot exercise this. tests/data/profdata/misc/v11_uniform_counters.profraw is a real v11 profraw from rustc 1.99.0-nightly (771916f90 2026-08-08), LLVM 23.1.0, with two uniform counters spliced in and the three v11 header fields set to match. Reverting the skip and rerunning v11_uniform_counter_section_is_skipped reproduces the panic above; with the skip in place both symbol names decode. Test vectors, as requested: tests/data/profdata/llvm-23 is populated from llvm/test/tools/llvm-profdata/Inputs at release/23.x, commit d8145e71418fb1e0a936adfb07dc5317113fc3b6. Filtering to the extensions llvm-22 carries gives 98 files, the same count and per-extension breakdown as llvm-22 (72 proftext, 9 profdata, 9 memprofraw, 4 profraw, and one each of v1, v2, v4, v10). Downloaded through the API and base64 decoded rather than over raw HTTP, since this was fetched on Windows and the binary vectors must not go through newline translation. All 22 binary vectors (profraw, profdata, memprofraw) were then checked back against their upstream blob SHAs and every one is byte identical. The committed proftext files are LF in the index, matching upstream. Also adds the __llvm_23 feature, the (23, nightly-2026-08-08) entry in SUPPORTED_LLVM_VERSIONS, and moves LATEST_SUPPORTED_VERSION to 23. CI runs --all-features so it picks the new feature up without a workflow change. Verification, on Windows x86_64: cargo test --release --test profdata v11_uniform_counter_section_is_skipped 1 passed, and FAILED with the panic above when the skip is reverted the rest of cargo test --release is unchanged from before this commit: merge, show_profdatas, show_proftexts and show_profraws fail identically because cargo profdata is unavailable here, everything else passes cargo profdata could not be used to drive the integration tests in this environment. cargo-binutils 0.4.0 panics inside clap on any cargo profdata -- <args> invocation, so the harness cannot shell out to it. As a substitute I ran llvm-profdata from the nightly toolchain directly over the new vectors and compared it against profparser show for each file, matching on hash, counter count, function count and block counts. Of the 13 binary vectors, 4 are accepted by llvm-profdata and all 4 agree exactly; the other 9 are rejected by llvm-profdata itself, which is expected for a directory that deliberately contains malformed and older-format inputs. So the new directory is verified against LLVM's own tool, but not yet through the repository's own harness on this machine. One thing worth flagging: thinlto_indirect_call_promotion.profraw hits an unimplemented!() in raw_profile.rs when parsed directly. That is not new here, the same vector is already in tests/data/profdata/llvm-22, and llvm-profdata rejects it too so check_command skips it. The uniform counter values themselves are still read and discarded rather than surfaced, which remains the open question from the original description. cargo fmt --check is clean. It was not clean before this commit: the earlier v11 header work left three spots unformatted in raw_profile.rs, including an error! call that lost four spaces of indentation relative to master. Those are fixed here rather than left for CI.
|
Yea, you're correct about the uniform counters, it breaks worse than missparsed names. Pushed 7200791. compiler-rt writes the body as data, PaddingBytesBeforeCounters, counters, PaddingBytesAfterCounters, bitmap, PaddingBytesAfterBitmapBytes, uniform counters, PaddingBytesAfterUniformCounters, names (the IOVec list in The bitmap sits immediately before the uniform counters and was never skipped either, so both are stepped over in the same place. Both are zero sized on an ordinary profile, which is why nothing noticed: the bitmap is only populated for MC/DC, and NumUniformCounters is 0 unless that section is emitted. A normal build can't produce a profile that exercises it, so the following was added Test vectors as you asked: Also added the One thing unable to be verified on my machine: cargo-binutils 0.4.0 panics inside clap on any Two notes. Still open from the original description: the uniform counter values are read and then discarded rather than surfaced. |
|
There'll be a new version of tarpaulin out later today with this change in btw |
3 participants
Fixes #81.
LLVM 23 bumped
INSTR_PROF_RAW_VERSIONto 11, so v11.profrawfiles currently fail to parse withNom(Satisfy)(preceded byconsistency check for reading counts failed). That breakscargo tarpaulin --engine llvmon current Rust nightly.Three changes are needed. The third is the one I did not expect:
1. Header — three new
uint64_tbeforeNamesSize:NumUniformCounters,PaddingBytesAfterUniformCounters,UniformCountersDelta.parse_headergates on>= 7,>= 9,>= 10, so with no>= 11branch everything after is shifted 24 bytes — includingcounters_delta, whichread_raw_countsuses for offset arithmetic. That is what trips the consistency check.2.
ProfileDatarecord —UniformCounterPtrafterCounterPtr, andOffloadDeviceWaveSize(auint16) afterNumValueSites[].3. The struct's tail padding moved. In v9/v10 the record ends on a 4-byte field at offset 60, so C pads it to 64 — which is exactly what the existing line in
parse_bytescompensates for:So that
TODOis answered: it is struct tail padding. In v11,OffloadDeviceWaveSizepushesNumBitmapBytesto offset 68, the struct ends at 72 which is already 8-aligned, and the 2 bytes of padding move inside the struct. Taking 4 bytes there as well over-consumes and desynchronises every record after the first — which is how I found it: with only changes 1 and 2 applied, record 0 parsed correctly and record 1 started at byte 260 instead of 256.Verification
Against a real v11
.profrawfromrustc 1.99.0-nightly (771916f90 2026-08-08), LLVM 23.1.0:NamesSize = 90, sane deltas);ProfileDatarecords decode correctly —NumCounters2 and 1, summing to the header'scounters_lenof 3;which matches what
--engine ptraceindependently reports for the same crate, so the numbers are right and not merely parseable.cargo test --releaseshows no regression: 4 integration tests (merge,show_profdatas,show_proftexts,show_profraws) fail identically on unpatched master in my environment becausecargo profdatais not installed — I checked the baseline specifically to be sure they were not mine. All other tests pass with and without the patch.Things I would want a second opinion on
OffloadDeviceWaveSizeare read and stored but otherwise unused. I established that reading them realigns parsing; I did not establish whether LLVM 23 ever emits meaningful uniform-counter data that should be surfaced rather than skipped. The names suggest a new counter representation, so if that is the case this PR is necessary but not sufficient.