[llvm-branch-commits] [libcxx] 7800ae0 - Revert "[ASan][libc++] Turn on ASan annotations for short strings"

via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Jan 18 13:21:30 PST 2024


Author: Vitaly Buka
Date: 2024-01-18T13:21:23-08:00
New Revision: 7800ae0a0f495bb2cc5cefd855339db72232640f

URL: https://github.com/llvm/llvm-project/commit/7800ae0a0f495bb2cc5cefd855339db72232640f
DIFF: https://github.com/llvm/llvm-project/commit/7800ae0a0f495bb2cc5cefd855339db72232640f.diff

LOG: Revert "[ASan][libc++] Turn on ASan annotations for short strings"

Added: 
    

Modified: 
    libcxx/include/string
    libcxx/test/support/asan_testing.h

Removed: 
    libcxx/test/libcxx/containers/strings/basic.string/asan_deque_integration.pass.cpp
    libcxx/test/libcxx/containers/strings/basic.string/asan_short.pass.cpp
    libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp


################################################################################
diff  --git a/libcxx/include/string b/libcxx/include/string
index 4116f350a80476..e97139206d4fa7 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -659,6 +659,7 @@ _LIBCPP_PUSH_MACROS
 #else
 #  define _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS
 #endif
+#define _LIBCPP_SHORT_STRING_ANNOTATIONS_ALLOWED false
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
@@ -1895,17 +1896,22 @@ private:
 #endif
   }
 
+  // ASan: short string is poisoned if and only if this function returns true.
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 bool __asan_short_string_is_annotated() const _NOEXCEPT {
+    return _LIBCPP_SHORT_STRING_ANNOTATIONS_ALLOWED && !__libcpp_is_constant_evaluated();
+  }
+
   _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())
+    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())
+    if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long()))
       __annotate_contiguous_container(data() + size() + 1, data() + capacity() + 1);
 #endif
   }
@@ -1913,7 +1919,7 @@ private:
   _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())
+    if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long()))
       __annotate_contiguous_container(data() + size() + 1, data() + size() + 1 + __n);
 #endif
   }
@@ -1921,7 +1927,7 @@ private:
   _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())
+    if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long()))
       __annotate_contiguous_container(data() + __old_size + 1, data() + size() + 1);
 #endif
   }

diff  --git a/libcxx/test/libcxx/containers/strings/basic.string/asan_deque_integration.pass.cpp b/libcxx/test/libcxx/containers/strings/basic.string/asan_deque_integration.pass.cpp
deleted file mode 100644
index b914609f35ddf3..00000000000000
--- a/libcxx/test/libcxx/containers/strings/basic.string/asan_deque_integration.pass.cpp
+++ /dev/null
@@ -1,182 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// REQUIRES: asan
-// UNSUPPORTED: c++03
-
-#include <cassert>
-#include <string>
-#include <array>
-#include <deque>
-#include "test_macros.h"
-#include "asan_testing.h"
-#include "min_allocator.h"
-
-// This tests exists to check if strings work well with deque, as those
-// may be partialy annotated, we cannot simply call
-// is_double_ended_contiguous_container_asan_correct, as it assumes that
-// object memory inside is not annotated, so we check everything in a more careful way.
-
-template <typename D>
-void verify_inside(D const& d) {
-  for (size_t i = 0; i < d.size(); ++i) {
-    assert(is_string_asan_correct(d[i]));
-  }
-}
-
-template <typename S, size_t N>
-S get_s(char c) {
-  S s;
-  for (size_t i = 0; i < N; ++i)
-    s.push_back(c);
-
-  return s;
-}
-
-template <class C, class S>
-void test_string() {
-  size_t const N = sizeof(S) < 256 ? (4096 / sizeof(S)) : 16;
-
-  {
-    C d1a(1), d1b(N), d1c(N + 1), d1d(32 * N);
-    verify_inside(d1a);
-    verify_inside(d1b);
-    verify_inside(d1c);
-    verify_inside(d1d);
-  }
-  {
-    C d2;
-    for (size_t i = 0; i < 16 * N; ++i) {
-      d2.push_back(get_s<S, 1>(i % 10 + 'a'));
-      verify_inside(d2);
-      d2.push_back(get_s<S, 222>(i % 10 + 'b'));
-      verify_inside(d2);
-
-      d2.pop_front();
-      verify_inside(d2);
-    }
-  }
-  {
-    C d3;
-    for (size_t i = 0; i < 16 * N; ++i) {
-      d3.push_front(get_s<S, 1>(i % 10 + 'a'));
-      verify_inside(d3);
-      d3.push_front(get_s<S, 222>(i % 10 + 'b'));
-      verify_inside(d3);
-
-      d3.pop_back();
-      verify_inside(d3);
-    }
-  }
-  {
-    C d4;
-    for (size_t i = 0; i < 16 * N; ++i) {
-      // When there is no SSO, all elements inside should not be poisoned,
-      // so we can verify deque poisoning.
-      d4.push_front(get_s<S, 333>(i % 10 + 'a'));
-      verify_inside(d4);
-      assert(is_double_ended_contiguous_container_asan_correct(d4));
-      d4.push_back(get_s<S, 222>(i % 10 + 'b'));
-      verify_inside(d4);
-      assert(is_double_ended_contiguous_container_asan_correct(d4));
-    }
-  }
-  {
-    C d5;
-    for (size_t i = 0; i < 5 * N; ++i) {
-      // In d4 we never had poisoned memory inside deque.
-      // Here we start with SSO, so part of the inside of the container,
-      // will be poisoned.
-      d5.push_front(S());
-      verify_inside(d5);
-    }
-    for (size_t i = 0; i < d5.size(); ++i) {
-      // We change the size to have long string.
-      // Memory owne by deque should not be poisoned by string.
-      d5[i].resize(1000);
-      verify_inside(d5);
-    }
-
-    assert(is_double_ended_contiguous_container_asan_correct(d5));
-
-    d5.erase(d5.begin() + 2);
-    verify_inside(d5);
-
-    d5.erase(d5.end() - 2);
-    verify_inside(d5);
-
-    assert(is_double_ended_contiguous_container_asan_correct(d5));
-  }
-  {
-    C d6a;
-    assert(is_double_ended_contiguous_container_asan_correct(d6a));
-
-    C d6b(N + 2, get_s<S, 1000>('a'));
-    d6b.push_front(get_s<S, 1001>('b'));
-    while (!d6b.empty()) {
-      d6b.pop_back();
-      assert(is_double_ended_contiguous_container_asan_correct(d6b));
-    }
-
-    C d6c(N + 2, get_s<S, 1002>('c'));
-    while (!d6c.empty()) {
-      d6c.pop_back();
-      assert(is_double_ended_contiguous_container_asan_correct(d6c));
-    }
-  }
-  {
-    C d7(9 * N + 2);
-
-    d7.insert(d7.begin() + 1, S());
-    verify_inside(d7);
-
-    d7.insert(d7.end() - 3, S());
-    verify_inside(d7);
-
-    d7.insert(d7.begin() + 2 * N, get_s<S, 1>('a'));
-    verify_inside(d7);
-
-    d7.insert(d7.end() - 2 * N, get_s<S, 1>('b'));
-    verify_inside(d7);
-
-    d7.insert(d7.begin() + 2 * N, 3 * N, get_s<S, 1>('c'));
-    verify_inside(d7);
-
-    // It may not be short for big element types, but it will be checked correctly:
-    d7.insert(d7.end() - 2 * N, 3 * N, get_s<S, 2>('d'));
-    verify_inside(d7);
-
-    d7.erase(d7.begin() + 2);
-    verify_inside(d7);
-
-    d7.erase(d7.end() - 2);
-    verify_inside(d7);
-  }
-}
-
-template <class S>
-void test_container() {
-  test_string<std::deque<S, std::allocator<S>>, S>();
-  test_string<std::deque<S, min_allocator<S>>, S>();
-  test_string<std::deque<S, safe_allocator<S>>, S>();
-}
-
-int main(int, char**) {
-  // Those tests support only types based on std::basic_string.
-  test_container<std::string>();
-  test_container<std::wstring>();
-#if TEST_STD_VER >= 11
-  test_container<std::u16string>();
-  test_container<std::u32string>();
-#endif
-#if TEST_STD_VER >= 20
-  test_container<std::u8string>();
-#endif
-
-  return 0;
-}

diff  --git a/libcxx/test/libcxx/containers/strings/basic.string/asan_short.pass.cpp b/libcxx/test/libcxx/containers/strings/basic.string/asan_short.pass.cpp
deleted file mode 100644
index 53c70bed189b5c..00000000000000
--- a/libcxx/test/libcxx/containers/strings/basic.string/asan_short.pass.cpp
+++ /dev/null
@@ -1,56 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// REQUIRES: asan
-// UNSUPPORTED: c++03
-
-// <string>
-
-// Basic test if ASan annotations work for short strings.
-
-#include <string>
-#include <cassert>
-#include <cstdlib>
-
-#include "asan_testing.h"
-#include "min_allocator.h"
-#include "test_iterators.h"
-#include "test_macros.h"
-
-extern "C" void __sanitizer_set_death_callback(void (*callback)(void));
-
-void do_exit() { exit(0); }
-
-int main(int, char**) {
-  {
-    typedef cpp17_input_iterator<char*> MyInputIter;
-    // Should not trigger ASan.
-    std::basic_string<char, std::char_traits<char>, safe_allocator<char>> v;
-    char i[] = {'a', 'b', 'c', 'd'};
-
-    v.insert(v.begin(), MyInputIter(i), MyInputIter(i + 4));
-    assert(v[0] == 'a');
-    assert(is_string_asan_correct(v));
-  }
-
-  __sanitizer_set_death_callback(do_exit);
-  {
-    using T     = char;
-    using C     = std::basic_string<T, std::char_traits<T>, safe_allocator<T>>;
-    const T t[] = {'a', 'b', 'c', 'd', 'e', 'f', 'g'};
-    C c(std::begin(t), std::end(t));
-    assert(is_string_asan_correct(c));
-    assert(__sanitizer_verify_contiguous_container(c.data(), c.data() + c.size() + 1, c.data() + c.capacity() + 1) !=
-           0);
-    volatile T foo = c[c.size() + 1]; // should trigger ASAN. Use volatile to prevent being optimized away.
-    assert(false);                    // if we got here, ASAN didn't trigger
-    ((void)foo);
-  }
-
-  return 0;
-}

diff  --git a/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp b/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp
deleted file mode 100644
index 5b1900fb00d5bb..00000000000000
--- a/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp
+++ /dev/null
@@ -1,182 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// REQUIRES: asan
-// UNSUPPORTED: c++03
-
-#include <cassert>
-#include <string>
-#include <vector>
-#include <array>
-#include "test_macros.h"
-#include "asan_testing.h"
-#include "min_allocator.h"
-
-// This tests exists to check if strings work well with vector, as those
-// may be partialy annotated, we cannot simply call
-// is_contiguous_container_asan_correct, as it assumes that
-// object memory inside is not annotated, so we check everything in a more careful way.
-
-template <typename D>
-void verify_inside(D const& d) {
-  for (size_t i = 0; i < d.size(); ++i) {
-    assert(is_string_asan_correct(d[i]));
-  }
-}
-
-template <typename S, size_t N>
-S get_s(char c) {
-  S s;
-  for (size_t i = 0; i < N; ++i)
-    s.push_back(c);
-
-  return s;
-}
-
-template <class C, class S>
-void test_string() {
-  size_t const N = sizeof(S) < 256 ? (4096 / sizeof(S)) : 16;
-
-  {
-    C d1a(1), d1b(N), d1c(N + 1), d1d(32 * N);
-    verify_inside(d1a);
-    verify_inside(d1b);
-    verify_inside(d1c);
-    verify_inside(d1d);
-  }
-  {
-    C d2;
-    for (size_t i = 0; i < 16 * N; ++i) {
-      d2.push_back(get_s<S, 1>(i % 10 + 'a'));
-      verify_inside(d2);
-      d2.push_back(get_s<S, 222>(i % 10 + 'b'));
-      verify_inside(d2);
-
-      d2.erase(d2.cbegin());
-      verify_inside(d2);
-    }
-  }
-  {
-    C d3;
-    for (size_t i = 0; i < 16 * N; ++i) {
-      d3.push_back(get_s<S, 1>(i % 10 + 'a'));
-      verify_inside(d3);
-      d3.push_back(get_s<S, 222>(i % 10 + 'b'));
-      verify_inside(d3);
-
-      d3.pop_back();
-      verify_inside(d3);
-    }
-  }
-  {
-    C d4;
-    for (size_t i = 0; i < 16 * N; ++i) {
-      // When there is no SSO, all elements inside should not be poisoned,
-      // so we can verify vector poisoning.
-      d4.push_back(get_s<S, 333>(i % 10 + 'a'));
-      verify_inside(d4);
-      assert(is_contiguous_container_asan_correct(d4));
-      d4.push_back(get_s<S, 222>(i % 10 + 'b'));
-      verify_inside(d4);
-      assert(is_contiguous_container_asan_correct(d4));
-    }
-  }
-  {
-    C d5;
-    for (size_t i = 0; i < 5 * N; ++i) {
-      // In d4 we never had poisoned memory inside vector.
-      // Here we start with SSO, so part of the inside of the container,
-      // will be poisoned.
-      d5.push_back(S());
-      verify_inside(d5);
-    }
-    for (size_t i = 0; i < d5.size(); ++i) {
-      // We change the size to have long string.
-      // Memory owne by vector should not be poisoned by string.
-      d5[i].resize(1000);
-      verify_inside(d5);
-    }
-
-    assert(is_contiguous_container_asan_correct(d5));
-
-    d5.erase(d5.begin() + 2);
-    verify_inside(d5);
-
-    d5.erase(d5.end() - 2);
-    verify_inside(d5);
-
-    assert(is_contiguous_container_asan_correct(d5));
-  }
-  {
-    C d6a;
-    assert(is_contiguous_container_asan_correct(d6a));
-
-    C d6b(N + 2, get_s<S, 1000>('a'));
-    d6b.push_back(get_s<S, 1001>('b'));
-    while (!d6b.empty()) {
-      d6b.pop_back();
-      assert(is_contiguous_container_asan_correct(d6b));
-    }
-
-    C d6c(N + 2, get_s<S, 1002>('c'));
-    while (!d6c.empty()) {
-      d6c.pop_back();
-      assert(is_contiguous_container_asan_correct(d6c));
-    }
-  }
-  {
-    C d7(9 * N + 2);
-
-    d7.insert(d7.begin() + 1, S());
-    verify_inside(d7);
-
-    d7.insert(d7.end() - 3, S());
-    verify_inside(d7);
-
-    d7.insert(d7.begin() + 2 * N, get_s<S, 1>('a'));
-    verify_inside(d7);
-
-    d7.insert(d7.end() - 2 * N, get_s<S, 1>('b'));
-    verify_inside(d7);
-
-    d7.insert(d7.begin() + 2 * N, 3 * N, get_s<S, 1>('c'));
-    verify_inside(d7);
-
-    // It may not be short for big element types, but it will be checked correctly:
-    d7.insert(d7.end() - 2 * N, 3 * N, get_s<S, 2>('d'));
-    verify_inside(d7);
-
-    d7.erase(d7.begin() + 2);
-    verify_inside(d7);
-
-    d7.erase(d7.end() - 2);
-    verify_inside(d7);
-  }
-}
-
-template <class S>
-void test_container() {
-  test_string<std::vector<S, std::allocator<S>>, S>();
-  test_string<std::vector<S, min_allocator<S>>, S>();
-  test_string<std::vector<S, safe_allocator<S>>, S>();
-}
-
-int main(int, char**) {
-  // Those tests support only types based on std::basic_string.
-  test_container<std::string>();
-  test_container<std::wstring>();
-#if TEST_STD_VER >= 11
-  test_container<std::u16string>();
-  test_container<std::u32string>();
-#endif
-#if TEST_STD_VER >= 20
-  test_container<std::u8string>();
-#endif
-
-  return 0;
-}

diff  --git a/libcxx/test/support/asan_testing.h b/libcxx/test/support/asan_testing.h
index 3785c1f9c20dea..6bfc8280a4ead3 100644
--- a/libcxx/test/support/asan_testing.h
+++ b/libcxx/test/support/asan_testing.h
@@ -56,16 +56,35 @@ TEST_CONSTEXPR bool is_double_ended_contiguous_container_asan_correct(const std:
 #endif
 
 #if TEST_HAS_FEATURE(address_sanitizer)
+template <typename S>
+bool is_string_short(S const& s) {
+  // We do not have access to __is_long(), but we can check if strings
+  // buffer is inside strings memory. If strings memory contains its content,
+  // SSO is in use. To check it, we can just confirm that the beginning is in
+  // the string object memory block.
+  // &s    - beginning of objects memory
+  // &s[0] - beginning of the buffer
+  // (&s+1) - end of objects memory
+  return (void*)std::addressof(s) <= (void*)std::addressof(s[0]) &&
+         (void*)std::addressof(s[0]) < (void*)(std::addressof(s) + 1);
+}
+
 template <typename ChrT, typename TraitsT, typename Alloc>
 TEST_CONSTEXPR bool is_string_asan_correct(const std::basic_string<ChrT, TraitsT, Alloc>& c) {
   if (TEST_IS_CONSTANT_EVALUATED)
     return true;
 
-  if (std::__asan_annotate_container_with_allocator<Alloc>::value)
-    return __sanitizer_verify_contiguous_container(c.data(), c.data() + c.size() + 1, c.data() + c.capacity() + 1) != 0;
-  else
-    return __sanitizer_verify_contiguous_container(
-               c.data(), c.data() + c.capacity() + 1, c.data() + c.capacity() + 1) != 0;
+  if (!is_string_short(c) || _LIBCPP_SHORT_STRING_ANNOTATIONS_ALLOWED) {
+    if (std::__asan_annotate_container_with_allocator<Alloc>::value)
+      return __sanitizer_verify_contiguous_container(c.data(), c.data() + c.size() + 1, c.data() + c.capacity() + 1) !=
+             0;
+    else
+      return __sanitizer_verify_contiguous_container(
+                 c.data(), c.data() + c.capacity() + 1, c.data() + c.capacity() + 1) != 0;
+  } else {
+    return __sanitizer_verify_contiguous_container(std::addressof(c), std::addressof(c) + 1, std::addressof(c) + 1) !=
+           0;
+  }
 }
 #else
 #  include <string>


        


More information about the llvm-branch-commits mailing list