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

[Kodé Williams via Phabricator via llvm-commits]

kodewilliams added a comment.

PTAL @metzman @Dor1s



================
Comment at: lib/fuzzer/FuzzerMutate.cpp:23
+const double kDefaultMutationWeight = 1;
+const double kDefaultMutationStat = 1 / (100 * 1000);
 
----------------
Dor1s wrote:
> metzman wrote:
> > Please add a comment to explain the significance of `100` and `1000` (frankly i don't know what the purpose is of either since we don't actually round anything).
> +1, what is it for?
It is just there to represent a usefulness ratio that is near to but not entirely useless. So that it still gets weight instead of calculating to 0.


================
Comment at: lib/fuzzer/FuzzerMutate.cpp:525
   for (int Iter = 0; Iter < 100; Iter++) {
-    auto M = &Mutators[Rand(Mutators.size())];
+    if (Options.UseWeightedMutations)
+      M = &Mutators[WeightedIndex()];
----------------
Dor1s wrote:
> How will this work during first 10000 runs? Do we have a uniform distribution by default?
AssignMutationWeights is called initially and sets all the mutation weights to the default, therefore they would be all be weighted the same and have the same probability before 10k runs.


================
Comment at: lib/fuzzer/FuzzerMutate.cpp:607
+      MutationWeights[i] =
+          (Stats[i] * 1000 / SumOfStats) + kDefaultMutationWeight;
+    }
----------------
Dor1s wrote:
> I don't understand why we need `+ kDefaultMutationWeight`, as well as why we multiply the value by `1000`.
I add the default to ensure that mutations  that are any amount more useful than the default get weighted higher in the distribution. No longer multiplying by 1000.


================
Comment at: test/fuzzer/fuzzer-weightedmutations.test:4
+# Weighted mutations only trigger after first 10,000 runs, hence flag.
+RUN: not %run %t-WeightedMutationsTest -use_weighted_mutations=1 -runs=100000 2>&1 | FileCheck %s
+
----------------
metzman wrote:
> Dor1s wrote:
> > metzman wrote:
> > > How long does it take to do 100k runs? Does this UnitTest take long?
> > > 
> > > Also, what behavior does this test verify? The only thing I can tell is that weighted mutations doesn't crash anything. 
> > > I think it would be nice to have a test of something more substantial if possible.
> > +1
> I think it will be hard to verify some of the behavior that I have observed is broken in this CL with an integration test. You may want to use a unittest for this purpose.
Real: 0m 0.157s
User: 0m 0.036s
Sys: 0m 0.076s


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D49621