Skip to content

[lldb][windows] print an error if python.dll is not in the DLL search path #164893

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Nov 3, 2025
Merged

[lldb][windows] print an error if python.dll is not in the DLL search path #164893

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3d2976b
[lldb][windows] print an error if python.dll is not in the DLL search…
charles-zablit Oct 23, 2025
d502ba9
fixup! [lldb][windows] print an error if python.dll is not in the DLL…
charles-zablit Oct 23, 2025
c73bf5f
fixup! [lldb][windows] print an error if python.dll is not in the DLL…
charles-zablit Oct 23, 2025
71178bb
fixup! [lldb][windows] print an error if python.dll is not in the DLL…
charles-zablit Oct 23, 2025
6071b08
fixup! [lldb][windows] print an error if python.dll is not in the DLL…
charles-zablit Oct 23, 2025
855a405
formatting fixup
charles-zablit Oct 24, 2025
6ee7fce
use IMPORTED_LIBRARY_LOCATION
charles-zablit Oct 24, 2025
8b34fea
check if the Python target is defined
charles-zablit Oct 27, 2025
2bde6e8
check if python.dll can be found after adding it to the search path
charles-zablit Nov 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lldb/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ if (LLDB_ENABLE_PYTHON)
"Path to python interpreter exectuable, relative to python's install prefix")
set(cachestring_LLDB_PYTHON_EXT_SUFFIX
"Filename extension for native code python modules")
set(cachestring_LLDB_PYTHON_SHARED_LIBRARY_NAME
"Filename of Python's shared library")

foreach(var LLDB_PYTHON_RELATIVE_PATH LLDB_PYTHON_EXE_RELATIVE_PATH LLDB_PYTHON_EXT_SUFFIX)
Copy link
Member

@mstorsjo mstorsjo Oct 24, 2025

Choose a reason for hiding this comment

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

If the cachestring_ above is to be used, we should include LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME in this loop too (if that makes sense? not familiar with what this does...). Otherwise the cachestring_ variable isn't used at all.

Copy link
Contributor Author

@charles-zablit charles-zablit Oct 24, 2025

Choose a reason for hiding this comment

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

Removed it, thanks 👍

if(NOT DEFINED ${var} AND NOT CMAKE_CROSSCOMPILING)
Expand All @@ -87,6 +89,8 @@ if (LLDB_ENABLE_PYTHON)
set(LLDB_PYTHON_EXT_SUFFIX "_d${LLDB_PYTHON_EXT_SUFFIX}")
endif()
endif()
set(LLDB_PYTHON_SHARED_LIBRARY_FILENAME
"python${Python3_VERSION_MAJOR}${Python3_VERSION_MINOR}${CMAKE_SHARED_LIBRARY_SUFFIX}")
Copy link
Member

@mstorsjo mstorsjo Oct 23, 2025

Choose a reason for hiding this comment

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

You may wish to include a cmake variable for the prefix as well; on mingw, it's libpython3.12.dll - I'm sure that there's a cmake variable that expands to lib for mingw but nothing for msvc like environments.

Also; if i understand this correctly, this variable gets set automatically, but is it possible to override this string through cmake parameters?

Copy link
Contributor Author

@charles-zablit charles-zablit Oct 23, 2025

Choose a reason for hiding this comment

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

I switched to using Python3_RUNTIME_LIBRARY which points directly to the correct dll. Hopefully that also works in mingw and other environment.

Also; if i understand this correctly, this variable gets set automatically, but is it possible to override this string through cmake parameters?

From my understanding of Cmake, it will be overwritten by the set, so the answer is no. Do you think it would be a useful variable to expose?

Copy link
Member

@mstorsjo mstorsjo Oct 24, 2025

Choose a reason for hiding this comment

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

If we pick the right name for all configurations, then no, we probably don't need to expose it. From a distributor freedom point of view, it maybe would be good to have the possibility to do that. But on the other hand, we can go with this first, and make it settable if someone later tells us they'd want to do that.

endif ()

if (LLDB_ENABLE_LUA)
Expand Down
3 changes: 3 additions & 0 deletions lldb/tools/driver/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ add_dependencies(lldb
if(DEFINED LLDB_PYTHON_DLL_RELATIVE_PATH)
target_compile_definitions(lldb PRIVATE LLDB_PYTHON_DLL_RELATIVE_PATH="${LLDB_PYTHON_DLL_RELATIVE_PATH}")
endif()
if(DEFINED LLDB_PYTHON_SHARED_LIBRARY_FILENAME)
target_compile_definitions(lldb PRIVATE LLDB_PYTHON_SHARED_LIBRARY_FILENAME="${LLDB_PYTHON_SHARED_LIBRARY_FILENAME}")
Copy link
Member

@mstorsjo mstorsjo Oct 24, 2025

Choose a reason for hiding this comment

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

This seems broken/mismatched right now; this uses LLDB_PYTHON_SHARED_LIBRARY_FILENAME while the rest of CMake, and the C++ code, uses LLDB_PYTHON_RUNTIME_LIBRARY_FILENAME.

Copy link
Contributor Author

@charles-zablit charles-zablit Oct 24, 2025

Choose a reason for hiding this comment

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

Fixed, thanks 👍

endif()

if(LLDB_BUILD_FRAMEWORK)
# In the build-tree, we know the exact path to the framework directory.
Expand Down
45 changes: 35 additions & 10 deletions lldb/tools/driver/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,8 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args, bool &exiting) {
return error;
}

#if defined(_WIN32) && defined(LLDB_PYTHON_DLL_RELATIVE_PATH)
#ifdef _WIN32
#ifdef LLDB_PYTHON_DLL_RELATIVE_PATH
/// Returns the full path to the lldb.exe executable.
inline std::wstring GetPathToExecutableW() {
// Iterate until we reach the Windows API maximum path length (32,767).
Expand All @@ -447,32 +448,51 @@ inline std::wstring GetPathToExecutableW() {
return L"";
}

/// Resolve the full path of the directory defined by
/// \brief 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() {
/// \return `true` if the library was added to the search path.
/// `false` otherwise.
bool AddPythonDLLToSearchPath() {
std::wstring modulePath = GetPathToExecutableW();
if (modulePath.empty()) {
llvm::errs() << "error: unable to find python.dll." << '\n';
return;
return false;
}

SmallVector<char, MAX_PATH> utf8Path;
if (sys::windows::UTF16ToUTF8(modulePath.c_str(), modulePath.length(),
utf8Path))
return;
return false;
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;
return false;

if (sys::fs::exists(utf8Path))
SetDllDirectoryW(widePath.data());
return SetDllDirectoryW(widePath.data());
return false;
}
#endif

#ifdef LLDB_PYTHON_SHARED_LIBRARY_FILENAME
/// Returns whether `python3x.dll` is in the DLL search path.
bool IsPythonDLLInPath() {
#define WIDEN2(x) L##x
#define WIDEN(x) WIDEN2(x)
WCHAR foundPath[MAX_PATH];
DWORD result =
SearchPathW(nullptr, WIDEN(LLDB_PYTHON_SHARED_LIBRARY_FILENAME), nullptr,
MAX_PATH, foundPath, nullptr);
#undef WIDEN2
#undef WIDEN

return result > 0;
}
#endif
#endif

std::string EscapeString(std::string arg) {
std::string::size_type pos = 0;
Expand Down Expand Up @@ -776,8 +796,13 @@ int main(int argc, char const *argv[]) {
"~/Library/Logs/DiagnosticReports/.\n");
#endif

#if defined(_WIN32) && defined(LLDB_PYTHON_DLL_RELATIVE_PATH)
AddPythonDLLToSearchPath();
#if defined(_WIN32) && defined(LLDB_PYTHON_SHARED_LIBRARY_FILENAME)
if (!IsPythonDLLInPath())
#ifdef LLDB_PYTHON_DLL_RELATIVE_PATH
Copy link
Member

@mstorsjo mstorsjo Oct 23, 2025

Choose a reason for hiding this comment

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

This condition nesting doesn't seem right. If we have LLDB_PYTHON_DLL_RELATIVE_PATH defined, but we don't have LLDB_PYTHON_SHARED_LIBRARY_FILENAME, then we no longer will call AddPythonDLLToSearchPath at all - which we did before. Not sure what the neatest form of this is; maybe an #elif defined(LLDB_PYTHON_DLL_RELATIVE_PATH) AddPythonDLLToSearchPath(); #endif?

Copy link
Contributor Author

@charles-zablit charles-zablit Oct 23, 2025

Choose a reason for hiding this comment

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

My initial assumption was that LLDB_PYTHON_SHARED_LIBRARY_FILENAME would always be set if LLDB_PYTHON_DLL_RELATIVE_PATH is set, because it's set if we are linking against Python.

To resolve this I've extracted the logic into a function which made it easier by using return to control the execution flow.

Copy link
Member

@mstorsjo mstorsjo Oct 24, 2025

Choose a reason for hiding this comment

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

Yes, practically speaking (as that variable is set automatically by cmake), it should always be defined. But it might be good to have the code robust for all potential combinations anyway?

Copy link
Contributor Author

@charles-zablit charles-zablit Oct 24, 2025

Choose a reason for hiding this comment

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

Yes, practically speaking (as that variable is set automatically by cmake), it should always be defined. But it might be good to have the code robust for all potential combinations anyway?

I agree, the new logic allows for either of them (or both) to be defined.

if (!AddPythonDLLToSearchPath())
#endif
llvm::errs() << "error: unable to find "
<< LLDB_PYTHON_SHARED_LIBRARY_FILENAME << ".\n";
#endif

// Parse arguments.
Expand Down
Loading