Skip to content

[ASan][libc++] Refactor of ASan annotation functions #74023

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 5, 2023

Conversation

AdvenamTacet
Copy link
Member

@AdvenamTacet AdvenamTacet commented Dec 1, 2023

This commit refactors the ASan annotation functions in libc++ to reduce unnecessary code duplication. Additionally it adds a small optimization.

  • Eliminates two redundant function versions by utilizing the [[maybe_unused]] attribute and guarding function bodies with #ifndef _LIBCPP_HAS_NO_ASAN.
  • Introduces an additional guard to an auxiliary function, allowing the removal of a no-ops function body. This approach avoids relying on the optimizer for code elimination.

Suggested by @ldionne in #73043

@AdvenamTacet AdvenamTacet added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 1, 2023
@AdvenamTacet AdvenamTacet requested a review from ldionne December 1, 2023 01:49
@AdvenamTacet AdvenamTacet self-assigned this Dec 1, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 1, 2023

@llvm/pr-subscribers-libcxx

Author: Tacet (AdvenamTacet)

Changes

This commit refactors the ASan annotation functions in libc++ to reduce unnecessary code duplication.

  • Eliminates two redundant function versions by utilizing the [[maybe_unused]] attribute and guarding function bodies with #ifndef _LIBCPP_HAS_NO_ASAN.
  • Introduces an additional guard to an auxiliary function, allowing the removal of a no-ops function body. This approach avoids relying on the optimizer for code elimination.

Suggested by @ldionne in #73043


Full diff: https://github.com/llvm/llvm-project/pull/74023.diff

2 Files Affected:

  • (modified) libcxx/include/deque (+15-12)
  • (modified) libcxx/include/vector (+8-9)
diff --git a/libcxx/include/deque b/libcxx/include/deque
index 1438f1e992e203f..5fd7b25cc81220a 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -966,25 +966,27 @@ public:
 // __asan_annotate_container_with_allocator to false.
 // For more details, see the "Using libc++" documentation page or
 // the documentation for __sanitizer_annotate_contiguous_container.
-#if !defined(_LIBCPP_HAS_NO_ASAN)
     _LIBCPP_HIDE_FROM_ABI void __annotate_double_ended_contiguous_container(
-        const void* __beg,
-        const void* __end,
-        const void* __old_con_beg,
-        const void* __old_con_end,
-        const void* __new_con_beg,
-        const void* __new_con_end) const {
+       [[__maybe_unused__]] const void* __beg,
+       [[__maybe_unused__]] const void* __end,
+       [[__maybe_unused__]] const void* __old_con_beg,
+       [[__maybe_unused__]] const void* __old_con_end,
+       [[__maybe_unused__]] const void* __new_con_beg,
+       [[__maybe_unused__]] const void* __new_con_end) const {
+#ifndef _LIBCPP_HAS_NO_ASAN
         if (__beg != nullptr && __asan_annotate_container_with_allocator<_Allocator>::value)
             __sanitizer_annotate_double_ended_contiguous_container(
                 __beg, __end, __old_con_beg, __old_con_end, __new_con_beg, __new_con_end);
+#endif
     }
-#else
-    _LIBCPP_HIDE_FROM_ABI void __annotate_double_ended_contiguous_container(
-        const void*, const void*, const void*, const void*, const void*, const void*) const _NOEXCEPT {}
-#endif // !defined(_LIBCPP_HAS_NO_ASAN)
 
     _LIBCPP_HIDE_FROM_ABI
-    void __annotate_from_to(size_type __beg, size_type __end, __asan_annotation_type __annotation_type, __asan_annotation_place __place) const _NOEXCEPT {
+    void __annotate_from_to(
+            [[__maybe_unused__]] size_type __beg,
+            [[__maybe_unused__]] size_type __end,
+            [[__maybe_unused__]] __asan_annotation_type __annotation_type,
+            [[__maybe_unused__]] __asan_annotation_place __place) const _NOEXCEPT {
+#ifndef _LIBCPP_HAS_NO_ASAN
         // __beg - index of the first item to annotate
         // __end - index behind the last item to annotate (so last item + 1)
         // __annotation_type - __asan_unposion or __asan_poison
@@ -1075,6 +1077,7 @@ public:
 
             __annotate_double_ended_contiguous_container(__mem_beg, __mem_end, __old_beg, __old_end, __new_beg, __new_end);
         }
+#endif
     }
 
     _LIBCPP_HIDE_FROM_ABI
diff --git a/libcxx/include/vector b/libcxx/include/vector
index 4ec6b602371eaee..fe3d3895be979a7 100644
--- a/libcxx/include/vector
+++ b/libcxx/include/vector
@@ -846,20 +846,19 @@ private:
     // __asan_annotate_container_with_allocator to false.
     // For more details, see the "Using libc++" documentation page or
     // the documentation for __sanitizer_annotate_contiguous_container.
-#ifndef _LIBCPP_HAS_NO_ASAN
+
     _LIBCPP_CONSTEXPR_SINCE_CXX20
-    void __annotate_contiguous_container(const void *__beg, const void *__end,
-                                         const void *__old_mid,
-                                         const void *__new_mid) const
+    void __annotate_contiguous_container([[__maybe_unused__]] const void *__beg,
+                                         [[__maybe_unused__]] const void *__end,
+                                         [[__maybe_unused__]] const void *__old_mid,
+                                         [[__maybe_unused__]] const void *__new_mid) const
     {
+#ifndef _LIBCPP_HAS_NO_ASAN
       if (!__libcpp_is_constant_evaluated() && __beg != nullptr && __asan_annotate_container_with_allocator<_Allocator>::value)
         __sanitizer_annotate_contiguous_container(__beg, __end, __old_mid, __new_mid);
-    }
-#else
-    _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI
-    void __annotate_contiguous_container(const void*, const void*, const void*,
-                                         const void*) const _NOEXCEPT {}
 #endif
+    }
+
     _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI
     void __annotate_new(size_type __current_size) const _NOEXCEPT {
       __annotate_contiguous_container(data(), data() + capacity(),

This commit refactors the ASan annotation functions in libc++ to improve code maintainability and reduce unnecessary code duplication.

- Eliminates two redundant function versions by utilizing the [[maybe_unused]] attribute and guarding function bodies with #ifndef _LIBCPP_HAS_NO_ASAN.
- Introduces an additional guard to an auxiliary function, allowing the removal of a no-ops function body. This approach avoids relying on the optimizer for code elimination.

Suggested by @ldionne in llvm#73043
@AdvenamTacet AdvenamTacet force-pushed the asan-optimization-refactoring branch from 6af6967 to 52245e6 Compare December 5, 2023 03:57
It wasn't catched by tests before, but the function should be hidden from ABI always.
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the patch. I think this makes it clearer that this code is really not used when asan is disabled.

Co-authored-by: Louis Dionne <[email protected]>
@ldionne ldionne merged commit 02f4b36 into llvm:main Dec 5, 2023
@AdvenamTacet AdvenamTacet deleted the asan-optimization-refactoring branch December 5, 2023 18:52
AdvenamTacet pushed a commit to trail-of-forks/llvm-project that referenced this pull request Dec 12, 2023
This commit refactors the ASan annotation function to reduce unnecessary code duplication.

Suggested here: llvm#73043
Related PR: llvm#74023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libc++] std::deque loops over the deque for ASAN even when annotations are disabled
3 participants