cargo-llvm-cov
cargo-llvm-cov copied to clipboard
taiki-e
[Feature request] Allow external profraws to be imported when using the `report` command
This can may be useful when using llvm-cov aside fuzz cargo command, especially when using the fuzz coverage command.
Why ?
I'm learning to do fuzzing on a library I'm creating, and while fuzz is a great tool to actually do fuzzing, it's not as good to show the results of the coverages in a meaningfull way. On the other hand, your tool allows to show the results of the coverage in an understandable way, but seems to only support well cargo run, cargo test, and cargo nextest.
So why not create a bridge between the two ?
How ?
The possibility i had in mind would be to have an argument --import / --external which could be used multiple times per command. Each time that argument is provided, the path of the profraw file should be put after.
I'm not sure if copying the profraw files to the the llvm-cov-target directory each time we use the report command is a good idea from a UX perspective, which is why i would suggest a flag argument --copy-all-external to be added as well.
PS
It seems that your tool allows to use external results (from what i can see here), but i didn't understood how to import my profdata / profraw into llvm-cov from there. Maybe this part is not even concerned with my feature request, idk.
What do you guys think about it ?
I'm happy to accept the patches needed to support cargo-fuzz.
Btw, does cargo-fuzz not work in a way like cargo-afl? https://github.com/taiki-e/cargo-llvm-cov?tab=readme-ov-file#get-coverage-of-afl-fuzzers
I have found a way similar, but i'm not a big fan of it :
source <(cargo llvm-cov show-env --export-prefix)
cargo +nightly fuzz build #build in release mode
cp ./target/x86_64-unknown-linux-gnu/release/<target> ./target/release/<target>
cp ./fuzz/coverage/<target>/raw/default-<target>.profraw ./target/default-<target>.profraw
cargo llvm-cov report --release
I mean, for AFL you didn't have to do the cp command everywhere.
~~Small question from my part : I have found that sometimes report needs the object file, but not always (after doing cargo llvm-cov test, it doesn't seems to keep the object file). Could you tell me under which circumstances it needs it and for why ?~~
Since the report needs the object files to function properly, maybe it's a good idea to add a warning so that the users also includes external object in this case ? Cargo fuzz generate it's own object files, that would maximize compatibility with the profraws. That would involve adding a new argument again, and considering I've already suggested adding two arguments, that's maybe too much.
cp ./target/x86_64-unknown-linux-gnu/release/
./target/release/
As for this cp, I guess passing --target x86_64-unknown-linux-gnu to cargo llvm-cov report should handle it well without this cp.
There is no limit on the number of flags to add, but it may be easier here to add one flag that is specific to integration with cargo-fuzz and handle everything as a feature of that flag, rather than adding multiple generic flags.
There is no limit on the number of flags to add, but it may be easier here to add one flag that is specific to integration with cargo-fuzz and handle everything as a feature of that flag, rather than adding multiple generic flags.
That seems to be the best thing to do, indeed.
I was thinking about a --template argument, which could set where to search for the objects and profraws.
The generics flags could be in case where no templates would be adequate for a situation, a custom configured cargo fuzz for example.
After some thoughts I think I will remove the --copy-external arguments from the proposition, as people could just rerun the with the same imports again. We have command history for a reason after all.
Given that there is a nextest subcommand already, would it not make more sense to add a cargo llvmcov fuzz subcommand directly?
I'm not sure that's the ideal way of doing this, as this tool is mainly designed for code coverage, hence analyze the fuzz results rather than fuzzing itself
(Sorry for the inactivity i had, i was busy with other things in the meantime)
Sure well cargo llvm-cov fuzz-coverage could work does make more sense. It would run cargo fuzz coverage and then report the code coverage in the usual way. From a UX perspective, I don't think you can do much better than that?
That would work but that means we also have to add a afl-coverage too, and doing this way makes me fear this tool will have to add a subcommand for each fuzzing tool in existence in the future.
The best compromise would be the template argument i've proposed earlier, as it doesn't require adding subcommands at all.
Yeah that seems independently useful because it is more general. But integration with other rust tools also seems valuable to me.
This comment says https://github.com/taiki-e/cargo-llvm-cov/pull/144#pullrequestreview-913381236
Ideally, I would like cargo-llvm-cov to work with other subcommands as well.
That is 2+ years ago, so the situation may have changed in the meantime. Nevertheless, having dedicated support for cargo fuzz seems useful to me and consistent with supporting cargo nextest.
First, as I said above, I'm open to accepting patches as needed for support.
I cannot specify too specifically which forms are preferred at this time, as it depends on the behaviors and forms of the underlying command.
In general, if the interface of the command changes significantly depending on the new flag (e.g., wasm-pack's case: https://github.com/taiki-e/cargo-llvm-cov/pull/338#issuecomment-1894238823), it should be a separate subcommand, but if it only changes a specific process, such as changing the location of artifacts search, the flag is easier to implement.
In afl's case, there is a way to work well using only the existing interface, and since their PR to implement subcommand was incomplete and stuck, we decided that documentation would suffice.
If someone wants to add a subcommand for afl, I'm open to accepting PR to do it, but the cost of the work of adding subcommand based interface is not always worth the benefit. For example, if forms of the cargo-llvm-cov and the underlying command are different, it is also necessary to define the subcommand interface (e.g., afl's case: https://github.com/taiki-e/cargo-llvm-cov/pull/352#pullrequestreview-1926080921).