[libcxx-commits] [libcxx] [libc++] Honor __SANITIZER_DISABLE_CONTAINER_OVERFLOW__ in libc++ (PR #168955)

[Louis Dionne via libcxx-commits]

================
@@ -0,0 +1,75 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+// REQUIRES: asan
+
+// Check that libc++ honors when __SANITIZER_DISABLE_CONTAINER_OVERFLOW__ is set
+// and disables the container overflow checks.
+
+// ADDITIONAL_COMPILE_FLAGS: -D__SANITIZER_DISABLE_CONTAINER_OVERFLOW__
+
+#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
----------------
ldionne wrote:

@DanBlackwell @padriff 

I realized something rather important while re-writing this test. Basically, `__SANITIZER_DISABLE_CONTAINER_OVERFLOW__` doesn't really work for any container that is (partly or fully) compiled in a separate library. That's kind of obvious when you think about it, but it means that e.g. `std::string` won't be able to honour this setting because the `char` specialization with default `char_traits` is partly instantiated in the dylib. That's really unfortunate.

So, basically, if `libc++.dylib` has been built with ASAN enabled, you'll be unable to turn off the sanitizer checks by defining `__SANITIZER_DISABLE_CONTAINER_OVERFLOW__` in your own translation unit. The reverse is not a problem per se: if `libc++.dylib` has been built with ASAN *disabled*, `std::string` will never enable container overflow checks, because it knows it can't actually implement it. Hence, it doesn't matter whether you define `__SANITIZER_DISABLE_CONTAINER_OVERFLOW__` or not, you're never getting any checks.

IMO this goes back to my initial comments about ASAN being a fundamentally ABI-affecting property that really needs "a different slice". Mixing TUs built with ASAN and TUs built without ASAN is always going to be fragile.

https://github.com/llvm/llvm-project/pull/168955