Upgrades the coverage map to Version 4 by richkadel · Pull Request #79365 · rust...

Contributor

richkadel commented 26 days ago

cc: @tmandry

richkadel commented 26 days ago

@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 commented 26 days ago

@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?

Thanks for this PR, getting nice results with it

wesleywiser commented 26 days ago

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 commented 26 days ago

Just some style recomendations

Thanks for catching these.

richkadel commented 25 days ago

@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.

richkadel commented 25 days ago

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 commented 25 days ago

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.

bors commented 24 days ago

Testing commit d334f58 with merge be11c42...

bors commented 24 days ago

Test failed - checks-actions

bors commented 24 days ago

Commit fdbc121 has been approved by wesleywiser

bors commented 24 days ago

Testing commit fdbc121 with merge 737462e...

bors commented 24 days ago

Test failed - checks-actions

richkadel commented 24 days ago

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 commented 24 days ago

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

richkadel commented 24 days ago

@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