[libc++] Honor __SANITIZER_DISABLE_CONTAINER_OVERFLOW__ in libc++ (#168955)

Address sanitizer recently got a new macro __SANITIZER_DISABLE_CONTAINER_OVERFLOW__
which is intended to disable container overflow checks in libraries (either the
standard library or user libraries that might provide such checks). This patch makes
libc++ honor that macro and, in addition, cleans up how these checks are enabled for
string (which is special) by introducing a macro just for string.

rdar://166234942

.github/workflows/libcxx-build-and-test.yaml

@@ -154,6 +154,8 @@ jobs:
           machine: llvm-premerge-libcxx-runners
         - config: 'generic-asan'
           machine: llvm-premerge-libcxx-runners
+        - config: 'generic-asan-in-tests-only'
+          machine: llvm-premerge-libcxx-runners
         - config: 'generic-tsan'
           machine: llvm-premerge-libcxx-runners
         - config: 'generic-ubsan'

libcxx/cmake/caches/Generic-asan-in-tests-only.cmake

@@ -0,0 +1,3 @@
+# Don't build the library itself with ASAN support, but enable ASAN in the test suite.
+set(LIBCXX_TEST_PARAMS "use_sanitizer=Address" CACHE STRING "")
+set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")

libcxx/include/__debug_utils/sanitizers.h

@@ -17,7 +17,32 @@
 #  pragma GCC system_header
 #endif
 
-#if __has_feature(address_sanitizer)
+// Within libc++, _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS determines whether the containers should
+// provide ASAN container overflow checks. That setting attempts to honour ASAN's documented option
+// __SANITIZER_DISABLE_CONTAINER_OVERFLOW__ which can be defined by users to disable container overflow
+// checks.
+//
+// However, since parts of some containers (e.g. std::string) are compiled separately into the built
+// library, there are caveats:
+// - __SANITIZER_DISABLE_CONTAINER_OVERFLOW__ can't always be honoured, i.e. if the built library
+//   was compiled with ASAN container checks, it's impossible to turn them off afterwards. We diagnose
+//   this with an error to avoid the proliferation of invalid configurations that appear to work.
+//
+// - The container overflow checks themselves are not always available even when the user is compiling
+//   with -fsanitize=address. If a container is compiled separately like std::string, it can't provide
+//   container checks unless the separately compiled code was built with container checks enabled. These
+//   containers need to also conditionalize whether they provide overflow checks on `_LIBCPP_INSTRUMENTED_WITH_ASAN`.
+#if __has_feature(address_sanitizer) && !defined(__SANITIZER_DISABLE_CONTAINER_OVERFLOW__)
+#  define _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS 1
+#else
+#  define _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS 0
+#endif
+
+#if _LIBCPP_INSTRUMENTED_WITH_ASAN && !_LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
+#  error "We can't disable ASAN container checks when libc++ has been built with ASAN container checks enabled"
+#endif
+
+#if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
 
 extern "C" {
 _LIBCPP_EXPORTED_FROM_ABI void
@@ -28,12 +53,12 @@ _LIBCPP_EXPORTED_FROM_ABI int
 __sanitizer_verify_double_ended_contiguous_container(const void*, const void*, const void*, const void*);
 }
 
-#endif // __has_feature(address_sanitizer)
+#endif // _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 // ASan choices
-#if __has_feature(address_sanitizer)
+#if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
 #  define _LIBCPP_HAS_ASAN_CONTAINER_ANNOTATIONS_FOR_ALL_ALLOCATORS 1
 #endif
 
@@ -57,7 +82,7 @@ _LIBCPP_HIDE_FROM_ABI void __annotate_double_ended_contiguous_container(
     const void* __last_old_contained,
     const void* __first_new_contained,
     const void* __last_new_contained) {
-#if !__has_feature(address_sanitizer)
+#if !_LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
   (void)__first_storage;
   (void)__last_storage;
   (void)__first_old_contained;
@@ -86,7 +111,7 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void __annotate_contiguous_c
     const void* __last_storage,
     const void* __old_last_contained,
     const void* __new_last_contained) {
-#if !__has_feature(address_sanitizer)
+#if !_LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
   (void)__first_storage;
   (void)__last_storage;
   (void)__old_last_contained;

libcxx/include/deque

@@ -953,7 +953,7 @@ private:
     (void)__end;
     (void)__annotation_type;
     (void)__place;
-#  if __has_feature(address_sanitizer)
+#  if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
     // __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
@@ -1046,23 +1046,23 @@ private:
       std::__annotate_double_ended_contiguous_container<_Allocator>(
           __mem_beg, __mem_end, __old_beg, __old_end, __new_beg, __new_end);
     }
-#  endif // __has_feature(address_sanitizer)
+#  endif // _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
   }
 
   _LIBCPP_HIDE_FROM_ABI void __annotate_new(size_type __current_size) const _NOEXCEPT {
     (void)__current_size;
-#  if __has_feature(address_sanitizer)
+#  if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
     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 // __has_feature(address_sanitizer)
+#  endif // _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
   }
 
   _LIBCPP_HIDE_FROM_ABI void __annotate_delete() const _NOEXCEPT {
-#  if __has_feature(address_sanitizer)
+#  if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
     if (empty()) {
       for (size_t __i = 0; __i < __map_.size(); ++__i) {
         __annotate_whole_block(__i, __asan_unposion);
@@ -1071,19 +1071,19 @@ 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 // __has_feature(address_sanitizer)
+#  endif // _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
   }
 
   _LIBCPP_HIDE_FROM_ABI void __annotate_increase_front(size_type __n) const _NOEXCEPT {
     (void)__n;
-#  if __has_feature(address_sanitizer)
+#  if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
     __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;
-#  if __has_feature(address_sanitizer)
+#  if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
     __annotate_from_to(__start_ + size(), __start_ + size() + __n, __asan_unposion, __asan_back_moved);
 #  endif
   }
@@ -1091,7 +1091,7 @@ private:
   _LIBCPP_HIDE_FROM_ABI void __annotate_shrink_front(size_type __old_size, size_type __old_start) const _NOEXCEPT {
     (void)__old_size;
     (void)__old_start;
-#  if __has_feature(address_sanitizer)
+#  if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
     __annotate_from_to(__old_start, __old_start + (__old_size - size()), __asan_poison, __asan_front_moved);
 #  endif
   }
@@ -1099,7 +1099,7 @@ private:
   _LIBCPP_HIDE_FROM_ABI void __annotate_shrink_back(size_type __old_size, size_type __old_start) const _NOEXCEPT {
     (void)__old_size;
     (void)__old_start;
-#  if __has_feature(address_sanitizer)
+#  if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
     __annotate_from_to(__old_start + size(), __old_start + __old_size, __asan_poison, __asan_back_moved);
 #  endif
   }
@@ -1112,7 +1112,7 @@ private:
   __annotate_whole_block(size_t __block_index, __asan_annotation_type __annotation_type) const _NOEXCEPT {
     (void)__block_index;
     (void)__annotation_type;
-#  if __has_feature(address_sanitizer)
+#  if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
     __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);
@@ -1125,7 +1125,7 @@ private:
     }
 #  endif
   }
-#  if __has_feature(address_sanitizer)
+#  if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
 
 public:
   _LIBCPP_HIDE_FROM_ABI bool __verify_asan_annotations() const _NOEXCEPT {
@@ -1187,7 +1187,7 @@ public:
   }
 
 private:
-#  endif // __has_feature(address_sanitizer)
+#  endif // _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
   _LIBCPP_HIDE_FROM_ABI bool __maybe_remove_front_spare(bool __keep_one = true) {
     if (__front_spare_blocks() >= 2 || (!__keep_one && __front_spare_blocks())) {
       __annotate_whole_block(0, __asan_unposion);

libcxx/include/string

@@ -678,14 +678,20 @@ basic_string<char32_t> operator""s( const char32_t *str, size_t len );
 _LIBCPP_PUSH_MACROS
 #  include <__undef_macros>
 
-#  if __has_feature(address_sanitizer) && _LIBCPP_INSTRUMENTED_WITH_ASAN
+// Since std::string is partially instantiated in the built library, we require that the library
+// has been built with ASAN support in order to enable the container checks in std::string.
+// Within the <string> implementation, use `_LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS_FOR_STRING`
+// to determine whether ASAN container checks should be provided.
+//
+// The _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS macro disables ASAN instrumentation for a specific
+// function, allowing memory accesses that would normally trigger ASan errors to proceed without
+// crashing. This is useful for accessing parts of objects memory, which should not be accessed,
+// such as unused bytes in short strings, that should never be accessed by other parts of the program.
+#  if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS && _LIBCPP_INSTRUMENTED_WITH_ASAN
+#    define _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS_FOR_STRING 1
 #    define _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS __attribute__((__no_sanitize__("address")))
-// This macro disables AddressSanitizer (ASan) instrumentation for a specific function,
-// allowing memory accesses that would normally trigger ASan errors to proceed without crashing.
-// This is useful for accessing parts of objects memory, which should not be accessed,
-// such as unused bytes in short strings, that should never be accessed
-// by other parts of the program.
 #  else
+#    define _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS_FOR_STRING 0
 #    define _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS
 #  endif
 
@@ -749,7 +755,7 @@ public:
   //
   // This string implementation doesn't contain any references into itself. It only contains a bit that says whether
   // it is in small or large string mode, so the entire structure is trivially relocatable if its members are.
-#  if __has_feature(address_sanitizer) && _LIBCPP_INSTRUMENTED_WITH_ASAN
+#  if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS_FOR_STRING
   // When compiling with AddressSanitizer (ASan), basic_string cannot be trivially
   // relocatable. Because the object's memory might be poisoned when its content
   // is kept inside objects memory (short string optimization), instead of in allocated
@@ -764,7 +770,7 @@ public:
       void>;
 #  endif
 
-#  if __has_feature(address_sanitizer) && _LIBCPP_INSTRUMENTED_WITH_ASAN
+#  if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS_FOR_STRING
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pointer __asan_volatile_wrapper(pointer const& __ptr) const {
     if (__libcpp_is_constant_evaluated())
       return __ptr;
@@ -2313,7 +2319,7 @@ private:
   __annotate_contiguous_container(const void* __old_mid, const void* __new_mid) const {
     (void)__old_mid;
     (void)__new_mid;
-#  if _LIBCPP_INSTRUMENTED_WITH_ASAN
+#  if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS_FOR_STRING
 #    if defined(__APPLE__)
     // TODO: remove after addressing issue #96099 (https://llvm.org/PR96099)
     if (!__is_long())

libcxx/test/extensions/libcxx/asan/diagnose_container_overflow_checks_disablement.verify.cpp

@@ -0,0 +1,22 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// XFAIL: FROZEN-CXX03-HEADERS-FIXME
+
+// Check that we diagnose when libc++ has been built with ASAN instrumentation
+// and the user requests turning off the ASAN container checks. Since that is
+// impossible to implement, we diagnose this with an error instead.
+//
+// REQUIRES: libcpp-instrumented-with-asan
+// ADDITIONAL_COMPILE_FLAGS: -D__SANITIZER_DISABLE_CONTAINER_OVERFLOW__
+
+#include <deque>
+#include <string>
+#include <vector>
+
+// expected-error@*:* {{We can't disable ASAN container checks when libc++ has been built with ASAN container checks enabled}}

libcxx/test/extensions/libcxx/asan/disable_container_overflow_checks.pass.cpp

@@ -0,0 +1,83 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// Check that libc++ honors when __SANITIZER_DISABLE_CONTAINER_OVERFLOW__ is set
+// and disables the container overflow checks.
+//
+// REQUIRES: asan
+// ADDITIONAL_COMPILE_FLAGS: -D__SANITIZER_DISABLE_CONTAINER_OVERFLOW__
+
+// When libc++ is built with ASAN instrumentation, we can't turn off the ASAN checks,
+// and that is diagnosed as an error.
+// XFAIL: libcpp-instrumented-with-asan
+
+// std::basic_string::data is const util C++17
+// UNSUPPORTED: c++03, c++11, c++14
+
+// The protocol checked by this test is specific to Clang and compiler-rt
+// UNSUPPORTED: gcc
+
+#include <deque>
+#include <string>
+#include <vector>
+
+// This check is somewhat weak because it would pass if we renamed the libc++-internal
+// macro and forgot to update this test. But it doesn't hurt to check it in addition to
+// the tests below.
+#if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS
+#  error "Container overflow checks should be disabled in libc++"
+#endif
+
+void vector() {
+  std::vector<int> v;
+  v.reserve(100);
+  int* data = v.data();
+
+  // This is illegal with respect to std::vector, but legal from the core language perspective since
+  // we do own that allocated memory and `int` is an implicit lifetime type. If container overflow
+  // checks are enabled, this would fail.
+  data[4] = 42;
+}
+
+// For std::string, we must use a custom char_traits class to reliably test this behavior. Since
+// std::string is externally instantiated in the built library, __SANITIZER_DISABLE_CONTAINER_OVERFLOW__
+// will not be honored for any function that happens to be in the built library. Using a custom
+// char_traits class ensures that this doesn't get in the way.
+struct my_char_traits : std::char_traits<char> {};
+
+void string() {
+  std::basic_string<char, my_char_traits> s;
+  s.reserve(100);
+  char* data = s.data();
+  data[4]    = 'x';
+}
+
+void deque() {
+  std::deque<int> d;
+  d.push_back(1);
+  d.push_back(2);
+  d.push_back(3);
+  int* last_element = &d[2];
+  d.pop_back();
+
+  // This reference is technically invalidated according to the library. However since
+  // we know std::deque is implemented using segments of a fairly large size and we know
+  // the non-erased elements are not invalidated by pop_front() (per the Standard), we can
+  // rely on the fact that the last element still exists in memory that is owned by the
+  // std::deque.
+  //
+  // If container overflow checks were enabled, this would obviously fail.
+  *last_element = 42;
+}
+
+int main(int, char**) {
+  vector();
+  string();
+  deque();
+  return 0;
+}

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

@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// REQUIRES: asan
+// REQUIRES: libcpp-instrumented-with-asan
 // UNSUPPORTED: c++03
 
 // Basic test if ASan annotations work for basic_string.

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

@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// REQUIRES: asan
+// REQUIRES: libcpp-instrumented-with-asan
 // UNSUPPORTED: c++03
 
 // <string>

libcxx/test/libcxx/type_traits/is_trivially_relocatable.compile.pass.cpp

@@ -103,7 +103,7 @@ static_assert(!std::__libcpp_is_trivially_relocatable<std::array<NotTriviallyCop
 static_assert(std::__libcpp_is_trivially_relocatable<std::array<std::unique_ptr<int>, 1> >::value, "");
 
 // basic_string
-#if !__has_feature(address_sanitizer) || !_LIBCPP_INSTRUMENTED_WITH_ASAN
+#if !_LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS_FOR_STRING
 struct MyChar {
   char c;
 };

libcxx/test/support/asan_testing.h

@@ -55,7 +55,7 @@ TEST_CONSTEXPR bool is_double_ended_contiguous_container_asan_correct(const std:
 }
 #endif
 
-#if TEST_HAS_FEATURE(address_sanitizer)
+#if TEST_HAS_FEATURE(address_sanitizer) && _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS_FOR_STRING
 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)

libcxx/utils/ci/run-buildbot

@@ -343,6 +343,13 @@ generic-asan)
     generate-cmake -C "${MONOREPO_ROOT}/libcxx/cmake/caches/Generic-asan.cmake"
     check-runtimes
 ;;
+generic-asan-in-tests-only)
+    # This builds the library without ASAN support, but then turns on ASAN support
+    # within the test suite.
+    clean
+    generate-cmake -C "${MONOREPO_ROOT}/libcxx/cmake/caches/Generic-asan-in-tests-only.cmake"
+    check-runtimes
+;;
 generic-msan)
     clean
     generate-cmake -C "${MONOREPO_ROOT}/libcxx/cmake/caches/Generic-msan.cmake"

libcxx/utils/libcxx/test/features/libcxx_macros.py

@@ -45,6 +45,7 @@ for macro, feature in macros.items():
 true_false_macros = {
     "_LIBCPP_HAS_THREAD_API_EXTERNAL": "libcpp-has-thread-api-external",
     "_LIBCPP_HAS_THREAD_API_PTHREAD": "libcpp-has-thread-api-pthread",
+    "_LIBCPP_INSTRUMENTED_WITH_ASAN": "libcpp-instrumented-with-asan",
 }
 for macro, feature in true_false_macros.items():
     features.append(