In D93879#2490294, @IanPudney wrote:

Dor1s, what do you suggest? I haven't been able to find a good way to pass this information to libFuzzer without extending the API. The best we came up with was to simulate a memcmp(), but it didn't seem to work very well.

Thanks for the context. If I understand correctly, the actual underlying goal is to pass an additional coverage signal to the fuzzing engine. If there is a way to achieve that without extending libFuzzer's API, would that suffice?

If this change moves forward, it will also need a test using LLVMFuzzerAddToDictionary. However, I'm personally not convinced in the use case for this.

lint

rebase

@morehouse @metzman Matt and Jonathan, could any of you please commit this? I don't have a checkout handy plus haven't figured out my current CLA situation.

Thanks for the patch. LGTM

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.

print "U" and "C" unconditionally

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 :)

addressing review feedback from Matt

Rebase

LGTM

LGTM, thanks for adding this back!

To be more constructive, do you have a use case where this option or its inverse is needed?

Fangrui, what made you think it's fine to remove documented options and break compatibility for the users of the tool?

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

Good job on uploading this for review, Ryan!

Alright, we've ended up disabling that behavior in honggfuzz build, so I'll just close this CL for now.

In D82685#2133565, @kcc wrote:

My preference would be to reject weird file names instead of adding this extra complexity.

Updated the version of ExecuteCommand with piped output.

Should be better now, ready for review again :)

Ready for review, PTAL.

Ready for review, PTAL.

Re-write to use fork-exec instead of system() on Linux.

If we still decide to proceed with this, would it make sense to expand it to sanitizer_coverage based on any sancov instrumentation being enabled? As you mentioned in the description, there might be users who manually enable certain sancov flags. I think it's good to be able to support those usecases too (e.g. other fuzzing engines).

What usecase(s) do you have for this in mind?

I've split the implementation into platform specific files, but I still don't like it. Plus, as Jonathan mentioned, it still has edge cases e.g. if a filename has a single quote. I'll give a shot at refactoring ExecuteCommand into fork-exec. It looks like I need to re-write only Posix implementation, as Fuchsia is already doing good and Windows is less prone to such issues due to its more restrictive file naming.

split the implementation into platform specific files

@leonardchan I've committed https://github.com/llvm/llvm-project/commit/6d0488f75bb2f37bcfe93fc8f59f6e78c9a0c939, that should help, I'll monitor the buildbot.

In D76651#1941953, @leonardchan wrote:

Hi. I think this patch might be leading to the linker error we're seeing:

>>> defined at FuzzedDataProvider.h:283 (/b/s/w/ir/k/recipe_cleanup/clangKqmrKj/lib/clang/11.0.0/include/fuzzer/FuzzedDataProvider.h:283)
>>>            obj/zircon/system/ulib/fs/journal/test/journal-replay-fuzzer_test.replay_fuzztest.cc.o:(FuzzedDataProvider::ConsumeBool())
>>> defined at FuzzedDataProvider.h:283 (/b/s/w/ir/k/recipe_cleanup/clangKqmrKj/lib/clang/11.0.0/include/fuzzer/FuzzedDataProvider.h:283)
>>>            obj/zircon/system/ulib/fs/journal/test/fuzz-utils.fuzzer_utils.cc.o:(.text+0x3F0)

ld.lld: error: duplicate symbol: FuzzedDataProvider::ConsumeData(void*, unsigned long)
>>> defined at FuzzedDataProvider.h:318 (/b/s/w/ir/k/recipe_cleanup/clangKqmrKj/lib/clang/11.0.0/include/fuzzer/FuzzedDataProvider.h:318)
>>>            obj/zircon/system/ulib/fs/journal/test/journal-replay-fuzzer_test.replay_fuzztest.cc.o:(FuzzedDataProvider::ConsumeData(void*, unsigned long))
>>> defined at FuzzedDataProvider.h:318 (/b/s/w/ir/k/recipe_cleanup/clangKqmrKj/lib/clang/11.0.0/include/fuzzer/FuzzedDataProvider.h:318)
>>>            obj/zircon/system/ulib/fs/journal/test/fuzz-utils.fuzzer_utils.cc.o:(.text+0x420)

... a bunch more

Would you mind taking a look? Thanks.

Builder: https://ci.chromium.org/p/fuchsia/builders/ci/clang_toolchain.fuchsia-x64-debug-subbuild/b8884854137402901888

rebase + add a comment for ConsumeProbability

thanks, Mitch!

The failure seems to be clang-tidy again -- I'm not going to change names of the methods. PTAL at lines 37-78, if you find time. The rest is existing code moved out of the class definition block. If no one takes a look, I'll land it in a couple days anyways, no worries.

updated CL description

rebase

make the linter happy #2

fix tidy error

PTAL, this is a small change requested/suggested by Mitch. I have another one ready (which is a large refactoring) that would come after.

TBR

Small change. PTAL, or I'll land it myself it a day or so :)

I'm gonna rebase and commit, since there isn't much to review compared to the PS#1 (just a test).

Thanks for the review. I can't think of a good application for the hash. Anything I come up with is better to be done either without hashing or even without FDP.

• Removed the hash
• Added test
• Renamed ConsumeMemory to ConsumeData, as the former sounds a bit ambiguous (like memory consumption is more about memory being allocated, while here it's just values being copied, i.e. data)

remove hash method, rename ConsumeMemory to ConsumeData, add test

Jonathan @metzman please take a look when you get a chance. I'll add the tests later. Two comments:

Self-approval for a quick fix (reverting two lines in https://reviews.llvm.org/D74137).

Looks like there is potentially relevant failure in: http://lab.llvm.org:8011/builders/clang-ppc64le-linux/builds/29265

I'm happy to take a review pass too. Just ping me when / if this is needed.

@kcc FYI

The next build succeeded. Must be a flake. And I'm seeing some other unrelated failures locally, but not this one. False alarm