Upgrades the coverage map to Version 4 #79365

richkadel · 2020-11-23T23:15:16Z

Changes the coverage map injected into binaries compiled with
-Zinstrument-coverage to LLVM Coverage Mapping Format, Version 4 (from
Version 3). Note, binaries compiled with this version will require LLVM
tools from at least LLVM Version 11.

r? @wesleywiser

Changes the coverage map injected into binaries compiled with `-Zinstrument-coverage` to LLVM Coverage Mapping Format, Version 4 (from Version 3). Note, binaries compiled with this version will require LLVM tools from at least LLVM Version 11.

richkadel · 2020-11-23T23:15:41Z

cc: @tmandry

richkadel · 2020-11-23T23:19:19Z

@wesleywiser - I confirmed the run-make-fulldeps/coverage* test suite still works (with a few changes to the LLVM IR test due to the format changes).

I also hand-checked the new coverage mapping format and LLVM IR changes on Windows and Mac.

richkadel · 2020-11-23T23:32:57Z

@wesleywiser @tmandry -

Do I need to implement a backward-compatible solution with LLVM 9 and 10?

I'm not sure how important it is to support these older versions, or if it's feasible (or practical) to require LLVM if users want to use -Zinstrument-coverage. What's the typical/best practice here?

jfrimmel

Just some style recomendations

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

catenacyber · 2020-11-24T13:14:03Z

Thanks for this PR, getting nice results with it

wesleywiser · 2020-11-24T13:47:00Z

Do I need to implement a backward-compatible solution with LLVM 9 and 10?

I'm not sure how important it is to support these older versions, or if it's feasible (or practical) to require LLVM if users want to use -Zinstrument-coverage. What's the typical/best practice here?

I opened a topic on Zulip to discuss this. IMO I think requiring LLVM 11 to use this feature is fine but we'll see what others think.

richkadel · 2020-11-24T18:27:56Z

Just some style recomendations

Thanks for catching these.

* `rustc` should now compile under LLVM 9 or 10 * Compiler generates an error if `-Z instrument-coverage` is specified but LLVM version is less than 11 * Coverage tests that require `-Z instrument-coverage` and run codegen should be skipped if LLVM version is less than 11

richkadel · 2020-11-24T20:00:38Z

@wesleywiser - FYI, I looked into adding a check in rustc_session::config but couldn't find a way to check the LLVM version number without adding unsafe ffi code. That's not common for the session crate, and it seemed redundant, so I left it out. The compiler appears to fail gracefully enough, if a user tries to use the -Z instrument-coverage flag with LLVM < 11.

This PR attempts to resolve the CI errors. I didn't try checking the LLVM version under mir-opt. I'm not sure if the mir-opt tests skip codegen or not. If they don't it may fail again, and I'll have to deal with it.

The other tests should be automatically skipped for LLVM < 11 though.

src/test/run-make-fulldeps/coverage-llvmir-base/Makefile

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

Co-authored-by: Wesley Wiser <wwiser@gmail.com>

compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp

richkadel · 2020-11-25T17:47:30Z

Here's the new error message, generated by Rust instead of LLVM...

thread 'rustc' panicked at 'assertion failed: `(left == right)`
  left: `3`,
 right: `4`: rustc option `-Z instrument-coverage` requires LLVM 11 or higher.', compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs:179:9
stack backtrace:
   0:     0x7f99ff4393b1 - std::backtrace_rs::backtrace::libunwind::trace::hfab685574dd18081
...

richkadel · 2020-11-25T17:50:19Z

Actually, not that it matters too much, but the assert output would likely be:

 left: `2`,
right: `3`,

because the version number is 0-based. I forced the error for testing by setting the expected version to Version 5 (0-based version number 4) in the output above.

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

wesleywiser · 2020-11-25T20:21:20Z

Thanks @richkadel!

@bors r+

bors · 2020-11-25T20:53:10Z

⌛ Testing commit d334f58 with merge be11c42f934494147a973ae5f53a151b51ff1c75...

bors · 2020-11-25T21:22:41Z

💔 Test failed - checks-actions

The angle brackets were confusing my IDE and I thought they were unnecessary. I was wrong.

wesleywiser · 2020-11-25T22:32:08Z

@bors r+

bors · 2020-11-25T22:32:10Z

📌 Commit fdbc121 has been approved by wesleywiser

bors · 2020-11-26T00:03:13Z

⌛ Testing commit fdbc121 with merge 737462e9563ba2d989b9bd1225a0103e5b9facab...

bors · 2020-11-26T00:10:22Z

💔 Test failed - checks-actions

richkadel · 2020-11-26T00:13:50Z

Build issue in auto (dist-powerpc64le-linux, ubuntu-latest-xl)?

Step 15/21 : RUN ./build-powerpc64le-toolchain.sh
 ---> Running in 715bcfb93bb7
+ source shared.sh
+ BINUTILS=2.32
+ GCC=5.3.0
+ TARGET=powerpc64le-linux-gnu
+ SYSROOT=/usr/local/powerpc64le-linux-gnu/sysroot
+ mkdir -p /usr/local/powerpc64le-linux-gnu/sysroot
+ pushd /usr/local/powerpc64le-linux-gnu/sysroot
+ centos_base=http://vault.centos.org/altarch/7.3.1611/os/ppc64le/Packages/
+ glibc_v=2.17-157.el7
+ kernel_v=3.10.0-514.el7
+ for package in 'glibc{,-devel,-headers}-$glibc_v' 'kernel-headers-$kernel_v'
/usr/local/powerpc64le-linux-gnu/sysroot /tmp
+ curl http://vault.centos.org/altarch/7.3.1611/os/ppc64le/Packages//glibc-2.17-157.el7.ppc64le.rpm
+ rpm2cpio -
+ cpio -idm
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100   276  100   276    0     0   1279      0 --:--:-- --:--:-- --:--:--  1277
argument is not an RPM package
cpio: premature end of archive
The command '/bin/sh -c ./build-powerpc64le-toolchain.sh' returned a non-zero code: 1
The command has failed after 5 attempts.
Error: Process completed with exit code 1.

richkadel · 2020-11-26T00:18:42Z

@Mark-Simulacrum - It looks like the last 3 jobs on the bors queue failed for the same reason.

richkadel · 2020-11-26T01:50:54Z

@jonas-schievink FYI, this was another one of several recently failed on powerpc.

I noticed you closed the tree. Thanks.

If/when resolved, I'd be thankful for a bors retry 🤞 🙂

jonas-schievink · 2020-11-26T02:01:47Z

@bors retry

…as-schievink Rollup of 10 pull requests Successful merges: - rust-lang#77758 (suggest turbofish syntax for uninferred const arguments) - rust-lang#79000 (Move lev_distance to rustc_ast, make non-generic) - rust-lang#79362 (Lower patterns before using the bound variable) - rust-lang#79365 (Upgrades the coverage map to Version 4) - rust-lang#79402 (Fix typos) - rust-lang#79412 (Clean up rustdoc tests by removing unnecessary features) - rust-lang#79413 (Fix persisted doctests on Windows / when using workspaces) - rust-lang#79420 (Fixes a word typo in librustdoc) - rust-lang#79421 (Fix docs formatting for `thir::pattern::_match`) - rust-lang#79428 (Fixup compiler docs) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Upgrades the coverage map to Version 4

1d8c381

Changes the coverage map injected into binaries compiled with `-Zinstrument-coverage` to LLVM Coverage Mapping Format, Version 4 (from Version 3). Note, binaries compiled with this version will require LLVM tools from at least LLVM Version 11.

rust-highfive assigned wesleywiser Nov 23, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 23, 2020

richkadel mentioned this pull request Nov 24, 2020

Implement support for LLVMs code coverage instrumentation #34701

Closed

Updated links to LLVM 11 docs and types

5d5dc4c

richkadel force-pushed the llvm-cov-map-version-4 branch from 1381a85 to 5d5dc4c Compare November 24, 2020 03:15

jfrimmel reviewed Nov 24, 2020

View reviewed changes

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs Outdated Show resolved Hide resolved

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs Outdated Show resolved Hide resolved

This was referenced Nov 24, 2020

Fuzzing and rust coverage OISF/suricata#5595

Closed

Rust coverage report (for Suricata) google/oss-fuzz#4697

Merged

wesleywiser reviewed Nov 25, 2020

View reviewed changes

src/test/run-make-fulldeps/coverage-llvmir-base/Makefile Outdated Show resolved Hide resolved

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs Outdated Show resolved Hide resolved

richkadel commented Nov 25, 2020

View reviewed changes

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs Outdated Show resolved Hide resolved

Apply suggestions from code review

b5fef37

Co-authored-by: Wesley Wiser <wwiser@gmail.com>

wesleywiser reviewed Nov 25, 2020

View reviewed changes

compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp Outdated Show resolved Hide resolved

Improved version check

b4668ec

wesleywiser reviewed Nov 25, 2020

View reviewed changes

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs Outdated Show resolved Hide resolved

richkadel commented Nov 25, 2020

View reviewed changes

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs Outdated Show resolved Hide resolved

replace assert with condition and fatal error

b1df6c0

richkadel commented Nov 25, 2020

View reviewed changes

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs Outdated Show resolved Hide resolved

Update compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

d334f58

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 25, 2020

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 25, 2020

fix URLs in doc comment

fdbc121

The angle brackets were confusing my IDE and I thought they were unnecessary. I was wrong.

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 25, 2020

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 26, 2020

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 26, 2020

This was referenced Nov 26, 2020

Rollup of 11 pull requests #79440

Closed

Rollup of 10 pull requests #79441

Merged

bors merged commit 0ae653a into rust-lang:master Nov 26, 2020

rustbot added this to the 1.50.0 milestone Nov 26, 2020

catenacyber mentioned this pull request Nov 30, 2020

Fuzz rust cov v2 OISF/suricata#5613

Closed

richkadel mentioned this pull request Feb 20, 2021

Upgrade to LLVM 12 #81451

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrades the coverage map to Version 4 #79365

Upgrades the coverage map to Version 4 #79365

richkadel commented Nov 23, 2020

richkadel commented Nov 23, 2020

richkadel commented Nov 23, 2020

richkadel commented Nov 23, 2020

jfrimmel left a comment

catenacyber commented Nov 24, 2020

wesleywiser commented Nov 24, 2020

richkadel commented Nov 24, 2020

richkadel commented Nov 24, 2020

richkadel commented Nov 25, 2020

richkadel commented Nov 25, 2020

wesleywiser commented Nov 25, 2020

bors commented Nov 25, 2020

bors commented Nov 25, 2020

wesleywiser commented Nov 25, 2020

bors commented Nov 25, 2020

bors commented Nov 26, 2020

bors commented Nov 26, 2020

richkadel commented Nov 26, 2020

richkadel commented Nov 26, 2020

richkadel commented Nov 26, 2020

jonas-schievink commented Nov 26, 2020

Upgrades the coverage map to Version 4 #79365

Upgrades the coverage map to Version 4 #79365

Conversation

richkadel commented Nov 23, 2020

richkadel commented Nov 23, 2020

richkadel commented Nov 23, 2020

richkadel commented Nov 23, 2020

jfrimmel left a comment

Choose a reason for hiding this comment

catenacyber commented Nov 24, 2020

wesleywiser commented Nov 24, 2020

richkadel commented Nov 24, 2020

richkadel commented Nov 24, 2020

richkadel commented Nov 25, 2020

richkadel commented Nov 25, 2020

wesleywiser commented Nov 25, 2020

bors commented Nov 25, 2020

bors commented Nov 25, 2020

wesleywiser commented Nov 25, 2020

bors commented Nov 25, 2020

bors commented Nov 26, 2020

bors commented Nov 26, 2020

richkadel commented Nov 26, 2020

richkadel commented Nov 26, 2020

richkadel commented Nov 26, 2020

jonas-schievink commented Nov 26, 2020