[rtsan] Explicitly link pthread in the tests and dynamic libraries #98256
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CC @vitalybuka |
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Chris Apple (cjappl) ChangesFollow up to #98219 This reverts commit 14f7450 Ensure that -pthread is explicitly linked when running the rtsan tests. Issue this fixes: Full diff: https://github.com/llvm/llvm-project/pull/98256.diff 3 Files Affected:
diff --git a/compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake b/compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
index c8bec41db36e9..bc152e304aaaf 100644
--- a/compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
+++ b/compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
@@ -32,9 +32,9 @@ set(ALL_ASAN_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${RISCV64}
${LOONGARCH64})
set(ALL_ASAN_ABI_SUPPORTED_ARCH ${X86_64} ${ARM64} ${ARM64_32})
set(ALL_DFSAN_SUPPORTED_ARCH ${X86_64} ${MIPS64} ${ARM64} ${LOONGARCH64})
-#set(ALL_RTSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${RISCV64}
-# ${MIPS32} ${MIPS64} ${PPC64} ${S390X} ${SPARC} ${SPARCV9} ${HEXAGON}
-# ${LOONGARCH64})
+set(ALL_RTSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${RISCV64}
+ ${MIPS32} ${MIPS64} ${PPC64} ${S390X} ${SPARC} ${SPARCV9} ${HEXAGON}
+ ${LOONGARCH64})
if(ANDROID)
set(OS_NAME "Android")
diff --git a/compiler-rt/lib/rtsan/CMakeLists.txt b/compiler-rt/lib/rtsan/CMakeLists.txt
index bd7358e86e59b..e434a1d11bb64 100644
--- a/compiler-rt/lib/rtsan/CMakeLists.txt
+++ b/compiler-rt/lib/rtsan/CMakeLists.txt
@@ -84,6 +84,7 @@ else()
${RTSAN_COMMON_RUNTIME_OBJECT_LIBS}
LINK_FLAGS ${RTSAN_LINK_FLAGS}
CFLAGS ${RTSAN_CFLAGS}
+ LINK_LIBS ${RTSAN_LINK_LIBS}
PARENT_TARGET rtsan)
endif()
diff --git a/compiler-rt/lib/rtsan/tests/CMakeLists.txt b/compiler-rt/lib/rtsan/tests/CMakeLists.txt
index e539ebc387e0a..a379e4029633e 100644
--- a/compiler-rt/lib/rtsan/tests/CMakeLists.txt
+++ b/compiler-rt/lib/rtsan/tests/CMakeLists.txt
@@ -36,6 +36,8 @@ set(RTSAN_UNITTEST_LINK_FLAGS
${SANITIZER_TEST_CXX_LIBRARIES}
-no-pie)
+append_list_if(COMPILER_RT_HAS_LIBPTHREAD -pthread RTSAN_UNITTEST_LINK_FLAGS)
+
if (APPLE)
add_weak_symbols("sanitizer_common" WEAK_SYMBOL_LINK_FLAGS)
list(APPEND RTSAN_UNITTEST_LINK_FLAGS ${WEAK_SYMBOL_LINK_FLAGS})
|
|
FYI I do not have a powerpc build to test on, so this is just a hunch. I do think all the signs point to this being the fix, though. |
|
|
vitalybuka
reviewed
Jul 10, 2024
vitalybuka
approved these changes
Jul 10, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/990 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/1051 Here is the relevant piece of the build log for the reference: |
|
On it, look for a follow up shortly |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/109/builds/783 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/176/builds/878 Here is the relevant piece of the build log for the reference: |
vitalybuka
pushed a commit
that referenced
this pull request
Jul 10, 2024
…et rid of atomic (#98264) Fixing test failures in #98256 There are two issues being fixed here: ``` ld.lld: error: undefined symbol: dlvsym ld.lld: error: undefined symbol: dlvsym ``` Fixed by linking dl ``` FAILED: ... -lstdc++ -no-pie -pthread -latomic -m64 /opt/rh/devtoolset-11/root/usr/lib/gcc/ppc64-redhat-linux/11/../../../../bin/ld: cannot find /usr/lib64/libatomic.so.1 ``` Fixed by removing atomic from the dependencies, I don't think we were even using it. I added in all of the dependencies used by the parent library in the tests, hopefully getting ahead of any other issues, adding rt and m, even if we haven't had issues with them yet
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
…lvm#98256) Follow up to llvm#98219 This reverts commit [14f7450](llvm@14f7450) Ensure that -pthread is explicitly linked when running the rtsan tests. Issue this fixes: ``` FAILED: compiler-rt/lib/rtsan/tests/Rtsan-powerpc64le-NoInstTest /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/runtimes/runtimes-bins/compiler-rt/lib/rtsan/tests/Rtsan-powerpc64le-NoInstTest cd /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/runtimes/runtimes-bins/compiler-rt/lib/rtsan/tests && /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/./bin/clang++ RtsanNoInstTestObjects.rtsan_preinit.cpp.powerpc64le.o RtsanNoInstTestObjects.rtsan_test_context.cpp.powerpc64le.o RtsanNoInstTestObjects.rtsan_test_main.cpp.powerpc64le.o RtsanNoInstTestObjects.gtest-all.cc.powerpc64le.o RtsanNoInstTestObjects.gmock-all.cc.powerpc64le.o /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/runtimes/runtimes-bins/compiler-rt/lib/rtsan/tests/libRTRtsanTest.powerpc64le.a -o /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/runtimes/runtimes-bins/compiler-rt/lib/rtsan/tests/./Rtsan-powerpc64le-NoInstTest -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -resource-dir=/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/./lib/../lib/clang/19 -Wl,-rpath,/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/./lib/../lib/clang/19/lib/powerpc64le-unknown-linux-gnu -lstdc++ -no-pie -latomic -m64 -fno-function-sections /usr/bin/ld: RtsanNoInstTestObjects.gtest-all.cc.powerpc64le.o: undefined reference to symbol 'pthread_getspecific@@GLIBC_2.17' //usr/lib64/libpthread.so.0: error adding symbols: DSO missing from command line ```
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
…et rid of atomic (llvm#98264) Fixing test failures in llvm#98256 There are two issues being fixed here: ``` ld.lld: error: undefined symbol: dlvsym ld.lld: error: undefined symbol: dlvsym ``` Fixed by linking dl ``` FAILED: ... -lstdc++ -no-pie -pthread -latomic -m64 /opt/rh/devtoolset-11/root/usr/lib/gcc/ppc64-redhat-linux/11/../../../../bin/ld: cannot find /usr/lib64/libatomic.so.1 ``` Fixed by removing atomic from the dependencies, I don't think we were even using it. I added in all of the dependencies used by the parent library in the tests, hopefully getting ahead of any other issues, adding rt and m, even if we haven't had issues with them yet
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up to #98219
This reverts commit 14f7450
Ensure that -pthread is explicitly linked when running the rtsan tests.
Issue this fixes: