[PATCH] D73776: Entropic: Boosting LibFuzzer Performance

[Kostya Serebryany via Phabricator via llvm-commits]

kcc added a comment.

Commenting on just to issues, not the hole patch.



================
Comment at: compiler-rt/lib/fuzzer/FuzzerCorpus.h:36
   size_t NumSuccessfullMutations = 0;
+  size_t TotalFuzzTime = 0; // in microseconds
   bool MayDeleteFile = false;
----------------
this is new in the patch, is it? 
While I completely understand why we'd want to use execution time as a signal for weights, 
it makes fuzzing process non-reproducible with a given seed, which I consider pretty bad. 
If we used 32- or 64- bit edge counters we could have substituted them for time, but alas, we use 8-bit ones. 


================
Comment at: compiler-rt/lib/fuzzer/FuzzerCorpus.h:70
+  void UpdateEnergy(size_t GlobalNumberOfFeatures) {
+    long double PreciseEnergy = 0.0L;
+    SumIncidence = 0;
----------------
I'm still worried about long double due to portability. 
Do you actually "know" that it's critical to use long double here? 


================
Comment at: compiler-rt/lib/fuzzer/FuzzerLoop.cpp:683
+  struct timeval TimeVal;
+  gettimeofday(&TimeVal, NULL);
+
----------------
for consistency, please use the C++ interface for getting current time (as elsewhere in the code).
But see above about my comment on time in gneral. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73776/new/

https://reviews.llvm.org/D73776