Skip to content
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

Handle source code coverage in const eval #73156

Open
tmandry opened this issue Jun 9, 2020 · 4 comments
Open

Handle source code coverage in const eval #73156

tmandry opened this issue Jun 9, 2020 · 4 comments
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...)

Comments

@tmandry
Copy link
Member

tmandry commented Jun 9, 2020

One open question around source code coverage instrumentation (#34701 / rust-lang/compiler-team#278) is how to handle code that is only run during const eval. For example:

const fn compute_foo(x: bool) -> i32 {
  if x {
    42
  } else {
    11
  }
}

#[cfg(test)]
mod tests {
  #[test]
  fn test_foo() {
    assert_eq!(compute_foo(false), 11);
    assert_eq!(compute_foo(true), 42);
  }
}

In a coverage report, we don't want the code in compute_foo to show as not being covered by tests because it was only "run" at compile time.

Approaches

I've talked about this with @petrhosek a couple times, he raised two possible approaches for both rustc and clang. I'll do my best to communicate them, but any mistakes are mine.

Emit a coverage profile at compile time

When code coverage instrumentation is enabled, another option can be specified which causes a profile to be emitted at compile time (in the same format as the profiles generated at runtime). This would include every counter hit while evaluating MIR during const eval. The profile could then be combined with profiles collected during runtime (which I believe tooling already exists for).

One benefit of this approach is that it allows us to handle counters hit at compile time differently from those hit at runtime. I can see this being useful e.g. if we were to use those counters for some kind of PGO pass in the MIR, though I'm not sure if that's a realistic use for them. There might be other reasons to differentiate.

A challenge with the approach is that we'll have to produce the profile format ourselves (we won't get the benefit of LLVM instrumentation doing it for us, like we do in the final binary).

Initialize runtime counters with compile time counts

Similar to before, we would count how many times each source code coverage region was evaluated during compile time. Instead of emitting a profile at compile time, we can simply initialize the (static) counter in the final binary to this value. So if that counter was evaluated 42 times, every profile emitted by the binary would start at 42 (and go up from there if the same counter were also hit at runtime). We might need to extend LLVM to support something like this.

The main benefit of this is simplicity: there are less "moving parts" for someone who wants to use source code coverage. We also don't have to teach the compiler how to emit coverage profiles.

One possible issue with this approach is how it might interact with linker garbage collection. We wouldn't want the linker to remove nonzero counters for never being referenced from any code in the binary.

@RalfJung
Copy link
Member

Cc @rust-lang/wg-const-eval

@RalfJung RalfJung added the A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) label Jun 10, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Jun 10, 2020

I don't see how we can do this in a way that doesn't clash with the query system, except by making the const_eval_raw query also emit that information. We can't store it in a side table while running the queries. Then codegen can invoke all those queries and take just the coverage information out of the result.

I guess as a first draft we could just runtime increment the counters by invoking the llvm instrumentation for the specific values at some point in the binary? Seems hacky, but would require no llvm changes giving us access to the counters, since we'd be invoking the appropriate intrinsics instead.

@gilescope
Copy link
Contributor

Earlier one thing I really liked about rustc was that it would complain about dead code that was not reachable by the tests. I always thought of that as compile time code coverage (Alas I think that property has disappeared). Would const eval code not evaluated during the compile count as dead code and could be reported that way?

@oli-obk
Copy link
Contributor

oli-obk commented Oct 15, 2020

This issue is not about the dead code lint, but instead about code coverage. It could be possible (once this issue is resolved) to use this coverage information for the dead code lint, but that would require the appropriate instrumentation, which we don't want to use during normal compilation (as it will slow down the compiler).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...)
Projects
None yet
Development

No branches or pull requests

4 participants