This is an archive of the discontinued LLVM Phabricator instance.

[libFuzzer] Added -print_full_coverage flag.
ClosedPublic

Authored by ryancao on Aug 13 2020, 1:15 PM.

Details

Summary

-print_full_coverage=1 produces a detailed branch coverage dump when run on a single file.
Uses same infrastructure as -print_coverage flag, but prints all branches (regardless of coverage status) in an easy-to-parse format.
Usage: For internal use with machine learning fuzzing models which require detailed coverage information on seed files to generate mutations.

Patch by Ryan Cao (@ryancao).

Diff Detail

Event Timeline

ryancao created this revision.Aug 13 2020, 1:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2020, 1:15 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
ryancao requested review of this revision.Aug 13 2020, 1:15 PM
Dor1s retitled this revision from Added -print-raw-coverage flag. to [libFuzzer] Added -print-raw-coverage flag..Aug 13 2020, 5:25 PM
Dor1s added a reviewer: morehouse.
Dor1s added a subscriber: kcc.

Good job on uploading this for review, Ryan!

You also need to add a test for the new functionality. Tests for libFuzzer are located in https://github.com/llvm/llvm-project/tree/master/compiler-rt/test/fuzzer

You can run them by executing ninja check-fuzzer command in your build directory

You can use https://github.com/llvm/llvm-project/blob/master/compiler-rt/test/fuzzer/coverage.test as an example

The tests are written using LIT, which is somewhat intuitive, but not always.

You can see some quick clarifications about it in go/llvm-dev-starting-kit, or from various sources on the internet e.g.

https://llvm.org/docs/TestingGuide.html

or
https://llvm.org/devmtg/2019-10/slides/Homerding-Kruse-LLVMTestingInfrastructureTutorial.pdf scroll to slide 16

compiler-rt/lib/fuzzer/FuzzerDriver.cpp
319

we should be able to re-use the existing RunOneTest function. You can explicitly set detect_leaks option to zero when print_full_coverage is used

compiler-rt/lib/fuzzer/FuzzerFlags.def
133

print_full_coverage sounds a bit better to me

compiler-rt/lib/fuzzer/FuzzerLoop.cpp
470

I don't think you need this call, as we just want to execute the input and get its coverage.

compiler-rt/lib/fuzzer/FuzzerTracePC.cpp
300

this function duplicates the most of the existing PrintCoverage function. Let's try to re-use the existing one, add CoveredPCs collection inside it, and branch the behavior at the end of the callback (where print calls are located) based on the value of the command line flag

ryancao updated this revision to Diff 287507.Aug 24 2020, 3:06 PM

Addressed @Dor1s comments. Extra files in the diff under llvm/ have not been touched.

Herald added a project: Restricted Project. · View Herald TranscriptAug 24 2020, 3:06 PM
ryancao retitled this revision from [libFuzzer] Added -print-raw-coverage flag. to [libFuzzer] Added -print_full_coverage flag..Aug 25 2020, 10:00 AM
ryancao edited the summary of this revision. (Show Details)
Dor1s added a comment.Sep 1 2020, 5:09 PM

@morehouse could you please take a look as well, when you get a chance?

compiler-rt/lib/fuzzer/FuzzerDriver.cpp
319

Why can't you use the existing RunOneTest as suggested in my previous comment here?

845

since we print the coverage information from within PrintFinalStats, it seems like we can just go through the if body starting on the line 817? This literally is a copy of it without some additional printing / parameters check.

compiler-rt/lib/fuzzer/FuzzerTracePC.cpp
272

let's rename full to something like PrintAllCounters

332

this comment is incorrect, just delete it

llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
842 ↗(On Diff #287507)

this and some other files look unrelated, can you exclude them from your commit please?

I'll take a look once the diff is fixed and Max's current comments are addressed.

ryancao updated this revision to Diff 291121.EditedSep 10 2020, 8:36 PM

Updated to address @Dor1s comments.

morehouse added inline comments.Sep 14 2020, 6:11 PM
compiler-rt/lib/fuzzer/FuzzerDriver.cpp
327

Why does the flag disable leak detection? We should either leave it enabled, or explain why it is disabled in the flag description.

compiler-rt/lib/fuzzer/FuzzerLoop.cpp
357–360
compiler-rt/lib/fuzzer/FuzzerTracePC.cpp
306

Nit: please remove curly braces

310

Do we need to exclude the U and C when the vectors are empty? It's slightly cleaner to exclude these if-statements.

compiler-rt/lib/fuzzer/FuzzerTracePC.h
97

Remove const, it is pointless for non-pointers.

compiler-rt/test/fuzzer/full-coverage.test
5

What does -mllvm -use-unknown-locations=Disable do, and why do we need it?

Dor1s updated this revision to Diff 300382.Oct 23 2020, 12:39 PM

addressing review feedback from Matt

Dor1s edited the summary of this revision. (Show Details)Oct 23 2020, 12:40 PM

Matt, I'm trying to finish this on behalf of Ryan. Marty has been enabling the use of that feature on ClusterFuzz already, so it makes sense to move forward with this. Please take another look when you can :)

compiler-rt/lib/fuzzer/FuzzerDriver.cpp
327

added a comment

compiler-rt/lib/fuzzer/FuzzerLoop.cpp
357–360

done!

compiler-rt/lib/fuzzer/FuzzerTracePC.cpp
272

here as well

306

done

310

I think it depends on the consuming side. Marty, what do you think?

compiler-rt/lib/fuzzer/FuzzerTracePC.h
97

done

compiler-rt/test/fuzzer/full-coverage.test
5

Does seem necessary!

mbarbella-chromium added inline comments.
compiler-rt/lib/fuzzer/FuzzerTracePC.cpp
310

This should be easy enough to handle, though it may need a small update to the training script. I agree that it's cleaner to output them unconditionally, but I don't have strong feelings either way.

Dor1s added inline comments.Oct 23 2020, 3:04 PM
compiler-rt/lib/fuzzer/FuzzerTracePC.cpp
310

I think leaving this is-is would be slightly better, as at least from the user perspective it'd be explicit that this code was executed and the counters were empty.

morehouse accepted this revision.Oct 23 2020, 3:53 PM
morehouse added inline comments.
compiler-rt/lib/fuzzer/FuzzerTracePC.cpp
310

Leaving as-is actually leaves it a mystery whether this code was executed or not. I was suggesting printing the U and C unconditionally (removing the if statements).

This revision is now accepted and ready to land.Oct 23 2020, 3:53 PM
Dor1s updated this revision to Diff 300427.Oct 23 2020, 4:03 PM

print "U" and "C" unconditionally

Dor1s added a comment.Oct 23 2020, 4:04 PM

Not that I like to merge CLs on Friday afternoon, but I'm OOO next week, so taking a shot now while I'm still online.

compiler-rt/lib/fuzzer/FuzzerTracePC.cpp
310

My bad, let me remove the conditions then. Thanks for clarifying this!

compiler-rt/test/fuzzer/full-coverage.test
5

something's wrong with me. I meant "does NOT seem necessary". Removed. Thanks Matt!

This revision was landed with ongoing or failed builds.Oct 23 2020, 4:09 PM
This revision was automatically updated to reflect the committed changes.