[PATCH] D20741: [LibFuzzer] Reimplement how the optional user functions are called.
Kostya Serebryany via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 1 12:44:27 PDT 2016
kcc added inline comments.
================
Comment at: lib/Fuzzer/FuzzerDriver.cpp:422
@@ +421,3 @@
+ assert(argc && argv && "Argument pointers cannot be nullptr");
+ fuzzer::ExternalFunctions EF;
+ EF.Init();
----------------
delcypher wrote:
> kcc wrote:
> > move all of this into the other FuzzerDriver.
> > This one is an atavism of my poor design choice in the past, let's not resurrect it.
> A problem with your suggestion is it makes calling the `LLVMFuzzerInitialize(int *argc, char ***argv)` function difficult because we don't have the `argv` anymore (inside `static int FuzzerDriver(const std::vector<std::string> &Args, UserCallback Callback, fuzzer::ExternalFunctions EF)` ). We can emulate `argc` using `Args.size()` but `argv` is more of pain because we have `std::vector<std::string>` not `std::vector<const char*>`.
>
> Also if the intention of `LLVMFuzzerInitialize(int *argc, char ***argv)` is to allow the command line arguments to be changed then what you're proposing doesn't work because the user needs to make the modifications before we create the `std::vector<std::string>` not after we create it.
Can you simply merge these two functions leaving just this one?
FuzzerDriver(int *argc, char ***argv, UserCallback Callback);
using std::vector there is unnecessary.
================
Comment at: lib/Fuzzer/FuzzerExtFunctions.h:26
@@ +25,3 @@
+ int (*LLVMFuzzerInitialize)(int *argc, char ***argv) = nullptr;
+ size_t (*LLVMFuzzerCustomMutator)(uint8_t *Data, size_t Size, size_t MaxSize,
+ unsigned int Seed) = nullptr;
----------------
We still have a duplication between here and FuzzerExtFunction.def
Can you use a bit more macro tricks to eliminate FuzzerExtFunction.def?
================
Comment at: lib/Fuzzer/FuzzerInternal.h:475
@@ -471,1 +474,3 @@
+ // Interface to functions that may or may not be available.
+ ExternalFunctions EF;
};
----------------
delcypher wrote:
> kcc wrote:
> > I did not suggest to pass it by value.
> > Can't it be created inside Fuzzer::Fuzzer with a default CTOR?
> >
> We can create an instance of `ExternalFunctions` inside the `Fuzzer` class in the default constructor but then we have the problem of also needing the call `LLVMFuzzerInitialize` which requires an instance of `ExternalFunctions` and that has to be done before we create the `Fuzzer` class.
>
> So either we need to make two instances of `ExternalFunctions` and initialize both of them (one inside the `Fuzzer` class and one outside) OR a single instance of `ExternalFunctions` which we initialize and pass to the `Fuzzer` class.
>
> I can do either. Decide which you'd prefer and I will implement that.
I'd prefer to have two instances of EF. That's a tiny bit slower at startup but leaves the code simpler.
http://reviews.llvm.org/D20741
More information about the llvm-commits
mailing list