From 7bf5f62574e23ba56447aca2530197d8577b59fd Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Mon, 30 Jan 2023 13:15:31 +0100 Subject: [PATCH] Revert "[ASan][libcxx] Annotating std::vector with all allocators" This caused false container-overflow errors when using a custom allocator that touches the memory on deallocation: GitHub Issue #60384 > 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. > > If you have any questions, please email: > - advenam.tacet@trailofbits.com > - disconnect3d@trailofbits.com > > Reviewed By: #libc, #sanitizers, philnik, vitalybuka > > Spies: EricWF, philnik, #sanitizers, libcxx-commits > > Differential Revision: https://reviews.llvm.org/D136765 This reverts commit 490555026821db47d1cf4bf08c219b3e56ec6b45. --- libcxx/include/vector | 19 +---- .../containers/sequences/vector/asan.pass.cpp | 91 ++++++++-------------- libcxx/test/support/min_allocator.h | 19 ----- 3 files changed, 34 insertions(+), 95 deletions(-) diff --git a/libcxx/include/vector b/libcxx/include/vector index e9019a8..4b7ae13 100644 --- a/libcxx/include/vector +++ b/libcxx/include/vector @@ -743,25 +743,8 @@ private: const void *__old_mid, const void *__new_mid) const { -# if _LIBCPP_CLANG_VER >= 1600 - if (!__libcpp_is_constant_evaluated() && __beg) - // Implementation of __sanitizer_annotate_contiguous_container function changed with commit - // rGdd1b7b797a116eed588fd752fbe61d34deeb24e4 and supports: - // - unaligned beginnings of buffers, - // - shared first/last granule (if memory granule is shared with a different object - // just after the end of unaligned end/before the unaligned beginning, memory of that object won't be poisoned). - // - // Therefore, check for standard allocator is not necessary. -# else - // TODO LLVM18: Remove the special-casing - // - // Vectors annotations rely on __sanitizer_annotate_contiguous_container function, - // its implementation from LLVM15 (and earlier) requires that - // beginning of a containers data buffer is aligned to shadow granularity and - // memory just after is not shared with another object. - // Default allocator satisfies that. + if (!__libcpp_is_constant_evaluated() && __beg && is_same::value) -# endif __sanitizer_annotate_contiguous_container(__beg, __end, __old_mid, __new_mid); } #else diff --git a/libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp b/libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp index 01da9d5..40275f5 100644 --- a/libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp +++ b/libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp @@ -29,65 +29,40 @@ void do_exit() { int main(int, char**) { -#if TEST_STD_VER >= 11 && TEST_CLANG_VER < 1600 - // TODO LLVM18: Remove the special-casing - { - typedef int T; - typedef std::vector> 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 + { + typedef int T; + typedef std::vector> 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); + } #endif -#if TEST_STD_VER >= 11 && TEST_CLANG_VER >= 1600 - // TODO LLVM18: Remove the TEST_CLANG_VER check - { - typedef int T; - typedef cpp17_input_iterator MyInputIter; - std::vector> 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 MyInputIter; - std::vector> 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 MyInputIter; - // Sould not trigger ASan. - std::vector 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 MyInputIter; + // Sould not trigger ASan. + std::vector 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 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 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/support/min_allocator.h b/libcxx/test/support/min_allocator.h index 7d51751..f27ac80 100644 --- a/libcxx/test/support/min_allocator.h +++ b/libcxx/test/support/min_allocator.h @@ -432,23 +432,4 @@ public: TEST_CONSTEXPR_CXX20 friend bool operator!=(explicit_allocator x, explicit_allocator y) {return !(x == y);} }; -template -class unaligned_allocator { -public: - static_assert(TEST_ALIGNOF(T) == 1, "Type T cannot be created on unaligned address (UB)"); - typedef T value_type; - - TEST_CONSTEXPR_CXX20 unaligned_allocator() TEST_NOEXCEPT {} - - template - TEST_CONSTEXPR_CXX20 explicit unaligned_allocator(unaligned_allocator) TEST_NOEXCEPT {} - - TEST_CONSTEXPR_CXX20 T* allocate(std::size_t n) { return static_cast(std::allocator().allocate(n + 1)) + 1; } - - TEST_CONSTEXPR_CXX20 void deallocate(T* p, std::size_t n) { std::allocator().deallocate(p - 1, n + 1); } - - TEST_CONSTEXPR_CXX20 friend bool operator==(unaligned_allocator, unaligned_allocator) { return true; } - TEST_CONSTEXPR_CXX20 friend bool operator!=(unaligned_allocator x, unaligned_allocator y) { return !(x == y); } -}; - #endif // MIN_ALLOCATOR_H -- 2.7.4