[rtsan] Add dl, rt, m, and log libraries as unit test dependencies, get rid of atomic #98264
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
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Chris Apple (cjappl) ChangesFixing test failures in #98256 There are two issues being fixed here: Fixed by linking dl 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 Full diff: https://github.com/llvm/llvm-project/pull/98264.diff 1 Files Affected:
diff --git a/compiler-rt/lib/rtsan/tests/CMakeLists.txt b/compiler-rt/lib/rtsan/tests/CMakeLists.txt
index a379e4029633e..9eda116541ae3 100644
--- a/compiler-rt/lib/rtsan/tests/CMakeLists.txt
+++ b/compiler-rt/lib/rtsan/tests/CMakeLists.txt
@@ -36,15 +36,17 @@ set(RTSAN_UNITTEST_LINK_FLAGS
${SANITIZER_TEST_CXX_LIBRARIES}
-no-pie)
+append_list_if(COMPILER_RT_HAS_LIBDL -ldl RTSAN_UNITTEST_LINK_FLAGS)
+append_list_if(COMPILER_RT_HAS_LIBRT -lrt RTSAN_UNITTEST_LINK_FLAGS)
+append_list_if(COMPILER_RT_HAS_LIBM -lm RTSAN_UNITTEST_LINK_FLAGS)
append_list_if(COMPILER_RT_HAS_LIBPTHREAD -pthread RTSAN_UNITTEST_LINK_FLAGS)
+append_list_if(COMPILER_RT_HAS_LIBLOG -llog 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})
list(APPEND RTSAN_UNITTEST_LINK_FLAGS ${DARWIN_osx_LINK_FLAGS})
list(APPEND RTSAN_UNITTEST_CFLAGS ${DARWIN_osx_CFLAGS})
-else()
- list(APPEND RTSAN_UNITTEST_LINK_FLAGS -latomic)
endif()
set(COMPILER_RT_GOOGLETEST_SOURCES ${COMPILER_RT_GTEST_SOURCE} ${COMPILER_RT_GMOCK_SOURCE})
|
|
CC @vitalybuka Really apologize for the churn. If there is a more deterministic way to do this other than "commit and see", please let me know. With some of these systems I'm at the mercy of the post-commit tests. Thanks for your continued help |
|
|
vitalybuka
approved these changes
Jul 10, 2024
vitalybuka
approved these changes
Jul 10, 2024
Would be possible to fix this? |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/109/builds/785 Here is the relevant piece of the build log for the reference: |
Fixed in my local gitconfig. I thought i just needed it in the git commits. Thanks for bringing attention to it |
Trying to fix this. I am considering disabling Android as well as PPC arch to go "back basics" then add the rest in later. Will start a new review with those changes. I want to get to some semblance of stability ASAP |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/176/builds/880 Here is the relevant piece of the build log for the reference: |
vitalybuka
pushed a commit
that referenced
this pull request
Jul 10, 2024
Follow on to #92460 (reporting old android builds failing) and #98264 (powerpc failing to discover gtests). Getting back to stability by getting back down to a very basic set of supported arches and OS's. In the future if there is demand we can build it back up. Especially when I understand how to test these systems, or have people who want to do the work on them. --------- Co-authored-by: David Trevelyan <[email protected]>
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
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
Follow on to llvm#92460 (reporting old android builds failing) and llvm#98264 (powerpc failing to discover gtests). Getting back to stability by getting back down to a very basic set of supported arches and OS's. In the future if there is demand we can build it back up. Especially when I understand how to test these systems, or have people who want to do the work on them. --------- Co-authored-by: David Trevelyan <[email protected]>
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.
Fixing test failures in #98256
There are two issues being fixed here:
Fixed by linking dl
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