mstorsjo wrote:
Now I've had time to study lldb-rpc-gen a little bit closer...
For myself and others, to clarify what lldb-rpc-gen does (as how command
options flow into it that aren't immediately obvious): It is a tool that
includes clang tooling libraries. It runs at the regular LLVM/LLDB build
directory, reading `compile_commans.json` from there, then using them to figure
out how to compile certain files, and use those to preprocess(?) and parse some
LLDB headers.
This is in a rather large contrast to other existing build time code generation
tools (like `llvm-tblgen` or `clang-tidy-confusable-chars-gen`); those are
relatively small executables that on their own generate code to be used during
the build - and those tools are mostly independent of the build environment.
Your initial attempts seem quite close to actually getting this right (already
attempting to use `get_host_tool_path` etc), but both the initial attempt and
this one missed one subtle detail: The cmake functions `get_host_tool_path` and
`setup_host_tool` do take a cmake variable name `LLDB_RPC_GEN_EXE` as input,
which allows the user to set this variable if they want to point to the tool,
but it doesn't set the final path in that variable, but in a different variable.
I managed to simplify the setup of all this, and got a cross build to actually
successfully build and use a nested cmake build to build `lldb-rpc-gen`, just
like it does for other tools. My diff to achieve this looks like this:
```diff
diff --git a/lldb/cmake/modules/LLDBConfig.cmake b/lldb/cmake/modules/LLDBConfig
.cmake
index 88b4eca4112c..f108110ff94b 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -334,11 +334,4 @@ else()
endif()
-if (CMAKE_CROSSCOMPILING)
- set(LLDB_BUILD_LLDBRPC OFF CACHE BOOL "")
- get_host_tool_path(lldb-rpc-gen LLDB_RPC_GEN_EXE lldb_rpc_gen_exe
lldb_rpc_gen_target)
-else()
- set(LLDB_BUILD_LLDBRPC ON CACHE BOOL "")
-endif()
-
include(LLDBGenerateConfig)
diff --git a/lldb/tools/CMakeLists.txt b/lldb/tools/CMakeLists.txt
index e2f039527ad7..d3096765ad8f 100644
--- a/lldb/tools/CMakeLists.txt
+++ b/lldb/tools/CMakeLists.txt
@@ -10,9 +10,7 @@ add_subdirectory(lldb-fuzzer EXCLUDE_FROM_ALL)
add_lldb_tool_subdirectory(lldb-instr)
add_lldb_tool_subdirectory(lldb-dap)
-if (LLDB_BUILD_LLDBRPC)
- add_lldb_tool_subdirectory(lldb-rpc-gen)
-endif()
+add_lldb_tool_subdirectory(lldb-rpc-gen)
if (LLDB_CAN_USE_LLDB_RPC_SERVER)
add_subdirectory(lldb-rpc)
endif()
diff --git a/lldb/tools/lldb-rpc-gen/CMakeLists.txt
b/lldb/tools/lldb-rpc-gen/CMakeLists.txt
index 4b2e5d7483a5..7a50072865ab 100644
--- a/lldb/tools/lldb-rpc-gen/CMakeLists.txt
+++ b/lldb/tools/lldb-rpc-gen/CMakeLists.txt
@@ -18,11 +18,6 @@ add_lldb_tool(lldb-rpc-gen
Support
)
-if (CMAKE_CROSSCOMPILING)
- setup_host_tool(lldb-rpc-gen LLDB_RPC_GEN_EXE lldb_rpc_gen_exe
lldb_rpc_gen_target)
-else()
- if (NOT DEFINED LLDB_RPC_GEN_EXE)
- set(LLDB_RPC_GEN_EXE $<TARGET_FILE:lldb-rpc-gen> CACHE STRING "Executable
that generates lldb-rpc-server")
- endif()
-endif()
+add_dependencies(lldb-rpc-gen clang-resource-headers)
+setup_host_tool(lldb-rpc-gen LLDB_RPC_GEN_EXE lldb_rpc_gen_exe
lldb_rpc_gen_target)
diff --git a/lldb/tools/lldb-rpc/LLDBRPCGeneration.cmake
b/lldb/tools/lldb-rpc/LLDBRPCGeneration.cmake
index a4cacf8692a8..91188ca393a1 100644
--- a/lldb/tools/lldb-rpc/LLDBRPCGeneration.cmake
+++ b/lldb/tools/lldb-rpc/LLDBRPCGeneration.cmake
@@ -1,9 +1,3 @@
-if (NOT DEFINED LLDB_RPC_GEN_EXE)
- message(FATAL_ERROR
- "Unable to generate lldb-rpc sources because LLDB_RPC_GEN_EXE is not
- defined. If you are cross-compiling, please build lldb-rpc-gen for your
host
- platform.")
-endif()
set(lldb_rpc_generated_dir "${CMAKE_CURRENT_BINARY_DIR}/generated")
set(lldb_rpc_server_generated_source_dir "${lldb_rpc_generated_dir}/server")
@@ -60,14 +54,14 @@ add_custom_command(OUTPUT ${lldb_rpc_gen_byproducts}
COMMAND ${CMAKE_COMMAND} -E make_directory
${lldb_rpc_server_generated_source_dir}
- COMMAND ${LLDB_RPC_GEN_EXE}
+ COMMAND ${lldb_rpc_gen_exe}
-p ${CMAKE_BINARY_DIR}
--output-dir=${lldb_rpc_generated_dir}
${sysroot_arg}
--extra-arg="-USWIG"
${api_headers}
- DEPENDS ${LLDB_RPC_GEN_EXE} ${api_headers}
+ DEPENDS ${lldb_rpc_gen_target} ${api_headers}
COMMENT "Generating sources for lldb-rpc-server..."
WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
)
```
I had to add the dependency on `clang-resource-headers`, so that the tool
`<builddir>/NATIVE/bin/lldb-rpc-gen` gets the clang resource headers it expects
in `<builddir>/NATIVE/lib/clang/<version>/include`.
However, even with all this in place, I fear that it may be brittle - in
particular during cross builds. (I wouldn't always assume that the clang
tooling flawlessly would figure out the ins and outs if I'm using an unusual
cross compiler.)
And secondly - building `lldb-rpc-gen` requires building most of LLVM and
Clang, which adds a lot of extra work to cross builds. Currently, when cross
compiling, it builds `llvm-tblgen`, `clang-tblgen` and `lldb-tblgen` and
similar tools natively - which is extra overhead to the build. But this is
usually just a couple hundred object files - so it's easier to just let cmake
do that, rather than to fiddle with manually pointing to a preexisting native
build (with the risk of it being out of sync etc). But here, this essentially
would make the nested native build duplicate 70% of the whole llvm build -
which is quite out of scope for what's probably expected.
I also see that the existing logic for setting up lldb-rpc here does seem to
try to account for this to some extent:
```
# In a cross-compile build, we need to skip building the generated
# lldb-rpc sources in the first phase of host build so that they can
# get built using the just-built Clang toolchain in the second phase.
if ((CMAKE_CROSSCOMPILING OR LLVM_HOST_TRIPLE MATCHES
"${LLVM_DEFAULT_TARGET_TRIPLE}") AND
```
But it's very unclear to me exactly how this is meant to work - the condition
seems to miss a negation with regards to `CMAKE_CROSS_COMPILING`? (And the
check for `LLVM_HOST_TRIPLE` and `LLVM_DEFAULT_TARGET_TRIPLE` seems unusual
here - we don't normally check `LLVM_DEFAULT_TARGET_TRIPLE` like that - just
checking `CMAKE_CROSSCOMPILING` should be enough.)
So we can probably do something like my inline diff above, to make the build
succeed _if_ we want to build this, but we probably should default to having
this disabled in general:
```diff
diff --git a/lldb/cmake/modules/LLDBConfig.cmake
b/lldb/cmake/modules/LLDBConfig.cmake
index f108110ff94b..388e37d0da78 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -323,14 +323,16 @@ else()
set(LLDB_CAN_USE_DEBUGSERVER OFF)
endif()
-# In a cross-compile build, we need to skip building the generated
-# lldb-rpc sources in the first phase of host build so that they can
-# get built using the just-built Clang toolchain in the second phase.
-if ((CMAKE_CROSSCOMPILING OR LLVM_HOST_TRIPLE MATCHES
"${LLVM_DEFAULT_TARGET_TRIPLE}") AND
- CMAKE_SYSTEM_NAME MATCHES
"AIX|Android|Darwin|FreeBSD|Linux|NetBSD|OpenBSD|Windows")
- set(LLDB_CAN_USE_LLDB_RPC_SERVER ON)
-else()
- set(LLDB_CAN_USE_LLDB_RPC_SERVER OFF)
+if (NOT DEFINED LLDB_CAN_USE_LLDB_RPC_SERVER)
+ # In a cross-compile build, we don't want to try to build a lldb-rpc-gen, as
+ # as that essentially forces rebuilding a whole separate native copy of
clang.
+ # Allow the caller to set this variable manually to opt in/out of it.
+ if (NOT CMAKE_CROSSCOMPILING AND
+ CMAKE_SYSTEM_NAME MATCHES
"AIX|Android|Darwin|FreeBSD|Linux|NetBSD|OpenBSD|Windows")
+ set(LLDB_CAN_USE_LLDB_RPC_SERVER ON)
+ else()
+ set(LLDB_CAN_USE_LLDB_RPC_SERVER OFF)
+ endif()
endif()
```
https://github.com/llvm/llvm-project/pull/151603
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
