This is an archive of the discontinued LLVM Phabricator instance.

[libFuzzer] Mutation tracking and logging implemented.
ClosedPublic

Authored by kodewilliams on Jun 11 2018, 2:48 PM.

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
metzman added inline comments.Jun 15 2018, 2:07 PM
lib/fuzzer/FuzzerMutate.cpp
72

Does anyone else find it cleaner to increment in MutateImpl (in FuzzerMutate.cpp), so that there are just 2 calls to IncrementCount?
I think there would be a negligible performance hit, but I think it would be worth it.

lib/fuzzer/FuzzerMutationStats.cpp
17

Let's change this to NAME_OF_STAT like the other C Macros in this progam.

Dor1s added inline comments.Jun 15 2018, 2:51 PM
lib/fuzzer/FuzzerMutate.cpp
72

Good point! Also, that code needs to be under if (Options.PrintMutationStats). So, having it in a single place instead of 10+ would be better.

kodewilliams marked 11 inline comments as done.

Fixed nits and made code more efficient.

Fixing context not available problem as per LLVM documentation.

Herald added subscribers: Restricted Project, llvm-commits. · View Herald TranscriptJun 15 2018, 3:50 PM
kodewilliams marked 6 inline comments as done.

More efficient code.

Removed a macro.

Is the whole patch uploaded? I can only see one file.

Encompasses all changes.

kodewilliams retitled this revision from Mutation Statistics to [libFuzzer] Mutation tracking and logging implemented.Jun 19 2018, 3:17 PM

@Dor1s I still think using a map that contains string -> count makes most sense.
What is the advantage of using an array? I don't consider the performance benefits worth it since that cost is paid instead when printing the name of a mutation (ie: when this option is not being used).
Additionally this approach is simpler, we only need 2 data structures instead of 3.

lib/fuzzer/FuzzerMutate.cpp
41

I think the names here should match the mutations exactly (minus the Mutate_ prefix I guess). So please change this to ChangeAsciiInteger.

44

Same as above, please change to CrossOver

55–56

Same as above, please change to AddWordFromTORC

59

Same as above, please change to Custom (I don't love the fact that its less descriptive than the name you used)

496

Even though the lookuptime is constant, I don't like that we are slowing down fuzzing even when this option is not used.
We wouldn't need to do this if the map was string -> enum Value.

lib/fuzzer/FuzzerMutate.h
116

Please remove this blank line so that there is just one blank line between the end of Muatator and private

lib/fuzzer/FuzzerMutationStats.cpp
24

Probably nicer to use the iterator as done in FuzzerMutate.cpp:540

30

Same as before: Probably nicer to use the iterator as done in FuzzerMutate.cpp:540

39

Please remove the paren around the first condition, i don't think it helps readability and is inconsistent with the second condition.

41

We can get rid of the else here.

46

Please remove the paren around the first condition, i don't think it helps readability and is inconsistent with the second condition.

48

We can get rid of the else here.

lib/fuzzer/FuzzerMutationStats.h
26

I'm a bit conflicted about using size_t here. On the one hand it is a nice way of saying the largest int type. On the other, this isn't really a size.

I still think using a map that contains string -> count makes most sense.

I meant unordered_map

Dor1s added a comment.Jun 19 2018, 8:38 PM

Kode, you also need to write a test for the functionality your're adding. Please check out the section regarding the tests in LLVM Dev Starting Kit. Then, take a look at the existing tests: https://github.com/llvm-mirror/compiler-rt/tree/master/test/fuzzer, especially at "fuzzer-finalstats.test".

While thinking about the test, I've realized the following. Since we eventually plan to remove the detailed statistics output (with mutation names), we will have to update the tests after that. The fact that we have that detailed output right now would also slightly complicate writing a test for you.

So, my proposal would be to remove the detailed output at all, just print those useful / total float numbers in a single line, as we discussed today. We still be using MutationType enum type for tracking mutation stats, but we can get rid of kMutationNames and the code that uses it. Struct Mutator would have both name and identifier. That would simplify the code and also automatically address some of Jonathan's comments.

lib/fuzzer/FuzzerMutate.h
113

Please see my main comment to this CL, and let's have both const char *Name; and MutationType Identifier; in this struct.

169

I've just realized that this lookup thing is unnecessary complicated which we'll eventually have to remove anyway. Let's do this now, please see my main comment to the CL.

lib/fuzzer/FuzzerMutationStats.cpp
39

Actually, inverse the condition and put it inside an assert:

assert(MType >= 0 && MType < MaxNumberOfMutationTypes);

46

Actually, inverse the condition and put it inside an assert:

assert(MType >= 0 && MType < MaxNumberOfMutationTypes);
lib/fuzzer/FuzzerMutationStats.h
26

yeah, uint64_t would be a better choice

kodewilliams marked 8 inline comments as done and an inline comment as not done.Jun 20 2018, 10:01 AM
kodewilliams added inline comments.
lib/fuzzer/FuzzerMutate.cpp
44

I had to change it because there is a function that exists called CrossOver and it was the cause of those template errors. Same goes for ChangeBinInt instead of using ChangeBinaryInteger.

496

The Mutator struct doesnt have a name anymore, it just has the function pointer and an identifier of type MutationType, and as a result, there has to be some lookup in the unordered_map for the name regardless

kodewilliams marked 13 inline comments as done.

Wrote tests for new feature and updated enums.

Dor1s added a comment.Jun 22 2018, 6:43 AM

Did you forget to do git add for the test file you've written?

lib/fuzzer/FuzzerMutate.cpp
33–35

Please use original string names, like the one on line 42, those look better. Plus, we shouldn't change any existing code without a strong reason.
Also, please make sure that every line is no longer than 80 characters. You can do it manually, or use clang-format utility for that, it should be in your `build/bin' directory, but be careful, as it can do some unnecessary changes which again we shouldn't do.

lib/fuzzer/FuzzerMutate.h
25

I think we don't need this comment and the one on line 31 anymore, please remove.

36

just call it NUMBER_OF_MUTATION_TYPESor maybe MUTATION_TYPES_NUMBER

lib/fuzzer/FuzzerMutationStats.cpp
25

It doesn't look like you've uploaded your change properly. All the code of MutationStats::PrintMutationCounts() should be highlighted as new, not as a context. Please upload a diff made against origin/master or origin/HEAD.

kodewilliams marked 4 inline comments as done.

Made requested changes.

Removed PrintMutationCounts function

Left some minor comments, looks good otherwise! Once Jonathan is also happy with that, we should add Matt (@morehouse) as the next review step.

lib/fuzzer/FuzzerLoop.cpp
667

I think we should put it under if (Options.PrintMutationUsefulness) for now, as the stats aren't used otherwise.

lib/fuzzer/FuzzerMutate.cpp
529

Let's put it under if (Options.PrintMutationUsefulness) for now, as the stats aren't used otherwise.

lib/fuzzer/FuzzerMutate.h
22

It would be simpler to do:

enum MutationType {
  ADD_WORD_FROM_MANUAL_DICTIONARY,
  ...
  etc
};
lib/fuzzer/FuzzerMutationStats.cpp
24

I think the current version looks unnecessary messy, let's just do something like:

// +1 is not significant, but prevents us from dividing by zero.
double Ratio = static_cast<double>(100 * UsefulMutations.at(i)) / (1 + TotalMutations.at(i));
lib/fuzzer/FuzzerMutationStats.h
2

please fix this first line to be the same length and format as it is in other files

test/fuzzer/fuzzer-mutationstats.test
4

add a comment before that line, something like "# Make sure there are some non-zero values in the usefulness percentages printed."

Dor1s added a comment.Jun 22 2018, 3:32 PM

Since Matt got automatically added to the subscribers, here is some context. Kode is an intern on our team who is looking into leveraging mutation usefulness statistics somehow to improve fuzzing efficiency. This step only adds stats computation under an experimental flag. The next step would be to enable those stats on ClusterFuzz and collect usefulness distribution from various fuzz targets running in various circumstances. While stats will be collected, Kode will be trying different approaches for mutation selection algorithm (instead of existing Rand()) and, as a result, expected to add that new algorithm under this or another experimental flag.

kodewilliams marked 6 inline comments as done.

Just addressed host comments and suggestions.

metzman added inline comments.Jun 22 2018, 4:20 PM
lib/fuzzer/FuzzerMutationStats.cpp
24

+1

30

I think every arithmetic operator in libFuzzer has spaces around it. Please do that here by adding spaces around the "-"

31

This if isn't needed. else is enough.
Please remove it

31

This if isn't needed, else is enough

test/fuzzer/fuzzer-mutationstats.test
3

We don't need -max_total_time=360 -print_final_stats=1 -print_mutation_stats=1
Remove them please

It looks like the line numbers on my last comments were messed up, trying again.

lib/fuzzer/FuzzerMutationStats.cpp
27

I think every arithmetic operator in libFuzzer has spaces around it. Please do that here by adding spaces around the "-"

28

This if isn't needed. else is enough.
Please remove it

30

I messed up with this comment please ignore

31

I messed up with this comment please ignore

kodewilliams marked 9 inline comments as done.

Still addressing comments.

metzman added inline comments.Jun 25 2018, 10:15 AM
lib/fuzzer/FuzzerMutationStats.cpp
24

Please rename Ratio since it is a percent not a ratio.
Also, what if you do 100. * UsefulMutations.at(i), do you still need to static_cast this?

test/fuzzer/fuzzer-mutationstats.test
4

Sorry, I think comments in this format begin with ; not #.
See afl-driver-stderr.test as an example.

kodewilliams marked 2 inline comments as done.

More comments :)

Dor1s accepted this revision.Jun 25 2018, 2:08 PM
Dor1s added a reviewer: morehouse.
Dor1s added inline comments.
lib/fuzzer/FuzzerMutate.cpp
544

nit: for const auto& M

test/fuzzer/fuzzer-mutationstats.test
4

Not a big deal, but looks like only AFL tests have that syntax, others use # for comments (search for ^# file:\.test file:^src/third_party/llvm/compiler-rt/test/fuzzer/ in on the internal CS)

This revision is now accepted and ready to land.Jun 25 2018, 2:08 PM
kodewilliams marked 2 inline comments as done.

Had to fix a test.

morehouse added inline comments.Jun 25 2018, 4:52 PM
lib/fuzzer/FuzzerDefs.h
15

Can we include this only in the files that need it?

lib/fuzzer/FuzzerFlags.def
156

Maybe print_mutation_stats would be a more accurate name?

lib/fuzzer/FuzzerLoop.cpp
369

Should this go before the short-circuit above? Otherwise these stats won't be printed if PrintFinalStats is false.

lib/fuzzer/FuzzerMutate.h
38

Maybe call this kNumMutationTypes or similar to distinguish it from the other enum values. That might help prevent future contributors from adding new values after it.

lib/fuzzer/FuzzerMutationStats.h
19

Can we just omit the destructor since it has an empty implementation anyway?

morehouse added inline comments.Jun 25 2018, 4:52 PM
lib/fuzzer/FuzzerMutationStats.cpp
19

Can we just do MutationStats MStats instead of making this on the heap?

26

Can we just do:

double UsefulPercentage = TotalMutations.at(i) ? ... : 0
27

The output will be easier to understand if you also print the mutation names.

test/fuzzer/fuzzer-mutationstats.test
6

I think this regex can be simplified to {{[0-9]+\.[0-9]+}}

kodewilliams added inline comments.Jun 25 2018, 5:11 PM
lib/fuzzer/FuzzerLoop.cpp
369

Mutation stats relies on final stats. This way, the same line doesnt have to be put into the code 10 times (like PrintFinalStats() is).

morehouse added inline comments.Jun 25 2018, 5:15 PM
lib/fuzzer/FuzzerLoop.cpp
369

I don't think moving this above the short circuit should affect that. It will just make your flag have the same behavior as -print_coverage, -dump_coverage, etc. which don't require -print_final_stats=1 to work.

kodewilliams marked 11 inline comments as done.
kodewilliams added inline comments.
lib/fuzzer/FuzzerMutationStats.cpp
27

I was told to remove the names from the output for parsing purposes, as it will make it easier to use in the latter phase of the project. Does this still stand?

morehouse added a subscriber: kcc.Jun 26 2018, 12:46 PM
morehouse added inline comments.
lib/fuzzer/FuzzerMutationStats.cpp
26

Please remove dead code.

27

I was interpreting this patch as an option to print human-readable stats similar to -print_final_stats.

@kcc, @Dor1s: Do we care if this information is human readable or not?

28

This formatting looks odd. Please use clang-format on your changes.

metzman added inline comments.Jun 26 2018, 12:50 PM
lib/fuzzer/FuzzerMutationStats.cpp
27

@Dor1s is OOO for 2 weeks.
But I think he was more concerned that the output be easy for libFuzzer to consume than human readable.
I believe he wanted it output as a CSV without any column names.

kodewilliams marked 5 inline comments as done.
morehouse accepted this revision.Jun 26 2018, 2:48 PM
morehouse added inline comments.
test/fuzzer/fuzzer-mutationstats.test
3

You should be able to remove -print_final_stats=1 now.

kcc added inline comments.Jun 26 2018, 5:21 PM
lib/fuzzer/FuzzerLoop.cpp
668

Shouldn't we call this near the call to RecordSuccessfulMutationSequence, or even inside it?
Also, I think it's find to collect the stats even when printing stats is not requested,
as soon as the stats collection is cheap. This way will have less code.

lib/fuzzer/FuzzerMutate.h
22

ouch. please find a way w/o introducing this enum

lib/fuzzer/FuzzerMutationStats.h
2

this is a bit over-engineered.
Just put this into FuzzerMutate.h and please make it simpler.

I guess it could be two extra Vector<uint64_t> objects inside MutationDispatcher
which you resize in the MutationDispatcher CTOR.

kodewilliams marked 2 inline comments as done.

Removed FuzzerMutationStats.* files and simplified implementation inside FuzzerMutate.

kodewilliams marked an inline comment as done.Jun 27 2018, 6:08 PM
kcc added inline comments.Jun 27 2018, 6:17 PM
lib/fuzzer/FuzzerMutate.cpp
34–35

This is still pretty gross. You don't need to assign numbers to mutations.
When you add a mutation to DefaultMutators that becomes the mutation's number.
And so you also should not need "int Identifier;" above.

69

you should use Mutators.size() here in stead of kNumMutationTypes

529

instead of having M.Identifier, change the line above

auto M = Mutators[Rand(Mutators.size())];

to something like

size_t MutatorIdx = Rand(Mutators.size());   
auto M = Mutators[MutatorIdx];

and then use MutatorIdx

lib/fuzzer/FuzzerMutate.h
22

you should not need this.

168

check text alignment.

lib/fuzzer/FuzzerOptions.h
56

stale?

kodewilliams marked 6 inline comments as done.Jun 27 2018, 7:35 PM
kodewilliams added inline comments.
lib/fuzzer/FuzzerMutate.cpp
529

@kcc I had been using the mutation identifier to increment useful counts but now since it is gone, the only way I can think of getting the correct index inside CountCurrentMutatorSequence is by looking up each element in the vector to get it's index then using that, which is expensive. Since this is not done a very large number of times, is it acceptable?

kodewilliams marked an inline comment as done.
kodewilliams marked an inline comment as done.Jun 29 2018, 8:58 AM
kcc added a comment.Jun 29 2018, 11:38 AM

the current patch is hard to review as it contains tons of formatting (or otherwise unrelated) fixed.
Plz don't mix formatting/refactoring with meaningful changes in one patch.

FuzzerDefs.h
19 ↗(On Diff #153415)

plz don't include unrelated changes, they make the review harder.

FuzzerMutate.cpp
68 ↗(On Diff #153415)

again, don;t include unrelated changes.
here and below.

kodewilliams marked 2 inline comments as done.

@kcc Insignificant changes are undone. PTAL.

kcc added a comment.Jun 29 2018, 1:18 PM

add a .lit test, please

FuzzerMutate.cpp
539 ↗(On Diff #153559)

still an unrelated change?

FuzzerMutate.h
98 ↗(On Diff #153559)

do you still need to move "private:"

134 ↗(On Diff #153559)

Now that you have CurrentMutatorIdxSequence you probably can get rid of CurrentMutatorSequence

kodewilliams marked 3 inline comments as done.

Re-added test and removed other insignificant changes.

kcc added a comment.Jul 2 2018, 6:04 PM

a couple of nits, then good to go.

lib/fuzzer/FuzzerMutate.cpp
550

the loop bound is a size_t, so please use size_t

lib/fuzzer/FuzzerMutate.h
110

Maybe try to find a better name.
(Nitpicking, feel free to ignore)

kodewilliams marked 2 inline comments as done.

Fixed final nits, good to go @kcc

kcc accepted this revision.Jul 3 2018, 3:56 PM

LGTM

kodewilliams added a comment.EditedJul 9 2018, 1:02 PM

@kcc @morehouse requesting these changes be landed.

kcc added a comment.Jul 9 2018, 1:13 PM

Matt, please land it

morehouse closed this revision.Jul 9 2018, 1:26 PM

Landed in r336597.

morehouse reopened this revision.Jul 9 2018, 3:38 PM

Reverted due to bot breakage in r336616.

FAIL: libFuzzer :: fuzzer-custommutator.test (6330 of 6439)
******************** TEST 'libFuzzer :: fuzzer-custommutator.test' FAILED ********************
Script:
--
: 'RUN: at line 1';     /b/sanitizer-x86_64-linux/build/llvm_build64/bin/clang  --driver-mode=g++ -std=c++11 -lstdc++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer -m64 /b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/test/fuzzer/CustomMutatorTest.cpp -o /b/sanitizer-x86_64-linux/build/compiler_rt_build/test/fuzzer/Output/fuzzer-custommutator.test.tmp-CustomMutatorTest
: 'RUN: at line 2';   not  /b/sanitizer-x86_64-linux/build/compiler_rt_build/test/fuzzer/Output/fuzzer-custommutator.test.tmp-CustomMutatorTest 2>&1 | FileCheck /b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/test/fuzzer/fuzzer-custommutator.test --check-prefix=LLVMFuzzerCustomMutator
--
Exit Code: 1

Command Output (stderr):
--
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/test/fuzzer/fuzzer-custommutator.test:4:26: error: expected string not found in input
LLVMFuzzerCustomMutator: BINGO
                         ^
<stdin>:8:1: note: scanning from here
=================================================================
^
<stdin>:9:9: note: possible intended match here
==57316==ERROR: AddressSanitizer: attempting double-free on 0x602000000050 in thread T0:
        ^

See http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fuzzer/builds/16996.

This revision is now accepted and ready to land.Jul 9 2018, 3:38 PM

@morehouse I have been looking over the code and I cannot seem to find a reason why that single test is failing. PTAL.

@kodewilliams have you rerun the tests to find the failing one?

I have seen the same failure when the commit was landed and I debugged it a little bit. The problem was that CustomMutatorTest.cpp's LLVMFuzzerCustomMutator implementation calls LLVMFuzzerMutate, which uses the default set of mutators. This causes a discrepancy: MutateImpl will log into CurrentMutatorIdxSequence that a mutator with an index between 0..12 (from the default set) was used, but when printing the sequence (in PrintMutationSequence), we only expect to find indexes from MutationDispatcher::Mutators. When using a custom mutator (like in this test), MutationDispatcher::Mutators only has one element. So PrintMutationSequence overflows the vector when it tries to use an index from 0..12. Add a if (M >= Mutators.size()) abort(); line into PrintMutationSequence and run the test to reproduce this.

@morehouse I have been looking over the code and I cannot seem to find a reason why that single test is failing. PTAL.

I suspect this has something to do with LLVMFuzzerMutate being called from CustomMutatorTest.cpp, which uses DefaultMutators instead of Mutators. So your CurrentMutatorIdxSequence vector gets indexes from both DefaultMutators and Mutators.

@morehouse tests are passing now.

morehouse added inline comments.Jul 11 2018, 2:09 PM
lib/fuzzer/FuzzerMutate.cpp
528–530

I don't like this solution. If we don't want to support mutation stats for custom mutators, we should print an error and exit before any fuzzing happens.

Otherwise, we should make stats work with custom mutators too.

metzman added inline comments.Jul 11 2018, 2:12 PM
lib/fuzzer/FuzzerMutate.cpp
528–530

+1 for exit.

kodewilliams marked 2 inline comments as done.

Tests now all pass and custom mutations can be tracked.

Dor1s updated this revision to Diff 155719.Jul 16 2018, 10:54 AM

Rebase and getting ready to commit.

Dor1s retitled this revision from [libFuzzer] Mutation tracking and logging implemented to [libFuzzer] Mutation tracking and logging implemented..Jul 16 2018, 10:55 AM
Dor1s edited the summary of this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.
Dor1s added a comment.Jul 16 2018, 1:00 PM

I'm seeing a build failure on one of the bots http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/13625/steps/build%20with%20ninja/logs/stdio :

[3726/4950] Building CXX object projects/compiler-rt/lib/fuzzer/CMakeFiles/RTfuzzer.x86_64.dir/FuzzerMutate.cpp.o
FAILED: projects/compiler-rt/lib/fuzzer/CMakeFiles/RTfuzzer.x86_64.dir/FuzzerMutate.cpp.o 
/b/sanitizer-x86_64-linux/build/clang_build/bin/clang++   -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iprojects/compiler-rt/lib/fuzzer -I/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer -I/usr/include/libxml2 -Iinclude -I/b/sanitizer-x86_64-linux/build/llvm/include -gmlt -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -Wall -Werror -std=c++11 -Wno-unused-parameter -O3    -UNDEBUG  -gmlt -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -Wall -Werror -std=c++11 -Wno-unused-parameter -m64 -fPIC -fno-builtin -fno-exceptions -fomit-frame-pointer -funwind-tables -fno-stack-protector -fno-sanitize=safe-stack -fvisibility=hidden -fno-lto -O3 -gline-tables-only -Wno-gnu -Wno-variadic-macros -Wno-c99-extensions -Wno-non-virtual-dtor -nostdinc++ -D_LIBCPP_ABI_VERSION=Fuzzer -fno-omit-frame-pointer -isystem /b/sanitizer-x86_64-linux/build/llvm_build_ninja/projects/compiler-rt/lib/fuzzer/libcxx_fuzzer_x86_64/include/c++/v1 -MD -MT projects/compiler-rt/lib/fuzzer/CMakeFiles/RTfuzzer.x86_64.dir/FuzzerMutate.cpp.o -MF projects/compiler-rt/lib/fuzzer/CMakeFiles/RTfuzzer.x86_64.dir/FuzzerMutate.cpp.o.d -o projects/compiler-rt/lib/fuzzer/CMakeFiles/RTfuzzer.x86_64.dir/FuzzerMutate.cpp.o -c /b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:33:64: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
          {&MutationDispatcher::Mutate_EraseBytes, "EraseBytes"},
                                                               ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:34:64: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
          {&MutationDispatcher::Mutate_InsertByte, "InsertByte"},
                                                               ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:36:33: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
           "InsertRepeatedBytes"},
                                ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:37:64: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
          {&MutationDispatcher::Mutate_ChangeByte, "ChangeByte"},
                                                               ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:38:62: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
          {&MutationDispatcher::Mutate_ChangeBit, "ChangeBit"},
                                                             ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:39:68: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
          {&MutationDispatcher::Mutate_ShuffleBytes, "ShuffleBytes"},
                                                                   ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:40:76: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
          {&MutationDispatcher::Mutate_ChangeASCIIInteger, "ChangeASCIIInt"},
                                                                           ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:41:75: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
          {&MutationDispatcher::Mutate_ChangeBinaryInteger, "ChangeBinInt"},
                                                                          ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:42:60: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
          {&MutationDispatcher::Mutate_CopyPart, "CopyPart"},
                                                           ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:43:62: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
          {&MutationDispatcher::Mutate_CrossOver, "CrossOver"},
                                                             ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:45:24: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
           "ManualDict"},
                       ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:47:26: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
           "PersAutoDict"},
                         ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:51:60: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
        {&MutationDispatcher::Mutate_AddWordFromTORC, "CMP"});
                                                           ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:54:69: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
    Mutators.push_back({&MutationDispatcher::Mutate_Custom, "Custom"});
                                                                    ^
/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMutate.cpp:60:72: error: missing field 'UsefulCount' initializer [-Werror,-Wmissing-field-initializers]
        {&MutationDispatcher::Mutate_CustomCrossOver, "CustomCrossOver"});
                                                                       ^
15 errors generated.
Dor1s reopened this revision.Jul 16 2018, 1:12 PM

Reverted in https://reviews.llvm.org/rCRT337206.

Kode, please add -DLLVM_ENABLE_WERROR=ON flag to your cmake command to reproduce the failure locally.

This revision is now accepted and ready to land.Jul 16 2018, 1:12 PM
Dor1s added a comment.Jul 16 2018, 1:19 PM

Actually, I think you might've already used that flag (it is used in LLVM Dev Starting Kit). In that case, you would need to add -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, I think.

Had to initialize the counters to silence build bot warnings and errors.

morehouse accepted this revision.Jul 17 2018, 10:44 AM

Hey Matt, do you think we can land this or should also wait for Kostya's approval?

I think it's fine to land. Kostya approved the previous version, and he probably won't mind the extra 0's in the initialization lists.

It isn't as clean looking, but it silences the warning, and it's probably good practice to specify all fields in the list anyway.

This revision was automatically updated to reflect the committed changes.