[PATCH] D20943: [LibFuzzer] Declare and use sanitizer functions in `fuzzer::ExternalFunctions`

[Dan Liew via llvm-commits]

delcypher added inline comments.

================
Comment at: lib/Fuzzer/FuzzerExtFunctions.h:21
@@ +20,3 @@
+  // Initialize function pointers. Functions that are not available will be set
+  // to nullptr.  Do not call this constructor  before ``main()`` has been
+  // entered.
----------------
kcc wrote:
> Why you can't call this before main()? 
> Just curious.
It is probably possible to call somewhere before `main()` but on OSX when running the uninstrumented test `ExternalFunctions::ExternalFunctions` tries to use the `Printf()` function to report that a function is missing. It seems the `Printf()` call crashes (trying to access an invalid address) when it is invoked from a global initializer. The crash happens when calling `vfprintf()`

I suspect something wasn't correctly initialized when the `Printf()` was called leading to the crash. So for safety I thought it best to leave a note saying not to try calling it before entering main().

================
Comment at: lib/Fuzzer/test/FuzzerUnittest.cpp:430
@@ +429,3 @@
+  // Make sure we free to avoid LSan firing.
+  std::unique_ptr<ExternalFunctions> t(new ExternalFunctions());
+  fuzzer::EF = t.get();
----------------
kcc wrote:
> Are you sure you need to do that? 
> Have you seen an lsan report of you don't?
> 
> 
> When you do 
>   global_var = new ... 
> there is no leak from lsan POV because the memory is reachable. 
@kcc: Good catch.

Originally the implementation of `main()` was like this

```
int main(int argc, char **argv) {
  fuzzer::EF = new ExternalFunctions();
  ::testing::InitGoogleTest(&argc, argv);
  int result =  RUN_ALL_TESTS();
  fuzzer::EF = nullptr;
  return result;
}
```

In this implementation LSan will fire because I set `fuzzer::EF` to `nullptr`. Because of that I started using a `std::unique_ptr<ExternalFunctions>` but then later I simplified the `main()` function to not set `fuzzer::EF` to `nullptr`. After doing that I forgot to check if the `std::unique_ptr<ExternalFunction>` was still needed. It isn't really needed anymore.

Would you like me to remove it?


http://reviews.llvm.org/D20943