[libFuzzer] Mutation tracking and logging implemented.

[kodewilliams]

Summary

Code now exists to track number of mutations that are used in fuzzing in total
and ones that produce new coverage. The stats are currently being dumped to the
command line.

Patch by Kodé Williams (@kodewilliams).

[metzman]

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]

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]

Fixed nits and made code more efficient.

[kodewilliams]

Fixing context not available problem as per LLVM documentation.

[kodewilliams]

More efficient code.

[kodewilliams]

Removed a macro.

[metzman]

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

[kodewilliams]

Encompasses all changes.

[metzman]

@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

52

Same as above, please change to AddWordFromTORC

55

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

490

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

115

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.

[metzman]

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

I meant unordered_map

[Dor1s]

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

112

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

170

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]

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.

490

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]

Wrote tests for new feature and updated enums.

[Dor1s]

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

lib/fuzzer/FuzzerMutate.cpp

34

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

27

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

38

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]

Made requested changes.

[kodewilliams]

Removed PrintMutationCounts function

[Dor1s]

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

3 ↗

(On Diff #152501)

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

[Dor1s]

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]

Just addressed host comments and suggestions.

[metzman]

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

2 ↗

(On Diff #152501)

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

[metzman]

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]

Still addressing comments.

[kodewilliams]

PTAL

[metzman]

lib/fuzzer/FuzzerMutationStats.cpp

25

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

3 ↗

(On Diff #152501)

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

[kodewilliams]

More comments :)

[Dor1s]

lib/fuzzer/FuzzerMutate.cpp

544

nit: for const auto& M

test/fuzzer/fuzzer-mutationstats.test

3 ↗

(On Diff #152501)

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)

[kodewilliams]

Had to fix a test.

[morehouse]

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]

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

5 ↗

(On Diff #152798)

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

[kodewilliams]

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]

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]

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]

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]

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.

[morehouse]

test/fuzzer/fuzzer-mutationstats.test

2 ↗

(On Diff #152956)

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

[kcc]

lib/fuzzer/FuzzerLoop.cpp

667

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]

Removed FuzzerMutationStats.* files and simplified implementation inside FuzzerMutate.

[kcc]

lib/fuzzer/FuzzerMutate.cpp

33

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.

66

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

531

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.

154

check text alignment.

lib/fuzzer/FuzzerOptions.h

56

stale?

[kodewilliams]

lib/fuzzer/FuzzerMutate.cpp

531

@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?

[kcc]

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]

@kcc Insignificant changes are undone. PTAL.

[kcc]

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]

Re-added test and removed other insignificant changes.

[kcc]

a couple of nits, then good to go.

lib/fuzzer/FuzzerMutate.cpp

549

the loop bound is a size_t, so please use size_t

lib/fuzzer/FuzzerMutate.h

91

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

[kodewilliams]

Fixed final nits, good to go @kcc

[kcc]

LGTM

[kodewilliams]

@kcc @morehouse requesting these changes be landed.

[kcc]

Matt, please land it

[morehouse]

Landed in r336597.

[morehouse]

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.

[kodewilliams]

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

[metzman]

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

[kubamracek]

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]

In D48054#1156815, @kodewilliams wrote:

@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.

[kodewilliams]

@morehouse tests are passing now.

[morehouse]

lib/fuzzer/FuzzerMutate.cpp

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]

lib/fuzzer/FuzzerMutate.cpp

530

+1 for exit.

[kodewilliams]

Tests now all pass and custom mutations can be tracked.

[morehouse]

LGTM

[Dor1s]

Rebase and getting ready to commit.

[Dor1s]

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]

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

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

[Dor1s]

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.

[kodewilliams]

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

[Dor1s]

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

[morehouse]

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.

lib/fuzzer/CMakeLists.txt

   FuzzerLoop.cpp
   FuzzerMerge.cpp
   FuzzerMutate.cpp
+  FuzzerMutationStats.cpp
   FuzzerSHA1.cpp
   FuzzerShmemFuchsia.cpp
   FuzzerShmemPosix.cpp

lib/fuzzer/FuzzerDefs.h

 #ifndef LLVM_FUZZER_DEFS_H
 #define LLVM_FUZZER_DEFS_H
 
+#include <array>
 #include <cassert>
 #include <cstddef>
 #include <cstdint>
 #include <cstring>
+#include <memory>
+#include <set>
 #include <string>
+#include <unordered_map>
 #include <vector>
-#include <set>
-#include <memory>
 
 // Platform detection.
 #ifdef __linux__

lib/fuzzer/FuzzerDriver.cpp

   Options.PrintNewCovPcs = Flags.print_pcs;
   Options.PrintNewCovFuncs = Flags.print_funcs;
   Options.PrintFinalStats = Flags.print_final_stats;
+  Options.PrintMutationStats = Flags.print_mutation_stats;
   Options.PrintCorpusStats = Flags.print_corpus_stats;
   Options.PrintCoverage = Flags.print_coverage;
   Options.DumpCoverage = Flags.dump_coverage;

lib/fuzzer/FuzzerFlags.def

 FUZZER_FLAG_INT(analyze_dict, 0, "Experimental")
 FUZZER_DEPRECATED_FLAG(use_clang_coverage)
 FUZZER_FLAG_STRING(data_flow_trace, "Experimental: use the data flow trace")
+FUZZER_FLAG_INT(print_mutation_stats, 0, "Experimental")

lib/fuzzer/FuzzerLoop.cpp

 #include "FuzzerIO.h"
 #include "FuzzerInternal.h"
 #include "FuzzerMutate.h"
+#include "FuzzerMutationStats.h"
 #include "FuzzerRandom.h"
 #include "FuzzerShmem.h"
 #include "FuzzerTracePC.h"

   Printf("stat::new_units_added:          %zd\n", NumberOfNewUnitsAdded);
   Printf("stat::slowest_unit_time_sec:    %zd\n", TimeOfLongestUnitInSeconds);
   Printf("stat::peak_rss_mb:              %zd\n", GetPeakRSSMb());
+
+  if (Options.PrintMutationStats)
+    MStats->PrintMutationCounts();
 }
 
 void Fuzzer::SetMaxInputLen(size_t MaxInputLen) {

                             /*DuringInitialCorpusExecution*/ false);
     if (NewCov) {
       ReportNewCoverage(&II, {CurrentUnitData, CurrentUnitData + Size});
+      MD.CountCurrentMutatorSequence();
       break;  // We will mutate this input more in the next rounds.
     }
     if (Options.ReduceDepth && !FoundUniqFeatures)

lib/fuzzer/FuzzerMutate.h

 
   Random &GetRand() { return Rand; }
 
-private:
+  void CountCurrentMutatorSequence();
 
   struct Mutator {
     size_t (MutationDispatcher::*Fn)(uint8_t *Data, size_t Size, size_t Max);
-    const char *Name;
+    MutationType Identifier;
   };
 
+
+private:
+
   size_t AddWordFromDictionary(Dictionary &D, uint8_t *Data, size_t Size,
                                size_t MaxSize);
   size_t MutateImpl(uint8_t *Data, size_t Size, size_t MaxSize,

   Vector<Mutator> DefaultMutators;
 };
 
+const std::unordered_map<MutationType, std::string> kMutationNames = {
+  {ManualDict, "AddWordFromManualDictionaryCount"},
+  {PersAutoDict, "AddWordFromPersistentAutoDictionaryCount"},
+  {CMP, "AddWordFromTORCCount"},
+  {ChangeAsciiInt, "ChangeASCIIIntegerCount"},
+  {ChangeBinInt, "ChangeBinaryIntegerCount"},
+  {ChangeBit, "ChangeBitCount"},
+  {CopyPart, "CopyPartCount"},
+  {ChangeByte, "ChangeByteCount"},
+  {CrossOver, "CrossOverCount"},
+  {CustomCrossOver, "CustomCrossOverCount"},
+  {CustomMutation, "CustomMutationCount"},
+  {EraseBytes, "EraseBytesCount"},
+  {InsertByte, "InsertByteCount"},
+  {InsertRepeatedBytes, "InsertRepeatedBytesCount"},
+  {ShuffleBytes, "ShuffleBytesCount"}
+};
+
 }  // namespace fuzzer
 
 #endif  // LLVM_FUZZER_MUTATE_H

lib/fuzzer/FuzzerMutate.cpp

 //===----------------------------------------------------------------------===//
 
 #include "FuzzerMutate.h"
+#include "FuzzerMutationStats.h"
 #include "FuzzerCorpus.h"
 #include "FuzzerDefs.h"
 #include "FuzzerExtFunctions.h"

   DefaultMutators.insert(
       DefaultMutators.begin(),
       {
-          {&MutationDispatcher::Mutate_EraseBytes, "EraseBytes"},
-          {&MutationDispatcher::Mutate_InsertByte, "InsertByte"},
+          {&MutationDispatcher::Mutate_EraseBytes, EraseBytes},
+          {&MutationDispatcher::Mutate_InsertByte, InsertByte},
           {&MutationDispatcher::Mutate_InsertRepeatedBytes,
-           "InsertRepeatedBytes"},
-          {&MutationDispatcher::Mutate_ChangeByte, "ChangeByte"},
-          {&MutationDispatcher::Mutate_ChangeBit, "ChangeBit"},
-          {&MutationDispatcher::Mutate_ShuffleBytes, "ShuffleBytes"},
-          {&MutationDispatcher::Mutate_ChangeASCIIInteger, "ChangeASCIIInt"},
-          {&MutationDispatcher::Mutate_ChangeBinaryInteger, "ChangeBinInt"},
-          {&MutationDispatcher::Mutate_CopyPart, "CopyPart"},
-          {&MutationDispatcher::Mutate_CrossOver, "CrossOver"},
+           InsertRepeatedBytes},
+          {&MutationDispatcher::Mutate_ChangeByte, ChangeByte},
+          {&MutationDispatcher::Mutate_ChangeBit, ChangeBit},
+          {&MutationDispatcher::Mutate_ShuffleBytes, ShuffleBytes},
+          {&MutationDispatcher::Mutate_ChangeASCIIInteger, ChangeAsciiInt},
+          {&MutationDispatcher::Mutate_ChangeBinaryInteger, ChangeBinInt},
+          {&MutationDispatcher::Mutate_CopyPart, CopyPart},
+          {&MutationDispatcher::Mutate_CrossOver, CrossOver},
           {&MutationDispatcher::Mutate_AddWordFromManualDictionary,
-           "ManualDict"},
+           ManualDict},
           {&MutationDispatcher::Mutate_AddWordFromPersistentAutoDictionary,
-           "PersAutoDict"},
+           PersAutoDict},
       });
   if(Options.UseCmp)
     DefaultMutators.push_back(
-        {&MutationDispatcher::Mutate_AddWordFromTORC, "CMP"});
+        {&MutationDispatcher::Mutate_AddWordFromTORC, CMP});
 
   if (EF->LLVMFuzzerCustomMutator)
-    Mutators.push_back({&MutationDispatcher::Mutate_Custom, "Custom"});
+    Mutators.push_back({&MutationDispatcher::Mutate_Custom, CustomMutation});
   else
     Mutators = DefaultMutators;
 
   if (EF->LLVMFuzzerCustomCrossOver)
     Mutators.push_back(
-        {&MutationDispatcher::Mutate_CustomCrossOver, "CustomCrossOver"});
+        {&MutationDispatcher::Mutate_CustomCrossOver, CustomCrossOver});
 }
 
 static char RandCh(Random &Rand) {

 
 size_t MutationDispatcher::Mutate_Custom(uint8_t *Data, size_t Size,
                                          size_t MaxSize) {
+  MStats->IncrementCount(CustomMutation, 0);
   return EF->LLVMFuzzerCustomMutator(Data, Size, MaxSize, Rand.Rand());
 }
 

     return 0;
   assert(NewSize <= MaxSize && "CustomCrossOver returned overisized unit");
   memcpy(Data, U.data(), NewSize);
+  MStats->IncrementCount(CustomCrossOver, 0);
   return NewSize;
 }
 

   size_t ShuffleStart = Rand(Size - ShuffleAmount);
   assert(ShuffleStart + ShuffleAmount <= Size);
   std::shuffle(Data + ShuffleStart, Data + ShuffleStart + ShuffleAmount, Rand);
+  MStats->IncrementCount(ShuffleBytes, 0);
   return Size;
 }
 

   // Erase Data[Idx:Idx+N].
   memmove(Data + Idx, Data + Idx + N, Size - Idx - N);
   // Printf("Erase: %zd %zd => %zd; Idx %zd\n", N, Size, Size - N, Idx);
+  MStats->IncrementCount(EraseBytes, 0);
   return Size - N;
 }
 

   // Insert new value at Data[Idx].
   memmove(Data + Idx + 1, Data + Idx, Size - Idx);
   Data[Idx] = RandCh(Rand);
+  MStats->IncrementCount(InsertByte, 0);
   return Size + 1;
 }
 

   uint8_t Byte = Rand.RandBool() ? Rand(256) : (Rand.RandBool() ? 0 : 255);
   for (size_t i = 0; i < N; i++)
     Data[Idx + i] = Byte;
+  MStats->IncrementCount(InsertRepeatedBytes, 0);
   return Size + N;
 }
 

   if (Size > MaxSize) return 0;
   size_t Idx = Rand(Size);
   Data[Idx] = RandCh(Rand);
+  MStats->IncrementCount(ChangeByte, 0);
   return Size;
 }
 

   if (Size > MaxSize) return 0;
   size_t Idx = Rand(Size);
   Data[Idx] ^= 1 << Rand(8);
+  MStats->IncrementCount(ChangeBit, 0);
   return Size;
 }
 
 size_t MutationDispatcher::Mutate_AddWordFromManualDictionary(uint8_t *Data,
                                                               size_t Size,
                                                               size_t MaxSize) {
+  MStats->IncrementCount(ManualDict, 0);
   return AddWordFromDictionary(ManualDictionary, Data, Size, MaxSize);
 }
 

                                 kCmpDictionaryEntriesDequeSize];
   DERef = DE;
   CurrentDictionaryEntrySequence.push_back(&DERef);
+  MStats->IncrementCount(CMP, 0);
   return Size;
 }
 
 size_t MutationDispatcher::Mutate_AddWordFromPersistentAutoDictionary(
     uint8_t *Data, size_t Size, size_t MaxSize) {
+  MStats->IncrementCount(PersAutoDict, 0);
   return AddWordFromDictionary(PersistentAutoDictionary, Data, Size, MaxSize);
 }
 

 
 size_t MutationDispatcher::Mutate_CopyPart(uint8_t *Data, size_t Size,
                                            size_t MaxSize) {
+  MStats->IncrementCount(CopyPart, 0);
   if (Size > MaxSize || Size == 0) return 0;
   // If Size == MaxSize, `InsertPartOf(...)` will
   // fail so there's no point using it in this case.

     Data[Idx] = (Val % 10) + '0';
     Val /= 10;
   }
+  MStats->IncrementCount(ChangeAsciiInt, 0);
   return Size;
 }
 

 size_t MutationDispatcher::Mutate_ChangeBinaryInteger(uint8_t *Data,
                                                       size_t Size,
                                                       size_t MaxSize) {
+  MStats->IncrementCount(ChangeBinInt, 0);
   if (Size > MaxSize) return 0;
   switch (Rand(4)) {
     case 3: return ChangeBinaryInteger<uint64_t>(Data, Size, Rand);

   assert(NewSize > 0 && "CrossOver returned empty unit");
   assert(NewSize <= MaxSize && "CrossOver returned overisized unit");
   memcpy(Data, U.data(), NewSize);
+  MStats->IncrementCount(CrossOver, 0);
   return NewSize;
 }
 

 
 void MutationDispatcher::PrintMutationSequence() {
   Printf("MS: %zd ", CurrentMutatorSequence.size());
-  for (auto M : CurrentMutatorSequence)
-    Printf("%s-", M.Name);
+  for (auto M : CurrentMutatorSequence) {
+    auto currentMutation = kMutationNames.find(M.Identifier);
+    Printf("%s-", currentMutation->second.c_str());
+  }
   if (!CurrentDictionaryEntrySequence.empty()) {
     Printf(" DE: ");
     for (auto DE : CurrentDictionaryEntrySequence) {

       {W, std::numeric_limits<size_t>::max()});
 }
 
+void MutationDispatcher::CountCurrentMutatorSequence() {
+  for (auto M : CurrentMutatorSequence)
+    // Increment using map look up and enum for index
+    MStats->IncrementCount(M.Identifier, 1);
+}
+
 }  // namespace fuzzer

lib/fuzzer/FuzzerMutationStats.h

+//===- FuzzerMutationStats.h - Header for mutation tracking ----------*- C++ -* ===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// fuzzer::MutationStats
+//===----------------------------------------------------------------------===//
+
+
+namespace fuzzer {
+
+enum MutationType {
+  ManualDict,
+  PersAutoDict,
+  CMP,
+  ChangeAsciiInt,
+  ChangeBinInt,
+  ChangeBit,
+  ChangeByte,
+  CopyPart,
+  CrossOver,
+  CustomCrossOver,
+  CustomMutation,
+  EraseBytes,
+  InsertByte,
+  InsertRepeatedBytes,
+  ShuffleBytes,
+  MaxNumberOfMutationTypes
+};
+
+class MutationStats {
+public:
+  ~MutationStats() {}
+  void PrintMutationCounts();
+  void IncTotalMutationCount(MutationType MType);
+  void IncUsefulMutationCount(MutationType MType);
+
+private:
+  // A total count of each mutation used in the fuzzing process.
+  std::array<size_t, MaxNumberOfMutationTypes> TotalMutations;
+  // The number of each mutation that resulted in new coverage.
+  std::array<size_t, MaxNumberOfMutationTypes> UsefulMutations;
+};
+
+extern MutationStats *MStats;
+
+}

lib/fuzzer/FuzzerMutationStats.cpp

+//===- FuzzerMutate.cpp - Mutate a test input -----------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// Mutate a test input.
+//===----------------------------------------------------------------------===//
+
+#include "FuzzerMutate.h"
+#include "FuzzerMutationStats.h"
+#include "FuzzerIO.h"
+
+#define NAMEOFSTAT(var) #var
+
+namespace fuzzer {
+
+MutationStats* MStats = new MutationStats();
+
+void MutationStats::PrintMutationCounts() {
+  // Outputs the number of each mutation used
+
+  Printf("\nTotal Mutations ----------\n");
+  for (int i = 0; i < MaxNumberOfMutationTypes; i++) {
+    auto current = kMutationNames.find(i);
+    Printf("%s: %d\n", current->second.c_str(), TotalMutations.at(i));
+  }
+
+  Printf("\nUseful Mutations ----------\n");
+  for (int i = 0; i < MaxNumberOfMutationTypes; i++) {
+    auto current = kMutationNames.find(i);
+    Printf("%s: %d\n", current->second.c_str(), UsefulMutations.at(i));
+  }
+
+}
+
+// Updates the count of mutations (total/useful) depending on type.
+// Types: (0) Total, (1) Useful
+void MutationStats::IncTotalMutationCount(MutationType MType) {
+  if ((MType < 0) || MType >= MaxNumberOfMutationTypes)
+    return;
+  else
+    TotalMutations[MType]++;
+}
+
+void MutationStats::IncTotalMutationCount(MutationType MType) {
+  if ((MType < 0) || MType >= MaxNumberOfMutationTypes)
+    return;
+  else
+    UsefulMutations[MType]++;
+}
+
+}  // namespace fuzzer

lib/fuzzer/FuzzerOptions.h

   bool PrintNewCovPcs = false;
   int PrintNewCovFuncs = 0;
   bool PrintFinalStats = false;
+  bool PrintMutationStats = false;
   bool PrintCorpusStats = false;
   bool PrintCoverage = false;
   bool DumpCoverage = false;