[libcxx-commits] [libcxx] c08d4ad - [ASan][libcxx] Annotating std::vector with all allocators

Nikolas Klauser via libcxx-commits libcxx-commits at lists.llvm.org
Thu May 4 17:44:55 PDT 2023


Author: Advenam Tacet
Date: 2023-05-04T17:44:06-07:00
New Revision: c08d4ad25cf3f335e9b2e7b1b149eb1b486868f1

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

LOG: [ASan][libcxx] Annotating std::vector with all allocators

This revision is a part of a series of patches extending
AddressSanitizer C++ container overflow detection
capabilities by adding annotations, similar to those existing
in std::vector, to std::string and std::deque collections.
These changes allow ASan to detect cases when the instrumented
program accesses memory which is internally allocated by
the collection but is still not in-use (accesses before or
after the stored elements for std::deque, or between the size and
capacity bounds for std::string).

The motivation for the 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 std::equals function
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.

In revision D132522, support for non-aligned memory buffers (sharing
first/last granule with other objects) was added, therefore the
check for standard allocator is not necessary anymore.
This patch removes the check in std::vector annotation member
function (__annotate_contiguous_container) to support
different allocators.

Additionally, this revision fixes unpoisoning in std::vector.
It guarantees that __alloc_traits::deallocate may access returned memory.
Originally suggested in D144155 revision.

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

Reviewed By: #libc, #sanitizers, philnik, vitalybuka, ldionne

Spies: mikhail.ramalho, manojgupta, ldionne, AntonBikineev, ayzhao, hans, EricWF, philnik, #sanitizers, libcxx-commits

Differential Revision: https://reviews.llvm.org/D136765

Added: 
    libcxx/test/libcxx/containers/sequences/vector/asan_turning_off.pass.cpp

Modified: 
    libcxx/include/vector
    libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/vector b/libcxx/include/vector
index 40538f211d48f..d90eb1666c360 100644
--- a/libcxx/include/vector
+++ b/libcxx/include/vector
@@ -301,6 +301,7 @@ template<class T, class charT> requires is-vector-bool-reference<T> // Since C++
 #include <__iterator/wrap_iter.h>
 #include <__memory/addressof.h>
 #include <__memory/allocate_at_least.h>
+#include <__memory/allocator_traits.h>
 #include <__memory/pointer_traits.h>
 #include <__memory/swap_allocator.h>
 #include <__memory/temp_value.h>
@@ -441,11 +442,11 @@ private:
       _LIBCPP_CONSTEXPR __destroy_vector(vector& __vec) : __vec_(__vec) {}
 
       _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void operator()() {
-          __vec_.__annotate_delete();
           std::__debug_db_erase_c(std::addressof(__vec_));
 
           if (__vec_.__begin_ != nullptr) {
             __vec_.__clear();
+            __vec_.__annotate_delete();
             __alloc_traits::deallocate(__vec_.__alloc(), __vec_.__begin_, __vec_.capacity());
           }
       }
@@ -745,8 +746,7 @@ private:
                                          const void *__old_mid,
                                          const void *__new_mid) const
     {
-
-      if (!__libcpp_is_constant_evaluated() && __beg && is_same<allocator_type, __default_allocator_type>::value)
+      if (!__libcpp_is_constant_evaluated() && __beg != nullptr && __asan_annotate_container_with_allocator<_Allocator>::value)
         __sanitizer_annotate_contiguous_container(__beg, __end, __old_mid, __new_mid);
     }
 #else
@@ -868,6 +868,7 @@ private:
     if (__alloc() != __c.__alloc())
     {
       __clear();
+      __annotate_delete();
       __alloc_traits::deallocate(__alloc(), this->__begin_, capacity());
       this->__begin_ = this->__end_ = __end_cap() = nullptr;
     }
@@ -956,6 +957,7 @@ vector<_Tp, _Allocator>::__vdeallocate() _NOEXCEPT
     if (this->__begin_ != nullptr)
     {
         clear();
+        __annotate_delete();
         __alloc_traits::deallocate(this->__alloc(), this->__begin_, capacity());
         this->__begin_ = this->__end_ = this->__end_cap() = nullptr;
     }

diff  --git a/libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp b/libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp
index 40275f58cb32c..5b58de0c86ac0 100644
--- a/libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp
+++ b/libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp
@@ -29,40 +29,52 @@ void do_exit() {
 
 int main(int, char**)
 {
-#if TEST_STD_VER >= 11
-    {
-        typedef int T;
-        typedef std::vector<T, min_allocator<T>> C;
-        const T t[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
-        C c(std::begin(t), std::end(t));
-        c.reserve(2*c.size());
-        volatile T foo = c[c.size()];    // bad, but not caught by ASAN
-        ((void)foo);
-    }
+#if TEST_STD_VER >= 11 && TEST_CLANG_VER >= 1600
+  // TODO LLVM18: Remove the special-casing
+  {
+    typedef int T;
+    typedef cpp17_input_iterator<T*> MyInputIter;
+    std::vector<T, min_allocator<T>> v;
+    v.reserve(1);
+    int i[] = {42};
+    v.insert(v.begin(), MyInputIter(i), MyInputIter(i + 1));
+    assert(v[0] == 42);
+    assert(is_contiguous_container_asan_correct(v));
+  }
+  {
+    typedef char T;
+    typedef cpp17_input_iterator<T*> MyInputIter;
+    std::vector<T, unaligned_allocator<T>> v;
+    v.reserve(1);
+    char i[] = {'a', 'b'};
+    v.insert(v.begin(), MyInputIter(i), MyInputIter(i + 2));
+    assert(v[0] == 'a');
+    assert(v[1] == 'b');
+    assert(is_contiguous_container_asan_correct(v));
+  }
 #endif
+  {
+    typedef cpp17_input_iterator<int*> MyInputIter;
+    // Sould not trigger ASan.
+    std::vector<int> v;
+    v.reserve(1);
+    int i[] = {42};
+    v.insert(v.begin(), MyInputIter(i), MyInputIter(i + 1));
+    assert(v[0] == 42);
+    assert(is_contiguous_container_asan_correct(v));
+  }
 
-    {
-        typedef cpp17_input_iterator<int*> MyInputIter;
-        // Sould not trigger ASan.
-        std::vector<int> v;
-        v.reserve(1);
-        int i[] = {42};
-        v.insert(v.begin(), MyInputIter(i), MyInputIter(i + 1));
-        assert(v[0] == 42);
-        assert(is_contiguous_container_asan_correct(v));
-    }
-
-    __sanitizer_set_death_callback(do_exit);
-    {
-        typedef int T;
-        typedef std::vector<T> C;
-        const T t[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
-        C c(std::begin(t), std::end(t));
-        c.reserve(2*c.size());
-        assert(is_contiguous_container_asan_correct(c));
-        assert(!__sanitizer_verify_contiguous_container( c.data(), c.data() + 1, c.data() + c.capacity()));
-        volatile T foo = c[c.size()]; // should trigger ASAN. Use volatile to prevent being optimized away.
-        assert(false);          // if we got here, ASAN didn't trigger
-        ((void)foo);
-    }
+  __sanitizer_set_death_callback(do_exit);
+  {
+    typedef int T;
+    typedef std::vector<T> C;
+    const T t[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
+    C c(std::begin(t), std::end(t));
+    c.reserve(2 * c.size());
+    assert(is_contiguous_container_asan_correct(c));
+    assert(!__sanitizer_verify_contiguous_container(c.data(), c.data() + 1, c.data() + c.capacity()));
+    volatile T foo = c[c.size()]; // should trigger ASAN. Use volatile to prevent being optimized away.
+    assert(false);                // if we got here, ASAN didn't trigger
+    ((void)foo);
+  }
 }

diff  --git a/libcxx/test/libcxx/containers/sequences/vector/asan_turning_off.pass.cpp b/libcxx/test/libcxx/containers/sequences/vector/asan_turning_off.pass.cpp
new file mode 100644
index 0000000000000..e2760271d60a5
--- /dev/null
+++ b/libcxx/test/libcxx/containers/sequences/vector/asan_turning_off.pass.cpp
@@ -0,0 +1,77 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+
+// <vector>
+
+// 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.
+
+#include <assert.h>
+#include <stdlib.h>
+#include <vector>
+#include <new>
+
+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) { return (T*)reuse_buffers.alloc(); }
+  void deallocate(T*, size_t) noexcept {}
+};
+
+#ifdef _LIBCPP_HAS_ASAN_CONTAINER_ANNOTATIONS_FOR_ALL_ALLOCATORS
+template <class T>
+struct std::__asan_annotate_container_with_allocator<user_allocator<T>> : false_type {};
+#endif
+
+int main(int, char**) {
+  using V = std::vector<int, user_allocator<int>>;
+
+  {
+    V* v = new (reuse_buffers.alloc()) V();
+    for (int i = 0; i < 100; i++)
+      v->push_back(i);
+  }
+  reuse_buffers.reset();
+  {
+    V v;
+    for (int i = 0; i < 1000; i++)
+      v.push_back(i);
+  }
+
+  return 0;
+}


        


More information about the libcxx-commits mailing list