[PATCH] D29751: [libFuzzer] Use dynamic loading for External Functions on Windows.

[Marcos Pividori via Phabricator via llvm-commits]

mpividori added inline comments.


================
Comment at: lib/Fuzzer/FuzzerExtFunctionsDlsymWin.cpp:45
+         Fn = GetProcAddress(Modules[i], #NAME "__dll");                       \
+      this->NAME = (decltype(ExternalFunctions::NAME)) Fn;                     \
+    }
----------------
zturner wrote:
> Should this be:
> 
> ```
> this->NAME = (decltype(ExternalFunctions::NAME)*) Fn;
> ```
> 
> ?  It looks like you're casting to a function instead of a function pointer.  (Not sure if there's a difference)
@zturner thanks for your feedback. `ExternalFunctions::NAME` are functions pointers, defined in `FuzzerExtFunctions.h` (members of the struct `ExternalFunctions`).


================
Comment at: lib/Fuzzer/FuzzerExtFunctionsDlsymWin.cpp:46
+      this->NAME = (decltype(ExternalFunctions::NAME)) Fn;                     \
+    }
+#include "FuzzerExtFunctions.def"
----------------
zturner wrote:
> What if it still can't find it?  You don't set `this->NAME` to anything, so it's uninitialized memory, but you don't indicate any kind of error.
> 
> Also, what happens if it could be found in more than one module but we choose the wrong one?  
@zturner Yes, `this->NAME` is set to `NULL`  if `GetProcAddress` can't find the function. This is ok, because these functions are optional.
If the functions is not found after considering all the modules, a warning is printed depending on the flag `WARN`. Is the same than for other platforms. For example, for Darwin, when using `dlsym`, or for linux, when considering weak symbols, if the function is not present, we set a `null` pointer.

The functions that we look for are:
 * sanitizer's functions, like: `__sanitizer_*` , `__lsan__*`, etc. Which are very unlikely to be defined in another module.
 * fuzzer's functions, like: `LLVMFuzzerInitialize`, `LLVMFuzzerCustomMutator`, etc.

Anyway, I could update the code to fail if it finds more than one reference for the same function in different modules.


https://reviews.llvm.org/D29751