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

[compiler-rt] Don't check COMPILER_RT_STANDALONE_BUILD for test deps #83651

Conversation

arichardson
Copy link
Member

With #83088, we now need the
runtimes to be built before running test if
COMPILER_RT_TEST_STANDALONE_BUILD_LIBS is true, since otherwise we
get failures running ninja check-all such as the following:

/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.fuzzer-x86_64.a: No such file or directory
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.xray-x86_64.a: No such file or directory
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.xray-basic-x86_64.a: No such file or directory
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.xray-fdr-x86_64.a: No such file or directory

This is a follow-up to 058e9b0 which started removing these checks
and it should make it easier to stop forcing COMPILER_RT_STANDALONE_BUILD
for runtimes builds in the future.

@llvmbot llvmbot added compiler-rt compiler-rt:asan Address sanitizer compiler-rt:fuzzer compiler-rt:ubsan Undefined behavior sanitizer xray PGO Profile Guided Optimizations compiler-rt:msan Memory sanitizer compiler-rt:sanitizer labels Mar 2, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 2, 2024

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

@llvm/pr-subscribers-pgo

Author: Alexander Richardson (arichardson)

Changes

With #83088, we now need the
runtimes to be built before running test if
COMPILER_RT_TEST_STANDALONE_BUILD_LIBS is true, since otherwise we
get failures running ninja check-all such as the following:

/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.fuzzer-x86_64.a: No such file or directory
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.xray-x86_64.a: No such file or directory
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.xray-basic-x86_64.a: No such file or directory
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.xray-fdr-x86_64.a: No such file or directory

This is a follow-up to 058e9b0 which started removing these checks
and it should make it easier to stop forcing COMPILER_RT_STANDALONE_BUILD
for runtimes builds in the future.


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

8 Files Affected:

  • (modified) compiler-rt/cmake/Modules/CompilerRTCompile.cmake (+2-1)
  • (modified) compiler-rt/lib/msan/tests/CMakeLists.txt (+1-3)
  • (modified) compiler-rt/test/asan_abi/CMakeLists.txt (+1-4)
  • (modified) compiler-rt/test/fuzzer/CMakeLists.txt (+13-8)
  • (modified) compiler-rt/test/memprof/CMakeLists.txt (+3-6)
  • (modified) compiler-rt/test/msan/CMakeLists.txt (+1-5)
  • (modified) compiler-rt/test/ubsan/CMakeLists.txt (+1-3)
  • (modified) compiler-rt/test/xray/CMakeLists.txt (+2-3)
diff --git a/compiler-rt/cmake/Modules/CompilerRTCompile.cmake b/compiler-rt/cmake/Modules/CompilerRTCompile.cmake
index 3d7528ad2e52c0..1629db18f1c2d9 100644
--- a/compiler-rt/cmake/Modules/CompilerRTCompile.cmake
+++ b/compiler-rt/cmake/Modules/CompilerRTCompile.cmake
@@ -65,8 +65,9 @@ function(clang_compile object_file source)
   cmake_parse_arguments(SOURCE "" "" "CFLAGS;DEPS" ${ARGN})
   get_filename_component(source_rpath ${source} REALPATH)
   if(NOT COMPILER_RT_STANDALONE_BUILD)
-    list(APPEND SOURCE_DEPS clang compiler-rt-headers)
+    list(APPEND SOURCE_DEPS clang)
   endif()
+  list(APPEND SOURCE_DEPS compiler-rt-headers)
   if (TARGET CompilerRTUnitTestCheckCxx)
     list(APPEND SOURCE_DEPS CompilerRTUnitTestCheckCxx)
   endif()
diff --git a/compiler-rt/lib/msan/tests/CMakeLists.txt b/compiler-rt/lib/msan/tests/CMakeLists.txt
index 1cb03d8323f651..0d43f432ab6a37 100644
--- a/compiler-rt/lib/msan/tests/CMakeLists.txt
+++ b/compiler-rt/lib/msan/tests/CMakeLists.txt
@@ -119,9 +119,7 @@ macro(add_msan_tests_for_arch arch kind cflags)
   set(MSAN_TEST_DEPS ${MSAN_TEST_OBJECTS} libcxx_msan_${arch}-build
                      ${MSAN_LOADABLE_SO}
 		     "${MSAN_LIBCXX_DIR}/libc++.a" "${MSAN_LIBCXX_DIR}/libc++abi.a")
-  if(NOT COMPILER_RT_STANDALONE_BUILD)
-    list(APPEND MSAN_TEST_DEPS msan)
-  endif()
+  list(APPEND MSAN_TEST_DEPS msan)
   get_target_flags_for_arch(${arch} TARGET_LINK_FLAGS)
   add_compiler_rt_test(MsanUnitTests "Msan-${arch}${kind}-Test" ${arch}
     OBJECTS ${MSAN_TEST_OBJECTS} "${MSAN_LIBCXX_DIR}/libc++.a" "${MSAN_LIBCXX_DIR}/libc++abi.a"
diff --git a/compiler-rt/test/asan_abi/CMakeLists.txt b/compiler-rt/test/asan_abi/CMakeLists.txt
index f28cf6cd1da6ea..1114ed1b82793d 100644
--- a/compiler-rt/test/asan_abi/CMakeLists.txt
+++ b/compiler-rt/test/asan_abi/CMakeLists.txt
@@ -9,10 +9,7 @@ macro(get_bits_for_arch arch bits)
   endif()
 endmacro()
 
-set(ASAN_ABI_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
-if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND ASAN_ABI_TEST_DEPS asan_abi)
-endif()
+set(ASAN_ABI_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS} asan_abi)
 
 set(ASAN_ABI_TEST_ARCH ${ASAN_ABI_SUPPORTED_ARCH})
 if(APPLE)
diff --git a/compiler-rt/test/fuzzer/CMakeLists.txt b/compiler-rt/test/fuzzer/CMakeLists.txt
index f0ba087a1b3260..5bcb44757bae46 100644
--- a/compiler-rt/test/fuzzer/CMakeLists.txt
+++ b/compiler-rt/test/fuzzer/CMakeLists.txt
@@ -1,12 +1,17 @@
-set(LIBFUZZER_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
+set(LIBFUZZER_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS} fuzzer)
+if (COMPILER_RT_HAS_UBSAN)
+  list(APPEND LIBFUZZER_TEST_DEPS ubsan)
+endif()
+if (COMPILER_RT_HAS_ASAN)
+  list(APPEND LIBFUZZER_TEST_DEPS asan)
+endif()
+if (COMPILER_RT_HAS_MSAN)
+  list(APPEND LIBFUZZER_TEST_DEPS msan)
+endif()
+if (COMPILER_RT_HAS_DFSAN)
+  list(APPEND LIBFUZZER_TEST_DEPS dfsan)
+endif()
 if (NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND LIBFUZZER_TEST_DEPS fuzzer asan ubsan)
-  if (COMPILER_RT_HAS_MSAN)
-    list(APPEND LIBFUZZER_TEST_DEPS msan)
-  endif()
-  if (COMPILER_RT_HAS_DFSAN)
-    list(APPEND LIBFUZZER_TEST_DEPS dfsan)
-  endif()
   if(NOT APPLE AND COMPILER_RT_HAS_LLD AND "lld" IN_LIST LLVM_ENABLE_PROJECTS)
     list(APPEND LIBFUZZER_TEST_DEPS lld)
   endif()
diff --git a/compiler-rt/test/memprof/CMakeLists.txt b/compiler-rt/test/memprof/CMakeLists.txt
index 8a29919b177024..3f0ba3812485d2 100644
--- a/compiler-rt/test/memprof/CMakeLists.txt
+++ b/compiler-rt/test/memprof/CMakeLists.txt
@@ -11,12 +11,9 @@ macro(get_bits_for_arch arch bits)
   endif()
 endmacro()
 
-set(MEMPROF_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
-if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND MEMPROF_TEST_DEPS memprof)
-  if(COMPILER_RT_HAS_LLD AND TARGET lld)
-    list(APPEND MEMPROF_TEST_DEPS lld)
-  endif()
+set(MEMPROF_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS} memprof)
+if(NOT COMPILER_RT_STANDALONE_BUILD AND COMPILER_RT_HAS_LLD AND TARGET lld)
+  list(APPEND MEMPROF_TEST_DEPS lld)
 endif()
 set(MEMPROF_DYNAMIC_TEST_DEPS ${MEMPROF_TEST_DEPS})
 
diff --git a/compiler-rt/test/msan/CMakeLists.txt b/compiler-rt/test/msan/CMakeLists.txt
index 01b832b5ae3f9d..9f784507c4ee1c 100644
--- a/compiler-rt/test/msan/CMakeLists.txt
+++ b/compiler-rt/test/msan/CMakeLists.txt
@@ -1,7 +1,7 @@
 set(MSAN_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 
 set(MSAN_TESTSUITES)
-set(MSAN_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
+set(MSAN_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS} msan)
 
 set(MSAN_TEST_ARCH ${MSAN_SUPPORTED_ARCH})
 if(APPLE)
@@ -41,10 +41,6 @@ foreach(arch ${MSAN_TEST_ARCH})
   endif()
 endforeach()
 
-if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND MSAN_TEST_DEPS msan)
-endif()
-
 if(COMPILER_RT_INCLUDE_TESTS AND
    COMPILER_RT_LIBCXX_PATH AND
    COMPILER_RT_LIBCXXABI_PATH)
diff --git a/compiler-rt/test/ubsan/CMakeLists.txt b/compiler-rt/test/ubsan/CMakeLists.txt
index b5040f79e607ba..52052c80960c24 100644
--- a/compiler-rt/test/ubsan/CMakeLists.txt
+++ b/compiler-rt/test/ubsan/CMakeLists.txt
@@ -23,9 +23,7 @@ macro(add_ubsan_testsuite test_mode sanitizer arch lld thinlto)
     ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
     ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME}/lit.site.cfg.py)
   list(APPEND UBSAN_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME})
-  if(NOT COMPILER_RT_STANDALONE_BUILD)
-    list(APPEND UBSAN_TEST_DEPS ${sanitizer})
-  endif()
+  list(APPEND UBSAN_TEST_DEPS ${sanitizer})
 endmacro()
 
 macro(add_ubsan_testsuites test_mode sanitizer arch)
diff --git a/compiler-rt/test/xray/CMakeLists.txt b/compiler-rt/test/xray/CMakeLists.txt
index b97659ae08f9bd..0c008b6ea5577b 100644
--- a/compiler-rt/test/xray/CMakeLists.txt
+++ b/compiler-rt/test/xray/CMakeLists.txt
@@ -3,12 +3,11 @@ set(XRAY_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 set(XRAY_TESTSUITES)
 set(XRAY_FDR_TESTSUITES)
 
-set(XRAY_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
-set(XRAY_FDR_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
+set(XRAY_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS} xray)
 
 if(NOT COMPILER_RT_STANDALONE_BUILD AND COMPILER_RT_BUILD_XRAY AND
    COMPILER_RT_HAS_XRAY)
-  list(APPEND XRAY_TEST_DEPS xray llvm-xray)
+  list(APPEND XRAY_TEST_DEPS llvm-xray)
 endif()
 
 set(XRAY_TEST_ARCH ${XRAY_SUPPORTED_ARCH})

Created using spr 1.3.6-beta.1

[skip ci]
Created using spr 1.3.6-beta.1
arichardson added a commit to arichardson/upstream-llvm-project that referenced this pull request Mar 7, 2024
With llvm#83088, we now need the
runtimes to be built before running test if
COMPILER_RT_TEST_STANDALONE_BUILD_LIBS is true, since otherwise we
get failures running `ninja check-all` such as the following:
```
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.fuzzer-x86_64.a: No such file or directory
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.xray-x86_64.a: No such file or directory
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.xray-basic-x86_64.a: No such file or directory
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.xray-fdr-x86_64.a: No such file or directory
```

This is a follow-up to 058e9b0 which started removing these checks
and it should make it easier to stop forcing COMPILER_RT_STANDALONE_BUILD
for runtimes builds in the future.

Pull Request: llvm#83651
Copy link
Collaborator

@vitalybuka vitalybuka left a comment

Choose a reason for hiding this comment

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

LGTM, but I bet we will brake some bots :)

vzakhari and others added 2 commits March 18, 2024 16:47
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.compiler-rt-dont-check-compiler_rt_standalone_build-for-test-deps to main March 18, 2024 23:48
@arichardson arichardson merged commit ba2dc29 into main Mar 18, 2024
4 of 5 checks passed
@arichardson arichardson deleted the users/arichardson/spr/compiler-rt-dont-check-compiler_rt_standalone_build-for-test-deps branch March 18, 2024 23:48
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
With llvm#83088, we now need the
runtimes to be built before running test if
COMPILER_RT_TEST_STANDALONE_BUILD_LIBS is true, since otherwise we
get failures running `ninja check-all` such as the following:
```
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.fuzzer-x86_64.a: No such file or directory
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.xray-x86_64.a: No such file or directory
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.xray-basic-x86_64.a: No such file or directory
/usr/bin/ld: cannot find .../compiler-rt/cmake-build-all-sanitizers/lib/linux/libclang_rt.xray-fdr-x86_64.a: No such file or directory
```

This is a follow-up to 058e9b0 which started removing these checks
and it should make it easier to stop forcing COMPILER_RT_STANDALONE_BUILD
for runtimes builds in the future.

Reviewed By: vitalybuka

Pull Request: llvm#83651
@thevinster
Copy link
Contributor

Echo-ing the comment made by @smeenai, checking for COMPILER_RT_HAS_MSAN or COMPILER_RT_HAS_DFSAN only checks whether the machine can support it, but not whether the user decides to build it (which ends up becoming a missing dep since we don't build for these sanitizers). Our workaround is to build it, but we don't need it, and it increases our build time. Have we thought about any other workarounds?

@arichardson
Copy link
Member Author

Echo-ing the comment made by @smeenai, checking for COMPILER_RT_HAS_MSAN or COMPILER_RT_HAS_DFSAN only checks whether the machine can support it, but not whether the user decides to build it (which ends up becoming a missing dep since we don't build for these sanitizers). Our workaround is to build it, but we don't need it, and it increases our build time. Have we thought about any other workarounds?

Does this refer to the changes in libfuzzer?

I guess the cleanest option here would be to have COMPILER_RT_HAS_MSAN be false when COMPILER_RT_TEST_STANDALONE_BUILD_LIBS (#83088) is set but msan runtimes are not being built.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler-rt:asan Address sanitizer compiler-rt:fuzzer compiler-rt:msan Memory sanitizer compiler-rt:sanitizer compiler-rt:ubsan Undefined behavior sanitizer compiler-rt PGO Profile Guided Optimizations xray
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants