This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] Refactor FuzzedDataProvider for better readability.
ClosedPublic

Authored by Dor1s on Mar 23 2020, 4:49 PM.

Diff Detail

Event Timeline

Dor1s created this revision.Mar 23 2020, 4:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2020, 4:49 PM
Herald added subscribers: Restricted Project, dberris. · View Herald Transcript
Dor1s updated this revision to Diff 252177.Mar 23 2020, 4:53 PM

updated CL description

Dor1s added a comment.Mar 23 2020, 9:55 PM

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.

hctim accepted this revision.Mar 24 2020, 2:03 PM

LGTM w/ one nit

compiler-rt/include/fuzzer/FuzzedDataProvider.h
63

Might be worth adding a comment here that probability is just a float between 0 and 1 [inclusive].

72

Speaking of, is it worth having a std::vector<const T> variant of this?

This revision is now accepted and ready to land.Mar 24 2020, 2:03 PM
Dor1s updated this revision to Diff 252509.Mar 25 2020, 12:04 AM
Dor1s marked 3 inline comments as done.

rebase + add a comment for ConsumeProbability

thanks, Mitch!

compiler-rt/include/fuzzer/FuzzedDataProvider.h
63

done

72

sounds reasonable, although a version with two iterators (.begin(), .end()) might be even better, but that's out of the scope of this CL

This revision was automatically updated to reflect the committed changes.

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

Dor1s added a comment.Mar 25 2020, 1:28 PM

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

Thank you, looking.

Thanks for the update! Looks to be green now.