[ASan][libc++] Annotating std::basic_string with all allocators (#7…

…5845)

This commit turns on ASan annotations in `std::basic_string` for all
allocators by default.

Originally suggested here: https://reviews.llvm.org/D146214

String annotations added here:
#72677

This commit is part of our efforts to support container annotations with
(almost) every allocator. Annotating `std::basic_string` with default
allocator is implemented in
#72677.

Additionally it removes `__begin != nullptr` because `data()` should
never return a nullptr.

Support in ASan API exists since
1c5ad6d.
This patch removes the check in std::basic_string annotation member
function (__annotate_contiguous_container) to support different
allocators.

You can turn off annotations for a specific allocator based on changes
from
2fa1bec.

The motivation for a research and those changes was a bug, found by
Trail of Bits, in a real code where an out-of-bounds read could happen
as two strings were compared via a call to `std::equal` that took
`iter1_begin`, `iter1_end`, `iter2_begin` iterators (with a custom
comparison function). When object `iter1` was longer than `iter2`, read
out-of-bounds on `iter2` could happen. Container sanitization would
detect it.

If you have any questions, please email:
- advenam.tacet@trailofbits.com
- disconnect3d@trailofbits.com

libcxx/include/string

@@ -1891,8 +1891,7 @@ private:
 #if !defined(_LIBCPP_HAS_NO_ASAN) && defined(_LIBCPP_INSTRUMENTED_WITH_ASAN)
     const void* __begin = data();
     const void* __end   = data() + capacity() + 1;
-    if (!__libcpp_is_constant_evaluated() && __begin != nullptr &&
-        is_same<allocator_type, __default_allocator_type>::value)
+    if (__asan_annotate_container_with_allocator<allocator_type>::value && !__libcpp_is_constant_evaluated())
       __sanitizer_annotate_contiguous_container(__begin, __end, __old_mid, __new_mid);
 #endif
   }

libcxx/test/libcxx/containers/strings/basic.string/asan.pass.cpp

@@ -0,0 +1,56 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// Basic test if ASan annotations work for basic_string.
+
+#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', 'e', 'f', 'g', 'h', 'i', 'j', 'a', 'b', 'c', 'd', 'e',
+                'f', 'g', 'h', 'i', 'j', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'};
+
+    v.insert(v.begin(), MyInputIter(i), MyInputIter(i + 29));
+    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', 'h', 'i', 'j', 'a', 'b', 'c', 'd', 'e',
+                   'f', 'g', 'h', 'i', 'j', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'};
+    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);
+    T foo = c[c.size() + 1]; // should trigger ASAN and call do_exit().
+    assert(false);           // if we got here, ASAN didn't trigger
+    ((void)foo);
+
+    return 0;
+  }
+}

libcxx/test/libcxx/containers/strings/basic.string/asan_turning_off.pass.cpp

@@ -0,0 +1,102 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// Test based on: https://bugs.chromium.org/p/chromium/issues/detail?id=1419798#c5
+// Some allocators during deallocation may not call destructors and just reuse memory.
+// In those situations, one may want to deactivate annotations for a specific allocator.
+// It's possible with __asan_annotate_container_with_allocator template class.
+// This test confirms that those allocators work after turning off annotations.
+//
+// A context to this test is a situations when memory is repurposed and destructors are not called.
+//   Related issue: https://github.com/llvm/llvm-project/issues/60384
+//
+// That issue appeared in the past and was addressed here: https://reviews.llvm.org/D145628
+//
+// There was also a discussion, if it's UB.
+//   Related discussion: https://reviews.llvm.org/D136765#4155262
+//   Related notes: https://eel.is/c++draft/basic.life#6
+// Probably it's no longer UB due a change in CWG2523.
+//   https://cplusplus.github.io/CWG/issues/2523.html
+//
+// Therefore we make sure that it works that way, also because people rely on this behavior.
+// Annotations are turned off only, if a user explicitly turns off annotations for a specific allocator.
+
+#include <assert.h>
+#include <stdlib.h>
+#include <string>
+#include <new>
+
+// Allocator with pre-allocated (with malloc in constructor) buffers.
+// Memory may be freed without calling destructors.
+struct reuse_allocator {
+  static size_t const N = 100;
+  reuse_allocator() {
+    for (size_t i = 0; i < N; ++i)
+      __buffers[i] = malloc(8 * 1024);
+  }
+  ~reuse_allocator() {
+    for (size_t i = 0; i < N; ++i)
+      free(__buffers[i]);
+  }
+  void* alloc() {
+    assert(__next_id < N);
+    return __buffers[__next_id++];
+  }
+  void reset() { __next_id = 0; }
+  void* __buffers[N];
+  size_t __next_id = 0;
+} reuse_buffers;
+
+template <typename T>
+struct user_allocator {
+  using value_type = T;
+  user_allocator() = default;
+  template <class U>
+  user_allocator(user_allocator<U>) {}
+  friend bool operator==(user_allocator, user_allocator) { return true; }
+  friend bool operator!=(user_allocator x, user_allocator y) { return !(x == y); }
+
+  T* allocate(size_t n) {
+    if (n * sizeof(T) > 8 * 1024)
+      throw std::bad_array_new_length();
+    return (T*)reuse_buffers.alloc();
+  }
+  void deallocate(T*, size_t) noexcept {}
+};
+
+// Turn off annotations for user_allocator:
+template <class T>
+struct std::__asan_annotate_container_with_allocator<user_allocator<T>> {
+  static bool const value = false;
+};
+
+int main(int, char**) {
+  using S = std::basic_string<char, std::char_traits<char>, user_allocator<char>>;
+
+  {
+    // Create a string with a buffer from reuse allocator object:
+    S* s = new (reuse_buffers.alloc()) S();
+    // Use string, so it's poisoned, if container annotations for that allocator are not turned off:
+    for (int i = 0; i < 40; i++)
+      s->push_back('a');
+  }
+  // Reset the state of the allocator, don't call destructors, allow memory to be reused:
+  reuse_buffers.reset();
+  {
+    // Create a next string with the same allocator, so the same buffer due to the reset:
+    S s;
+    // Use memory inside the string again, if it's poisoned, an error will be raised:
+    for (int i = 0; i < 60; i++)
+      s.push_back('a');
+  }
+
+  return 0;
+}

libcxx/test/support/asan_testing.h

@@ -75,7 +75,7 @@ TEST_CONSTEXPR bool is_string_asan_correct(const std::basic_string<ChrT, TraitsT
     return true;
 
   if (!is_string_short(c) || _LIBCPP_SHORT_STRING_ANNOTATIONS_ALLOWED) {
-    if (std::is_same<Alloc, std::allocator<ChrT>>::value)
+    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