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

[test] Don't check COMPILER_RT_STANDALONE_BUILD for deps #66259

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

vitalybuka
Copy link
Collaborator

COMPILER_RT_STANDALONE_BUILD is only needed to be
checked for dependencies outside of compiler-rt.

COMPILER_RT_STANDALONE_BUILD is only needed to be
checked for dependencies outside of compiler-rt.
@vitalybuka vitalybuka requested review from a team as code owners September 13, 2023 17:23
@llvmbot llvmbot added compiler-rt PGO Profile Guided Optimizations compiler-rt:sanitizer labels Sep 13, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 13, 2023

@llvm/pr-subscribers-pgo

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

Changes COMPILER_RT_STANDALONE_BUILD is only needed to be checked for dependencies outside of compiler-rt.

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

11 Files Affected:

  • (modified) compiler-rt/lib/msan/tests/CMakeLists.txt (+1-3)
  • (modified) compiler-rt/test/asan/CMakeLists.txt (+1-1)
  • (modified) compiler-rt/test/dfsan/CMakeLists.txt (+1-3)
  • (modified) compiler-rt/test/hwasan/CMakeLists.txt (+1-1)
  • (modified) compiler-rt/test/lsan/CMakeLists.txt (+3-5)
  • (modified) compiler-rt/test/metadata/CMakeLists.txt (+1-3)
  • (modified) compiler-rt/test/profile/CMakeLists.txt (+2-1)
  • (modified) compiler-rt/test/safestack/CMakeLists.txt (+1-2)
  • (modified) compiler-rt/test/tsan/CMakeLists.txt (+1-3)
  • (modified) compiler-rt/test/ubsan/CMakeLists.txt (+1-3)
  • (modified) compiler-rt/test/ubsan_minimal/CMakeLists.txt (+1-3)
diff --git a/compiler-rt/lib/msan/tests/CMakeLists.txt b/compiler-rt/lib/msan/tests/CMakeLists.txt
index 6c0520d98426315..6ef63ff8216638e 100644
--- a/compiler-rt/lib/msan/tests/CMakeLists.txt
+++ b/compiler-rt/lib/msan/tests/CMakeLists.txt
@@ -79,9 +79,7 @@ macro(msan_link_shared so_list so_name arch kind)
   cmake_parse_arguments(SOURCE "" "" "OBJECTS;LINK_FLAGS;DEPS" ${ARGN})
   set(output_so "${CMAKE_CURRENT_BINARY_DIR}/${so_name}.${arch}${kind}.so")
   get_target_flags_for_arch(${arch} TARGET_LINK_FLAGS)
-  if(NOT COMPILER_RT_STANDALONE_BUILD)
-    list(APPEND SOURCE_DEPS msan)
-  endif()
+  list(APPEND SOURCE_DEPS msan)
   clang_link_shared(${output_so}
                 OBJECTS ${SOURCE_OBJECTS}
                 LINK_FLAGS ${COMPILER_RT_UNITTEST_LINK_FLAGS} ${TARGET_LINK_FLAGS} ${SOURCE_LINK_FLAGS}
diff --git a/compiler-rt/test/asan/CMakeLists.txt b/compiler-rt/test/asan/CMakeLists.txt
index 70c282a621c371a..d8c779c4ad6c5bf 100644
--- a/compiler-rt/test/asan/CMakeLists.txt
+++ b/compiler-rt/test/asan/CMakeLists.txt
@@ -24,8 +24,8 @@ macro(get_bits_for_arch arch bits)
 endmacro()
 
 set(ASAN_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
+list(APPEND ASAN_TEST_DEPS asan)
 if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND ASAN_TEST_DEPS asan)
   if(NOT APPLE AND COMPILER_RT_HAS_LLD AND "lld" IN_LIST LLVM_ENABLE_PROJECTS)
     list(APPEND ASAN_TEST_DEPS lld)
   endif()
diff --git a/compiler-rt/test/dfsan/CMakeLists.txt b/compiler-rt/test/dfsan/CMakeLists.txt
index 168f046238dbbe6..14e431a0375b9b6 100644
--- a/compiler-rt/test/dfsan/CMakeLists.txt
+++ b/compiler-rt/test/dfsan/CMakeLists.txt
@@ -21,9 +21,7 @@ foreach(arch ${DFSAN_TEST_ARCH})
 endforeach()
 
 set(DFSAN_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
-if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND DFSAN_TEST_DEPS dfsan)
-endif()
+list(APPEND DFSAN_TEST_DEPS dfsan)
 
 add_lit_testsuite(check-dfsan "Running the DataFlowSanitizer tests"
   ${DFSAN_TESTSUITES}
diff --git a/compiler-rt/test/hwasan/CMakeLists.txt b/compiler-rt/test/hwasan/CMakeLists.txt
index 34911a307003ba0..927d0d74bd6ec71 100644
--- a/compiler-rt/test/hwasan/CMakeLists.txt
+++ b/compiler-rt/test/hwasan/CMakeLists.txt
@@ -21,8 +21,8 @@ foreach(arch ${HWASAN_TEST_ARCH})
 endforeach()
 
 set(HWASAN_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
+list(APPEND HWASAN_TEST_DEPS hwasan)
 if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND HWASAN_TEST_DEPS hwasan)
   if(COMPILER_RT_HAS_LLD AND "lld" IN_LIST LLVM_ENABLE_PROJECTS)
     list(APPEND HWASAN_TEST_DEPS lld)
   endif()
diff --git a/compiler-rt/test/lsan/CMakeLists.txt b/compiler-rt/test/lsan/CMakeLists.txt
index 5b4fee764843b17..dd16aab4c27a2db 100644
--- a/compiler-rt/test/lsan/CMakeLists.txt
+++ b/compiler-rt/test/lsan/CMakeLists.txt
@@ -42,11 +42,9 @@ foreach(arch ${LSAN_TEST_ARCH})
 endforeach()
 
 set(LSAN_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
-if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND LSAN_TEST_DEPS lsan)
-  append_list_if(COMPILER_RT_HAS_ASAN asan LSAN_TEST_DEPS)
-  append_list_if(COMPILER_RT_HAS_HWASAN hwasan LSAN_TEST_DEPS)
-endif()
+list(APPEND LSAN_TEST_DEPS lsan)
+append_list_if(COMPILER_RT_HAS_ASAN asan LSAN_TEST_DEPS)
+append_list_if(COMPILER_RT_HAS_HWASAN hwasan LSAN_TEST_DEPS)
 add_lit_testsuite(check-lsan "Running the LeakSanitizer tests"
   ${LSAN_TESTSUITES}
   DEPENDS ${LSAN_TEST_DEPS})
diff --git a/compiler-rt/test/metadata/CMakeLists.txt b/compiler-rt/test/metadata/CMakeLists.txt
index 4a17b0e92445478..4245937f3ce568e 100644
--- a/compiler-rt/test/metadata/CMakeLists.txt
+++ b/compiler-rt/test/metadata/CMakeLists.txt
@@ -5,9 +5,7 @@ if(CAN_TARGET_x86_64)
   set(METADATA_LIT_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
 
   set(METADATA_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
-  if (NOT COMPILER_RT_STANDALONE_BUILD)
-    list(APPEND METADATA_TEST_DEPS asan ubsan)
-  endif()
+  list(APPEND METADATA_TEST_DEPS asan ubsan)
 
   set(SANITIZER_COMMON_TEST_TARGET_ARCH ${X86_64})
   get_test_cc_for_arch(x86_64 METADATA_TEST_TARGET_CC METADATA_TEST_TARGET_CFLAGS)
diff --git a/compiler-rt/test/profile/CMakeLists.txt b/compiler-rt/test/profile/CMakeLists.txt
index 1b61d1ea0728eb4..975e4c42f4b640f 100644
--- a/compiler-rt/test/profile/CMakeLists.txt
+++ b/compiler-rt/test/profile/CMakeLists.txt
@@ -4,8 +4,9 @@ set(PROFILE_LIT_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
 set(PROFILE_TESTSUITES)
 # Profile tests rely on the compiler-rt-headers being in the resource directory
 set(PROFILE_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS} compiler-rt-headers)
+list(APPEND PROFILE_TEST_DEPS profile)
 if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND PROFILE_TEST_DEPS profile llvm-profdata llvm-cov)
+  list(APPEND PROFILE_TEST_DEPS llvm-profdata llvm-cov)
   if(NOT APPLE AND COMPILER_RT_HAS_LLD AND "lld" IN_LIST LLVM_ENABLE_PROJECTS)
     list(APPEND PROFILE_TEST_DEPS lld)
   endif()
diff --git a/compiler-rt/test/safestack/CMakeLists.txt b/compiler-rt/test/safestack/CMakeLists.txt
index d1a33b89e83a965..2746102bd89b3bc 100644
--- a/compiler-rt/test/safestack/CMakeLists.txt
+++ b/compiler-rt/test/safestack/CMakeLists.txt
@@ -2,9 +2,8 @@ set(SAFESTACK_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 set(SAFESTACK_LIT_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
 
 set(SAFESTACK_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
+list(APPEND SAFESTACK_TEST_DEPS safestack)
 if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND SAFESTACK_TEST_DEPS safestack)
-
   # Some tests require LTO, so add a dependency on the relevant LTO plugin.
   if(LLVM_ENABLE_PIC)
     if(LLVM_BINUTILS_INCDIR)
diff --git a/compiler-rt/test/tsan/CMakeLists.txt b/compiler-rt/test/tsan/CMakeLists.txt
index 25e95aa98cf9501..ebd8ddf639df993 100644
--- a/compiler-rt/test/tsan/CMakeLists.txt
+++ b/compiler-rt/test/tsan/CMakeLists.txt
@@ -4,9 +4,7 @@ set(TSAN_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
 if(${COMPILER_RT_DEFAULT_TARGET_ARCH} MATCHES "(x86_64|s390x)")
   list(APPEND TSAN_TEST_DEPS GotsanRuntimeCheck)
 endif()
-if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND TSAN_TEST_DEPS tsan)
-endif()
+list(APPEND TSAN_TEST_DEPS tsan)
 if(COMPILER_RT_LIBCXX_PATH AND
    COMPILER_RT_LIBCXXABI_PATH AND
    COMPILER_RT_TEST_COMPILER_ID STREQUAL "Clang"
diff --git a/compiler-rt/test/ubsan/CMakeLists.txt b/compiler-rt/test/ubsan/CMakeLists.txt
index d14aa149d978cab..b5040f79e607ba5 100644
--- a/compiler-rt/test/ubsan/CMakeLists.txt
+++ b/compiler-rt/test/ubsan/CMakeLists.txt
@@ -86,9 +86,7 @@ macro(add_ubsan_device_testsuite test_mode sanitizer platform arch)
     ${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})
   add_lit_testsuite(check-ubsan-${test_mode}-${platform}-${arch}
     "UBSan ${CONFIG_NAME} tests"
     ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME}/
diff --git a/compiler-rt/test/ubsan_minimal/CMakeLists.txt b/compiler-rt/test/ubsan_minimal/CMakeLists.txt
index 522426602a759e4..c2debeae07e82ba 100644
--- a/compiler-rt/test/ubsan_minimal/CMakeLists.txt
+++ b/compiler-rt/test/ubsan_minimal/CMakeLists.txt
@@ -7,9 +7,7 @@ endif()
 
 set(UBSAN_TESTSUITES)
 set(UBSAN_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
-if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND UBSAN_TEST_DEPS ubsan-minimal)
-endif()
+list(APPEND UBSAN_TEST_DEPS ubsan-minimal)
 
 foreach(arch ${UBSAN_TEST_ARCH})
   get_test_cc_for_arch(${arch} UBSAN_TEST_TARGET_CC UBSAN_TEST_TARGET_CFLAGS)

@vitalybuka vitalybuka merged commit 058e9b0 into llvm:main Sep 15, 2023
5 checks passed
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
COMPILER_RT_STANDALONE_BUILD is only needed to be
checked for dependencies outside of compiler-rt.
@smeenai
Copy link
Collaborator

smeenai commented Sep 19, 2023

This change exposed a bug, where e.g.

append_list_if(COMPILER_RT_HAS_HWASAN hwasan LSAN_TEST_DEPS)
is checking COMPILER_RT_HAS_HWASAN to decide whether to add hwasan as a dependency. However, COMPILER_RT_HAS_HWASAN only says whether the current OS and architecture supports hwasan, not whether it's actually being built (which is determined by COMPILER_RT_SANITIZERS_TO_BUILD. Consequently, even if you're not building hwasan, you try to add a dependency on it, and the build fails cos the target doesn't exist. How would you suggest fixing that?

zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
COMPILER_RT_STANDALONE_BUILD is only needed to be
checked for dependencies outside of compiler-rt.
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
COMPILER_RT_STANDALONE_BUILD is only needed to be
checked for dependencies outside of compiler-rt.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants