-
-
Notifications
You must be signed in to change notification settings - Fork 181
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
LLVM coverage instrumentation #549
Comments
https://hammer-vlsi.readthedocs.io/en/stable/CoverageMappingFormat.html some docs and the rust issue I keep having to find again rust-lang/rust#34701 |
Is there already a branch where we can try this? |
Not yet, given my current work schedule I'm hoping to have something early January |
Work schedule and covid restrictions came back to bite me I'm likely to be abroad away from my home until march or april until I can return. I plan on starting the work to make it so tarpaulin compiles on other OSs even if it doesn't do anything and then bring in llvm coverage resources found: |
So I've merged the first PR, didn't want to be constantly building up a giant PR that becomes hard to merge 😅. This one simply adds a future option (not visible to the user) to set the tracing engine. It has 3 options Auto, Ptrace and llvm (and with #675 I hope to add probe-rs). So currently this is hard set to Ptrace, but if it's set to Auto or llvm and the compiler version is compatible it will build the test binaries with the llvm-cov instrumentation included. If set to llvm and the compiler isn't compatible it prints an error log warning the user the compiler doesn't support it and falls back to ptrace. There are some issues with doctests marked as should_panic and the llvm coverage but everything else works fine and generates the profdata files for the tests and passes as expected. Looking at what I can see of the profdata it looks like I have all the info to parse it with the binary and I'd ideally like to do that from the get-go to avoid making the user install the llvm-tools component and using llvm-profdata. Another benefit is it also gives me ability to access the information I need to derive other coverage metrics than line coverage which would be a big future win. I haven't found a source for the format other than the llvm source code which isn't quite a joy to read... |
So as another update I've just merged #786 which allows for selecting the coverage engine to use and actually runs the llvm-cov instrumented binaries and just prints out the path to the test executable and the profraw files it generates for each test. It doesn't yet collect the metrics from within those files. I've also made the flag to select the engine backend hidden so people who want to experiment can but for normal users they won't be able to access incomplete features. For my aim to parse the formats in Rust to avoid installing extra tools, I've been working on https://github.com/xd009642/llvm-profparser I can now parse and merge the profraw files and the consistency tests show the implementation matches that of the llvm tools using their own test vectors. The next step is getting the counter to source mapping which is compiled into the test binary to turn those coverage counters into actual coverage stats. Once that's done I just need to tidy up the error handling as there's a few unwraps/todos in the code still and it should be ready to integrate. |
Another small update, the llvm-profparser parsing of the ELF sections has progressed and I'm close. Just failing to exactly see how the two parsed formats interlink as the counters seem different. I expect this is to do with the addition/subtraction expressions being like span deltas for coverage and I might be able to reduce those expressions down so the counter counts match. We'll see, I want to check the LLVM code/docs to double check the correctness/implementation of that. So failing the motivation to take that final jump tonight I published an alpha verion of my llvm-profparser crate and integrated it into tarpaulin. It panics due to a
EDIT: Also it's in the intuitively named |
Also, after some work with a very-early adopter interested in windows support there's now a commit pushed that simplifies that and should hopefully mean windows users can get to the same |
I see that rustc coverage instrumentation was stabilized in the 1.60 release today. Is it fair to say that what is included is sufficient to switch the underlying coverage support and implement this feature? |
The short answer: no. The long answer: needing to install cargo-binutils separately and rely on that being stable or finding the correct llvm-tools in the rust install (handling distro installs or rustup) felt like a pretty poor solution to me (partially because I already know there will be a large number of issues where the answer is "run So I went straight for the solution of implementing the profdata/profraw parsing in https://github.com/xd009642/llvm-profparser which currently works with llvm 11, 12, 13 and 14 formats (passing all their test suite). I also correctly seem to parse the ELF headers LLVM adds to the binary so the missing piece is mapping between the profdata and the binary source locations and then tarpaulin will work with llvm coverage on any installed compiler version that has the llvm coverage without need for any additional tools. Parsing the profraw and profdata was actually reasonably quick it's just finding time for the last bits as I don' have much personal time right now |
Well after a bunch of work over last week I now have the very initial version working all the way through. Anyone who wants to test and report any issues would be appreciated. I'll be running it through some tests and polishing more before release but it's the home stretch. The branch to install from for testing is |
Some more movement on this, I've fixed an issue with it on windows. Doing some more tests make sure everything passes CI then will look at moving off alpha versions and doing an initial release with using llvm coverage as a non-default option. And then making default when I've had more a chance to look at the output at a lower level. |
@xd009642 I think I've found a bug:
I minimized the issue to this: https://github.com/orium/cargo-rdme/blob/bug-tarpaulin-llvm-profparser/src/main.rs#L6-L12 |
Awesome, thanks @orium. I've done a tweak on my local copy and cargo-rdme is not getting beyond that point so hopefully it's fixed. I've also spotted out a windows issue in report generation so I'll update when I've got an updated version with both fixes in 👍 |
No worries 👍. I can confirm the bug is gone. Later today I'll run all tests and see if any issue pops up. Something that I also want to test is |
So provided the This probably makes |
I tested the This is the output from tarpaulin (commit 9fe80a5):
Note that the
but since binary This is the report from tarpaulin: If you run
This is the report |
Anyway I can help debug the OS error? |
It may take a bit of println debugging in the code, but a first step would be to run |
All of the doc tests do seem to run (all of the ones named |
Seems to be failing in let file = unsafe { MmapOptions::new().map(&file)? }; It's trying to open the .dSYM, but isn't that a folder? |
Ahhh I remember when I first worked on mac coverage a few years ago there was a separate dSYM file for debug info not a folder. So that's from an early PR adding in the binary analysis side of tarpaulin before trying to sort tracing. I'll just remove that as all it can do is marginally improve the llvm line coverage. Thanks for tracking it down 🎉 |
Happy to help! Can't wait to use LLVM coverage more |
Which LLVM compiler can be considered "compatible" though? I have LLVM installed on Mac M1, but getting a warning
Which results in an ERR still.
UPD I've discovered this piece: Lines 37 to 40 in b97f20a
So this works for me:
|
Have you installed from the feature branch instead of latest tarpaulin release? And compatibility is purely on the rustc version it doesn't look at llvm versioning at all. So on feature branch it's nightly > 1.50 or stable > 1.60. I guess you've not installed using the git url and branch https://github.com/xd009642/tarpaulin/blob/feat/llvm-profparser-integration/src/cargo.rs#L37-L41 |
Did anybody test it with docker (macOs M1). I tried it in VM (parallels + Linux ubuntu arm64) and docker (macOs m1) and I'm getting errors. Is it even supported in the mentioned configurations? It compiles fine on macOs (m1) but doesn't seem to be working with docker/linux arm (running in macos M1)?
Logs:
|
A version from |
Well the last 2 CI runs have passed, so it looks like I'm on the final stretch of clean up before merge 🎉 |
Hey :) thanks for your hard work! I tried this Dockerfile on a M1 and it lead to the same error as in version 0.21:
|
And it's merged. I aim on doing a release later this week (likely tomorrow), but I'll be keeping this issue open for anything that pops up after initial release |
Awesome work @xd009642. I'm now running the llvm engine and it works well. In particular I can now get coverage information for binaries launched by the integration tests 🎉. When do you intend make the llvm engine the default? Also, I've noticed warnings when I run the integration tests of https://github.com/orium/cargo-rdme. Those integration tests execute the
|
So coverage from binaries launched should have worked before with the So far the llvm engine isn't default because:
So I'm not sure when it will change over, the fact that 2. means some functionality is lost at the expense of more accurate results is a tradeoff. Also, there are people relying on the behaviour in 2. for coverage in production so a breaking release is something that has to be weighed up. I'll try out cargo-rdme tomorrow, there does seem to be some kinks to work out with some projects but unfortunately I've been unable to recreate them |
The ptrace engine segmentation faults for me: #966 (comment) |
I seem to be getting the same error as @karolh2000. Compiling in a Debian VM running on an M1 mac (no docker):
|
@karolh2000 @fisherdarling I may have fixed this issue in #1124 if you want to test it, otherwise it will be in the next release (whenever that is) |
Looks like that got me past the earlier errors @xd009642! Here's what I'm getting now:
Thanks for working on this and for the quick turnaround :) |
Huh interesting, I figured tests running and passing on linux aarch64 would be enough to prove it worked, looks like I need to tweak CI some more to find the target where it doesn't build and fix that. I'll do that tonight after work 👍 |
@fisherdarling and the fix is available on develop now! |
Closing this in favour of the more specific issues that have popped up |
So when llvm coverage instrumentation is stable on rust we can look at removing the ptrace based coverage completely and instead relying on llvm and rustc to help. This has a few advantages
In this instance tarpaulin should make the whole experience still nicer because:
OSX TODOs:
Windows TODOs
Linux TODOs
General TODO
The text was updated successfully, but these errors were encountered: