[rtsan] Link in proper CXX ABI library #109715
Merged
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) ChangesBased on an issue reported in #109529 (comment, build problem ) We seemingly were having issues with compiling/linking against different ABIs: I cannot reproduce this issue locally, but this fix seems to be in the same general area. This new pattern was found in all of the other sanitizer's cmakelists.txt, making me think this is our problem Full diff: https://github.com/llvm/llvm-project/pull/109715.diff 1 Files Affected:
diff --git a/compiler-rt/lib/rtsan/CMakeLists.txt b/compiler-rt/lib/rtsan/CMakeLists.txt
index 0fc3a3f8f48960..d4296f56acd30d 100644
--- a/compiler-rt/lib/rtsan/CMakeLists.txt
+++ b/compiler-rt/lib/rtsan/CMakeLists.txt
@@ -27,7 +27,8 @@ set(RTSAN_CFLAGS
set(RTSAN_LINK_FLAGS ${COMPILER_RT_COMMON_LINK_FLAGS})
set(RTSAN_LINK_LIBS
${COMPILER_RT_UNWINDER_LINK_LIBS}
- ${COMPILER_RT_CXX_LINK_LIBS})
+ ${SANITIZER_CXX_ABI_LIBRARIES}
+ ${SANITIZER_COMMON_LINK_LIBS})
append_rtti_flag(OFF RTSAN_CFLAGS)
|
|
@zeroomega Can you please try this patch and see if it works to fix your issue? I cannot repro locally. |
|
CC @davidtrevelyan for review |
|
Also adding @petrhosek as it looks like he performed the original change to the sanitizers to use this version of CXX ABI linking |
It still failed at the same spot. https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci.shadow/clang-mac-x64/b8735970862707274593/overview |
|
Thanks for looking, I will continue investigating. Is there any easy way for me to run builds against this system for me to verify any potential fixes without bugging you? |
If you have a branch in your public repo, I can launch a build job for you. You can ping me on discord. I am 'zeroomega86' on discord. Please rebase your branch against upstream main branch. e.g. I uses zeroomega@642d4c8 to instrument the build file. The build task is https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci.shadow/clang-mac-x64/b8735967159680657121/overview You can see output: |
|
Thanks @zeroomega - looking into this now a bit further. I will ping you when I have something to build. |
|
I am stopping for the night with no significant progress, I'll pick it back up tomorrow. |
MaskRay
approved these changes
Sep 24, 2024
|
@MaskRay or @vitalybuka should it be possible to use the STL, like We introduced But after doing some searching, it seems like not many STL containers/algorithms are in compiler-rt. I'm wondering if we made a mistake going this direction or if we just have some easily solvable configuration issue. |
|
@cjappl I did some experiment later and I discovered that if I change In our build setup, we set Is there a reason why on mac it requires to build rtsan as a shared library but static for other platform? |
There could be multiple problems:
I'd suggest removing it. |
|
Many thanks @vitalybuka - understood. Thanks everyone for the valuable inputs, I've drafted a PR for removing the |
cjappl
pushed a commit
that referenced
this pull request
Sep 24, 2024
cjappl
added a commit
that referenced
this pull request
Sep 25, 2024
Follow on to #109715 This better matches this same variable in asan, ubsan, hwasan, and nsan. Shows the logical coupling, and describes them as "dynamic only" which is their intent.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/1090 Here is the relevant piece of the build log for the reference |
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.
Based on an issue reported in #109529 (comment, build problem )
We seemingly were having issues with compiling/linking against different ABIs:
I cannot reproduce this issue locally, but this fix seems to be in the same general area.
This new pattern was found in all of the other sanitizer's cmakelists.txt, making me think this is our problem