[llvm] [Windows] Avoid loading shared system libraries from user directory (PR #90520)

via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 30 06:36:32 PDT 2024


https://github.com/jofrn updated https://github.com/llvm/llvm-project/pull/90520

>From 4c78b147d8cc7682cfacb8a206b472b04a3a5c21 Mon Sep 17 00:00:00 2001
From: jofernau <jofernau at amd.com>
Date: Mon, 29 Apr 2024 13:17:40 -0700
Subject: [PATCH 1/5] [Windows] Find absolute path to dll

---
 llvm/lib/Support/CMakeLists.txt      | 16 ++++++++++++++++
 llvm/lib/Support/Windows/Signals.inc |  9 ++++++---
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Support/CMakeLists.txt b/llvm/lib/Support/CMakeLists.txt
index 03e888958a0711..46a9ffea50fc39 100644
--- a/llvm/lib/Support/CMakeLists.txt
+++ b/llvm/lib/Support/CMakeLists.txt
@@ -41,6 +41,22 @@ if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
   set(system_libs ${system_libs} psapi shell32 ole32 uuid advapi32 ws2_32)
+  # find dbgcore.dll and dbghelp.dll together in their system folder,
+  # required for lib/Support/Windows/Signals.inc.
+  set(tempsuffixes ${CMAKE_FIND_LIBRARY_SUFFIXES})
+  set(CMAKE_FIND_LIBRARY_SUFFIXES .DLL ${CMAKE_FIND_LIBRARY_SUFFIXES})
+  find_library(DBGCORE dbgcore REQUIRED NO_CACHE)
+  cmake_path(GET DBGCORE PARENT_PATH DBGSYSPATH)
+  find_library(DBGHELP dbghelp REQUIRED HINTS "${DBGSYSPATH}" NO_DEFAULT_PATH NO_CACHE)
+  cmake_path(CONVERT "${DBGCORE}" TO_NATIVE_PATH_LIST DBGCORE)
+  cmake_path(CONVERT "${DBGHELP}" TO_NATIVE_PATH_LIST DBGHELP)
+  string(REPLACE "\\" "\\\\" DBGCORE "${DBGCORE}")
+  string(REPLACE "\\" "\\\\" DBGHELP "${DBGHELP}")
+  message("-DDBGCORE is ${DBGCORE}")
+  message("-DDBGHELP is ${DBGHELP}")
+  add_compile_definitions(DBGCOREDLL="${DBGCORE}"
+                          DBGHELPDLL="${DBGHELP}")
+  set(CMAKE_FIND_LIBRARY_SUFFIXES ${tempsuffixes})
 elseif( CMAKE_HOST_UNIX )
   if( HAVE_LIBRT )
     set(system_libs ${system_libs} rt)
diff --git a/llvm/lib/Support/Windows/Signals.inc b/llvm/lib/Support/Windows/Signals.inc
index 34635b5aba7a1b..c4cfc19553b9ad 100644
--- a/llvm/lib/Support/Windows/Signals.inc
+++ b/llvm/lib/Support/Windows/Signals.inc
@@ -168,10 +168,13 @@ static bool isDebugHelpInitialized() {
 }
 
 static bool load64BitDebugHelp(void) {
-  HMODULE hLib = ::LoadLibraryW(L"Dbghelp.dll");
-  if (hLib) {
+  HMODULE hLibCore = ::LoadLibraryW(L"" DBGCOREDLL);
+  if (hLibCore) {
     fMiniDumpWriteDump =
-        (fpMiniDumpWriteDump)::GetProcAddress(hLib, "MiniDumpWriteDump");
+        (fpMiniDumpWriteDump)::GetProcAddress(hLibCore, "MiniDumpWriteDump");
+  }
+  HMODULE hLib = ::LoadLibraryW(L"" DBGHELPDLL);
+  if (hLib) {
     fStackWalk64 = (fpStackWalk64)::GetProcAddress(hLib, "StackWalk64");
     fSymGetModuleBase64 =
         (fpSymGetModuleBase64)::GetProcAddress(hLib, "SymGetModuleBase64");

>From 501b15280cdba7aaa8a63b2d7300074af4db8686 Mon Sep 17 00:00:00 2001
From: jofernau <jofernau at amd.com>
Date: Mon, 29 Apr 2024 15:40:01 -0700
Subject: [PATCH 2/5] [Windows] Restrict search path of LoadLibraryW.

LoadLibraryW search will be more portable than searching during buildtime. This modifies its search set to omit the current directory.
---
 llvm/lib/Support/CMakeLists.txt      | 16 ----------------
 llvm/lib/Support/InitLLVM.cpp        |  4 ++++
 llvm/lib/Support/Windows/Signals.inc |  4 ++--
 3 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/llvm/lib/Support/CMakeLists.txt b/llvm/lib/Support/CMakeLists.txt
index 46a9ffea50fc39..03e888958a0711 100644
--- a/llvm/lib/Support/CMakeLists.txt
+++ b/llvm/lib/Support/CMakeLists.txt
@@ -41,22 +41,6 @@ if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
   set(system_libs ${system_libs} psapi shell32 ole32 uuid advapi32 ws2_32)
-  # find dbgcore.dll and dbghelp.dll together in their system folder,
-  # required for lib/Support/Windows/Signals.inc.
-  set(tempsuffixes ${CMAKE_FIND_LIBRARY_SUFFIXES})
-  set(CMAKE_FIND_LIBRARY_SUFFIXES .DLL ${CMAKE_FIND_LIBRARY_SUFFIXES})
-  find_library(DBGCORE dbgcore REQUIRED NO_CACHE)
-  cmake_path(GET DBGCORE PARENT_PATH DBGSYSPATH)
-  find_library(DBGHELP dbghelp REQUIRED HINTS "${DBGSYSPATH}" NO_DEFAULT_PATH NO_CACHE)
-  cmake_path(CONVERT "${DBGCORE}" TO_NATIVE_PATH_LIST DBGCORE)
-  cmake_path(CONVERT "${DBGHELP}" TO_NATIVE_PATH_LIST DBGHELP)
-  string(REPLACE "\\" "\\\\" DBGCORE "${DBGCORE}")
-  string(REPLACE "\\" "\\\\" DBGHELP "${DBGHELP}")
-  message("-DDBGCORE is ${DBGCORE}")
-  message("-DDBGHELP is ${DBGHELP}")
-  add_compile_definitions(DBGCOREDLL="${DBGCORE}"
-                          DBGHELPDLL="${DBGHELP}")
-  set(CMAKE_FIND_LIBRARY_SUFFIXES ${tempsuffixes})
 elseif( CMAKE_HOST_UNIX )
   if( HAVE_LIBRT )
     set(system_libs ${system_libs} rt)
diff --git a/llvm/lib/Support/InitLLVM.cpp b/llvm/lib/Support/InitLLVM.cpp
index b7e463a19122db..8f4b765e7b5561 100644
--- a/llvm/lib/Support/InitLLVM.cpp
+++ b/llvm/lib/Support/InitLLVM.cpp
@@ -43,6 +43,10 @@ InitLLVM::InitLLVM(int &Argc, const char **&Argv,
   assert(!Initialized && "InitLLVM was already initialized!");
   Initialized = true;
 #endif
+#ifdef _WIN32
+  // Avoid searching the directory from which the application is loaded.
+  SetDllDirectoryA("");
+#endif
 #ifdef __MVS__
   // Bring stdin/stdout/stderr into a known state.
   sys::AddSignalHandler(CleanupStdHandles, nullptr);
diff --git a/llvm/lib/Support/Windows/Signals.inc b/llvm/lib/Support/Windows/Signals.inc
index c4cfc19553b9ad..2f4d1951d78f2b 100644
--- a/llvm/lib/Support/Windows/Signals.inc
+++ b/llvm/lib/Support/Windows/Signals.inc
@@ -168,12 +168,12 @@ static bool isDebugHelpInitialized() {
 }
 
 static bool load64BitDebugHelp(void) {
-  HMODULE hLibCore = ::LoadLibraryW(L"" DBGCOREDLL);
+  HMODULE hLibCore = ::LoadLibraryW(L"Dbgcore.dll");
   if (hLibCore) {
     fMiniDumpWriteDump =
         (fpMiniDumpWriteDump)::GetProcAddress(hLibCore, "MiniDumpWriteDump");
   }
-  HMODULE hLib = ::LoadLibraryW(L"" DBGHELPDLL);
+  HMODULE hLib = ::LoadLibraryW(L"Dbghelp.dll");
   if (hLib) {
     fStackWalk64 = (fpStackWalk64)::GetProcAddress(hLib, "StackWalk64");
     fSymGetModuleBase64 =

>From b8868fd2f226b7c464b971953a58878466129fc4 Mon Sep 17 00:00:00 2001
From: jofernau <jofernau at amd.com>
Date: Tue, 30 Apr 2024 05:11:24 -0700
Subject: [PATCH 3/5] [Windows] Restrict app load directory from dll search
 path (in addition to pwd)

---
 llvm/lib/Support/InitLLVM.cpp | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Support/InitLLVM.cpp b/llvm/lib/Support/InitLLVM.cpp
index 8f4b765e7b5561..e136f3c449808d 100644
--- a/llvm/lib/Support/InitLLVM.cpp
+++ b/llvm/lib/Support/InitLLVM.cpp
@@ -44,8 +44,24 @@ InitLLVM::InitLLVM(int &Argc, const char **&Argv,
   Initialized = true;
 #endif
 #ifdef _WIN32
-  // Avoid searching the directory from which the application is loaded.
-  SetDllDirectoryA("");
+  // Avoid searching user directories for shared libraries:
+  //   Avoid searching the current directory:
+  (void)SetDllDirectoryA("");
+  wchar_t CurrentPath[MAX_PATH];
+  (void)GetCurrentDirectory(MAX_PATH, CurrentPath);
+  //   Avoid searching the directory from which the application is loaded:
+  wchar_t Appname[MAX_PATH];
+  (void)GetModuleFileName(NULL, Appname, MAX_PATH);
+  std::array<wchar_t, MAX_PATH> AN;
+  std::copy(std::begin(Appname), std::end(Appname), std::begin(AN));
+  std::string LP{AN.begin(), AN.end()};
+  LP = LP.substr(0, LP.rfind('\\'));
+  auto Path = std::wstring(LP.begin(), LP.end());
+  auto LoadPath = Path.c_str();
+  (void)SetCurrentDirectory(LoadPath);
+  (void)SetDllDirectoryA("");
+  (void)SetCurrentDirectory(CurrentPath); // reset cwd
+  // FIXME: check for errors
 #endif
 #ifdef __MVS__
   // Bring stdin/stdout/stderr into a known state.

>From 9411b4f1582188dba656658f871fc777c983f351 Mon Sep 17 00:00:00 2001
From: jofernau <jofernau at amd.com>
Date: Tue, 30 Apr 2024 05:27:07 -0700
Subject: [PATCH 4/5] [Windows] Use LoadLibraryExA and SEARCH_SYSTEM32

---
 llvm/lib/Support/InitLLVM.cpp        | 20 --------------------
 llvm/lib/Support/Windows/Signals.inc |  4 ++--
 2 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/llvm/lib/Support/InitLLVM.cpp b/llvm/lib/Support/InitLLVM.cpp
index e136f3c449808d..b7e463a19122db 100644
--- a/llvm/lib/Support/InitLLVM.cpp
+++ b/llvm/lib/Support/InitLLVM.cpp
@@ -43,26 +43,6 @@ InitLLVM::InitLLVM(int &Argc, const char **&Argv,
   assert(!Initialized && "InitLLVM was already initialized!");
   Initialized = true;
 #endif
-#ifdef _WIN32
-  // Avoid searching user directories for shared libraries:
-  //   Avoid searching the current directory:
-  (void)SetDllDirectoryA("");
-  wchar_t CurrentPath[MAX_PATH];
-  (void)GetCurrentDirectory(MAX_PATH, CurrentPath);
-  //   Avoid searching the directory from which the application is loaded:
-  wchar_t Appname[MAX_PATH];
-  (void)GetModuleFileName(NULL, Appname, MAX_PATH);
-  std::array<wchar_t, MAX_PATH> AN;
-  std::copy(std::begin(Appname), std::end(Appname), std::begin(AN));
-  std::string LP{AN.begin(), AN.end()};
-  LP = LP.substr(0, LP.rfind('\\'));
-  auto Path = std::wstring(LP.begin(), LP.end());
-  auto LoadPath = Path.c_str();
-  (void)SetCurrentDirectory(LoadPath);
-  (void)SetDllDirectoryA("");
-  (void)SetCurrentDirectory(CurrentPath); // reset cwd
-  // FIXME: check for errors
-#endif
 #ifdef __MVS__
   // Bring stdin/stdout/stderr into a known state.
   sys::AddSignalHandler(CleanupStdHandles, nullptr);
diff --git a/llvm/lib/Support/Windows/Signals.inc b/llvm/lib/Support/Windows/Signals.inc
index 2f4d1951d78f2b..4852aa4a80ae9c 100644
--- a/llvm/lib/Support/Windows/Signals.inc
+++ b/llvm/lib/Support/Windows/Signals.inc
@@ -168,12 +168,12 @@ static bool isDebugHelpInitialized() {
 }
 
 static bool load64BitDebugHelp(void) {
-  HMODULE hLibCore = ::LoadLibraryW(L"Dbgcore.dll");
+  HMODULE hLibCore = ::LoadLibraryExA("Dbgcore.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
   if (hLibCore) {
     fMiniDumpWriteDump =
         (fpMiniDumpWriteDump)::GetProcAddress(hLibCore, "MiniDumpWriteDump");
   }
-  HMODULE hLib = ::LoadLibraryW(L"Dbghelp.dll");
+  HMODULE hLib = ::LoadLibraryExA("Dbghelp.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
   if (hLib) {
     fStackWalk64 = (fpStackWalk64)::GetProcAddress(hLib, "StackWalk64");
     fSymGetModuleBase64 =

>From 8a296181c3d0f3a1816b9546a354159050956628 Mon Sep 17 00:00:00 2001
From: jofernau <jofernau at amd.com>
Date: Tue, 30 Apr 2024 06:32:20 -0700
Subject: [PATCH 5/5] [Windows] Forwarded function is resolved to sys32

This was only required when referencing the shared libraries with absolute paths. The forwarded function no longer needs to be looked up separately since restricting the search path forwards functions properly.
---
 llvm/lib/Support/Windows/Signals.inc | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/Support/Windows/Signals.inc b/llvm/lib/Support/Windows/Signals.inc
index 4852aa4a80ae9c..cf9c2d37ae7acc 100644
--- a/llvm/lib/Support/Windows/Signals.inc
+++ b/llvm/lib/Support/Windows/Signals.inc
@@ -168,13 +168,10 @@ static bool isDebugHelpInitialized() {
 }
 
 static bool load64BitDebugHelp(void) {
-  HMODULE hLibCore = ::LoadLibraryExA("Dbgcore.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
-  if (hLibCore) {
-    fMiniDumpWriteDump =
-        (fpMiniDumpWriteDump)::GetProcAddress(hLibCore, "MiniDumpWriteDump");
-  }
   HMODULE hLib = ::LoadLibraryExA("Dbghelp.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
   if (hLib) {
+    fMiniDumpWriteDump =
+        (fpMiniDumpWriteDump)::GetProcAddress(hLib, "MiniDumpWriteDump");
     fStackWalk64 = (fpStackWalk64)::GetProcAddress(hLib, "StackWalk64");
     fSymGetModuleBase64 =
         (fpSymGetModuleBase64)::GetProcAddress(hLib, "SymGetModuleBase64");



More information about the llvm-commits mailing list