[PATCH] Add a Fuzzer library

David Blaikie dblaikie at gmail.com
Mon Jan 26 15:59:51 PST 2015


It'd probably be good to have the build infrastructure go in with the source - having non-building/non-testing source in tree isn't terribly useful.


================
Comment at: lib/Fuzzer/FuzzerIO.cpp:16
@@ +15,3 @@
+
+std::vector<std::string> ListFilesInDir(const std::string &Dir) {
+  std::vector<std::string> V;
----------------
Dir could be StringRef here, maybe?

================
Comment at: lib/Fuzzer/FuzzerIO.cpp:34
@@ +33,3 @@
+
+void WriteToFile(const Unit &U, std::string Path) {
+  std::ofstream OF(Path);
----------------
Path should be passed by const ref here?

================
Comment at: lib/Fuzzer/FuzzerInternal.h:38
@@ +37,3 @@
+  struct FuzzingOptions {
+    int Verbosity = 1;
+    int MaxLen = 0;
----------------
I suspect MSVC 2012 (2013? whatever our min support is) doesn't support non-static data member initializers.

================
Comment at: lib/Fuzzer/FuzzerInternal.h:52
@@ +51,3 @@
+  size_t CorpusSize() const { return Corpus.size(); }
+  void ReadDir(const std::string &Path) {
+    ReadDirToVectorOfUnits(Path.c_str(), &Corpus);
----------------
StringPiece

================
Comment at: lib/Fuzzer/FuzzerInternal.h:62
@@ +61,3 @@
+  void WriteToOutputCorpus(const Unit &U);
+  static void WriteToCrash(const Unit &U, const char *Prefix);
+
----------------
StringPiece? Maybe?

================
Comment at: lib/Fuzzer/FuzzerLoop.cpp:113
@@ +112,3 @@
+  size_t NewUnits = 0;
+  for (size_t Iter = 0; Iter < Options.MutateDepth; Iter++) {
+    Mutate(U, Options.MaxLen);
----------------
idiomatic != rather than <? And prefix increment rather than post increment?

(& maybe use a name like 'i' which I think is idiomatic LLVM for index variables - since this is an index, not an iterator)

(similar comments on lots of other loops)

================
Comment at: lib/Fuzzer/FuzzerMain.cpp:25
@@ +24,3 @@
+struct FlagDescription {
+  const char *Name;
+  const char *Description;
----------------
StringRef (for both of these)?

================
Comment at: lib/Fuzzer/FuzzerMain.cpp:60
@@ +59,3 @@
+    std::cerr << "  " << D.Name;
+    for (size_t i = 0; i < MaxFlagLen - strlen(D.Name); i++)
+      std::cerr << " ";
----------------
Compute the bound once rather than on every iteration?

================
Comment at: lib/Fuzzer/FuzzerMutate.cpp:34
@@ +33,3 @@
+
+void Mutate(Unit *U, size_t MaxLen) {
+  assert(MaxLen > 0);
----------------
pass U by reference?

================
Comment at: lib/Fuzzer/FuzzerMutate.cpp:38
@@ +37,3 @@
+  switch (rand() % 3) {
+    case 0:
+      if (U->size()) U->erase(U->begin() + rand() % U->size());
----------------
Is this clang-format/LLVM's formatting for case? I would've expected the 'case' to be at the same level as the 'switch'.

================
Comment at: lib/Fuzzer/FuzzerUtil.cpp:42
@@ +41,3 @@
+  }
+  return std::to_string(h1) + std::to_string(h2);
+}
----------------
Could be marginally more efficient to use an ostringstream here to avoid two allocations? (Well, I guess they'll be short strings, etc... - so maybe not a problem)

http://reviews.llvm.org/D7184

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list