Skip to content

Some Rust functions still missing coverage (-Z instrument-coverage) #79617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
richkadel opened this issue Dec 2, 2020 · 1 comment
Closed
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) C-bug Category: This is a bug.

Comments

@richkadel
Copy link
Contributor

richkadel commented Dec 2, 2020

PR #79109 included improvements to the Coverage Map Generation to identify uncovered functions, and inject Unreachable code regions for them.

mapgen.rs identifies the Unreachable functions by querying the tcx for the DefIds of all MIR, and subtracting the DefIds of all code-generated functions (per tcx.collect_and_partition_mono_items).

Then mapgen.rs has to identify at least one Instance/DefId already code-generated and in the current CoverageCx to which to add the Unreachable function code regions. Code regions can only be added to the LLVM coverage map if associated with a known, code-generated, function; and the known function cannot be a generic function. There are a couple of other constraints that must be followed, as discussed in the mapgen.rs comments.

Some functions are still missing coverage, for example, see the results for the range at:
https://github.com/google/json5format/blob/920d8880ba2f39cd1cc405efb90aa3f91b22aba4/src/parser.rs#L22-L162

I don't see how the documented constraints apply in these cases, but there certainly are unique characteristics that may contribute to the coverage loss.

The amount of code without coverage has been minimized overall, and it may not be a priority to resolve these remaining gaps; but if and when it becomes a priority, I recommend:

  1. Identify missing coverage like the example below (there may be others in this and/or other code bases) and deduce the root cause of the missing coverage.
  2. Determine how to inject additional "Unreachable" coverage regions.
  • In the case below, most of the missing coverage is inside a lazy_static! { ... } block. Macros are likely to interfere with coverage analysis, but perhaps there are workarounds.
  • exact_match() is outside the lazy_static!, but only called from within the lazy_static!, which may (or may not) contribute to the lost coverage.
  • matches_unquoted_property_name() is also missing coverage, and is called from another function that has coverage (though according to that function's coverage, matches_unquoted_property_name() was never actually called during the test execution). I would have expected to find the DefId for the MIR for this function, and gotten coverage. Perhaps the function was inlined before the InstrumentCoverage pass? I'm really not sure at this point.

Example to reproduce the issue:

(This is not MCVE, sorry, but it should be reproducible.)

I tried this code (assumes the default compiler includes PR #79109):

$ cd $HOME
$ git clone [email protected]:google/json5format.git
$ cd json5format
$ RUSTFLAGS="-Zinstrument-coverage" cargo build --example formatjson5
$ LLVM_PROFILE_FILE="formatjson5.profraw" ./target/debug/examples/formatjson5 path_to_some.json # or json5 file
$ llvm-profdata merge -sparse formatjson5.profraw -o formatjson5.profdata
$ llvm-cov show --use-color --instr-profile=formatjson5.profdata target/debug/examples/formatjson5 --show-line-counts-or-regions  --Xdemangler=rustfilt --show-instantiations | less -R

I expected to see this happen: Ideally, all functions would be covered

Instead, this happened: Some functions are still missing coverage, for example, see the results for the range at: https://github.com/google/json5format/blob/920d8880ba2f39cd1cc405efb90aa3f91b22aba4/src/parser.rs#L22-L162

Meta

I'm logging this issue based on the current version of PR #79109, which I anticipate landing very soon, and I don't expect to resolve this issue in that PR. To run the test described above will require a Rust nightly distribution from early- to mid-December 2020 or later, after PR #79109 has landed in the nightly distro.

@richkadel richkadel added the C-bug Category: This is a bug. label Dec 2, 2020
@wesleywiser wesleywiser added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Dec 2, 2020
@richkadel
Copy link
Contributor Author

This issue described a general problem that has been resolved by PR #83774. I confirmed this by re-running the test in this issue's description.

(Note there is a separate, related issue, that was also mostly resolved by #83774, except for one case. See #83985 (comment). So issue #83985 remains open to address that specific edge case.)

@tmandry: THIS issue (#79617) can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

2 participants