[ASan][libc++] Optimization of container annotations

[AdvenamTacet]

This commit implements conditional compilation for ASan helper code.

As convey to me by @EricWF, string benchmarks with UBSan have been experiencing significant performance hit after the commit with ASan string annotations. This is likely due to the fact that no-op ASan code is not optimized out with UBSan. To address this issue, this commit conditionalizes the inclusion of ASan helper function bodies using #ifdef directives. This approach allows us to selectively include only the ASan code when it's actually required, thereby enhancing optimizations and improving performance.

While issue was noticed in string benchmarks, I expect same overhead (just less noticeable) in other containers, therefore std::vector and std::deque have same changes.

To see impact of that change run string.libcxx.out with UBSan and --benchmark_filter=BM_StringAssign or --benchmark_filter=BM_StringConstruct.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Tacet (AdvenamTacet)

Changes

This commit implements conditional compilation for ASan helper code.

As convey to me by @EricWF, string benchmarks with UBSan have been experiencing significant performance hit after the commit with ASan string annotations. This is likely due to the fact that no-op ASan code is not optimized out with UBSan. To address this issue, this commit conditionalizes the inclusion of ASan helper function bodies using #ifdef directives. This approach allows us to selectively include only the ASan code when it's actually required, thereby enhancing optimizations and improving performance.

While issue was noticed in string benchmarks, I expect same overhead (just less noticeable) in other containers, therefore std::vector and std::deque have same changes.

To see impact of that change run string.libcxx.out with UBSan and --benchmark_filter=BM_StringAssign or --benchmark_filter=BM_StringConstruct.

---

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

3 Files Affected:

• (modified) libcxx/include/deque (+27)
• (modified) libcxx/include/string (+11)
• (modified) libcxx/include/vector (+11)

diff --git a/libcxx/include/deque b/libcxx/include/deque
index d0520b635bcc8f..fca8b3d6e2c737 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -998,15 +998,19 @@ private:
   }
 
   _LIBCPP_HIDE_FROM_ABI void __annotate_new(size_type __current_size) const _NOEXCEPT {
+    (void)__current_size;
+#ifndef _LIBCPP_HAS_NO_ASAN
     if (__current_size == 0)
       __annotate_from_to(0, __map_.size() * __block_size, __asan_poison, __asan_back_moved);
     else {
       __annotate_from_to(0, __start_, __asan_poison, __asan_front_moved);
       __annotate_from_to(__start_ + __current_size, __map_.size() * __block_size, __asan_poison, __asan_back_moved);
     }
+#endif
   }
 
   _LIBCPP_HIDE_FROM_ABI void __annotate_delete() const _NOEXCEPT {
+#ifndef _LIBCPP_HAS_NO_ASAN
     if (empty()) {
       for (size_t __i = 0; __i < __map_.size(); ++__i) {
         __annotate_whole_block(__i, __asan_unposion);
@@ -1015,30 +1019,52 @@ private:
       __annotate_from_to(0, __start_, __asan_unposion, __asan_front_moved);
       __annotate_from_to(__start_ + size(), __map_.size() * __block_size, __asan_unposion, __asan_back_moved);
     }
+#endif
   }
 
   _LIBCPP_HIDE_FROM_ABI void __annotate_increase_front(size_type __n) const _NOEXCEPT {
+    (void)__n;
+#ifndef _LIBCPP_HAS_NO_ASAN
     __annotate_from_to(__start_ - __n, __start_, __asan_unposion, __asan_front_moved);
+#endif
   }
 
   _LIBCPP_HIDE_FROM_ABI void __annotate_increase_back(size_type __n) const _NOEXCEPT {
+    (void)__n;
+#ifndef _LIBCPP_HAS_NO_ASAN
     __annotate_from_to(__start_ + size(), __start_ + size() + __n, __asan_unposion, __asan_back_moved);
+#endif
   }
 
   _LIBCPP_HIDE_FROM_ABI void __annotate_shrink_front(size_type __old_size, size_type __old_start) const _NOEXCEPT {
+    (void)__old_size;
+    (void)__old_start;
+#ifndef _LIBCPP_HAS_NO_ASAN
     __annotate_from_to(__old_start, __old_start + (__old_size - size()), __asan_poison, __asan_front_moved);
+#endif
   }
 
   _LIBCPP_HIDE_FROM_ABI void __annotate_shrink_back(size_type __old_size, size_type __old_start) const _NOEXCEPT {
+    (void)__old_size;
+    (void)__old_start;
+#ifndef _LIBCPP_HAS_NO_ASAN
     __annotate_from_to(__old_start + size(), __old_start + __old_size, __asan_poison, __asan_back_moved);
+#endif
   }
 
   _LIBCPP_HIDE_FROM_ABI void __annotate_poison_block(const void* __beginning, const void* __end) const _NOEXCEPT {
+    (void)__beginning;
+    (void)__end;
+#ifndef _LIBCPP_HAS_NO_ASAN
     __annotate_double_ended_contiguous_container(__beginning, __end, __beginning, __end, __end, __end);
+#endif
   }
 
   _LIBCPP_HIDE_FROM_ABI void
   __annotate_whole_block(size_t __block_index, __asan_annotation_type __annotation_type) const _NOEXCEPT {
+    (void)__block_index;
+    (void)__annotation_type;
+#ifndef _LIBCPP_HAS_NO_ASAN
     __map_const_iterator __block_it = __map_.begin() + __block_index;
     const void* __block_start       = std::__to_address(*__block_it);
     const void* __block_end         = std::__to_address(*__block_it + __block_size);
@@ -1049,6 +1075,7 @@ private:
       __annotate_double_ended_contiguous_container(
           __block_start, __block_end, __block_start, __block_start, __block_start, __block_end);
     }
+#endif
   }
 #if !defined(_LIBCPP_HAS_NO_ASAN)
 
diff --git a/libcxx/include/string b/libcxx/include/string
index fdffca5aed18be..c676182fba8bac 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1903,23 +1903,34 @@ private:
   }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __annotate_new(size_type __current_size) const _NOEXCEPT {
+    (void) __current_size;
+#if !defined(_LIBCPP_HAS_NO_ASAN) && defined(_LIBCPP_INSTRUMENTED_WITH_ASAN)
     if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long()))
       __annotate_contiguous_container(data() + capacity() + 1, data() + __current_size + 1);
+#endif
   }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __annotate_delete() const _NOEXCEPT {
+#if !defined(_LIBCPP_HAS_NO_ASAN) && defined(_LIBCPP_INSTRUMENTED_WITH_ASAN)
     if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long()))
       __annotate_contiguous_container(data() + size() + 1, data() + capacity() + 1);
+#endif
   }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __annotate_increase(size_type __n) const _NOEXCEPT {
+    (void) __n;
+#if !defined(_LIBCPP_HAS_NO_ASAN) && defined(_LIBCPP_INSTRUMENTED_WITH_ASAN)
     if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long()))
       __annotate_contiguous_container(data() + size() + 1, data() + size() + 1 + __n);
+#endif
   }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __annotate_shrink(size_type __old_size) const _NOEXCEPT {
+    (void) __old_size;
+#if !defined(_LIBCPP_HAS_NO_ASAN) && defined(_LIBCPP_INSTRUMENTED_WITH_ASAN)
     if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long()))
       __annotate_contiguous_container(data() + __old_size + 1, data() + size() + 1);
+#endif
   }
 
   template <size_type __a>
diff --git a/libcxx/include/vector b/libcxx/include/vector
index 3abc917f5c0e18..0098273a195ff8 100644
--- a/libcxx/include/vector
+++ b/libcxx/include/vector
@@ -845,19 +845,30 @@ private:
   }
 
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __annotate_new(size_type __current_size) const _NOEXCEPT {
+    (void)__current_size;
+#ifndef _LIBCPP_HAS_NO_ASAN
     __annotate_contiguous_container(data(), data() + capacity(), data() + capacity(), data() + __current_size);
+#endif
   }
 
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __annotate_delete() const _NOEXCEPT {
+#ifndef _LIBCPP_HAS_NO_ASAN
     __annotate_contiguous_container(data(), data() + capacity(), data() + size(), data() + capacity());
+#endif
   }
 
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __annotate_increase(size_type __n) const _NOEXCEPT {
+    (void)__n;
+#ifndef _LIBCPP_HAS_NO_ASAN
     __annotate_contiguous_container(data(), data() + capacity(), data() + size(), data() + size() + __n);
+#endif
   }
 
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __annotate_shrink(size_type __old_size) const _NOEXCEPT {
+    (void)__old_size;
+#ifndef _LIBCPP_HAS_NO_ASAN
     __annotate_contiguous_container(data(), data() + capacity(), data() + __old_size, data() + size());
+#endif
   }
 
   struct _ConstructTransaction {

[AdvenamTacet]

String changes from that PR are probably no longer needed, because of ongoing work on zero cost string ASan annotations.

Not sure if we want to include that in std::vector and std::deque, probably if we want to do something, we want to do same thing as in std::string, inside std::vector and std::deque.

@EricWF if you agree, please close this PR without a merge.

[EricWF]

Yep. I think this is superseded.

[ldionne]

After discussing with Eric, I think we should first try this solution. Based on https://godbolt.org/z/dEo34aP1a, I think we get back almost all the code size benefits with almost no complexity.

[ldionne]

Sorry for doing this unilaterally, this is apparently quite time sensitive and we want to measure whether this change helps. If it doesn't, we can go all the way to #76165 but the downsides of that patch are quite significant.

[AdvenamTacet]

Then we should also merge #76192 with green CI as it's the only known to me case of growing stack frame. (Possibly it's not the only one, but the only one I'm aware of.)

Edit: improved solution is use of lambda.