-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lldb][windows] add support for out of PATH python.dll resolution #162509
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[lldb][windows] add support for out of PATH python.dll resolution #162509
Conversation
@llvm/pr-subscribers-lldb Author: Charles Zablit (charles-zablit) ChangesThis patch adds the If this variable is set and the resolved path points to an existing directory, we call This, combined with Full diff: https://github.com/llvm/llvm-project/pull/162509.diff 3 Files Affected:
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;
|
lldb/cmake/modules/AddLLDB.cmake
Outdated
There was a problem hiding this comment.
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
lldb/tools/driver/Driver.cpp
Outdated
There was a problem hiding this comment.
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
?
lldb/tools/driver/Driver.cpp
Outdated
} | ||
|
||
#ifdef _WIN32 | ||
// Returns the full path to the lldb.exe executable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// 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) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
lldb/tools/driver/Driver.cpp
Outdated
void AddPythonDLLToSearchPath() { | ||
std::wstring modulePath = GetPathToExecutableW(); | ||
if (modulePath.empty()) { | ||
WithColor::error() << "Unable to find python: " << GetLastError() << '\n'; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
✅ With the latest revision this PR passed the C/C++ code formatter. |
This patch adds the
LLDB_PYTHON_DLL_RELATIVE_PATH
Cmake variable which is the relative path to the directory containingpython.dll
. The path is relative to the directory containinglldb.exe
.If this variable is set and the resolved path points to an existing directory, we call
SetDllDirectoryW
to addpython.dll
to the list of DLL search paths.This, combined with
liblldb.dll
being delay loaded, allows to packagepython.dll
with thellvm
installer.