[asan/win] Delay load dbghelp.dll to delay ucrtbase.dll initialization

[rnk]

Summary

ASan needs to initialize before ucrtbase.dll so that it can intercept all of
its heap allocations. New versions of dbghelp.dll depend on ucrtbase.dll, which
means both of those DLLs will initialize before the dynamic ASan runtime. With
delayloading, we delay the ucrtbase initialization and can intercept its
allocations.

Eventually, I would like to remove our dbghelp.dll dependency in favor of
always using llvm-symbolizer.exe, but this seems like an acceptable interim
solution.

Fixes PR30903

[kubamracek]

I don't think adding those huge binary files into the repository is a good idea. Can the test use the DLLs from a system location?

[rnk]

In D26473#591156, @kubabrecka wrote:

I don't think adding those huge binary files into the repository is a good idea. Can the test use the DLLs from a system location?

No, my system copy of dbghelp is not new enough to reproduce the issue. All tests fail reliably on systems with a new enough version of dbghelp. I can delete the test if you think 1MB of data is too much cost to pay for this, but we open ourselves up to regressions, as the bots probably don't have "new" dbghelp.

[rnk]

Also, this isn't sufficient to fix the issue in static builds. I'm rewriting this to use LoadLibrary / GetProcAddress.

[rnk]

• Use LoadLibrary instead of /delayload:dbghelp.dll

[rnk]

• Remove stray header

[kubamracek]

Ok, now that you've removed the dependency on dbghelp.dll, is the test (and the binary files) still needed? I mean, now, it doesn't matter what dbghelp.dll depends on.

[rnk]

In D26473#591256, @kubabrecka wrote:

Ok, now that you've removed the dependency on dbghelp.dll, is the test (and the binary files) still needed? I mean, now, it doesn't matter what dbghelp.dll depends on.

I don't really see how the code change affects the usefulness of the test. You seem strongly opposed to having a few test binaries in the repo, though. Maybe a better test would be to ensure that the dynamic asan runtime depends only on a whitelist of DLLs, such as kernel32 et al.

[kubamracek]

In D26473#592220, @rnk wrote:

Maybe a better test would be to ensure that the dynamic asan runtime depends only on a whitelist of DLLs, such as kernel32 et al.

That does sounds like a better test.

[aaron.ballman]

Modulo @kubabrecka 's comments, LGTM

lib/sanitizer_common/sanitizer_dbghelp.h

28 ↗

(On Diff #77420)

I love this pattern. We should clean up some of LLVM's header similarly.

lib/sanitizer_common/sanitizer_symbolizer_win.cc

67 ↗

(On Diff #77420)

Should we also assert that the returned pointer is not null?

[rnk]

• Rewrite test with llvm-readobj
• CHECK our dbghelp imports

compiler-rt/trunk/lib/sanitizer_common/sanitizer_dbghelp.h

+//===-- sanitizer_dbghelp.h ------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Wrappers for lazy loaded dbghelp.dll. Provides function pointers and a
+// callback to initialize them.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef SANITIZER_SYMBOLIZER_WIN_H
+#define SANITIZER_SYMBOLIZER_WIN_H
+
+#if !SANITIZER_WINDOWS
+#error "sanitizer_dbghelp.h is a Windows-only header"
+#endif
+
+#define WIN32_LEAN_AND_MEAN
+#include <windows.h>
+#include <dbghelp.h>
+
+namespace __sanitizer {
+
+extern decltype(::StackWalk64) *StackWalk64;
+extern decltype(::SymCleanup) *SymCleanup;
+extern decltype(::SymFromAddr) *SymFromAddr;
+extern decltype(::SymFunctionTableAccess64) *SymFunctionTableAccess64;
+extern decltype(::SymGetLineFromAddr64) *SymGetLineFromAddr64;
+extern decltype(::SymGetModuleBase64) *SymGetModuleBase64;
+extern decltype(::SymGetSearchPathW) *SymGetSearchPathW;
+extern decltype(::SymInitialize) *SymInitialize;
+extern decltype(::SymSetOptions) *SymSetOptions;
+extern decltype(::SymSetSearchPathW) *SymSetSearchPathW;
+extern decltype(::UnDecorateSymbolName) *UnDecorateSymbolName;
+
+}  // namespace __sanitizer
+
+#endif  // SANITIZER_SYMBOLIZER_WIN_H

compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_win.cc

 //===-- sanitizer_symbolizer_win.cc ---------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file is shared between AddressSanitizer and ThreadSanitizer
 // run-time libraries.
 // Windows-specific implementation of symbolizer parts.
 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_platform.h"
 #if SANITIZER_WINDOWS
-#define WIN32_LEAN_AND_MEAN
-#include <windows.h>
-#include <dbghelp.h>
-#pragma comment(lib, "dbghelp.lib")
 
+#include "sanitizer_dbghelp.h"
 #include "sanitizer_symbolizer_internal.h"
 
 namespace __sanitizer {
 
+decltype(::StackWalk64) *StackWalk64;
+decltype(::SymCleanup) *SymCleanup;
+decltype(::SymFromAddr) *SymFromAddr;
+decltype(::SymFunctionTableAccess64) *SymFunctionTableAccess64;
+decltype(::SymGetLineFromAddr64) *SymGetLineFromAddr64;
+decltype(::SymGetModuleBase64) *SymGetModuleBase64;
+decltype(::SymGetSearchPathW) *SymGetSearchPathW;
+decltype(::SymInitialize) *SymInitialize;
+decltype(::SymSetOptions) *SymSetOptions;
+decltype(::SymSetSearchPathW) *SymSetSearchPathW;
+decltype(::UnDecorateSymbolName) *UnDecorateSymbolName;
+
 namespace {
 
 class WinSymbolizerTool : public SymbolizerTool {
  public:
   bool SymbolizePC(uptr addr, SymbolizedStack *stack) override;
   bool SymbolizeData(uptr addr, DataInfo *info) override {
     return false;
   }

 }  // namespace
 
 // Initializes DbgHelp library, if it's not yet initialized. Calls to this
 // function should be synchronized with respect to other calls to DbgHelp API
 // (e.g. from WinSymbolizerTool).
 void InitializeDbgHelpIfNeeded() {
   if (is_dbghelp_initialized)
     return;
+
+  HMODULE dbghelp = LoadLibraryA("dbghelp.dll");
+  CHECK(dbghelp && "failed to load dbghelp.dll");
+
+#define DBGHELP_IMPORT(name)                                                  \
+  do {                                                                        \
+    name =                                                                    \
+        reinterpret_cast<decltype(::name) *>(GetProcAddress(dbghelp, #name)); \
+    CHECK(name != nullptr);                                                   \
+  } while (0)
+  DBGHELP_IMPORT(StackWalk64);
+  DBGHELP_IMPORT(SymCleanup);
+  DBGHELP_IMPORT(SymFromAddr);
+  DBGHELP_IMPORT(SymFunctionTableAccess64);
+  DBGHELP_IMPORT(SymGetLineFromAddr64);
+  DBGHELP_IMPORT(SymGetModuleBase64);
+  DBGHELP_IMPORT(SymGetSearchPathW);
+  DBGHELP_IMPORT(SymInitialize);
+  DBGHELP_IMPORT(SymSetOptions);
+  DBGHELP_IMPORT(SymSetSearchPathW);
+  DBGHELP_IMPORT(UnDecorateSymbolName);
+#undef DBGHELP_IMPORT
+
   if (!TrySymInitialize()) {
     // OK, maybe the client app has called SymInitialize already.
     // That's a bit unfortunate for us as all the DbgHelp functions are
     // single-threaded and we can't coordinate with the app.
     // FIXME: Can we stop the other threads at this point?
     // Anyways, we have to reconfigure stuff to make sure that SymInitialize
     // has all the appropriate options set.
     // Cross our fingers and reinitialize DbgHelp.

compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc

 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_platform.h"
 #if SANITIZER_WINDOWS
 
 #define WIN32_LEAN_AND_MEAN
 #define NOGDI
 #include <windows.h>
-#include <dbghelp.h>
 #include <io.h>
 #include <psapi.h>
 #include <stdlib.h>
 
 #include "sanitizer_common.h"
+#include "sanitizer_dbghelp.h"
 #include "sanitizer_libc.h"
 #include "sanitizer_mutex.h"
 #include "sanitizer_placement_new.h"
 #include "sanitizer_procmaps.h"
 #include "sanitizer_stacktrace.h"
 #include "sanitizer_symbolizer.h"
 
 namespace __sanitizer {
#else
   stack_frame.AddrPC.Offset = ctx.Eip;
   stack_frame.AddrFrame.Offset = ctx.Ebp;
   stack_frame.AddrStack.Offset = ctx.Esp;
 #endif
   stack_frame.AddrPC.Mode = AddrModeFlat;
   stack_frame.AddrFrame.Mode = AddrModeFlat;
   stack_frame.AddrStack.Mode = AddrModeFlat;
   while (StackWalk64(machine_type, GetCurrentProcess(), GetCurrentThread(),
-                     &stack_frame, &ctx, NULL, &SymFunctionTableAccess64,
-                     &SymGetModuleBase64, NULL) &&
+                     &stack_frame, &ctx, NULL, SymFunctionTableAccess64,
+                     SymGetModuleBase64, NULL) &&
          size < Min(max_depth, kStackTraceMax)) {
     trace_buffer[size++] = (uptr)stack_frame.AddrPC.Offset;
   }
 }
 #endif  // #if !SANITIZER_GO
 
 void ReportFile::Write(const char *buffer, uptr length) {
   SpinMutexLock l(mu);

compiler-rt/trunk/test/CMakeLists.txt

 
 # When ANDROID, we build tests with the host compiler (i.e. CMAKE_C_COMPILER),
 # and run tests with tools from the host toolchain.
 if(NOT ANDROID)
   if(NOT COMPILER_RT_STANDALONE_BUILD)
     # Use LLVM utils and Clang from the same build tree.
     list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS
       clang clang-headers FileCheck count not llvm-config llvm-nm llvm-objdump
-      llvm-symbolizer compiler-rt-headers sancov)
+      llvm-readobj llvm-symbolizer compiler-rt-headers sancov)
     if (COMPILER_RT_HAS_PROFILE)
       list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS profile)
     endif()
     if (WIN32)
       list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS KillTheDoctor)
     endif()
   endif()
   if(CMAKE_HOST_UNIX)

compiler-rt/trunk/test/asan/TestCases/Windows/delay_dbghelp.cc

+// Build an executable with ASan, then extract the DLLs that it depends on.
+// RUN: %clang_cl_asan %s -Fe%t.exe
+// RUN: llvm-readobj -coff-imports %t.exe | grep Name: | sed -e 's/ *Name: *//' > %t
+//
+// Make sure the binary doesn't depend on dbghelp directly.
+// RUN: not grep dbghelp.dll %t
+//
+// Make sure any clang_rt DLLs it depends on don't depend on dbghelp. In the
+// static build, there won't be any clang_rt DLLs.
+// RUN: not grep cl""ang_rt %t || \
+// RUN:   grep cl""ang_rt %t | xargs which | \
+// RUN:   xargs llvm-readobj -coff-imports | not grep dbghelp.dll %t
+
+extern "C" int puts(const char *);
+
+int main() {
+  puts("main");
+}