[PATCH] D85928: [libFuzzer] Added -print_full_coverage flag.

[Matt Morehouse via Phabricator via llvm-commits]

morehouse added inline comments.


================
Comment at: compiler-rt/lib/fuzzer/FuzzerDriver.cpp:327
+  } else {
+    F->TryDetectingAMemoryLeak(U.data(), U.size(), true);
+  }
----------------
Why does the flag disable leak detection?  We should either leave it enabled, or explain why it is disabled in the flag description.


================
Comment at: compiler-rt/lib/fuzzer/FuzzerLoop.cpp:357-360
+  if (Options.PrintFullCoverage)
+    TPC.PrintCoverage(true);
   if (Options.PrintCoverage)
+    TPC.PrintCoverage(false);
----------------



================
Comment at: compiler-rt/lib/fuzzer/FuzzerTracePC.cpp:300
+      } else {
+        CoveredPCs.push_back(TE->PC);
+      }
----------------
Nit: please remove curly braces


================
Comment at: compiler-rt/lib/fuzzer/FuzzerTracePC.cpp:304
+    if (PrintAllCounters) {
+      if (UncoveredPCs.size() > 0) {
+        Printf("U");
----------------
Do we need to exclude the U and C when the vectors are empty?  It's slightly cleaner to exclude these if-statements.


================
Comment at: compiler-rt/lib/fuzzer/FuzzerTracePC.h:97
 
-  void PrintCoverage();
+  void PrintCoverage(const bool PrintAllCounters);
 
----------------
Remove const, it is pointless for non-pointers.


================
Comment at: compiler-rt/test/fuzzer/full-coverage.test:4
+XFAIL: s390x
+RUN: %cpp_compiler -mllvm -use-unknown-locations=Disable %S/DSO1.cpp -fPIC %ld_flags_rpath_so1 -O0 -shared -o %dynamiclib1
+RUN: %cpp_compiler -mllvm -use-unknown-locations=Disable %S/DSO2.cpp -fPIC %ld_flags_rpath_so2 -O0 -shared -o %dynamiclib2
----------------
What does `-mllvm -use-unknown-locations=Disable` do, and why do we need it?


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

https://reviews.llvm.org/D85928