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

[mpividori]

Summary

I replace the weak aliases with dynamic loading. Weak aliases were generating some problems when linking for MT on Windows. For MT, compiler-rt's libraries are statically linked to the main executable the same than libFuzzer, so if we use weak aliases, we are providing two different default functions for the same weak functions and the linker fails.

So, I decided to re implement ExternalFunctions() using dynamic loading, so it works in both cases (MD and MT).
Also, I think dynamic loading is clearer, since we are not defining any auxiliary external function, and we don't need to deal with weak aliases.

It is equivalent to the implementation using dlsym(RTLD_DEFAULT, FnName) for Posix.

[rnk]

Are the .def files missing?

[mpividori]

@rnk We don't need .def files, because we use dllexport.

[rnk]

In D29751#672292, @mpividori wrote:

@rnk We don't need .def files, because we use dllexport.

Ignore me, FuzzerExtFunctions.def is already in the repo. I thought it was a new thing.

[mpividori]

@zturner @rnk Ok. This is the final diff necessary to support MT. What do you think?

[zturner]

lib/Fuzzer/FuzzerExtFunctionsDlsymWin.cpp

45 ↗

(On Diff #87769)

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)

46 ↗

(On Diff #87769)

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?

[amccarth]

Looks right to me, but I'll leave it to kcc or zturner to accept.

lib/Fuzzer/FuzzerExtFunctionsDlsymWin.cpp

48 ↗

(On Diff #87769)

I thought the LLVM pattern was to have the #undef in the .def file itself.

That's just a style question. This code is correct.

[mpividori]

lib/Fuzzer/FuzzerExtFunctionsDlsymWin.cpp

45 ↗

(On Diff #87769)

@zturner thanks for your feedback. ExternalFunctions::NAME are functions pointers, defined in FuzzerExtFunctions.h (members of the struct ExternalFunctions).

46 ↗

(On Diff #87769)

@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.

[zturner]

lib/Fuzzer/FuzzerExtFunctionsDlsymWin.cpp

46 ↗

(On Diff #87769)

No, because that would require searching a potentially huge module list even if you find it on the first one. We should optimize for the normal case, not the exceptional case. I don't know what the best solution is, just wanted to make sure you think about the possibility that the function exists in multiple modules.

[mpividori]

lib/Fuzzer/FuzzerExtFunctionsDlsymWin.cpp

46 ↗

(On Diff #87769)

@zturner yes, exactly. Is the same than for dlsym(RTLD_DEFAULT, FnName),:
``
"... There are two special pseudo-handles, RTLD_DEFAULT and RTLD_NEXT. The former will find the first occurrence of the desired symbol using the default library search order. ..."
``
So, it finds the first occurrence.

llvm/trunk/lib/Fuzzer/CMakeLists.txt

 set(LIBFUZZER_FLAGS_BASE "${CMAKE_CXX_FLAGS}")
 # Disable the coverage and sanitizer instrumentation for the fuzzer itself.
 set(CMAKE_CXX_FLAGS "${LIBFUZZER_FLAGS_BASE} -fno-sanitize-coverage=trace-pc-guard,edge,trace-cmp,indirect-calls,8bit-counters -Werror")
 if( LLVM_USE_SANITIZE_COVERAGE )
   if(NOT "${LLVM_USE_SANITIZER}" STREQUAL "Address")
     message(FATAL_ERROR
       "LibFuzzer and its tests require LLVM_USE_SANITIZER=Address and "
       "LLVM_USE_SANITIZE_COVERAGE=YES to be set."
       )
   endif()
   add_library(LLVMFuzzerNoMainObjects OBJECT
     FuzzerCrossOver.cpp
     FuzzerDriver.cpp
     FuzzerExtFunctionsDlsym.cpp
+    FuzzerExtFunctionsDlsymWin.cpp
     FuzzerExtFunctionsWeak.cpp
-    FuzzerExtFunctionsWeakAlias.cpp
     FuzzerIO.cpp
     FuzzerIOPosix.cpp
     FuzzerIOWindows.cpp
     FuzzerLoop.cpp
     FuzzerMerge.cpp
     FuzzerMutate.cpp
     FuzzerSHA1.cpp
     FuzzerShmemPosix.cpp

llvm/trunk/lib/Fuzzer/FuzzerExtFunctionsDlsymWin.cpp

+//===- FuzzerExtFunctionsDlsymWin.cpp - Interface to external functions ---===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// Implementation using dynamic loading for Windows.
+//===----------------------------------------------------------------------===//
+#include "FuzzerDefs.h"
+#if LIBFUZZER_WINDOWS
+
+#include "FuzzerExtFunctions.h"
+#include "FuzzerIO.h"
+#include "Windows.h"
+#include "Psapi.h"
+
+namespace fuzzer {
+
+ExternalFunctions::ExternalFunctions() {
+  HMODULE Modules[1024];
+  DWORD BytesNeeded;
+  HANDLE CurrentProcess = GetCurrentProcess();
+
+  if (!EnumProcessModules(CurrentProcess, Modules, sizeof(Modules),
+                          &BytesNeeded)) {
+    Printf("EnumProcessModules failed (error: %d).\n", GetLastError());
+    exit(1);
+  }
+
+  if (sizeof(Modules) < BytesNeeded) {
+    Printf("Error: the array is not big enough to hold all loaded modules.\n");
+    exit(1);
+  }
+
+  for (size_t i = 0; i < (BytesNeeded / sizeof(HMODULE)); i++)
+  {
+    FARPROC Fn;
+#define EXT_FUNC(NAME, RETURN_TYPE, FUNC_SIG, WARN)                            \
+    if (this->NAME == nullptr) {                                               \
+      Fn = GetProcAddress(Modules[i], #NAME);                                  \
+      if (Fn == nullptr)                                                       \
+         Fn = GetProcAddress(Modules[i], #NAME "__dll");                       \
+      this->NAME = (decltype(ExternalFunctions::NAME)) Fn;                     \
+    }
+#include "FuzzerExtFunctions.def"
+#undef EXT_FUNC
+  }
+
+#define EXT_FUNC(NAME, RETURN_TYPE, FUNC_SIG, WARN)                            \
+  if (this->NAME == nullptr && WARN)                                           \
+    Printf("WARNING: Failed to find function \"%s\".\n", #NAME);
+#include "FuzzerExtFunctions.def"
+#undef EXT_FUNC
+}
+
+} // namespace fuzzer
+
+#endif // LIBFUZZER_WINDOWS