[WIP] Enable python for LLDB.#332
Conversation
efriedma-quic
force-pushed
the
lldb-enable-python
branch
from
5384e66 to
1407cb4
Compare
jonathonpenix
left a comment
jonathonpenix
left a comment
There was a problem hiding this comment.
lldb tests still fail on x86 hosts due to the default target triple; there should be some way to fix this, but the comment indicates we already have a workaround for this?
When I poked at this/from discussing with Ted in the past, I don't remember seeing a nice way to do this.
At the time, I hacked around this via LLVM_TARGET_TRIPLE_ENV--it's by no means robust/complete, but I think the code I was playing with is here if it's helpful. I think I remember this working, but it isn't exactly pretty.
There was a problem hiding this comment.
This change looks good to me I think/seems like it should fix the issue for our specific setup.
I'm a bit curious if this has issues if someone tried to build LLDB standalone via add_subdirectory (since I think then we'll hit set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin) in LLDBStandalone.cmake?).
But, given LLVM_RUNTIME_OUTPUT_INTDIR seems to be used quite a bit in lldb, I don't think that's unique to this change/seems like a separate issue.
The change to lldb/CMakeLists.txt fixes the path issue: it makes sure to copy the python scripts into the build directory relative to the lldb executable, instead of trying to indirectly figure out where lldb will be installed. I think this can be upstreamed. Also added a clang wrapper script to fix lldb. This seems to mostly work? I'm seeing a few local failures, not sure why.
efriedma-quic
force-pushed
the
lldb-enable-python
branch
from
1407cb4 to
26251a0
Compare
There was a problem hiding this comment.
Maybe would be worth replacing the x86_64-linux-gnu with some CMake var that contains the triple? Not sure if there is something in cmake proper that will be set here, but I imagine something from LLVM (LLVM_HOST_TRIPLE?) might be ok too?
I think whenever we turn on the LLDB tests, they'll get run in AArch64 Linux, not sure about Windows.
There was a problem hiding this comment.
I don't think we need the wrapper on aarch64 linux; we default to the native target.
Also, it seems like this doesn't work completely; some invocations of clang don't go through the wrapper, so I'm not sure this is actually what we want to do; I just wanted to throw this on the buildbot to see what happened. And what happened is that eld tests failed. :(
There was a problem hiding this comment.
Oh, oops--forgot about that on aarch64.
FWIW, I think that eld issue should be resolved at this point (I think I remember seeing it in some other PRs in the past but seems to be gone from our nightlies now) if you want to rerun
2 participants
The change to lldb/CMakeLists.txt fixes the path issue: it makes sure to copy the python scripts into the build directory relative to the lldb executable, instead of trying to indirectly figure out where lldb will be installed. I think this can be upstreamed.
lldb tests still fail on x86 hosts due to the default target triple; there should be some way to fix this, but the comment indicates we already have a workaround for this?