-
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
Changes from 1 commit
37cf46e
2f7a6f1
e9625a4
93c0a4b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any particular reason for using 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This used to be gated behind a
I agree, this is much better 👍 |
||
endif() | ||
endif() | ||
if(CLANG_LINK_CLANG_DYLIB) | ||
target_link_libraries(${name} PRIVATE clang-cpp) | ||
else() | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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 | ||||||
|
// Returns the full path to the lldb.exe executable | |
// 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.
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.
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.
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:
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
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 thanks!