[libcxx-commits] [libcxx] [ASan][libc++] Optimize `__annotate_delete` for the default allocator (PR #76176)

[via libcxx-commits]

https://github.com/AdvenamTacet updated https://github.com/llvm/llvm-project/pull/76176

>From d55a13c8b35394d6e6cdc0f3042d2167bcc81bb1 Mon Sep 17 00:00:00 2001
From: Advenam Tacet <advenam.tacet at trailofbits.com>
Date: Thu, 21 Dec 2023 19:44:34 +0100
Subject: [PATCH 1/3] [ASan][libc++] Optimize `__annotate_delete` for the
 default allocator

This commit optimizes the ASan helper functions `__annotate_delete()`, in `std::basic_string`, `std::vector` and `std::deque`, by adding `if` statements to prevent unpoisoning of memory for the default allocator. Unpoisoning is not required by the default allocator, and since it is widely used, this optimization should yield a meaningful performance improvement.

The optimization was suggested by @EricWF.
---
 libcxx/include/deque  | 3 +++
 libcxx/include/string | 6 ++++--
 libcxx/include/vector | 4 +++-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/libcxx/include/deque b/libcxx/include/deque
index fca8b3d6e2c737..36e7e6dc6621fa 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -1011,6 +1011,9 @@ private:
 
   _LIBCPP_HIDE_FROM_ABI void __annotate_delete() const _NOEXCEPT {
 #ifndef _LIBCPP_HAS_NO_ASAN
+    // The default allocator does not require unpoisoning before returning memory.
+    if _LIBCPP_CONSTEXPR (is_same<allocator_type, allocator<_Tp>>::value)
+      return;
     if (empty()) {
       for (size_t __i = 0; __i < __map_.size(); ++__i) {
         __annotate_whole_block(__i, __asan_unposion);
diff --git a/libcxx/include/string b/libcxx/include/string
index c676182fba8bac..8a5e4c5e530ce2 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1912,8 +1912,10 @@ private:
 
   _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);
+    // The default allocator does not require unpoisoning before returning memory.
+    if _LIBCPP_CONSTEXPR (!is_same<allocator_type, allocator<__default_allocator_type>>::value)
+      if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long()))
+        __annotate_contiguous_container(data() + size() + 1, data() + capacity() + 1);
 #endif
   }
 
diff --git a/libcxx/include/vector b/libcxx/include/vector
index 0098273a195ff8..dea2ea3a13b6d0 100644
--- a/libcxx/include/vector
+++ b/libcxx/include/vector
@@ -853,7 +853,9 @@ private:
 
   _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());
+    // The default allocator does not require unpoisoning before returning memory.
+    if _LIBCPP_CONSTEXPR (!is_same<allocator_type, __default_allocator_type>::value)
+      __annotate_contiguous_container(data(), data() + capacity(), data() + size(), data() + capacity());
 #endif
   }
 

>From 418cdd7ac97f492231ff145007df306197ff1b13 Mon Sep 17 00:00:00 2001
From: Advenam Tacet <advenam.tacet at trailofbits.com>
Date: Thu, 21 Dec 2023 22:21:31 +0100
Subject: [PATCH 2/3] C++03 fix

---
 libcxx/include/deque  | 2 +-
 libcxx/include/string | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/include/deque b/libcxx/include/deque
index 36e7e6dc6621fa..790c45c26d80fa 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -1012,7 +1012,7 @@ private:
   _LIBCPP_HIDE_FROM_ABI void __annotate_delete() const _NOEXCEPT {
 #ifndef _LIBCPP_HAS_NO_ASAN
     // The default allocator does not require unpoisoning before returning memory.
-    if _LIBCPP_CONSTEXPR (is_same<allocator_type, allocator<_Tp>>::value)
+    if _LIBCPP_CONSTEXPR (is_same<allocator_type, allocator<_Tp> >::value)
       return;
     if (empty()) {
       for (size_t __i = 0; __i < __map_.size(); ++__i) {
diff --git a/libcxx/include/string b/libcxx/include/string
index 8a5e4c5e530ce2..5c2a6bea90e6c9 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1913,7 +1913,7 @@ private:
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __annotate_delete() const _NOEXCEPT {
 #if !defined(_LIBCPP_HAS_NO_ASAN) && defined(_LIBCPP_INSTRUMENTED_WITH_ASAN)
     // The default allocator does not require unpoisoning before returning memory.
-    if _LIBCPP_CONSTEXPR (!is_same<allocator_type, allocator<__default_allocator_type>>::value)
+    if _LIBCPP_CONSTEXPR (!is_same<allocator_type, allocator<__default_allocator_type> >::value)
       if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long()))
         __annotate_contiguous_container(data() + size() + 1, data() + capacity() + 1);
 #endif

>From 629e00786578ff0cbb64b107ce21678862c0fd71 Mon Sep 17 00:00:00 2001
From: Advenam Tacet <advenam.tacet at trailofbits.com>
Date: Thu, 21 Dec 2023 23:01:13 +0100
Subject: [PATCH 3/3] if constexpr since C++17

---
 libcxx/include/deque  | 2 +-
 libcxx/include/string | 2 +-
 libcxx/include/vector | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libcxx/include/deque b/libcxx/include/deque
index 790c45c26d80fa..73d9a3b3920882 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -1012,7 +1012,7 @@ private:
   _LIBCPP_HIDE_FROM_ABI void __annotate_delete() const _NOEXCEPT {
 #ifndef _LIBCPP_HAS_NO_ASAN
     // The default allocator does not require unpoisoning before returning memory.
-    if _LIBCPP_CONSTEXPR (is_same<allocator_type, allocator<_Tp> >::value)
+    if _LIBCPP_CONSTEXPR_SINCE_CXX17 (is_same<allocator_type, allocator<_Tp> >::value)
       return;
     if (empty()) {
       for (size_t __i = 0; __i < __map_.size(); ++__i) {
diff --git a/libcxx/include/string b/libcxx/include/string
index 5c2a6bea90e6c9..3e1d468892d0a3 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1913,7 +1913,7 @@ private:
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __annotate_delete() const _NOEXCEPT {
 #if !defined(_LIBCPP_HAS_NO_ASAN) && defined(_LIBCPP_INSTRUMENTED_WITH_ASAN)
     // The default allocator does not require unpoisoning before returning memory.
-    if _LIBCPP_CONSTEXPR (!is_same<allocator_type, allocator<__default_allocator_type> >::value)
+    if _LIBCPP_CONSTEXPR_SINCE_CXX17 (!is_same<allocator_type, allocator<__default_allocator_type> >::value)
       if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long()))
         __annotate_contiguous_container(data() + size() + 1, data() + capacity() + 1);
 #endif
diff --git a/libcxx/include/vector b/libcxx/include/vector
index dea2ea3a13b6d0..5b248ef9d6aa62 100644
--- a/libcxx/include/vector
+++ b/libcxx/include/vector
@@ -854,7 +854,7 @@ private:
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __annotate_delete() const _NOEXCEPT {
 #ifndef _LIBCPP_HAS_NO_ASAN
     // The default allocator does not require unpoisoning before returning memory.
-    if _LIBCPP_CONSTEXPR (!is_same<allocator_type, __default_allocator_type>::value)
+    if _LIBCPP_CONSTEXPR_SINCE_CXX17 (!is_same<allocator_type, __default_allocator_type>::value)
       __annotate_contiguous_container(data(), data() + capacity(), data() + size(), data() + capacity());
 #endif
   }