Skip to content
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

[runtimes] Allow building against an installed LLVM tree #86209

Open
wants to merge 2 commits into
base: users/arichardson/spr/main.runtimes-allow-building-against-an-installed-llvm-tree
Choose a base branch
from

Conversation

arichardson
Copy link
Member

I am currently trying to test the LLVM runtimes (including compiler-rt)
against an installed LLVM tree rather than a build tree (since that is
no longer available). Currently, the runtimes build of compiler-rt assumes
that LLVM_BINARY_DIR is writable since it uses configure_file() to write
there during the CMake configure stage. Instead, generate this file inside
CMAKE_CURRENT_BINARY_DIR, which will match LLVM_BINARY_DIR when invoked
from llvm/runtimes/CMakeLists.txt.

I also needed to make a minor change to the hwasan tests: hwasan_symbolize
was previously found in the LLVM_BINARY_DIR, but since it is generated as
part of the compiler-rt build it is now inside the CMake build directory
instead. I fixed this by passing the output directory to lit as
config.compiler_rt_bindir and using llvm_config.add_tool_substitutions().

For testing that we no longer write to the LLVM install directory as
part of testing or configuration, I created a read-only bind mount and
configured the runtimes builds as follows:

$ sudo mount --bind --read-only ~/llvm-install /tmp/upstream-llvm-readonly
$ cmake -DCMAKE_BUILD_TYPE=Debug \
  -DCMAKE_C_COMPILER=/tmp/upstream-llvm-readonly/bin/clang \
  -DCMAKE_CXX_COMPILER=/tmp/upstream-llvm-readonly/bin/clang++ \
  -DLLVM_INCLUDE_TESTS=TRUE -DLLVM_ENABLE_ASSERTIONS=TRUE \
  -DCOMPILER_RT_INCLUDE_TESTS=TRUE -DCOMPILER_RT_DEBUG=OFF \
  -DLLVM_ENABLE_RUNTIMES=compiler-rt \
  -DLLVM_BINARY_DIR=/tmp/upstream-llvm-readonly \
  -G Ninja -S ~/upstream-llvm-project/runtimes \
  -B ~/upstream-llvm-project/runtimes/cmake-build-debug-llvm-git

Created using spr 1.3.6-beta.1
@arichardson arichardson requested a review from a team as a code owner March 21, 2024 22:23
@llvmbot llvmbot added cmake Build system in general and CMake in particular compiler-rt compiler-rt:hwasan Hardware-assisted address sanitizer compiler-rt:sanitizer labels Mar 21, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 21, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Alexander Richardson (arichardson)

Changes

I am currently trying to test the LLVM runtimes (including compiler-rt)
against an installed LLVM tree rather than a build tree (since that is
no longer available). Currently, the runtimes build of compiler-rt assumes
that LLVM_BINARY_DIR is writable since it uses configure_file() to write
there during the CMake configure stage. Instead, generate this file inside
CMAKE_CURRENT_BINARY_DIR, which will match LLVM_BINARY_DIR when invoked
from llvm/runtimes/CMakeLists.txt.

I also needed to make a minor change to the hwasan tests: hwasan_symbolize
was previously found in the LLVM_BINARY_DIR, but since it is generated as
part of the compiler-rt build it is now inside the CMake build directory
instead. I fixed this by passing the output directory to lit as
config.compiler_rt_bindir and using llvm_config.add_tool_substitutions().

For testing that we no longer write to the LLVM install directory as
part of testing or configuration, I created a read-only bind mount and
configured the runtimes builds as follows:

$ sudo mount --bind --read-only ~/llvm-install /tmp/upstream-llvm-readonly
$ cmake -DCMAKE_BUILD_TYPE=Debug \
  -DCMAKE_C_COMPILER=/tmp/upstream-llvm-readonly/bin/clang \
  -DCMAKE_CXX_COMPILER=/tmp/upstream-llvm-readonly/bin/clang++ \
  -DLLVM_INCLUDE_TESTS=TRUE -DLLVM_ENABLE_ASSERTIONS=TRUE \
  -DCOMPILER_RT_INCLUDE_TESTS=TRUE -DCOMPILER_RT_DEBUG=OFF \
  -DLLVM_ENABLE_RUNTIMES=compiler-rt \
  -DLLVM_BINARY_DIR=/tmp/upstream-llvm-readonly \
  -G Ninja -S ~/upstream-llvm-project/runtimes \
  -B ~/upstream-llvm-project/runtimes/cmake-build-debug-llvm-git

Full diff: https://github.com/llvm/llvm-project/pull/86209.diff

4 Files Affected:

  • (modified) compiler-rt/cmake/Modules/AddCompilerRT.cmake (+1)
  • (modified) compiler-rt/test/hwasan/lit.cfg.py (+7)
  • (modified) compiler-rt/test/lit.common.configured.in (+1)
  • (modified) runtimes/CMakeLists.txt (+18-10)
diff --git a/compiler-rt/cmake/Modules/AddCompilerRT.cmake b/compiler-rt/cmake/Modules/AddCompilerRT.cmake
index 567b123b7abcac..4c3752d14a4789 100644
--- a/compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ b/compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -769,6 +769,7 @@ function(configure_compiler_rt_lit_site_cfg input output)
 
   string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} COMPILER_RT_RESOLVED_TEST_COMPILER ${COMPILER_RT_TEST_COMPILER})
   string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} COMPILER_RT_RESOLVED_OUTPUT_DIR ${COMPILER_RT_OUTPUT_DIR})
+  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} COMPILER_RT_RESOLVED_EXEC_OUTPUT_DIR ${COMPILER_RT_EXEC_OUTPUT_DIR})
   string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR ${output_dir})
 
   configure_lit_site_cfg(${input} ${output})
diff --git a/compiler-rt/test/hwasan/lit.cfg.py b/compiler-rt/test/hwasan/lit.cfg.py
index 594f3294a84ac1..96cc2f477ad445 100644
--- a/compiler-rt/test/hwasan/lit.cfg.py
+++ b/compiler-rt/test/hwasan/lit.cfg.py
@@ -2,6 +2,9 @@
 
 import os
 
+from lit.llvm import llvm_config
+from lit.llvm.subst import ToolSubst, FindTool
+
 # Setup config name.
 config.name = "HWAddressSanitizer" + getattr(config, "name_suffix", "default")
 
@@ -74,6 +77,10 @@ def build_invocation(compile_flags):
     ("%env_hwasan_opts=", "env HWASAN_OPTIONS=" + default_hwasan_opts_str)
 )
 
+# Ensure that we can use hwasan_symbolize from the expected location
+llvm_config.add_tool_substitutions([ToolSubst("hwasan_symbolize", unresolved="fatal")],
+                                   search_dirs=[config.compiler_rt_bindir])
+
 # Default test suffixes.
 config.suffixes = [".c", ".cpp"]
 
diff --git a/compiler-rt/test/lit.common.configured.in b/compiler-rt/test/lit.common.configured.in
index 8955efa3eafd18..e146e847aa00e6 100644
--- a/compiler-rt/test/lit.common.configured.in
+++ b/compiler-rt/test/lit.common.configured.in
@@ -28,6 +28,7 @@ set_default("python_executable", "@Python3_EXECUTABLE@")
 set_default("compiler_rt_debug", @COMPILER_RT_DEBUG_PYBOOL@)
 set_default("compiler_rt_intercept_libdispatch", @COMPILER_RT_INTERCEPT_LIBDISPATCH_PYBOOL@)
 set_default("compiler_rt_output_dir", "@COMPILER_RT_RESOLVED_OUTPUT_DIR@")
+set_default("compiler_rt_bindir", "@COMPILER_RT_RESOLVED_EXEC_OUTPUT_DIR@")
 set_default("compiler_rt_libdir", "@COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR@")
 set_default("emulator", "@COMPILER_RT_EMULATOR@")
 set_default("asan_shadow_scale", "@COMPILER_RT_ASAN_SHADOW_SCALE@")
diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt
index 6f24fbcccec955..3195a293c0adfa 100644
--- a/runtimes/CMakeLists.txt
+++ b/runtimes/CMakeLists.txt
@@ -218,6 +218,22 @@ foreach(entry ${runtimes})
 endforeach()
 
 if(LLVM_INCLUDE_TESTS)
+  # Add lit if needed before adding any runtimes since their CMake tests
+  # configuration might depend on lit being present.
+  if (NOT HAVE_LLVM_LIT)
+    # If built by manually invoking cmake on this directory, we don't have
+    # llvm-lit. If invoked via llvm/runtimes, the toplevel llvm cmake
+    # invocation already generated the llvm-lit script.
+    set(LLVM_LIT_OUTPUT_DIR ${CMAKE_CURRENT_BINARY_DIR}/bin)
+    add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit
+                     ${CMAKE_CURRENT_BINARY_DIR}/llvm-lit)
+    # Ensure that the testsuites use the local lit rather than
+    # LLVM_INSTALL_DIR/bin/llvm-lit (which may not exist if LLVM_BINARY_DIR
+    # points at an installed LLVM tree rather than a build tree.
+    get_llvm_lit_path(_base_dir _file_name)
+    set(LLVM_EXTERNAL_LIT "${_base_dir}/${_file_name}" CACHE STRING "Command used to spawn lit" FORCE)
+  endif()
+
   set(LIT_ARGS_DEFAULT "-sv --show-xfail --show-unsupported")
   if (MSVC OR XCODE)
     set(LIT_ARGS_DEFAULT "${LIT_ARGS_DEFAULT} --no-progress-bar")
@@ -251,14 +267,6 @@ if(LLVM_INCLUDE_TESTS)
   # and we know the total set of lit testsuites.
   umbrella_lit_testsuite_end(check-runtimes)
 
-  if (NOT HAVE_LLVM_LIT)
-    # If built by manually invoking cmake on this directory, we don't have
-    # llvm-lit. If invoked via llvm/runtimes, the toplevel llvm cmake
-    # invocation already generated the llvm-lit script.
-    add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit
-                     ${CMAKE_CURRENT_BINARY_DIR}/llvm-lit)
-  endif()
-
   get_property(LLVM_RUNTIMES_LIT_TESTSUITES GLOBAL PROPERTY LLVM_RUNTIMES_LIT_TESTSUITES)
   string(REPLACE ";" "\n" LLVM_RUNTIMES_LIT_TESTSUITES "${LLVM_RUNTIMES_LIT_TESTSUITES}")
   file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/lit.tests ${LLVM_RUNTIMES_LIT_TESTSUITES})
@@ -288,10 +296,10 @@ if(SUB_COMPONENTS)
   if(LLVM_RUNTIMES_TARGET)
     configure_file(
       ${CMAKE_CURRENT_SOURCE_DIR}/Components.cmake.in
-      ${LLVM_BINARY_DIR}/runtimes/${LLVM_RUNTIMES_TARGET}/Components.cmake)
+      ${CMAKE_CURRENT_BINARY_DIR}/runtimes/${LLVM_RUNTIMES_TARGET}/Components.cmake)
   else()
     configure_file(
       ${CMAKE_CURRENT_SOURCE_DIR}/Components.cmake.in
-      ${LLVM_BINARY_DIR}/runtimes/Components.cmake)
+      ${CMAKE_CURRENT_BINARY_DIR}/runtimes/Components.cmake)
   endif()
 endif()

Comment on lines 230 to 234
# Ensure that the testsuites use the local lit rather than
# LLVM_INSTALL_DIR/bin/llvm-lit (which may not exist if LLVM_BINARY_DIR
# points at an installed LLVM tree rather than a build tree.
get_llvm_lit_path(_base_dir _file_name)
set(LLVM_EXTERNAL_LIT "${_base_dir}/${_file_name}" CACHE STRING "Command used to spawn lit" FORCE)
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand. We must have lit available since we are in the runtimes/CMakeLists.txt? I actually don't understand why the if (NOT HAVE_LLVM_LIT) branch is ever taken. Can you shed some light on this?

Copy link
Member Author

Choose a reason for hiding this comment

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

This should be explained by the existing comment:

If built by manually invoking cmake on this directory, we don't have
llvm-lit. If invoked via llvm/runtimes, the toplevel llvm cmake
invocation already generated the llvm-lit script.

Copy link
Member

Choose a reason for hiding this comment

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

Ahhhhh. I missed the distinction between llvm/runtimes and just runtimes. This could be made clearer by saying something like

If built with the runtimes build (rooted at runtimes/CMakeLists.txt), we don't have
llvm-lit. If built with the bootstrapping build (rooted at llvm/CMakeLists.txt), the
top-level llvm CMake invocation already generated the llvm-lit script.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry for the delay here, was waiting to land the dependent PRs. Updated now.

Created using spr 1.3.6-beta.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake Build system in general and CMake in particular compiler-rt:hwasan Hardware-assisted address sanitizer compiler-rt:sanitizer compiler-rt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants