[ASan][libc++] Turn off SSO annotations for Apple platforms #96269
Conversation
This file contains 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
This commit disables short string AddressSanitizer annotations on Apple platforms as a temporary solution to the problem reported in issue llvm#96099. For more information on Apple's block implementation, please refer to [`clang/docs/Block-ABI-Apple.rst`](/clang/docs/Block-ABI-Apple.rst). The core issue lies in the fact that blocks are unaware of their content, causing AddressSanitizer errors when blocks are moved using `memmove`. I believe - and I'm not alone - that the issue should ideally be addressed within the block moving logic. However, if a timely resolution is not feasible, this temporary fix can be used. Before merging, we should ensure that a more permanent solution cannot be implemented in time and that this change effectively resolves the issue.
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Tacet (AdvenamTacet) ChangesThis commit disables short string AddressSanitizer annotations on Apple platforms as a temporary solution to the problem reported in issue #96099. For more information on Apple's block implementation, please refer to I believe - and I'm not alone - that the issue should ideally be addressed within the block moving logic. However, if a timely resolution is not feasible, this temporary fix can be used. Before merging, we should ensure that a more permanent solution cannot be implemented in time and that this change effectively resolves the issue. Full diff: https://github.com/llvm/llvm-project/pull/96269.diff 1 Files Affected:
diff --git a/libcxx/include/string b/libcxx/include/string
index 56307b391a3e5..6b442c51c607f 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1930,6 +1930,10 @@ private:
(void)__old_mid;
(void)__new_mid;
#if !defined(_LIBCPP_HAS_NO_ASAN) && defined(_LIBCPP_INSTRUMENTED_WITH_ASAN)
+ #if defined(__APPLE__)
+ if(!__is_long())
+ return;
+ #endif
std::__annotate_contiguous_container<_Allocator>(data(), data() + capacity() + 1, __old_mid, __new_mid);
#endif
}
|
|
I have no strong opinion if this should be merged; it should be @ldionne's call. #91702 seems like a nice fix to me. I don't know how the asan runtime is deployed on apple devices – if it ships with Xcode, then that should work well over there deployment-wise (assuming #91702 is shipped at the same time as the short string asan annotations.) |
ldionne
approved these changes
Jul 19, 2024
|
@AdvenamTacet Please disable email hiding, it's a LLVM policy to have proper emails associated to commits. You can use an alternate email address for contributing to LLVM if you don't want to leak your normal email address in commit logs. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/1857 Here is the relevant piece of the build log for the reference: |
|
I don't have access to a computer until next week, so I cannot look closer at that failure, but it seems unrelated at first glance. About email: so far when I was landing my PRs I just made sure that I land them with proper email. If that's problematic, I can replace my private one with another and unhide emails. However, I am not a fan of that as unfortunately I have to leak all my emails and I don't think I can enforce which one is used by the person performing merge. I just want to point out that GitHubs granularity is terrible and problematic here. I wish I could unhide some emails / allow to merge with email used in the PR (from CLI it's actually possible as I don't block pushes with my emails in commits). |
sgundapa
pushed a commit
to sgundapa/upstream_effort
that referenced
this pull request
Jul 23, 2024
This commit disables short string AddressSanitizer annotations on Apple platforms as a temporary solution to the problem reported in llvm#96099. For more information on Apple's block implementation, please refer to clang/docs/Block-ABI-Apple.rst [1]. The core issue lies in the fact that blocks are unaware of their content, causing AddressSanitizer errors when blocks are moved using `memmove`. I believe - and I'm not alone - that the issue should ideally be addressed within the block moving logic. However, this patch provides a temporary fix until a proper resolution exists in the Blocks runtime. [1]: https://github.com/llvm/llvm-project/blob/main/clang/docs/Block-ABI-Apple.rst
yuxuanchen1997
pushed a commit
that referenced
this pull request
Jul 25, 2024
Summary: This commit disables short string AddressSanitizer annotations on Apple platforms as a temporary solution to the problem reported in #96099. For more information on Apple's block implementation, please refer to clang/docs/Block-ABI-Apple.rst [1]. The core issue lies in the fact that blocks are unaware of their content, causing AddressSanitizer errors when blocks are moved using `memmove`. I believe - and I'm not alone - that the issue should ideally be addressed within the block moving logic. However, this patch provides a temporary fix until a proper resolution exists in the Blocks runtime. [1]: https://github.com/llvm/llvm-project/blob/main/clang/docs/Block-ABI-Apple.rst Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251313
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.
This commit disables short string AddressSanitizer annotations on Apple platforms as a temporary solution to the problem reported in issue #96099.
For more information on Apple's block implementation, please refer to
clang/docs/Block-ABI-Apple.rst. The core issue lies in the fact that blocks are unaware of their content, causing AddressSanitizer errors when blocks are moved usingmemmove.I believe - and I'm not alone - that the issue should ideally be addressed within the block moving logic. However, if a timely resolution is not feasible, this temporary fix can be used. Before merging, we should ensure that a more permanent solution cannot be implemented in time and that this change effectively resolves the issue.