Stabilize -Z instrument-coverage as -C instrument-coverage

[joshtriplett]

(Tracking issue for instrument-coverage: #79121)

This PR stabilizes support for instrumentation-based code coverage, previously provided via the -Z instrument-coverage option. (Continue supporting -Z instrument-coverage for compatibility for now, but show a deprecation warning for it.)

Many, many people have tested this support, and there are numerous reports of it working as expected.

Move the documentation from the unstable book to stable rustc documentation. Update uses and documentation to use the -C option.

Addressing questions raised in the tracking issue:

If/when stabilized, will the compiler flag be updated to -C instrument-coverage? (If so, the -Z variant could also be supported for some time, to ease migrations for existing users and scripts.)

This stabilization PR updates the option to -C and keeps the -Z variant to ease migration.

The Rust coverage implementation depends on (and automatically turns on) -Z symbol-mangling-version=v0. Will stabilizing this feature depend on stabilizing v0 symbol-mangling first? If so, what is the current status and timeline?

This stabilization PR depends on #90128 , which stabilizes -C symbol-mangling-version=v0 (but does not change the default symbol-mangling-version).

The Rust coverage implementation implements the latest version of LLVM's Coverage Mapping Format (version 4), which forces a dependency on LLVM 11 or later. A compiler error is generated if attempting to compile with coverage, and using an older version of LLVM.

Given that LLVM 13 has now been released, requiring LLVM 11 for coverage support seems like a reasonable requirement. If people don't have at least LLVM 11, nothing else breaks; they just can't use coverage support. Given that coverage support currently requires a nightly compiler and LLVM 11 or newer, allowing it on a stable compiler built with LLVM 11 or newer seems like an improvement.

The tracking issue and the issue label A-code-coverage link to a few open issues related to instrument-coverage, but none of them seem like showstoppers. All of them seem like improvements and refinements we can make after stabilization.

The original -Z instrument-coverage support went through a compiler-team MCP at rust-lang/compiler-team#278 . Based on that, @pnkfelix suggested that this needed a stabilization PR and a compiler-team FCP.

[rust-highfive]

Some changes occured to rustc_codegen_gcc

cc @antoyo

[rust-highfive]

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

[davidtwco]

Changes in this PR look good to me.

[antoyo]

The gcc part looks good to me.

[richkadel]

You may also need to stablize the #[no_coverage] attribute as well. See #84605. It's a minimal solution at present, but I think sufficient for the time being. Stablizing will allow you to move the references to that attribute to the stable docs, resolving the broken link here.

[Mark-Simulacrum]

The documentation I think currently points users towards installing/using https://crates.io/crates/cargo-cov, which looks both somewhat old and is not a rust-lang crate. I think we generally try to stay away from third party tools being directly referenced by our documentation, and especially it worries me when it says "may eat your laundry". We also point towards https://crates.io/crates/cargo-binutils and json5format, the latter of which seems entirely unnecessary.

I also note that the recommended path is making use of the llvm-tools-preview component, which I am somewhat uncomfortable with: as it notes, it's intentionally a preview component and while we have no firm policy, we don't treat it as really stable. Obviously, users can find llvm-cov in distros and such too, but that feels a little bit outside the beaten path. This component is not currently nightly-only, but only because it's introduction predates the ability to declare components as nightly only, I think.

In general, I think usage of LLVM tools (e.g., profdata, cov) should be OK -- we may want to consider uplifting them to a separate component and we should check with upstream that they're expected to stick around. (For example, the split-dwarf effort discovered that the LLVM tool it wanted to use is not well maintained, IIRC). But I would like us to drop references to third-party crates or consider whether they should be blessed from the documentation before we stabilize.

I think regardless, we need a description in the documentation for this flag as to what is considered stable (and if LLVM upstream or whatever changes, we need to workaround and patch to keep same behavior) and what isn't. This will be useful for future PR review making changes to this feature, such as #90047.

[richkadel]

@Mark-Simulacrum - These are all good concerns/recommendations.

Since I wrote most of the initial docs, its reasonable someone might want me to make at least some of the required changes, but...

I'll be out of office for about 3 weeks, without a laptop, starting tonight. (I'll probably check email via my phone, periodically, in case there's a question I can answer.)

Please keep @tmandry in the loop. He may be able to help with some parts of this. And if I can help when I return, I will.

Thanks!

[joshtriplett]

I'm happy to update the documentation.

[cuviper]

requiring LLVM 11 for coverage support seems like a reasonable requirement.

We're also bumping the overall minimum to LLVM 11 in #90062.

[joshtriplett]

@richkadel I've updated the link to point to #[no_coverage] in the unstable book for the moment. We can always stabilize #[no_coverage] after this, but I'd like to avoid combining the two stabilizations.

[joshtriplett]

@Mark-Simulacrum The documentation doesn't point towards the cargo-cov crate; it's using cargo-cov from cargo-binutils.

I've rewritten the documentation to use the LLVM tools directly. It still mentions the existence of llvm-tools-preview, but notes that it is still experimental.

[joshtriplett]

I think I've now addressed all of the open issues, except that the example still uses json5format. I do think it's important that the example uses something with a command-line tool, tests, and doctests; it'd be trivial to replace the command-line example with something like this:

fn main() {
    if let arg = std::env::args().nth(1).next() {
        println!("{}", arg);
    } else {
        println!("no argument");
    }
}

But such an example wouldn't show how to handle tests or doctests.

We've had this as the unstable documentation for quite some time, and while unstable features do get less usage, the unstable book is still official Rust documentation. I would propose that we merge the (now-rewritten) version, complete with the example using json5format; I don't think doing so amounts to blessing the json5format crate as anything other than "a crate on which we're demonstrating the use of instrument-coverage".

[pietroalbini]

I have a couple questions around this stabilization:

• Programs instrumented with this flag are currently emitting version 4 of the coverage mapping format, but there is already a PR bumping the format to version 6 (Update CoverageMappingFormat to Version6 #90047). What's the expected stability of the format of the emitted coverage files?

• The implementation seems to be tied with LLVM (the environment variable is called LLVM_PROFILE_FILE, instrumented programs rely on compiler-rt...), how is this feature expected to work with alternate codegen backends?

[joshtriplett]

@pietroalbini I would expect a relatively recent stable version of LLVM tooling to be required; in particular, I would not expect it to be required to work with tools from the oldest version of LLVM we support. I'll add documentation noting that the format will evolve over time, but will always work with a stable LLVM.

Regarding other codegen backends, in theory they could emit data in the same format. I would expect otherwise that they should produce an error comparable to the one produced by a build with an old LLVM.

[pietroalbini]

I would expect a relatively recent stable version of LLVM tooling to be required; in particular, I would not expect it to be required to work with tools from the oldest version of LLVM we support. I'll add documentation noting that the format will evolve over time, but will always work with a stable LLVM.

Hmm, I'm worried about emitting an unstable format from a stable flag, especially one that won't be niche like this one.

Thinking about the experience for end users, we recommend installing the tools required to use the generated files from the user's distribution, but for example neither Debian nor Ubuntu have LLVM 13 tools available in their stable distribution, and Debian lacks LLVM 12 tools too. Other distributions also likely ship older LLVM versions. If the compiler is free to bump the version of the format, those users will have their coverage setup break when updating the stable compiler.

Bumping the format without considering backward compatibility will also break everyone who uses third party tools to parse those files. cargo-tarpaulin, a fairly popular code coverage tool for Rust, is developing a pure-Rust parser for the format to avoid requiring the LLVM tools being installed, and I could definitely see tools being written to convert from the coverage information Rust emits into other formats if someone needs to integrate Rust code coverage inside a larger project.

Right now using this flag in a supported way (without llvm-tools-preview) implies coverage support will break eventually if someone updates the stable compiler, which I don't think is appropriate for a stable flag.

Regarding other codegen backends, in theory they could emit data in the same format. I would expect otherwise that they should produce an error comparable to the one produced by a build with an old LLVM.

Hmm, yeah. Would we expect the environment variable to still be LLVM_PROFILE_FLAG then?

[tmandry]

There are two formats, the profile data itself and the source mapping.

As I understand it, the profiles have no backwards compatibility guarantees. There is a version number that gets bumped with every change that is not backwards compatible, but older versions are not supported. The best way of guaranteeing compatibility for these is to release llvm-profdata with the Rust compiler. There's a decent chance that using tooling from the same major version of LLVM will work, but I don't know if that's guaranteed (@petrhosek may know more).

For the source mapping I'm not sure about backwards compatibility, but if it's the same story then we should release llvm-cov as well.

[joshtriplett]

To clarify: my expectation would be that we would occasionally bump the compatibility requirements of the format when there is a specific need to do so, such as new functionality we require, and I would expect us to not keep the version requirement in lockstep with our baseline requirement for LLVM, but I would still expect us to put thought into bumping the format compatibility forward. If, for example, cargo tarpaulin had support for a certain format version, I think we would take that into account before bumping our compatibility requirements past that.

Long-term, I do think either something like cargo tarpaulin or something distributed by rustup would be the smoothest experience. But I don't think we need to wait for that before shipping support for an otherwise widely used format.

Regarding the environment variable, I had interpreted LLVM_PROFILE_FILE as "file to put the profile in LLVM format" rather than "file where the LLVM-backend-generated code should put the profile".

[pietroalbini]

Long-term, I do think either something like cargo tarpaulin or something distributed by rustup would be the smoothest experience. But I don't think we need to wait for that before shipping support for an otherwise widely used format.

As I understand it, the profiles have no backwards compatibility guarantees. There is a version number that gets bumped with every change that is not backwards compatible, but older versions are not supported. The best way of guaranteeing compatibility for these is to release llvm-profdata with the Rust compiler. There's a decent chance that using tooling from the same major version of LLVM will work, but I don't know if that's guaranteed (petrhosek may know more).

For the source mapping I'm not sure about backwards compatibility, but if it's the same story then we should release llvm-cov as well.

Shipping llvm-profdata and llvm-cov in a coverage-tools rustup component would definitely alleviate the concerns for end users, and it'd be great to do it along with this stabilization.

To clarify: my expectation would be that we would occasionally bump the compatibility requirements of the format when there is a specific need to do so, such as new functionality we require, and I would expect us to not keep the version requirement in lockstep with our baseline requirement for LLVM, but I would still expect us to put thought into bumping the format compatibility forward. If, for example, cargo tarpaulin had support for a certain format version, I think we would take that into account before bumping our compatibility requirements past that.

Taking into account other tools is good, but we don't know all of the tools that will rely on the format and not all of those tools will be publicly available. Our stability guarantee can't be limited to just tools we're aware of.

Also, can we actually tell LLVM not to generate files in a newer format if we don't have a specific need for the changes? Otherwise we'd be forced to either make the breaking change or avoid bumping LLVM as a whole.

Regarding the environment variable, I had interpreted LLVM_PROFILE_FILE as "file to put the profile in LLVM format" rather than "file where the LLVM-backend-generated code should put the profile".

Good point! I didn't see it that way, but it makes sense!

[wesleywiser]

Also, can we actually tell LLVM not to generate files in a newer format if we don't have a specific need for the changes? Otherwise we'd be forced to either make the breaking change or avoid bumping LLVM as a whole.

Yes, we're actually still using version 4 even though the latest available version in LLVM 13 is version 6.

[richkadel]

FYI while it's likely true that the coverage mapping format can be controlled because we generate it, the profraw data generated by the running binary is not under rustc control. I'm not aware of any kind of flag or environment variable to tell the llvm runtime libraries what output format you want for that, which is why we probably need to bundle the required tools matching the version of llvm used by a given version of rustc.

[bors]

⌛ Testing commit 3187f41 with merge 7c88966249ed78522e3f23357a849800e61acffd...

[bors]

💥 Test timed out

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[wesleywiser]

Looks like i686-msvc-1 timed out while running stage 2 tests.

@bors retry