[TySan][CMake] Depend on tysan for check-tysan in runtimes build #143597
Conversation
The runtimes build expects libclang_rt.tysan.a to be available, but the check-tysan target does not actually depend on it when built using a runtimes build with LLVM_ENABLE_RUNTIMES pointing at ./llvm. This means we get test failures when running check-compiler-rt due to the missing static archive. This patch also makes check-tysan depend on tysan when we are using the runtimes build.
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Aiden Grossman (boomanaiden154) ChangesThe runtimes build expects libclang_rt.tysan.a to be available, but the check-tysan target does not actually depend on it when built using a runtimes build with LLVM_ENABLE_RUNTIMES pointing at ./llvm. This means we get test failures when running check-compiler-rt due to the missing static archive. This patch also makes check-tysan depend on tysan when we are using the runtimes build. This is causing premerge failures currently since we recently migrated to the runtimes build. Full diff: https://github.com/llvm/llvm-project/pull/143597.diff 1 Files Affected:
diff --git a/compiler-rt/test/tysan/CMakeLists.txt b/compiler-rt/test/tysan/CMakeLists.txt
index 76f57501e854e..6f1beef65ee46 100644
--- a/compiler-rt/test/tysan/CMakeLists.txt
+++ b/compiler-rt/test/tysan/CMakeLists.txt
@@ -21,7 +21,7 @@ foreach(arch ${TYSAN_TEST_ARCH})
endforeach()
set(TYSAN_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
-if(NOT COMPILER_RT_STANDALONE_BUILD)
+if(NOT COMPILER_RT_STANDALONE_BUILD OR LLVM_RUNTIMES_BUILD)
list(APPEND TYSAN_TEST_DEPS tysan)
endif()
|
| @@ -21,7 +21,7 @@ foreach(arch ${TYSAN_TEST_ARCH}) | |||
| endforeach() | |||
|
|
|||
| set(TYSAN_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS}) | |||
| if(NOT COMPILER_RT_STANDALONE_BUILD) | |||
| if(NOT COMPILER_RT_STANDALONE_BUILD OR LLVM_RUNTIMES_BUILD) | |||
There was a problem hiding this comment.
I'm a bit confused: Shouldn't COMPILER_RT_STANDALONE_BUILD always be false for a runtimes build?
There was a problem hiding this comment.
Potentially? It definitely is set to true though.
It looks like the runtimes build is setup in such a way that the check below ends up firing because for whatever reason CMAKE_SOURCE_DIR is set to compiler-rt/
llvm-project/compiler-rt/CMakeLists.txt
Line 21 in a7f495f
There was a problem hiding this comment.
Interesting! Standalone is a separate, third build type next to runtimes and project, so I'm quite surprised that the runtimes build also identifies as a standalone build.
There was a problem hiding this comment.
I think the runtimes build uses the standalone build, so a runtimes build makes standalone compiler-rt cmake builds.
There was a problem hiding this comment.
All the other sanitizers do this unconditionally, so can we just remove the if?
There was a problem hiding this comment.
Looks like it. I was looking at rtsan for what I thought was a simple example, but it seems like its the odd one out.
Not sure if there was a reason for doing it this way in the first place though (some dependency on clang)?
…m#143597) The runtimes build expects libclang_rt.tysan.a to be available, but the check-tysan target does not actually depend on it when built using a runtimes build with LLVM_ENABLE_RUNTIMES pointing at ./llvm. This means we get test failures when running check-compiler-rt due to the missing static archive. This patch also makes check-tysan depend on tysan when we are using the runtimes build. This is causing premerge failures currently since we recently migrated to the runtimes build.
…m#143597) The runtimes build expects libclang_rt.tysan.a to be available, but the check-tysan target does not actually depend on it when built using a runtimes build with LLVM_ENABLE_RUNTIMES pointing at ./llvm. This means we get test failures when running check-compiler-rt due to the missing static archive. This patch also makes check-tysan depend on tysan when we are using the runtimes build. This is causing premerge failures currently since we recently migrated to the runtimes build.
…m#143597) The runtimes build expects libclang_rt.tysan.a to be available, but the check-tysan target does not actually depend on it when built using a runtimes build with LLVM_ENABLE_RUNTIMES pointing at ./llvm. This means we get test failures when running check-compiler-rt due to the missing static archive. This patch also makes check-tysan depend on tysan when we are using the runtimes build. This is causing premerge failures currently since we recently migrated to the runtimes build.
The runtimes build expects libclang_rt.tysan.a to be available, but the check-tysan target does not actually depend on it when built using a runtimes build with LLVM_ENABLE_RUNTIMES pointing at ./llvm. This means we get test failures when running check-compiler-rt due to the missing static archive.
This patch also makes check-tysan depend on tysan when we are using the runtimes build.
This is causing premerge failures currently since we recently migrated to the runtimes build.