knurling-rs / defmt

Efficient, deferred formatting for logging on embedded systems
https://defmt.ferrous-systems.com/
Apache License 2.0
819 stars 75 forks source link

LLVM instrumentation coverage on embedded tests #631

Open xd009642 opened 2 years ago

xd009642 commented 2 years ago

Relevant crate: defmt-test (although could be relevant for any generated runners etc).

In the past year rustc has added the unstable instrument-coverage feature for collecting code coverage: docs. This works reasonably well and generally provides more accurate results than process tracing tools. The way it works is to associate regions of code with some counters, add some extra functions and then the compiler adds in function calls to record when each location is ran. When the process exits it saves the results to a file and then using a combination of the instrumented binary and this file you can get the code coverage results.

Now I've done a little bit of investigation in a separate project https://github.com/xd009642/llvm-embedded-coverage which demonstrates how to remove the File IO. LLVM provides some external functions/symbols (docs: https://clang.llvm.org/docs/SourceBasedCodeCoverage.html ) which you can use to instead operate on a buffer.

The way I envisage this feature would allow people to run their tests on target, and then the buffer with the profraw data be transferred over JTAG/USB/whatever to the host machine where it is collected and saved to a file default.profraw to match the current UX of using it in rustc. The transferring of the data down the wire and signalling it shouldn't be printed but saved to a file is currently an unknown for me - I'm not familiar enough with defmt's workings.

There's currently some usability faff. The serialization isn't streaming so you need the entire buffer available and the size cannot be known before compilation (depends on the amount of code included, dead-code elimination etc can change this). So without the alloc feature you may find the buffer is too small to serialize and you can't collect coverage. This is currently the only big issue I see, for these instances I've made the max buffer size configurable via an env var, if it's too small the program panics with a message saying how big it should be.

It does without saying I anticipate this feature to be optionally compiled in only if -Z instrument-coverage is present in the rust flags, so for normal users this should have no effect. I'd also be very interested in implementing this as I've already done a bunch of work with the format and this has significant overlap with some of the work I may have to do for my main project https://github.com/xd009642/tarpaulin

xd009642 commented 2 years ago

From some discussion that went on I've upped the PoC to make it no_std, in a branch and ran into https://github.com/rust-lang/rust/issues/81684 I'll still be looking into this but it may be more a distant dream than something easily graspable. EDIT: although https://github.com/Amanieu/minicov may provide a solution and be perfectly workable.

EDIT: I have llvm coverage compiling for thumbv7em-none-eabihf I just need to find a dev board with more flash to test it https://github.com/xd009642/llvm-embedded-coverage/tree/feat/make-no-std

japaric commented 2 years ago

As a data point:

I looked into -Z instrument-coverage and also minicov months ago. The main issue I saw at the time was that -Z instrument-coverage increased program size (Flash size) and RAM usage (iirc? .bss?) in a way that's proportional to the amount of source code, e.g. number of dependencies. So having a 100kLoCs svd2rust crate in the dependency graph made the program not fit in the target, regardless of whether the crate's API was used or not. Unsure if that's just a bug.

What seemed more promising in terms of Flash & memory footprint was directly using one of LLVM coverage passes, boolean flags something? I forget the flag name. That produced branch coverage data but it was not in any standard format like profdata so extra work would be needed to convert the raw data into something that other tools accept. I did not look into that.

xd009642 commented 2 years ago

It will increase bss for the counter variables. One solution for that is to add the #[no_coverage] attribute to any svd2rust generated code, but I'm not sure if it's stable yet but that would reduce the impact of the problem.

I'll have to look at the llvm coverage passes and see how they work, it's not an area I've looked at!