Skip to content

Conversation

charles-zablit
Copy link
Contributor

This patch adds the LLDB_PYTHON_DLL_RELATIVE_PATH Cmake variable which is the relative path to the directory containing python.dll. The path is relative to the directory containing lldb.exe.

If this variable is set and the resolved path points to an existing directory, we call SetDllDirectoryW to add python.dll to the list of DLL search paths.

This, combined with liblldb.dll being delay loaded, allows to package python.dll with the llvm installer.

@llvmbot
Copy link
Member

llvmbot commented Oct 8, 2025

@llvm/pr-subscribers-lldb

Author: Charles Zablit (charles-zablit)

Changes

This patch adds the LLDB_PYTHON_DLL_RELATIVE_PATH Cmake variable which is the relative path to the directory containing python.dll. The path is relative to the directory containing lldb.exe.

If this variable is set and the resolved path points to an existing directory, we call SetDllDirectoryW to add python.dll to the list of DLL search paths.

This, combined with liblldb.dll being delay loaded, allows to package python.dll with the llvm installer.


Full diff: https://github.com/llvm/llvm-project/pull/162509.diff

3 Files Affected:

  • (modified) lldb/cmake/modules/AddLLDB.cmake (+6)
  • (modified) lldb/tools/driver/CMakeLists.txt (+4)
  • (modified) lldb/tools/driver/Driver.cpp (+49)
diff --git a/lldb/cmake/modules/AddLLDB.cmake b/lldb/cmake/modules/AddLLDB.cmake
index 28bf8d816d89a..4d2036e68704f 100644
--- a/lldb/cmake/modules/AddLLDB.cmake
+++ b/lldb/cmake/modules/AddLLDB.cmake
@@ -167,6 +167,12 @@ function(add_lldb_executable name)
   )
 
   target_link_libraries(${name} PRIVATE ${ARG_LINK_LIBS})
+  if(WIN32)
+    list(FIND ARG_LINK_LIBS liblldb LIBLLD_INDEX)
+    if(NOT LIBLLD_INDEX EQUAL -1)
+      target_link_options(${name} PRIVATE "/DELAYLOAD:$<TARGET_FILE_BASE_NAME:liblldb>.dll")
+    endif()
+  endif()
   if(CLANG_LINK_CLANG_DYLIB)
     target_link_libraries(${name} PRIVATE clang-cpp)
   else()
diff --git a/lldb/tools/driver/CMakeLists.txt b/lldb/tools/driver/CMakeLists.txt
index 5c0cac484cc94..941ef0d83385b 100644
--- a/lldb/tools/driver/CMakeLists.txt
+++ b/lldb/tools/driver/CMakeLists.txt
@@ -34,6 +34,10 @@ add_dependencies(lldb
   ${tablegen_deps}
 )
 
+if(DEFINED LLDB_PYTHON_DLL_RELATIVE_PATH)
+  target_compile_definitions(lldb PRIVATE LLDB_PYTHON_DLL_RELATIVE_PATH="${LLDB_PYTHON_DLL_RELATIVE_PATH}")
+endif()
+
 if(LLDB_BUILD_FRAMEWORK)
   # In the build-tree, we know the exact path to the framework directory.
   # The installed framework can be in different locations.
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index 16cc736441b59..df31d649ada48 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -22,11 +22,15 @@
 #include "lldb/Host/MainLoop.h"
 #include "lldb/Host/MainLoopBase.h"
 #include "lldb/Utility/Status.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/Windows/WindowsSupport.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -426,6 +430,47 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args, bool &exiting) {
   return error;
 }
 
+#ifdef _WIN32
+// Returns the full path to the lldb.exe executable
+inline std::wstring GetPathToExecutableW() {
+  // Iterate until we reach the Windows max path length (32,767).
+  std::vector<WCHAR> buffer;
+  buffer.resize(MAX_PATH);
+  while (buffer.size() < 32767) {
+    if (GetModuleFileNameW(NULL, buffer.data(), buffer.size()) < buffer.size())
+      return std::wstring(buffer.begin(), buffer.end());
+    buffer.resize(buffer.size() * 2);
+  }
+  return L"";
+}
+
+// Resolve the full path of the directory defined by
+// LLDB_PYTHON_DLL_RELATIVE_PATH. If it exists, add it to the list of DLL search
+// directories.
+void AddPythonDLLToSearchPath() {
+  std::wstring modulePath = GetPathToExecutableW();
+  if (modulePath.empty()) {
+    WithColor::error() << "Unable to find python: " << GetLastError() << '\n';
+    return;
+  }
+
+  SmallVector<char, MAX_PATH> utf8Path;
+  if (sys::windows::UTF16ToUTF8(modulePath.c_str(), modulePath.length(),
+                                utf8Path))
+    return;
+  sys::path::remove_filename(utf8Path);
+  sys::path::append(utf8Path, LLDB_PYTHON_DLL_RELATIVE_PATH);
+  sys::fs::make_absolute(utf8Path);
+
+  SmallVector<wchar_t, 1> widePath;
+  if (sys::windows::widenPath(utf8Path.data(), widePath))
+    return;
+
+  if (sys::fs::exists(utf8Path))
+    SetDllDirectoryW(widePath.data());
+}
+#endif
+
 std::string EscapeString(std::string arg) {
   std::string::size_type pos = 0;
   while ((pos = arg.find_first_of("\"\\", pos)) != std::string::npos) {
@@ -728,6 +773,10 @@ int main(int argc, char const *argv[]) {
                         "~/Library/Logs/DiagnosticReports/.\n");
 #endif
 
+#ifdef _WIN32
+  AddPythonDLLToSearchPath();
+#endif
+
   // Parse arguments.
   LLDBOptTable T;
   unsigned MissingArgIndex;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: missing a B -- LIBLLDB_INDEX

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be an ifdef?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should, I'm surprised that it worked before.

}

#ifdef _WIN32
// Returns the full path to the lldb.exe executable
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Returns the full path to the lldb.exe executable
// Returns the full path to the lldb.exe executable.

// Iterate until we reach the Windows max path length (32,767).
std::vector<WCHAR> buffer;
buffer.resize(MAX_PATH);
while (buffer.size() < 32767) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if MAX_PATH is 32767 isn't this condition always false?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a doubling on failure, so this shouldn't be always false.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAX_PATH is defined in the Windows API as 260. However, a lot of the Windows API functions can return Unicode paths that are up to 32,767 bytes long.

I have updated the comments to explain this better.

void AddPythonDLLToSearchPath() {
std::wstring modulePath = GetPathToExecutableW();
if (modulePath.empty()) {
WithColor::error() << "Unable to find python: " << GetLastError() << '\n';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We typically with the word "error:" in the error color, and not the entire message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by using llvm::errs() and changing the message to start with error:

Copy link

github-actions bot commented Oct 9, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@charles-zablit charles-zablit merged commit ec28b95 into llvm:main Oct 9, 2025
9 checks passed
@dzhidzhoev
Copy link
Member

Please note that this patch has broken lldb-remote-linux-win.

C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\tools\driver\Driver.cpp(465): error C2065: 'LLDB_PYTHON_DLL_RELATIVE_PATH': undeclared identifier

https://lab.llvm.org/buildbot/#/builders/197/builds/9689

Could you please take a look at that?

@charles-zablit
Copy link
Contributor Author

Please note that this patch has broken lldb-remote-linux-win.

C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\tools\driver\Driver.cpp(465): error C2065: 'LLDB_PYTHON_DLL_RELATIVE_PATH': undeclared identifier

lab.llvm.org/buildbot#/builders/197/builds/9689

Could you please take a look at that?

Thank you for letting me know, I have opened a PR for the fix here:

charles-zablit added a commit that referenced this pull request Oct 9, 2025
This patch fixes the `'LLDB_PYTHON_DLL_RELATIVE_PATH': undeclared
identifier` error introduced by
#162509.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Oct 9, 2025
This patch fixes the `'LLDB_PYTHON_DLL_RELATIVE_PATH': undeclared
identifier` error introduced by
llvm/llvm-project#162509.
svkeerthy pushed a commit that referenced this pull request Oct 9, 2025
…62509)

This patch adds the `LLDB_PYTHON_DLL_RELATIVE_PATH` Cmake variable which
is the relative path to the directory containing `python.dll`. The path
is relative to the directory containing `lldb.exe`.

If this variable is set and the resolved path points to an existing
directory, we call `SetDllDirectoryW` to add `python.dll` to the list of
DLL search paths.

This, combined with `liblldb.dll` being delay loaded, allows to package
`python.dll` with the `llvm` installer.
svkeerthy pushed a commit that referenced this pull request Oct 9, 2025
This patch fixes the `'LLDB_PYTHON_DLL_RELATIVE_PATH': undeclared
identifier` error introduced by
#162509.
charles-zablit added a commit to charles-zablit/llvm-project that referenced this pull request Oct 10, 2025
…vm#162509)

This patch adds the `LLDB_PYTHON_DLL_RELATIVE_PATH` Cmake variable which
is the relative path to the directory containing `python.dll`. The path
is relative to the directory containing `lldb.exe`.

If this variable is set and the resolved path points to an existing
directory, we call `SetDllDirectoryW` to add `python.dll` to the list of
DLL search paths.

This, combined with `liblldb.dll` being delay loaded, allows to package
`python.dll` with the `llvm` installer.
if(WIN32)
list(FIND ARG_LINK_LIBS liblldb LIBLLDB_INDEX)
if(NOT LIBLLDB_INDEX EQUAL -1)
target_link_options(${name} PRIVATE "/DELAYLOAD:$<TARGET_FILE_BASE_NAME:liblldb>.dll")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason for using $<TARGET_FILE_BASE_NAME:liblldb>.dll rather than the more straightforward $<TARGET_FILE_NAME:liblldb>?

This patch broke compilation for mingw targets, as these options are specific to the MSVC style build case - and for that case, the DLL name we'd like to pass here has a lib prefix, i.e. liblldb.dll, not lldb.dll. So instead of trying to manually reapply a prefix and add .dll at the end, it seems more straightforward to me to just use $<TARGET_FILE_NAME:liblldb> which should give the right string in both cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to be gated behind a if(MSVC) check in the downstream patch: swiftlang@af218c3. I think that with #162831 we will get a complete solution as if LLDB_PYTHON_DLL_RELATIVE_PATH is not set, the mechanism to add to the DLL search path will not even be compiled. If it's set, then it will only work for MSVC and mingw and there will be a warning if it's a different compiler.

which should give the right string in both cases?

I agree, this is much better 👍

@mstorsjo
Copy link
Member

This, combined with liblldb.dll being delay loaded, allows to package python.dll with the llvm installer.

Just for reference - for llvm-mingw, we've been bundling a python install in our toolchain as well, but we've solved this slightly differently. We've just copied python.dll (but nothing else from the Python install) into the LLVM tool bin directory. That, together with the LLDB_PYTHON_HOME flag, has been enough for finding the bundled python install in a different directory. This solution is kinda neat though, and would allow skipping copying the python.dll.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants