[PATCH] D49621: [libFuzzer] Initial implementation of weighted mutation leveraging during runtime.

[Jonathan Metzman via Phabricator via llvm-commits]

metzman added inline comments.


================
Comment at: lib/fuzzer/FuzzerMutate.cpp:36
       {
-          {&MutationDispatcher::Mutate_EraseBytes, "EraseBytes", 0, 0},
-          {&MutationDispatcher::Mutate_InsertByte, "InsertByte", 0, 0},
+          {&MutationDispatcher::Mutate_EraseBytes, "EraseBytes", 1, 1},
+          {&MutationDispatcher::Mutate_InsertByte, "InsertByte", 1, 1},
----------------
Can you leave a comment here about why we are starting with a `UsefulCount` and a `TotalCount` of 1?


================
Comment at: lib/fuzzer/FuzzerMutate.cpp:43
+          {&MutationDispatcher::Mutate_ShuffleBytes, "ShuffleBytes", 1, 1},
           {&MutationDispatcher::Mutate_ChangeASCIIInteger, "ChangeASCIIInt", 0,
+           1},
----------------
Why do `ChangeASCIIInt` and `ChangeBinInt` start with a `UsefulCount` of `0` when everything else starts with `1`?


================
Comment at: lib/fuzzer/FuzzerMutate.cpp:603
+  for (size_t i = 0; i < Stats.size(); i++)
+    MutationWeights[i] = (Stats[i] * 1.0) / SumOfStats;
+}
----------------
I don't think we need to multiply by 1.0, right? I don't think it (or the parentheses) does anything, please remove them if I am correct.


================
Comment at: lib/fuzzer/FuzzerMutate.cpp:607
+size_t MutationDispatcher::WeightedIndex() {
+  Random Rand(time(0));
+  std::discrete_distribution<size_t> SelectIndex(MutationWeights.begin(),
----------------
Kode, I don't think this is what Max meant. I believe that here you need to use the rand obtained by calling `GetRand`.
It's possible, though unlikely, that not doing this was causing the test flakiness you were seeing before. 
I would run the previously flaky test in a loop and ensure that the tests pass every time, since if there is any flake we don't know about, it will be more painful to get rid of later rather than now.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D49621