[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