Skip to content
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

[Feature request] Allow external profraws to be imported when using the report command #415

Open
NovaliX-Dev opened this issue Jan 24, 2025 · 6 comments
Labels
C-enhancement Category: A new feature or an improvement for an existing one

Comments

@NovaliX-Dev
Copy link

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 ?

@taiki-e
Copy link
Owner

taiki-e commented Jan 25, 2025

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

@taiki-e taiki-e added the C-enhancement Category: A new feature or an improvement for an existing one label Jan 25, 2025
@NovaliX-Dev
Copy link
Author

NovaliX-Dev commented Jan 25, 2025

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 ?

@NovaliX-Dev
Copy link
Author

NovaliX-Dev commented Jan 25, 2025

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.

@taiki-e
Copy link
Owner

taiki-e commented Jan 25, 2025

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.

@taiki-e
Copy link
Owner

taiki-e commented Jan 25, 2025

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.

@NovaliX-Dev
Copy link
Author

NovaliX-Dev commented Jan 25, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: A new feature or an improvement for an existing one
Projects
None yet
Development

No branches or pull requests

2 participants