Merge pull request #62294 from grynspan/jgrynspan/dbghelp-pass-handle

Refactor `_swift_withWin32DbgHelpLibrary()` to avoid using `GetCurrentProcess()` per Microsoft documentation

Author: grynspan

stdlib/public/runtime/Errors.cpp

@@ -99,8 +99,8 @@ static bool getSymbolNameAddr(llvm::StringRef libraryName,
 #if defined(_WIN32)
   char szUndName[1024];
   DWORD dwResult;
-  dwResult = _swift_withWin32DbgHelpLibrary([&] (bool isInitialized) -> DWORD {
-    if (!isInitialized) {
+  dwResult = _swift_win32_withDbgHelpLibrary([&] (HANDLE hProcess) -> DWORD {
+    if (!hProcess) {
       return 0;
     }
 

stdlib/public/runtime/ImageInspection.h

@@ -29,6 +29,12 @@
 #include <memory>
 #include <type_traits>
 
+#if defined(_WIN32)
+#define WIN32_LEAN_AND_MEAN
+#define NOMINMAX
+#include <Windows.h>
+#endif
+
 namespace swift {
 
 /// This is a platform independent version of Dl_info from dlfcn.h
@@ -112,55 +118,73 @@ int lookupSymbol(const void *address, SymbolInfo *info);
 /// Configure the environment to allow calling into the Debug Help library.
 ///
 /// \param body A function to invoke. This function attempts to first initialize
-///   the Debug Help library. The result of that operation is passed to this
-///   function.
+///   the Debug Help library. If it did so successfully, the handle used during
+///   initialization is passed to this function and should be used with
+///   subsequent calls to the Debug Help library. Do not close this handle.
 /// \param context A caller-supplied value to pass to \a body.
 ///
 /// On Windows, the Debug Help library (DbgHelp.lib) is not thread-safe. All
 /// calls into it from the Swift runtime and stdlib should route through this
 /// function.
+///
+/// This function sets the Debug Help library's options by calling
+/// \c SymSetOptions() before \a body is invoked, and then resets them back to
+/// their old value before returning. \a body can also call \c SymSetOptions()
+/// if needed.
 SWIFT_RUNTIME_STDLIB_SPI
-void _swift_withWin32DbgHelpLibrary(
-  void (* body)(bool isInitialized, void *context), void *context);
+void _swift_win32_withDbgHelpLibrary(
+  void (* body)(HANDLE hProcess, void *context), void *context);
 
 /// Configure the environment to allow calling into the Debug Help library.
 ///
 /// \param body A function to invoke. This function attempts to first initialize
-///   the Debug Help library. The result of that operation is passed to this
-///   function.
+///   the Debug Help library. If it did so successfully, the handle used during
+///   initialization is passed to this function and should be used with
+///   subsequent calls to the Debug Help library. Do not close this handle.
 ///
 /// On Windows, the Debug Help library (DbgHelp.lib) is not thread-safe. All
 /// calls into it from the Swift runtime and stdlib should route through this
 /// function.
-static inline void _swift_withWin32DbgHelpLibrary(
-  const std::function<void(bool /*isInitialized*/)> &body) {
-  _swift_withWin32DbgHelpLibrary([](bool isInitialized, void *context) {
-    auto bodyp = reinterpret_cast<std::function<void(bool)> *>(context);
-    (* bodyp)(isInitialized);
+///
+/// This function sets the Debug Help library's options by calling
+/// \c SymSetOptions() before \a body is invoked, and then resets them back to
+/// their old value before returning. \a body can also call \c SymSetOptions()
+/// if needed.
+static inline void _swift_win32_withDbgHelpLibrary(
+  const std::function<void(HANDLE /*hProcess*/)> &body) {
+  _swift_win32_withDbgHelpLibrary([](HANDLE hProcess, void *context) {
+    auto bodyp = reinterpret_cast<std::function<void(HANDLE)> *>(context);
+    (* bodyp)(hProcess);
   }, const_cast<void *>(reinterpret_cast<const void *>(&body)));
 }
 
 /// Configure the environment to allow calling into the Debug Help library.
 ///
 /// \param body A function to invoke. This function attempts to first initialize
-///   the Debug Help library. The result of that operation is passed to this
-///   function.
+///   the Debug Help library. If it did so successfully, the handle used during
+///   initialization is passed to this function and should be used with
+///   subsequent calls to the Debug Help library. Do not close this handle.
 ///
 /// \returns Whatever is returned from \a body.
 ///
 /// On Windows, the Debug Help library (DbgHelp.lib) is not thread-safe. All
 /// calls into it from the Swift runtime and stdlib should route through this
 /// function.
+///
+/// This function sets the Debug Help library's options by calling
+/// \c SymSetOptions() before \a body is invoked, and then resets them back to
+/// their old value before returning. \a body can also call \c SymSetOptions()
+/// if needed.
 template <
   typename F,
-  typename R = typename std::result_of_t<F&(bool /*isInitialized*/)>,
+  typename R = typename std::result_of_t<F&(HANDLE /*hProcess*/)>,
   typename = typename std::enable_if_t<!std::is_same<void, R>::value>
 >
-static inline R _swift_withWin32DbgHelpLibrary(const F& body) {
+static inline R _swift_win32_withDbgHelpLibrary(const F& body) {
   R result;
 
-  _swift_withWin32DbgHelpLibrary([&body, &result] (bool isInitialized) {
-    result = body(isInitialized);
+  _swift_win32_withDbgHelpLibrary([&body, &result] (HANDLE hProcess) {
+    result = body(hProcess);
   });
 
   return result;

stdlib/public/runtime/ImageInspectionCOFF.cpp

@@ -40,30 +40,25 @@ int swift::lookupSymbol(const void *address, SymbolInfo *info) {
   info->symbolAddress = dli_saddr;
   return 1;
 #elif defined(_WIN32)
-  return _swift_withWin32DbgHelpLibrary([&] (bool isInitialized) {
+  return _swift_win32_withDbgHelpLibrary([&] (HANDLE hProcess) {
     static const constexpr size_t kSymbolMaxNameLen = 1024;
 
-    if (!isInitialized) {
+    if (!hProcess) {
       return 0;
     }
 
-    char buffer[sizeof(SYMBOL_INFO) + kSymbolMaxNameLen];
-    PSYMBOL_INFO pSymbol = reinterpret_cast<PSYMBOL_INFO>(buffer);
-    pSymbol->SizeOfStruct = sizeof(SYMBOL_INFO);
-    pSymbol->MaxNameLen = kSymbolMaxNameLen;
-
-    DWORD64 dwDisplacement = 0;
-
-    if (SymFromAddr(GetCurrentProcess(),
-                    reinterpret_cast<const DWORD64>(address),
-                    &dwDisplacement, pSymbol) == FALSE) {
+    SYMBOL_INFO_PACKAGE symbol = {};
+    symbol.si.SizeOfStruct = sizeof(SYMBOL_INFO);
+    symbol.si.MaxNameLen = MAX_SYM_NAME;
+    if (SymFromAddr(hProcess, reinterpret_cast<const DWORD64>(address),
+                    nullptr, &symbol.si) == FALSE) {
       return 0;
     }
 
     info->fileName = NULL;
-    info->baseAddress = reinterpret_cast<void *>(pSymbol->ModBase);
-    info->symbolName.reset(_strdup(pSymbol->Name));
-    info->symbolAddress = reinterpret_cast<void *>(pSymbol->Address);
+    info->baseAddress = reinterpret_cast<void *>(symbol.si.ModBase);
+    info->symbolName.reset(_strdup(symbol.si.Name));
+    info->symbolAddress = reinterpret_cast<void *>(symbol.si.Address);
 
     return 1;
   });
@@ -74,16 +69,58 @@ int swift::lookupSymbol(const void *address, SymbolInfo *info) {
 
 #if defined(_WIN32)
 static LazyMutex mutex;
-static bool isDbgHelpInitialized = false;
+static HANDLE dbgHelpHandle = nullptr;
 
-void swift::_swift_withWin32DbgHelpLibrary(
-  void (* body)(bool isInitialized, void *context), void *context) {
+void swift::_swift_win32_withDbgHelpLibrary(
+  void (* body)(HANDLE hProcess, void *context), void *context) {
   mutex.withLock([=] () {
-    if (!isDbgHelpInitialized) {
-      SymSetOptions(SYMOPT_UNDNAME | SYMOPT_DEFERRED_LOADS);
-      isDbgHelpInitialized = SymInitialize(GetCurrentProcess(), nullptr, true);
+    // If we have not previously created a handle to use with the library, do so
+    // now. This handle belongs to the Swift runtime and should not be closed by
+    // `body` (or anybody else.)
+    if (!dbgHelpHandle) {
+      // Per the documentation for the Debug Help library, we should not use the
+      // current process handle because other subsystems might also use it and
+      // end up stomping on each other. So we'll try to duplicate that handle to
+      // get a unique one that still fulfills the needs of the library. If that
+      // fails (presumably because the current process doesn't have the
+      // PROCESS_DUP_HANDLE access right) then fall back to using the original
+      // process handle and hope nobody else is using it too.
+      HANDLE currentProcess = GetCurrentProcess();
+      if (!DuplicateHandle(currentProcess, currentProcess, currentProcess,
+                           &dbgHelpHandle, 0, false, DUPLICATE_SAME_ACCESS)) {
+        dbgHelpHandle = currentProcess;
+      }
+    }
+
+    // If we have not previously initialized the Debug Help library, do so now.
+    bool isDbgHelpInitialized = false;
+    if (dbgHelpHandle) {
+      isDbgHelpInitialized = SymInitialize(dbgHelpHandle, nullptr, true);
+    }
+
+    if (isDbgHelpInitialized) {
+      // Set the library's options to what the Swift runtime generally expects.
+      // If the options aren't going to change, we can skip the call and save a
+      // few CPU cycles on the library call.
+      constexpr const DWORD options = SYMOPT_UNDNAME | SYMOPT_DEFERRED_LOADS;
+      DWORD oldOptions = SymGetOptions();
+      if (oldOptions != options) {
+        SymSetOptions(options);
+      }
+
+      body(dbgHelpHandle, context);
+
+      // Before returning, reset the library's options back to their previous
+      // value. No need to call if the options didn't change because LazyMutex
+      // is not recursive, so there shouldn't be an outer call expecting the
+      // original options, and a subsequent call to this function will set them
+      // to the defaults above.
+      if (oldOptions != options) {
+        SymSetOptions(oldOptions);
+      }
+    } else {
+      body(nullptr, context);
     }
-    body(isDbgHelpInitialized, context);
   });
 }
 #endif