[PATCH] D26956: Initial changes for porting libFuzzer to Windows.

[Reid Kleckner via llvm-commits]

rnk added inline comments.


================
Comment at: lib/Fuzzer/CMakeLists.txt:12
   add_library(LLVMFuzzerNoMainObjects OBJECT
     FuzzerCrossOver.cpp
     FuzzerDriver.cpp
----------------
zturner wrote:
> I mentioned below that we should not have the preprocessor definitions use to conditionally compile code or not, but instead use CMake.  Here is how you would do it.  You might want to skip thsi comment for now and come back to it after you read the rest of the comments.
> 
> ```
> set(FUZZER_SOURCES
>     FuzzerCrossOver.cpp
>     FuzzerDriver.cpp
>     FuzzerIO.cpp
>     FuzzerLoop.cpp
>     FuzzerMutate.cpp
>     FuzzerSHA1.cpp
>     FuzzerTracePC.cpp
>     FuzzerTraceState.cpp
>     FuzzerUtil.cpp
> )
> 
> if (CMAKE_SYSTEM_NAME MATCHES "Windows")
>     list(APPEND FUZZER_SOURCES
>         FuzzerUtilWindows.cpp
>         FuzzerIOWindows.cpp
>         FuzzerExtFunctionsWindows.cpp
>     )    
> else()
>     list(APPEND FUZZER_SOURCES
>         FuzzerUtilPosix.cpp
>         FuzzerIOPosix.cpp
>     )
> 
>     if (CMAKE_SYSTEM_NAME MATCHES "Linux")
>         FuzzerUtilLinux.cpp
>         FuzzerExtFunctionsLinux.cpp
>     elseif(CMAKE_SYSTEM_NAME MATCHES "Darwin")
>         FuzzerUtilDarwin.cpp
>         FuzzerExtFunctionsApple.cpp
>     endif()
> endif()
> 
> add_library(LLVMFuzzerNoMainObjects OBJECT ${FUZZER_SOURCES})
> ```
> 
> This way you can remove all the `#if <foo>` from the individual source files.
FWIW, the sanitizer code takes the opposite approach, specifically to simplify build logic.


================
Comment at: lib/Fuzzer/FuzzerUtilWindows.cpp:32
+LONG WINAPI SEGVHandler(PEXCEPTION_POINTERS ExceptionInfo) {
+  switch (ExceptionInfo->ExceptionRecord->ExceptionCode) {
+    case EXCEPTION_ACCESS_VIOLATION:
----------------
I think it would be better to draw this interface boundary at a higher level. Something where we pass in the handle_* flags, install a single exception handler, have one switch, and let it dispatch based on whether it's supposed to catch that type of exception. Then the cross-platform API would be something like InitializeSignalHandlers().

This would require hosting the Flags code out of FuzzerDriver.cpp, so we should ask @kcc about it.


================
Comment at: lib/Fuzzer/FuzzerUtilWindows.cpp:148
+void SetSigTermHandler() {
+  if (!SetConsoleCtrlHandler(TERMHandler, TRUE)) {
+    DWORD LastError = GetLastError();
----------------
For example, this handler is completely redundant with the SigInt handler. Only one will actually be called.


Repository:
  rL LLVM

https://reviews.llvm.org/D26956