[Support] Lazy load of dbghlp.dll on Windows

[lenykholodov]

Summary

This patch changes linkage with dbghlp.dll for clang from static (at load time) to on demand (at the first use of required functions). Clang uses dbghlp.dll only in minor use-cases. First of all in case of crash and in case of plugin load. The dbghlp.dll library can be absent on system. In this case clang will fail to load. With lazy load of dbghlp.dll clang can work even if dbghlp.dll is not available.

[zturner]

Why not just make it delay load dll instead of all the LoadLibrary stuff?

[lenykholodov]

In D10737#195096, @zturner wrote:

Why not just make it delay load dll instead of all the LoadLibrary stuff?

This was done for supporting mingw distributions without 64-bit versions of the DebugHlp APIs (please, see lib/Support/Windows/Signals.inc line 49 and below). Existing code base has already implemented dll loading part for some cases so I have prepared the patch as close to current implementation as possible.

[zturner]

I don't really know much about MinGW, and i also am not in a position to be
able to look at any code right now as I'm technically OOO for another 3
weeks. That said, if it is possible to make this work as a delayload dll,
it would be much preferred, even if it strays from some existing
implementation that does something different. With MSVC, this is as simple
as adding a linker flag, and you don't have to change even 1 line of code.
I'm not sure if MinGW toolchain supports this, but please check first. I'll
leave the final lgtm to someone else on this review.

[rnk]

Can you remove the configure and cmake test for libimagehlp along with this? Don't worry about regenerating autoconf, I can get Eric to do that.

[lenykholodov]

In D10737#195406, @zturner wrote:

I don't really know much about MinGW, and i also am not in a position to be
able to look at any code right now as I'm technically OOO for another 3
weeks. That said, if it is possible to make this work as a delayload dll,
it would be much preferred, even if it strays from some existing
implementation that does something different. With MSVC, this is as simple
as adding a linker flag, and you don't have to change even 1 line of code.
I'm not sure if MinGW toolchain supports this, but please check first. I'll
leave the final lgtm to someone else on this review.

I have checked MinGW possibilities and have not found easy way for delay loading. So at this moment I see following aproaches for this issue:

1. Leave manual loading for MinGW in the code but use MSVC -delayload linker flag. The main drawback of this approach is two different ways for implementing lazy loading of imghelp.dll.
2. Use dlltool (https://sourceware.org/binutils/docs/binutils/dlltool.html) for MinGW to generate imghelp.a file with lazy loading and simple -delayload linker flag for MSVC. This approach removes manual loading code from the patch, but adds configuration code for launching dlltool and most probably def file. Moreover, some of MinGW implementations don't provide --output-delaylib parameter (for example, 4.3 and lower). So building of LLVM may be impossible for this versions.
3. Use manual loading for both compilers. This approach has evident benefit with same implementation for both compilers (msvc, mingw) and without using of extra tools.

[lenykholodov]

libimagehlp was removed from configure and cmake files.

[lenykholodov]

The additional issue with automatic DLL delay loading is error handling. By default, application will fail in case of DLL absense, if there are no hooks or SEH handlers (https://msdn.microsoft.com/en-us/library/3aeywt27.aspx). So for automatic delay-loaded DLLs we have to write such hooks/handlers. Manual loading aproach doesn't have such drawback.

[rnk]

I'm fine with the dynamic approach over the delayload approach. We use this DLL so infrequently that it's fine.

configure

8661 ↗

(On Diff #28798)

configure is a generated file. You want to change llvm/autoconf/configure.ac, and let Eric run autoconf to update this file.

[lenykholodov]

• configure changes were removed;
• autoconf/configure.ac tesing of imghelp was removed.

[rnk]

Thanks, lgtm!

llvm/trunk/autoconf/configure.ac

 dnl===-----------------------------------------------------------------------===
 dnl===
 dnl=== SECTION 5: Check for libraries
 dnl===
 dnl===-----------------------------------------------------------------------===
 
 AC_CHECK_LIB(m,sin)
 if test "$llvm_cv_os_type" = "MingW" ; then
-  AC_CHECK_LIB(imagehlp, main)
   AC_CHECK_LIB(ole32, main)
   AC_CHECK_LIB(psapi, main)
   AC_CHECK_LIB(shell32, main)
 fi
 
 dnl dlopen() is required for plugin support.
 AC_SEARCH_LIBS(dlopen,dl,LLVM_DEFINE_SUBST([HAVE_DLOPEN],[1],
                [Define if dlopen() is available on this platform.]),

llvm/trunk/cmake/config-ix.cmake

else ()
 
   # We don't have an disassembler for all architectures yet.
   if (EXISTS ${CMAKE_SOURCE_DIR}/lib/Target/${LLVM_NATIVE_ARCH}/Disassembler/CMakeLists.txt)
     set(LLVM_NATIVE_DISASSEMBLER LLVMInitialize${LLVM_NATIVE_ARCH}Disassembler)
   endif ()
 endif ()
 
 if( MINGW )
-  set(HAVE_LIBIMAGEHLP 1)
   set(HAVE_LIBPSAPI 1)
   set(HAVE_LIBSHELL32 1)
   # TODO: Check existence of libraries.
   #   include(CheckLibraryExists)
-  #   CHECK_LIBRARY_EXISTS(imagehlp ??? . HAVE_LIBIMAGEHLP)
 endif( MINGW )
 
 if (NOT HAVE_STRTOLL)
   # Use _strtoi64 if strtoll is not available.
   check_symbol_exists(_strtoi64 stdlib.h have_strtoi64)
   if (have_strtoi64)
     set(HAVE_STRTOLL 1)
     set(strtoll "_strtoi64")

llvm/trunk/include/llvm/Config/config.h.cmake

 #cmakedefine HAVE_INTTYPES_H ${HAVE_INTTYPES_H}
 
 /* Define to 1 if you have the `isatty' function. */
 #cmakedefine HAVE_ISATTY 1
 
 /* Define if you have the libdl library or equivalent. */
 #cmakedefine HAVE_LIBDL ${HAVE_LIBDL}
 
-/* Define to 1 if you have the `imagehlp' library (-limagehlp). */
-#cmakedefine HAVE_LIBIMAGEHLP ${HAVE_LIBIMAGEHLP}
-
 /* Define to 1 if you have the `m' library (-lm). */
 #undef HAVE_LIBM
 
 /* Define to 1 if you have the `ole32' library (-lole32). */
 #undef HAVE_LIBOLE32
 
 /* Define to 1 if you have the `psapi' library (-lpsapi). */
 #cmakedefine HAVE_LIBPSAPI ${HAVE_LIBPSAPI}

llvm/trunk/include/llvm/Config/config.h.in

 #undef HAVE_INTTYPES_H
 
 /* Define to 1 if you have the `isatty' function. */
 #undef HAVE_ISATTY
 
 /* Define if libedit is available on this platform. */
 #undef HAVE_LIBEDIT
 
-/* Define to 1 if you have the `imagehlp' library (-limagehlp). */
-#undef HAVE_LIBIMAGEHLP
-
 /* Define to 1 if you have the `m' library (-lm). */
 #undef HAVE_LIBM
 
 /* Define to 1 if you have the `ole32' library (-lole32). */
 #undef HAVE_LIBOLE32
 
 /* Define to 1 if you have the `psapi' library (-lpsapi). */
 #undef HAVE_LIBPSAPI

llvm/trunk/lib/Support/CMakeLists.txt

 set(system_libs)
 if( NOT MSVC )
   if( MINGW )
-    set(system_libs ${system_libs} imagehlp psapi shell32 ole32)
+    set(system_libs ${system_libs} psapi shell32 ole32)
   elseif( CMAKE_HOST_UNIX )
     if( HAVE_LIBRT )
       set(system_libs ${system_libs} rt)
     endif()
     if( HAVE_LIBDL )
       set(system_libs ${system_libs} ${CMAKE_DL_LIBS})
     endif()
     if(LLVM_ENABLE_TERMINFO)

llvm/trunk/lib/Support/Windows/DynamicLibrary.inc

 #else
  #include <dbghelp.h>
 #endif
 
 #ifdef _MSC_VER
  #include <ntverp.h>
 #endif
 
-#ifdef __MINGW32__
- #if (HAVE_LIBIMAGEHLP != 1)
-  #error "libimagehlp.a should be present"
- #endif
-#else
- #pragma comment(lib, "dbghelp.lib")
-#endif
-
 namespace llvm {
 using namespace sys;
 
 //===----------------------------------------------------------------------===//
 //=== WARNING: Implementation here must contain only Win32 specific code
 //===          and must not be UNIX code.
 //===----------------------------------------------------------------------===//
 
+typedef BOOL (WINAPI *fpEnumerateLoadedModules)(HANDLE,PENUMLOADED_MODULES_CALLBACK64,PVOID);
+static fpEnumerateLoadedModules fEnumerateLoadedModules;
 static DenseSet<HMODULE> *OpenedHandles;
 
+static bool loadDebugHelp(void) {
+  HMODULE hLib = ::LoadLibraryW(L"Dbghelp.dll");
+  if (hLib) {
+    fEnumerateLoadedModules = (fpEnumerateLoadedModules)
+      ::GetProcAddress(hLib, "EnumerateLoadedModules64");
+  }
+  return fEnumerateLoadedModules != 0;
+}
+
 static BOOL CALLBACK
-ELM_Callback(WIN32_ELMCB_PCSTR ModuleName, ULONG_PTR ModuleBase,
+ELM_Callback(WIN32_ELMCB_PCSTR ModuleName, DWORD64 ModuleBase,
              ULONG ModuleSize, PVOID UserContext) {
   OpenedHandles->insert((HMODULE)ModuleBase);
   return TRUE;
 }
 
 DynamicLibrary DynamicLibrary::getPermanentLibrary(const char *filename,
                                                    std::string *errMsg) {
   SmartScopedLock<true> lock(*SymbolsMutex);
 
   if (!filename) {
     // When no file is specified, enumerate all DLLs and EXEs in the process.
     if (OpenedHandles == 0)
       OpenedHandles = new DenseSet<HMODULE>();
 
-    EnumerateLoadedModules(GetCurrentProcess(), ELM_Callback, 0);
+    if (!fEnumerateLoadedModules) {
+      if (!loadDebugHelp()) {
+        assert(false && "These APIs should always be available");
+        return DynamicLibrary();
+      }
+    }
+
+    fEnumerateLoadedModules(GetCurrentProcess(), ELM_Callback, 0);
     // Dummy library that represents "search all handles".
     // This is mostly to ensure that the return value still shows up as "valid".
     return DynamicLibrary(&OpenedHandles);
   }
 
   SmallVector<wchar_t, MAX_PATH> filenameUnicode;
   if (std::error_code ec = windows::UTF8ToUTF16(filename, filenameUnicode)) {
     SetLastError(ec.value());

llvm/trunk/lib/Support/Windows/Signals.inc

  #include <imagehlp.h>
 #else
  #include <dbghelp.h>
 #endif
 #include <psapi.h>
 
 #ifdef _MSC_VER
  #pragma comment(lib, "psapi.lib")
- #pragma comment(lib, "dbghelp.lib")
 #elif __MINGW32__
- #if ((HAVE_LIBIMAGEHLP != 1) || (HAVE_LIBPSAPI != 1))
-  #error "libimagehlp.a & libpsapi.a should be present"
+ #if (HAVE_LIBPSAPI != 1)
+  #error "libpsapi.a should be present"
  #endif
  // The version of g++ that comes with MinGW does *not* properly understand
  // the ll format specifier for printf. However, MinGW passes the format
  // specifiers on to the MSVCRT entirely, and the CRT understands the ll
  // specifier. So these warnings are spurious in this case. Since we compile
  // with -Wall, this will generate these warnings which should be ignored. So
  // we will turn off the warnings for this just file. However, MinGW also does
  // not support push and pop for diagnostics, so we have to manually turn it
typedef struct _tagSTACKFRAME64 {
    ADDRESS64   AddrBStore;
    PVOID       FuncTableEntry;
    DWORD64     Params[4];
    BOOL        Far;
    BOOL        Virtual;
    DWORD64     Reserved[3];
    KDHELP64    KdHelp;
  } STACKFRAME64, *LPSTACKFRAME64;
+ #endif // !defined(__MINGW64_VERSION_MAJOR)
+#endif // __MINGW32__
 
 typedef BOOL (__stdcall *PREAD_PROCESS_MEMORY_ROUTINE64)(HANDLE hProcess,
                       DWORD64 qwBaseAddress, PVOID lpBuffer, DWORD nSize,
                       LPDWORD lpNumberOfBytesRead);
 
 typedef PVOID (__stdcall *PFUNCTION_TABLE_ACCESS_ROUTINE64)( HANDLE ahProcess,
                       DWORD64 AddrBase);
 
 typedef DWORD64 (__stdcall *PGET_MODULE_BASE_ROUTINE64)(HANDLE hProcess,
                       DWORD64 Address);
 
 typedef DWORD64 (__stdcall *PTRANSLATE_ADDRESS_ROUTINE64)(HANDLE hProcess,
                       HANDLE hThread, LPADDRESS64 lpaddr);
 
 typedef BOOL (WINAPI *fpStackWalk64)(DWORD, HANDLE, HANDLE, LPSTACKFRAME64,
                       PVOID, PREAD_PROCESS_MEMORY_ROUTINE64,
                       PFUNCTION_TABLE_ACCESS_ROUTINE64,
                       PGET_MODULE_BASE_ROUTINE64,
                       PTRANSLATE_ADDRESS_ROUTINE64);
-static fpStackWalk64 StackWalk64;
+static fpStackWalk64 fStackWalk64;
 
 typedef DWORD64 (WINAPI *fpSymGetModuleBase64)(HANDLE, DWORD64);
-static fpSymGetModuleBase64 SymGetModuleBase64;
+static fpSymGetModuleBase64 fSymGetModuleBase64;
 
 typedef BOOL (WINAPI *fpSymGetSymFromAddr64)(HANDLE, DWORD64,
                       PDWORD64, PIMAGEHLP_SYMBOL64);
-static fpSymGetSymFromAddr64 SymGetSymFromAddr64;
+static fpSymGetSymFromAddr64 fSymGetSymFromAddr64;
 
 typedef BOOL (WINAPI *fpSymGetLineFromAddr64)(HANDLE, DWORD64,
                       PDWORD, PIMAGEHLP_LINE64);
-static fpSymGetLineFromAddr64 SymGetLineFromAddr64;
+static fpSymGetLineFromAddr64 fSymGetLineFromAddr64;
 
 typedef PVOID (WINAPI *fpSymFunctionTableAccess64)(HANDLE, DWORD64);
-static fpSymFunctionTableAccess64 SymFunctionTableAccess64;
+static fpSymFunctionTableAccess64 fSymFunctionTableAccess64;
+
+typedef DWORD (WINAPI *fpSymSetOptions)(DWORD);
+static fpSymSetOptions fSymSetOptions;
+
+typedef BOOL (WINAPI *fpSymInitialize)(HANDLE, PCSTR, BOOL);
+static fpSymInitialize fSymInitialize;
 
 static bool load64BitDebugHelp(void) {
   HMODULE hLib = ::LoadLibraryW(L"Dbghelp.dll");
   if (hLib) {
-    StackWalk64 = (fpStackWalk64)
+    fStackWalk64 = (fpStackWalk64)
                       ::GetProcAddress(hLib, "StackWalk64");
-    SymGetModuleBase64 = (fpSymGetModuleBase64)
+    fSymGetModuleBase64 = (fpSymGetModuleBase64)
                       ::GetProcAddress(hLib, "SymGetModuleBase64");
-    SymGetSymFromAddr64 = (fpSymGetSymFromAddr64)
+    fSymGetSymFromAddr64 = (fpSymGetSymFromAddr64)
                       ::GetProcAddress(hLib, "SymGetSymFromAddr64");
-    SymGetLineFromAddr64 = (fpSymGetLineFromAddr64)
+    fSymGetLineFromAddr64 = (fpSymGetLineFromAddr64)
                       ::GetProcAddress(hLib, "SymGetLineFromAddr64");
-    SymFunctionTableAccess64 = (fpSymFunctionTableAccess64)
+    fSymFunctionTableAccess64 = (fpSymFunctionTableAccess64)
                      ::GetProcAddress(hLib, "SymFunctionTableAccess64");
+    fSymSetOptions = (fpSymSetOptions)::GetProcAddress(hLib, "SymSetOptions");
+    fSymInitialize = (fpSymInitialize)::GetProcAddress(hLib, "SymInitialize");
   }
-  return StackWalk64 != NULL;
+  return fStackWalk64 && fSymInitialize && fSymSetOptions;
 }
- #endif // !defined(__MINGW64_VERSION_MAJOR)
-#endif // __MINGW32__
 
 // Forward declare.
 static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep);
 static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType);
 
 // InterruptFunction - The function to call if ctrl-c is pressed.
 static void (*InterruptFunction)() = 0;
 
static void PrintStackTraceForThread(llvm::raw_ostream &OS, HANDLE hProcess,
   DWORD machineType;
 #if defined(_M_X64)
   machineType = IMAGE_FILE_MACHINE_AMD64;
 #else
   machineType = IMAGE_FILE_MACHINE_I386;
 #endif
 
   // Initialize the symbol handler.
-  SymSetOptions(SYMOPT_DEFERRED_LOADS | SYMOPT_LOAD_LINES);
-  SymInitialize(hProcess, NULL, TRUE);
+  fSymSetOptions(SYMOPT_DEFERRED_LOADS | SYMOPT_LOAD_LINES);
+  fSymInitialize(hProcess, NULL, TRUE);
 
   while (true) {
-    if (!StackWalk64(machineType, hProcess, hThread, &StackFrame, Context, NULL,
-                     SymFunctionTableAccess64, SymGetModuleBase64, NULL)) {
+    if (!fStackWalk64(machineType, hProcess, hThread, &StackFrame, Context, 0,
+                      fSymFunctionTableAccess64, fSymGetModuleBase64, 0)) {
       break;
     }
 
     if (StackFrame.AddrFrame.Offset == 0)
       break;
 
     using namespace llvm;
     // Print the PC in hexadecimal.

 #elif defined(_M_IX86)
     OS << format(" (0x%08lX 0x%08lX 0x%08lX 0x%08lX)",
             static_cast<DWORD>(StackFrame.Params[0]),
             static_cast<DWORD>(StackFrame.Params[1]),
             static_cast<DWORD>(StackFrame.Params[2]),
             static_cast<DWORD>(StackFrame.Params[3]));
 #endif
     // Verify the PC belongs to a module in this process.
-    if (!SymGetModuleBase64(hProcess, PC)) {
+    if (!fSymGetModuleBase64(hProcess, PC)) {
       OS << " <unknown module>\n";
       continue;
     }
 
     // Print the symbol name.
     char buffer[512];
     IMAGEHLP_SYMBOL64 *symbol = reinterpret_cast<IMAGEHLP_SYMBOL64 *>(buffer);
     memset(symbol, 0, sizeof(IMAGEHLP_SYMBOL64));
     symbol->SizeOfStruct = sizeof(IMAGEHLP_SYMBOL64);
     symbol->MaxNameLength = 512 - sizeof(IMAGEHLP_SYMBOL64);
 
     DWORD64 dwDisp;
-    if (!SymGetSymFromAddr64(hProcess, PC, &dwDisp, symbol)) {
+    if (!fSymGetSymFromAddr64(hProcess, PC, &dwDisp, symbol)) {
       OS << '\n';
       continue;
     }
 
     buffer[511] = 0;
     if (dwDisp > 0)
       OS << format(", %s() + 0x%llX bytes(s)", (const char*)symbol->Name,
                    dwDisp);
     else
       OS << format(", %s", (const char*)symbol->Name);
 
     // Print the source file and line number information.
     IMAGEHLP_LINE64 line = {};
     DWORD dwLineDisp;
     line.SizeOfStruct = sizeof(line);
-    if (SymGetLineFromAddr64(hProcess, PC, &dwLineDisp, &line)) {
+    if (fSymGetLineFromAddr64(hProcess, PC, &dwLineDisp, &line)) {
       OS << format(", %s, line %lu", line.FileName, line.LineNumber);
       if (dwLineDisp > 0)
         OS << format(" + 0x%lX byte(s)", dwLineDisp);
     }
 
     OS << '\n';
   }
 }
static void InitializeThreading() {
 
   // Now's the time to create the critical section. This is the first time
   // through here, and there's only one thread.
   InitializeCriticalSection(&CriticalSection);
   CriticalSectionInitialized = true;
 }
 
 static void RegisterHandler() {
-#if __MINGW32__ && !defined(__MINGW64_VERSION_MAJOR)
-  // On MinGW.org, we need to load up the symbols explicitly, because the
-  // Win32 framework they include does not have support for the 64-bit
-  // versions of the APIs we need.  If we cannot load up the APIs (which
-  // would be unexpected as they should exist on every version of Windows
-  // we support), we will bail out since there would be nothing to report.
+  // If we cannot load up the APIs (which would be unexpected as they should
+  // exist on every version of Windows we support), we will bail out since
+  // there would be nothing to report.
   if (!load64BitDebugHelp()) {
     assert(false && "These APIs should always be available");
     return;
   }
-#endif
 
   if (RegisteredUnhandledExceptionFilter) {
     EnterCriticalSection(&CriticalSection);
     return;
   }
 
   InitializeThreading();
 

llvm/trunk/tools/llvm-symbolizer/LLVMSymbolize.cpp

 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include <sstream>
 #include <stdlib.h>
 
 #if defined(_MSC_VER)
 #include <Windows.h>
 #include <DbgHelp.h>
+#pragma comment(lib, "dbghelp.lib")
 #endif
 
 namespace llvm {
 namespace symbolize {
 
 static bool error(std::error_code ec) {
   if (!ec)
     return false;