Skip to content

Reland "[compiler-rt] Allow running tests without installing first" #88075

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

Conversation

arichardson
Copy link
Member

Currently, the testsuite uses the default runtimes path to find the
runtimes libraries which may or may not match the just-built runtimes.
This change uses the -resource-dir flag for clang whenever
COMPILER_RT_TEST_STANDALONE_BUILD_LIBS is set to ensure that we are
actually testing the currently built libraries rather than the ones
bundled with ${COMPILER_RT_TEST_COMPILER}.

The existing logic works fine when clang and compiler-rt share the same
build directory ``-DLLVM_ENABLE_PROJECTS=clang;compiler-rt`, but when
building compiler-rt separately we need to tell the compiler used for
the tests where it can find the just-built libraries.

This reduces the fixes check-all failures to one in my configuration:

cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -G Ninja
-DCMAKE_C_COMPILER=$HOME/output/upstream-llvm/bin/clang
-DCMAKE_CXX_COMPILER=$HOME/output/upstream-llvm/bin/clang++
-DCOMPILER_RT_INCLUDE_TESTS=ON
-DLLVM_EXTERNAL_LIT=$HOME/build/upstream-llvm-project-build/bin/llvm-lit
-DLLVM_CMAKE_DIR=$HOME/output/upstream-llvm
-DCOMPILER_RT_DEBUG=OFF
-S $HOME/src/upstream-llvm-project/compiler-rt
-B $HOME/src/upstream-llvm-project/compiler-rt/cmake-build-all-sanitizers

This relands the previous PR with fixes for Windows.
Depends on #88074 to be merged
first for GCC buildbots.

Created using spr 1.3.6-beta.1

[skip ci]
Created using spr 1.3.6-beta.1
@llvmbot
Copy link
Member

llvmbot commented Apr 9, 2024

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

Author: Alexander Richardson (arichardson)

Changes

Currently, the testsuite uses the default runtimes path to find the
runtimes libraries which may or may not match the just-built runtimes.
This change uses the -resource-dir flag for clang whenever
COMPILER_RT_TEST_STANDALONE_BUILD_LIBS is set to ensure that we are
actually testing the currently built libraries rather than the ones
bundled with ${COMPILER_RT_TEST_COMPILER}.

The existing logic works fine when clang and compiler-rt share the same
build directory ``-DLLVM_ENABLE_PROJECTS=clang;compiler-rt`, but when
building compiler-rt separately we need to tell the compiler used for
the tests where it can find the just-built libraries.

This reduces the fixes check-all failures to one in my configuration:

cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -G Ninja
-DCMAKE_C_COMPILER=$HOME/output/upstream-llvm/bin/clang
-DCMAKE_CXX_COMPILER=$HOME/output/upstream-llvm/bin/clang++
-DCOMPILER_RT_INCLUDE_TESTS=ON
-DLLVM_EXTERNAL_LIT=$HOME/build/upstream-llvm-project-build/bin/llvm-lit
-DLLVM_CMAKE_DIR=$HOME/output/upstream-llvm
-DCOMPILER_RT_DEBUG=OFF
-S $HOME/src/upstream-llvm-project/compiler-rt
-B $HOME/src/upstream-llvm-project/compiler-rt/cmake-build-all-sanitizers

This relands the previous PR with fixes for Windows.
Depends on #88074 to be merged
first for GCC buildbots.


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

7 Files Affected:

  • (modified) compiler-rt/CMakeLists.txt (+23)
  • (modified) compiler-rt/cmake/Modules/AddCompilerRT.cmake (+1)
  • (modified) compiler-rt/test/CMakeLists.txt (-8)
  • (modified) compiler-rt/test/fuzzer/lit.cfg.py (+1)
  • (modified) compiler-rt/test/lit.common.cfg.py (+67-22)
  • (modified) compiler-rt/test/lit.common.configured.in (+1)
  • (modified) compiler-rt/test/safestack/lit.cfg.py (+8-2)
diff --git a/compiler-rt/CMakeLists.txt b/compiler-rt/CMakeLists.txt
index f4e92f14db8570..8649507ce1c79b 100644
--- a/compiler-rt/CMakeLists.txt
+++ b/compiler-rt/CMakeLists.txt
@@ -584,6 +584,29 @@ string(APPEND COMPILER_RT_TEST_COMPILER_CFLAGS " ${stdlib_flag}")
 string(REPLACE " " ";" COMPILER_RT_UNITTEST_CFLAGS "${COMPILER_RT_TEST_COMPILER_CFLAGS}")
 set(COMPILER_RT_UNITTEST_LINK_FLAGS ${COMPILER_RT_UNITTEST_CFLAGS})
 
+option(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS
+  "When set to ON and testing in a standalone build, test the runtime \
+  libraries built by this standalone build rather than the runtime libraries \
+  shipped with the compiler (used for testing). When set to OFF and testing \
+  in a standalone build, test the runtime libraries shipped with the compiler \
+  (used for testing). This option has no effect if the compiler and this \
+  build are configured to use the same runtime library path."
+  ON)
+if (COMPILER_RT_TEST_STANDALONE_BUILD_LIBS)
+  # Ensure that the unit tests can find the sanitizer headers prior to installation.
+  list(APPEND COMPILER_RT_UNITTEST_CFLAGS "-I${CMAKE_CURRENT_LIST_DIR}/include")
+  # Ensure that unit tests link against the just-built runtime libraries instead
+  # of the ones bundled with the compiler by overriding the resource directory.
+  #
+  if ("${COMPILER_RT_TEST_COMPILER_ID}" MATCHES "Clang")
+    list(APPEND COMPILER_RT_UNITTEST_LINK_FLAGS "-resource-dir=${COMPILER_RT_OUTPUT_DIR}")
+  endif()
+  get_compiler_rt_output_dir(${COMPILER_RT_DEFAULT_TARGET_ARCH} rtlib_dir)
+  if (NOT WIN32)
+    list(APPEND COMPILER_RT_UNITTEST_LINK_FLAGS "-Wl,-rpath,${rtlib_dir}")
+  endif()
+endif()
+
 if(COMPILER_RT_USE_LLVM_UNWINDER)
   # We're linking directly against the libunwind that we're building so don't
   # try to link in the toolchain's default libunwind which may be missing.
diff --git a/compiler-rt/cmake/Modules/AddCompilerRT.cmake b/compiler-rt/cmake/Modules/AddCompilerRT.cmake
index e0400a8ea95222..567b123b7abcac 100644
--- a/compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ b/compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -768,6 +768,7 @@ function(configure_compiler_rt_lit_site_cfg input output)
   get_compiler_rt_output_dir(${COMPILER_RT_DEFAULT_TARGET_ARCH} output_dir)
 
   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_LIBRARY_OUTPUT_DIR ${output_dir})
 
   configure_lit_site_cfg(${input} ${output})
diff --git a/compiler-rt/test/CMakeLists.txt b/compiler-rt/test/CMakeLists.txt
index c186be1e44fd9a..edc007aaf477a7 100644
--- a/compiler-rt/test/CMakeLists.txt
+++ b/compiler-rt/test/CMakeLists.txt
@@ -1,14 +1,6 @@
 # Needed for lit support in standalone builds.
 include(AddLLVM)
 
-option(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS
-  "When set to ON and testing in a standalone build, test the runtime \
-  libraries built by this standalone build rather than the runtime libraries \
-  shipped with the compiler (used for testing). When set to OFF and testing \
-  in a standalone build, test the runtime libraries shipped with the compiler \
-  (used for testing). This option has no effect if the compiler and this \
-  build are configured to use the same runtime library path."
-  ON)
 pythonize_bool(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS)
 
 pythonize_bool(LLVM_ENABLE_EXPENSIVE_CHECKS)
diff --git a/compiler-rt/test/fuzzer/lit.cfg.py b/compiler-rt/test/fuzzer/lit.cfg.py
index 9084254b3b15cd..0c0550ac729d61 100644
--- a/compiler-rt/test/fuzzer/lit.cfg.py
+++ b/compiler-rt/test/fuzzer/lit.cfg.py
@@ -106,6 +106,7 @@ def generate_compiler_cmd(is_cpp=True, fuzzer_enabled=True, msan_enabled=False):
     return " ".join(
         [
             compiler_cmd,
+            config.target_cflags,
             std_cmd,
             "-O2 -gline-tables-only",
             sanitizers_cmd,
diff --git a/compiler-rt/test/lit.common.cfg.py b/compiler-rt/test/lit.common.cfg.py
index 0ac20a9831d950..28f126a11b16b9 100644
--- a/compiler-rt/test/lit.common.cfg.py
+++ b/compiler-rt/test/lit.common.cfg.py
@@ -14,6 +14,27 @@
 import lit.util
 
 
+def get_path_from_clang(args, allow_failure):
+    clang_cmd = [
+        config.clang.strip(),
+        f"--target={config.target_triple}",
+        *args,
+    ]
+    path = None
+    try:
+        result = subprocess.run(
+            clang_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True
+        )
+        path = result.stdout.decode().strip()
+    except subprocess.CalledProcessError as e:
+        msg = f"Failed to run {clang_cmd}\nrc:{e.returncode}\nstdout:{e.stdout}\ne.stderr{e.stderr}"
+        if allow_failure:
+            lit_config.warning(msg)
+        else:
+            lit_config.fatal(msg)
+    return path, clang_cmd
+
+
 def find_compiler_libdir():
     """
     Returns the path to library resource directory used
@@ -26,26 +47,6 @@ def find_compiler_libdir():
         # TODO: Support other compilers.
         return None
 
-    def get_path_from_clang(args, allow_failure):
-        clang_cmd = [
-            config.clang.strip(),
-            f"--target={config.target_triple}",
-        ]
-        clang_cmd.extend(args)
-        path = None
-        try:
-            result = subprocess.run(
-                clang_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True
-            )
-            path = result.stdout.decode().strip()
-        except subprocess.CalledProcessError as e:
-            msg = f"Failed to run {clang_cmd}\nrc:{e.returncode}\nstdout:{e.stdout}\ne.stderr{e.stderr}"
-            if allow_failure:
-                lit_config.warning(msg)
-            else:
-                lit_config.fatal(msg)
-        return path, clang_cmd
-
     # Try using `-print-runtime-dir`. This is only supported by very new versions of Clang.
     # so allow failure here.
     runtime_dir, clang_cmd = get_path_from_clang(
@@ -130,6 +131,7 @@ def push_dynamic_library_lookup_path(config, new_path):
     config.available_features.add("shell")
 
 target_is_msvc = bool(re.match(r".*-windows-msvc$", config.target_triple))
+target_is_windows = bool(re.match(r".*-windows.*$", config.target_triple))
 
 compiler_id = getattr(config, "compiler_id", None)
 if compiler_id == "Clang":
@@ -168,10 +170,53 @@ def push_dynamic_library_lookup_path(config, new_path):
             r"/i386(?=-[^/]+$)", "/x86_64", config.compiler_rt_libdir
         )
 
+
+# Check if the test compiler resource dir matches the local build directory
+# (which happens with -DLLVM_ENABLE_PROJECTS=clang;compiler-rt) or if we are
+# using an installed clang to test compiler-rt standalone. In the latter case
+# we may need to override the resource dir to match the path of the just-built
+# compiler-rt libraries.
+test_cc_resource_dir, _ = get_path_from_clang(
+    shlex.split(config.target_cflags) + ["-print-resource-dir"], allow_failure=True
+)
+# Normalize the path for comparison
+if test_cc_resource_dir is not None:
+    test_cc_resource_dir = os.path.realpath(test_cc_resource_dir)
+if lit_config.debug:
+    lit_config.note(f"Resource dir for {config.clang} is {test_cc_resource_dir}")
+local_build_resource_dir = os.path.realpath(config.compiler_rt_output_dir)
+if test_cc_resource_dir != local_build_resource_dir and config.test_standalone_build_libs:
+    if config.compiler_id == "Clang":
+        if lit_config.debug:
+            lit_config.note(
+                f"Overriding test compiler resource dir to use "
+                f'libraries in "{config.compiler_rt_libdir}"'
+            )
+        # Ensure that we use the just-built static libraries when linking by
+        # overriding the Clang resource directory. Additionally, we want to use
+        # the builtin headers shipped with clang (e.g. stdint.h), so we
+        # explicitly add this as an include path (since the headers are not
+        # going to be in the current compiler-rt build directory).
+        # We also tell the linker to add an RPATH entry for the local library
+        # directory so that the just-built shared libraries are used.
+        config.target_cflags += f" -nobuiltininc"
+        config.target_cflags += f" -I{config.compiler_rt_src_root}/include"
+        config.target_cflags += f" -idirafter {test_cc_resource_dir}/include"
+        config.target_cflags += f" -resource-dir={config.compiler_rt_output_dir}"
+        if not target_is_windows:
+            # Avoid passing -rpath on Windows where it is not supported.
+            config.target_cflags += f" -Wl,-rpath,{config.compiler_rt_libdir}"
+    else:
+        lit_config.warning(
+            f"Cannot override compiler-rt library directory with non-Clang "
+            f"compiler: {config.compiler_id}"
+        )
+
+
 # Ask the compiler for the path to libraries it is going to use. If this
 # doesn't match config.compiler_rt_libdir then it means we might be testing the
 # compiler's own runtime libraries rather than the ones we just built.
-# Warn about about this and handle appropriately.
+# Warn about this and handle appropriately.
 compiler_libdir = find_compiler_libdir()
 if compiler_libdir:
     compiler_rt_libdir_real = os.path.realpath(config.compiler_rt_libdir)
@@ -182,7 +227,7 @@ def push_dynamic_library_lookup_path(config, new_path):
             f'compiler-rt libdir:  "{compiler_rt_libdir_real}"'
         )
         if config.test_standalone_build_libs:
-            # Use just built runtime libraries, i.e. the the libraries this built just built.
+            # Use just built runtime libraries, i.e. the libraries this build just built.
             if not config.test_suite_supports_overriding_runtime_lib_path:
                 # Test suite doesn't support this configuration.
                 # TODO(dliew): This should be an error but it seems several bots are
diff --git a/compiler-rt/test/lit.common.configured.in b/compiler-rt/test/lit.common.configured.in
index fff5dc6cc75094..8889b816b149fc 100644
--- a/compiler-rt/test/lit.common.configured.in
+++ b/compiler-rt/test/lit.common.configured.in
@@ -27,6 +27,7 @@ set_default("compiler_id", "@COMPILER_RT_TEST_COMPILER_ID@")
 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_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/compiler-rt/test/safestack/lit.cfg.py b/compiler-rt/test/safestack/lit.cfg.py
index adf27a0d7e5eab..aadb8bf0d5c77b 100644
--- a/compiler-rt/test/safestack/lit.cfg.py
+++ b/compiler-rt/test/safestack/lit.cfg.py
@@ -13,10 +13,16 @@
 
 # Add clang substitutions.
 config.substitutions.append(
-    ("%clang_nosafestack ", config.clang + " -O0 -fno-sanitize=safe-stack ")
+    (
+        "%clang_nosafestack ",
+        config.clang + config.target_cflags + " -O0 -fno-sanitize=safe-stack ",
+    )
 )
 config.substitutions.append(
-    ("%clang_safestack ", config.clang + " -O0 -fsanitize=safe-stack ")
+    (
+        "%clang_safestack ",
+        config.clang + config.target_cflags + " -O0 -fsanitize=safe-stack ",
+    )
 )
 
 if config.lto_supported:

@arichardson
Copy link
Member Author

@mstorsjo It would be great if you could test this on windows.

@mstorsjo
Copy link
Member

mstorsjo commented Apr 9, 2024

@mstorsjo It would be great if you could test this on windows.

Thanks, this does seem to run fine in my configuration!

@arichardson
Copy link
Member Author

@mstorsjo It would be great if you could test this on windows.

Thanks, this does seem to run fine in my configuration!

Thank you very much for testing!

Created using spr 1.3.6-beta.1

[skip ci]
Created using spr 1.3.6-beta.1
@arichardson arichardson changed the base branch from users/arichardson/spr/main.reland-compiler-rt-allow-running-tests-without-installing-first to main April 12, 2024 20:20
@arichardson arichardson merged commit dfafe38 into main Apr 12, 2024
@arichardson arichardson deleted the users/arichardson/spr/reland-compiler-rt-allow-running-tests-without-installing-first branch April 12, 2024 20:20
bazuzi pushed a commit to bazuzi/llvm-project that referenced this pull request Apr 15, 2024
Currently, the testsuite uses the default runtimes path to find the
runtimes libraries which may or may not match the just-built runtimes.
This change uses the `-resource-dir` flag for clang whenever
`COMPILER_RT_TEST_STANDALONE_BUILD_LIBS` is set to ensure that we are
actually testing the currently built libraries rather than the ones
bundled with `${COMPILER_RT_TEST_COMPILER}`.

The existing logic works fine when clang and compiler-rt share the same
build directory ``-DLLVM_ENABLE_PROJECTS=clang;compiler-rt`, but when
building compiler-rt separately we need to tell the compiler used for
the tests where it can find the just-built libraries.

This reduces the fixes check-all failures to one in my configuration:
```
cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -G Ninja
-DCMAKE_C_COMPILER=$HOME/output/upstream-llvm/bin/clang
-DCMAKE_CXX_COMPILER=$HOME/output/upstream-llvm/bin/clang++
-DCOMPILER_RT_INCLUDE_TESTS=ON
-DLLVM_EXTERNAL_LIT=$HOME/build/upstream-llvm-project-build/bin/llvm-lit
-DLLVM_CMAKE_DIR=$HOME/output/upstream-llvm
-DCOMPILER_RT_DEBUG=OFF
-S $HOME/src/upstream-llvm-project/compiler-rt
-B $HOME/src/upstream-llvm-project/compiler-rt/cmake-build-all-sanitizers
```

This relands the previous PR with fixes for Windows.
Depends on llvm#88074 to be merged
first for GCC buildbots.

Pull Request: llvm#88075
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants