Add cross-platform Coverage CI#842
Merged
mjp41 merged 11 commits intomicrosoft:mainfrom May 5, 2026
Merged
Conversation
Adds a coverage workflow that mirrors the regular ctest invocation across the CI matrix and surfaces a per-PR coverage delta. Coverage is advisory only and never gates a PR. Build wiring (CMakeLists.txt, cmake/run_coverage.cmake) - New SNMALLOC_COVERAGE option (Clang/AppleClang only) adding -fprofile-instr-generate -fcoverage-mapping to compile and link. - Per-test LLVM_PROFILE_FILE so each test's profraws live in their own subdir and can be invalidated independently by the cached runner. - Global SNMALLOC_TEST_BINARIES property collects every test target so the new `coverage` custom target can pass them all to llvm-cov without globbing build outputs. - run_coverage.cmake drives ctest under coverage with sha256-keyed per-test caching, then merges per-test profdata and exports a unified coverage.json. Hashes are only cached when ctest reports overall success, so a timed-out or crashed test does not get its partial profraw frozen as the canonical answer for that binary. Merge tool (.github/scripts/merge_coverage.py + tests) - Per-line set-union merger of llvm-cov JSON exports across platforms: merged.executable = ⋃ executable lines, merged.covered = ⋃ covered lines, with covered ⊆ executable asserted at merge. - Emits per-platform breakdown alongside the union for the comment renderer. - 16 pytest cases cover union semantics, invariant enforcement, rendering, and determinism. Reusable workflows (reusable-cmake-build.yml, reusable-vm-build.yml) - New coverage-mode (off | tests | tests+selfhost) and coverage-artifact-name inputs; artifact name validated against ^[A-Za-z0-9_-]+$. - When coverage-mode != off: forces Debug, sets SNMALLOC_COVERAGE=ON, invokes the `coverage` target instead of plain ctest. - A single self-host step (always sets LLVM_PROFILE_FILE; harmless when not a coverage build) plus a separate post-step that exports selfhost.json only under coverage-mode == 'tests+selfhost'. Coverage workflow (.github/workflows/coverage.yml) - pull_request + nightly schedule + workflow_dispatch. - Matrix: ubuntu-24.04, macos-14 (brew llvm@19, absolute tool paths), linux-self-host-shim, linux-self-host-shim-checks (adds SNMALLOC_MEMCPY_BOUNDS=ON + SNMALLOC_CHECK_LOADS=ON, runs tests+selfhost), freebsd-14 (absolute /usr/local/llvm19/bin paths), netbsd-10 (pkgsrc clang/clang++), and a Windows clang-cl pre-gate build that exercises configure-time code paths without producing coverage. - Merge job: deterministic find | sort over downloaded artifacts, produces a coverage-merged artifact consumed by the commenter. Comment workflow (.github/workflows/coverage-comment.yml) - workflow_run trigger keeps write permissions out of the build job (read-all token in coverage.yml; pull-requests:write + issues:write here, no contents). - Validates artifact size/structure and the snmalloc-coverage-bot marker before posting. - PR path: dual-marker find-or-create with 3-attempt 409/403 backoff and pagination via github.paginate(). - Tracking-issue path (vars.COVERAGE_TRACKING_ISSUE) for nightly runs and pushes to the default branch. Marker-string coupling between the Python merger and the JS commenter is documented at both call sites; maintainers must update them in lockstep.
GitHub Actions expressions only accept single-quoted strings, but the ctest invocation contains single quotes (the -E regex). Move the off-vs-coverage selection from a expression into a shell inside the script body.
- Validation regex: allow `.` in coverage-artifact-name so matrix labels like `ubuntu-24.04` pass. - FreeBSD: llvm19 port installs versioned binaries (clang19, clang++19, llvm-cov19, llvm-profdata19) directly under /usr/local/bin/, not /usr/local/llvm19/bin/. Update absolute paths. - macOS: brew clang on the Apple SDK warns -Wundef on the SDK's `__STDC_VERSION__` check because brew clang doesn't treat the SDK as system headers by default. Export SDKROOT in the dependencies step so brew clang picks up the SDK as a sysroot (system include path), suppressing -Wundef in those headers. - Self-host coverage export: ls with two glob arguments returns nonzero when one glob has no match (e.g. .dylib on Linux), and pipefail propagated the failure. Use bash nullglob + array instead. Co-authored-by: Copilot <copilot@github.com>
pkgsrc's `llvm`/`clang` packages do not bundle libclang_rt.profile.a; the profile runtime lives in the separate `compiler-rt` package. Add it to pkg_add so -fprofile-instr-generate links cleanly. If this leg goes red because pkgsrc cannot resolve `compiler-rt`, the fallback is to fetch it from the NetBSD binary package mirror. Co-authored-by: Copilot <copilot@github.com>
pkgsrc's compiler-rt-19 ships a profile runtime in which __llvm_profile_raw_version is declared hidden but not defined. This makes every -fprofile-instr-generate shared library unlinkable — including libsnmallocshim.so during the regular build, not just the self-host step: R_X86_64_PC32 against undefined hidden symbol `__llvm_profile_raw_version` can not be used when making a shared object There is no fix on our side. The leg can be reinstated once pkgsrc ships a fixed compiler-rt, or by adding an in-VM compiler-rt build from llvm-project source (rejected for now — ~10 min per CI run for marginal NetBSD-pal coverage).
llvm-cov export emits JSON by default; the -format flag only accepts text|lcov, so -format=json failed CLI parsing. Co-authored-by: Copilot <copilot@github.com>
Drop build-only and contribute Windows coverage to the merged report. Exercises the Windows PAL surface, which no other leg in the matrix touches. windows-2022 runners ship LLVM (clang-cl, llvm-profdata, llvm-cov under C:\Program Files\LLVM\bin, on PATH) and ninja preinstalled, so no \`dependencies:\` step is needed.
- Windows: pass absolute -DLLVM_COV / -DLLVM_PROFDATA paths instead of relying on PATH ordering inside the runner image. - Self-host step: tighten the if condition so it doesn't run a pointless self-host build when self-host=true is requested alongside an in-progress coverage build (the coverage-mode=='tests+selfhost' branch already covers that). Add a comment explaining the coupling with the Export step. - test_all_platforms_empty: assert covered ⊆ executable invariant for consistency with every other test in the suite. Co-authored-by: Copilot <copilot@github.com>
The coverage matrix had two redundant Linux legs: - plain ubuntu-24.04 ctest, fully covered by macOS/FreeBSD/Windows ctest - linux-self-host-shim with no extra mitigations The remaining linux-self-host-shim-checks leg mirrors main.yml's self-host job (SNMALLOC_MEMCPY_BOUNDS=ON + SNMALLOC_CHECK_LOADS=ON) and is the only leg that exercises the bounds-checked memcpy and load-check paths. Also fix the self-host export: llvm-cov export was only given the first shim variant as -object, silently dropping coverage mappings for libsnmallocshim-checks.so and libsnmallocshim-checks-memcpy-only.so even though the for-loop ran ninja under each. Pass every built shim as a separate -object.
The Windows job passed -DLLVM_PROFDATA=C:/Program Files/LLVM/bin/llvm-profdata.exe
via a YAML folded scalar. The unquoted space survived YAML but split
at shell expansion of ${{ inputs.extra-cmake-flags }} in the
reusable workflow, so CMake received -DLLVM_PROFDATA=C:/Program plus
a phantom positional. Configure passed (find_program does not
override a preset cache value, even a bogus one), but the coverage
target later failed in execute_process with:
llvm-profdata merge failed (exit no such file or directory)
The windows-2022 runner image installs LLVM with C:\Program Files\LLVM\bin
already on PATH, so find_program(NAMES ... llvm-profdata) resolves
correctly without any override.
The 'coverage-mode' input had three values (off / tests / tests+selfhost) that conflated two orthogonal concerns: 1. Is this a coverage build at all? 2. Should self-host run? (2) is already expressed by the existing 'self-host' boolean input, so 'tests+selfhost' was redundant — it just meant 'coverage && self-host'. Replace with a 'coverage' boolean. Self-host now runs whenever the caller asks for it, regardless of coverage. The export step's gate becomes 'coverage && self-host'. The validate step drops its enum case, keeping the artifact-name regex check. No behavioural change for any existing call site: coverage-mode: 'off' -> coverage: false (default) coverage-mode: 'tests' -> coverage: true, self-host: false coverage-mode: 'tests+selfhost' -> coverage: true, self-host: true
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a coverage workflow that mirrors the regular ctest invocation across the CI matrix and surfaces a per-PR coverage delta. Coverage is advisory only and never gates a PR.
Build wiring (CMakeLists.txt, cmake/run_coverage.cmake)
coveragecustom target can pass them all to llvm-cov without globbing build outputs.Merge tool (.github/scripts/merge_coverage.py + tests)
Reusable workflows (reusable-cmake-build.yml, reusable-vm-build.yml)
coveragetarget instead of plain ctest.Coverage workflow (.github/workflows/coverage.yml)
Comment workflow (.github/workflows/coverage-comment.yml)
Marker-string coupling between the Python merger and the JS commenter is documented at both call sites; maintainers must update them in lockstep.