Fix scope leak via forwarded CcInfo and broken HTML links (#5) - #6
Merged
Merged
Conversation
Two defects reported in issue #5, both visible in the baselibs report: 1. A workspace rule that only forwards the CcInfo of a third-party library (baselibs third_party/openssl applies a transition that way) made that library's headers the workspace target's direct_public_headers, and the scope aspect took them as declared headers: 72 external/openssl+ files in the report. 2. The reporter let llvm-cov read sources through the workspace directory. Generated headers (bazel-out/.../_virtual_includes/) exist there only after an earlier local build and external repositories never do, so those pages were silently not rendered while the index still linked them (94 dead links). Scope aspect: - Only files a workspace target declares itself (srcs/hdrs, including headers vendored from an external repository) enter the allowlist. Forwarded headers do not. - A header exposed through strip_include_prefix / include_prefix is recorded in a new path map (<name>_path_map.txt): the generated _virtual_includes/ path the compiler names, and the declared header it stands for. Only paths the target generated itself are mapped. - The source files are exported (source_files output group) and the reporter wrapper adds them to the reporter's runfiles. Reporter: - Stages every in-scope source from its runfiles under the raw covmap layout and points llvm-cov at that directory, so every page can be rendered regardless of the workspace directory's state. - Files the HTML pages under coverage/<canonical path>.html, rewrites the index links and the pages' asset links, and names files canonically in LCOV, text summary and page titles: headers behind include prefixes appear under their declared path, never under _virtual_includes/ or a machine-specific directory. A row whose source could not be read keeps its numbers but loses the dead link. - Exclusion filters match one compiled file exactly (anchored on the recorded compilation directory and the staging root); an excluded foo/bar.h no longer suppresses an in-scope src/foo/bar.h. - A file compiled under two paths is reported once (declared path preferred), with a warning. Tests: 5 new analysis tests / fixtures (forwarded external library, include_prefix mapping, path map and source_files outputs), 19 new reporter unit tests (selection, exclusion regex, staging, page relocation, path map, summary naming), and the integration workspace gained a forwarded third-party library plus a vendored external header with hand-derived ground truth and a link-integrity check over the generated index. Validated on baselibs (//score/hash/..., //score/flatbuffers/..., //score/static_reflection_with_serialization/...): 440 index links, all resolving; no external/openssl+ entries; no bazel-out/ or absolute paths; the 15 flatbuffers headers under external/flatbuffers+/include/flatbuffers/, each once. Docs: tool requirements (scope_transitive, scope_excludes, report_allowlist, report_relative_paths), architecture, known problems, release notes 0.2.0, verification report inventory. Fixes #5 Signed-off-by: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com>
dcalavrezo-qorix
requested review from
AlexanderLanin,
MaximilianSoerenPollak and
nradakovic
as code owners
September 15, 2026 06:48
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
A file in the scope allowlist for which no test binary and no baseline archive carries a coverage mapping cannot be rendered by llvm-cov, not even at 0 %: a header no translation unit includes, or template code that is never instantiated. Until now such a file was silently absent from every report format, which contradicts the promise that untested in-scope files show up. baselibs' flatbuffers/buffer_ref.h, a vendored public API header nothing in baselibs uses, went unnoticed that way. - reporter: the selection computes the allowlisted files without any data. Headers whose same-named source file has data (foo.h next to a compiled foo.cpp) hold declarations only and are kept apart, so the actionable list is not buried under them. Both lists are written to text_report/unmapped_files.txt and text_report/declaration_only_headers.txt (always present) and the reporter warns about the first. - generate_coverage_html: prints the unmapped files, copies both lists into the archive, and hands them to the summary. - coverage_summary: a table row "In-scope files without coverage data" and two collapsible sections; older reports without the lists render as before. Tests: selection, reporter main, summary rendering and loading, generator wiring and archive layout; the integration workspace gains a template-only header nobody includes (src/unused_api.h) and checks the lists, the summary and that no LCOV record is invented for it. Docs: new tool_req__coverage_report_unmapped, known problems, release notes 0.2.0, user manual, verification report inventory. Follow-up of #5 (discussion of vendored third-party headers). Signed-off-by: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com>
Contributor
Author
|
Second commit: in-scope files without any coverage data are now reported instead of silently missing (reporter warning, |
Follow-up on the unmapped-files diagnostic, driven by what it showed on
baselibs (377 entries for three packages, most of them not findings):
- The list is now "<category>\t<path>" with three categories:
declaration-only (a header whose same-named source has data),
empty-translation-unit (a source compiled into a baseline archive whose
object carries no coverage mapping, i.e. the placeholder .cpp of a
header-only library), and no-data (the findings: a header nothing
includes, template code never instantiated, a source never built).
Only no-data is warned about and counted in the job summary table;
each category gets its own collapsible section.
- Real bug found on the way: llvm-cov rejects an archive as a whole
("no coverage data found") as soon as ONE member has no __llvm_covmap
section. A library with a placeholder .cpp next to real sources
therefore silently lost the 0 % baseline of all its other files. The
reporter now inspects every member's ELF section table and passes only
members with a mapping to llvm-cov, generalising the rlib expansion
(tool_req__coverage_report_rlib_expansion v2). The dropped members are
what identifies the empty-translation-unit category.
Tests: synthetic ELF64 objects for the covmap probe and archive
splitting (rlib, plain, mixed, all-empty, executable); selection with
the three categories; summary loader and renderer; generator wiring.
The integration workspace's uncovered library gains a placeholder
.cpp, which without the archive fix removes uncovered.cpp from the
golden LCOV, and the archive check expects the categorised list.
Docs: tool_req__coverage_report_unmapped and _rlib_expansion, known
problems, release notes, verification inventory.
Signed-off-by: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com>
Contributor
Author
|
Third commit. The diagnostic list is now categorised ( |
On baselibs the placeholder .cpp of a header-only library usually does carry a coverage mapping, for the inline functions of the headers it includes, while the file itself has no regions. Deciding "compiled but no code of its own" by the absence of a mapping therefore caught 37 of 82 such files and left the rest among the findings. The category is now decided by the object being a member of a baseline archive at all, and renamed compiled-without-code; the mapping probe is kept for what it is needed for, handing llvm-cov only loadable members. The archive reader also skips the GNU symbol table member, which was counted as a member without mapping and made every archive look mixed. baselibs, whole tree: 531 files reported, all index links valid, baseline 446 files (431 before the archive split), findings 243, of which 172 are template headers of score/language/futurecpp. Signed-off-by: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com>
Contributor
Author
|
Fourth commit: the |
MaximilianSoerenPollak
previously approved these changes
Sep 16, 2026
MaximilianSoerenPollak
left a comment
MaximilianSoerenPollak
left a comment
Contributor
There was a problem hiding this comment.
Talked about in chats.
Reasoning for changes made sense, unclear passages were explained.
Lead with what changed as a reader without C++ background would read it, then explain the new unmapped_files.txt and its three categories (no-data, declaration-only, compiled-without-code) with what each means and what to do. The technical bullets stay under a separate heading for integrators. Signed-off-by: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com>
baselibs' futurecpp tests compile the library's headers through a test-only twin target (futurecpp_internal: same hdrs behind strip_include_prefix, test copts). The coverage mapping therefore names bazel-out/.../_virtual_includes/futurecpp_internal/score/<h>.hpp. The scope's path map only knows trees of targets the aspect visited, so the reporter took these for unknown files, excluded them as out of scope, and then listed 172 futurecpp headers as having no coverage data, although 68 tests exercise them. The reporter now resolves such a name by its path below the tree: the single allowlisted file that ends with it (shorter tails are tried for an include_prefix). A tail matching several in-scope files is left unresolved and warned about. The resolved names go into the path map, so LCOV, HTML and summary use the declared header path. Tests: resolver (unique, ambiguous, include_prefix, no match), reporter main with a foreign tree and no path map entry. The integration workspace's test now compiles the vendored header through a test-only twin target, which reproduces the futurecpp pattern: without the fix the header vanishes from the golden LCOV. Docs: tool_req__coverage_report_relative_paths, known problems, release notes (plain words and integrator details), inventory. Signed-off-by: Dan Calavrezo <195309321+dcalavrezo-qorix@users.noreply.github.com>
Contributor
Author
|
Sixth commit: headers compiled through the |
Contributor
Author
|
Whole-tree baselibs with the sixth commit (all 300 tests, CI exclusions applied): 640 files in the report, every index link valid; 109 futurecpp headers recovered through the twin-target fallback, futurecpp now has 116 files with data; raw line coverage 87.76 %. |
MaximilianSoerenPollak
approved these changes
Sep 16, 2026
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
2 participants
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.
Fixes #5.
Root causes
Both symptoms come from the baselibs report of eclipse-score/baselibs#584:
external/openssl+entries. baselibs wraps@openssl//:cryptoin a workspace rule that only forwards the library'sCcInfo(to apply a transition). That makes OpenSSL's headers the workspace target'sdirect_public_headers, and the scope aspect treated them as declared headers.bazel-out/.../_virtual_includes/...andexternal/...). The reporter let llvm-cov read sources through the workspace directory.bazel-out/exists there only after an earlier local build (which is why it worked on a developer machine and not in CI) and external repositories never do; llvm-cov skips the page silently but still emits the index row.Changes
Scope aspect
srcs/hdrs, including headers vendored from an external repository, baselibs#558) enter the allowlist. Headers a wrapper rule merely forwards do not.<name>_path_map.txt: for a header behindstrip_include_prefix/include_prefix, the generated_virtual_includes/path the compiler records and the declared header it stands for. Only paths the target generated itself (owner) are mapped.source_filesoutput group); the reporter wrapper puts them into the reporter's runfiles.Reporter
coverage/<canonical path>.html, rewrites index links and the pages'style.css/control.jslinks, and names files canonically in LCOV, text summary and page titles. Headers behind include prefixes appear under their declared path (external/flatbuffers+/include/flatbuffers/base.h), never under_virtual_includes/or a machine directory. A row whose source could not be read keeps its numbers but loses the dead link.foo/bar.halso suppressed an in-scopesrc/foo/bar.h.Consumers instantiate the same two macros; no change on their side. Justifications written against a
_virtual_includes/path must move to the declared path (release notes 0.2.0).Acceptance criteria of #5
bazel-out/**/_virtual_includes/**inputs resolve to a clickable page for the underlying filecoverage/src/vendored/include/vendored/inline_math.h.htmlandcoverage/external/itest_external+/include/vext/vext.h.htmllinked and present; baselibs: 15 flatbuffers headers underexternal/flatbuffers+/include/flatbuffers/external/**of third-party targets excluded from index and totalsitest_externallibrary executed by the test, absent from LCOV and HTML; baselibs: 0external/openssl+entrieshrefofindex.htmland the stylesheet link of every page; baselibs: 440 links, all resolveinclude_prefix, outputs), 19 new reporter unit tests, integration fixtures with hand-derived ground truthNote on the second criterion: a header a workspace target declares from an external repository (
//score/flatbuffers:flatbufferscppin baselibs, the subject of baselibs#558) stays in scope and is reported under itsexternal/<repo>/...source path. That is the target's own code; what is excluded is code reached only through a dependency, forwarded or not.Verification
bazel test //score_coverage/... //tools/...integration_tests/run_integration_test.sh//score/hash/... //score/flatbuffers/... //score/static_reflection_with_serialization/...with this branchexternal/openssl+,bazel-out/or absolute paths; 26 headers mapped to their declared path