[libcxx-commits] [libcxx] 7bf5f62 - Revert "[ASan][libcxx] Annotating std::vector with all allocators"
Hans Wennborg via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Jan 30 04:25:03 PST 2023
Author: Hans Wennborg
Date: 2023-01-30T13:24:44+01:00
New Revision: 7bf5f62574e23ba56447aca2530197d8577b59fd
URL: https://github.com/llvm/llvm-project/commit/7bf5f62574e23ba56447aca2530197d8577b59fd
DIFF: https://github.com/llvm/llvm-project/commit/7bf5f62574e23ba56447aca2530197d8577b59fd.diff
LOG: 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 at trailofbits.com
> - disconnect3d at 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.
Added:
Modified:
libcxx/include/vector
libcxx/test/libcxx/containers/sequences/vector/asan.pass.cpp
libcxx/test/support/min_allocator.h
Removed:
################################################################################
diff --git a/libcxx/include/vector b/libcxx/include/vector
index e9019a8ae303..4b7ae130a7bc 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
diff erent 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<allocator_type, __default_allocator_type>::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 01da9d52e8f0..40275f58cb32 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<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
+ {
+ 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);
+ }
#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<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/support/min_allocator.h b/libcxx/test/support/min_allocator.h
index 7d517518f052..f27ac80c0464 100644
--- a/libcxx/test/support/min_allocator.h
+++ b/libcxx/test/support/min_allocator.h
@@ -432,23 +432,4 @@ class explicit_allocator
TEST_CONSTEXPR_CXX20 friend bool operator!=(explicit_allocator x, explicit_allocator y) {return !(x == y);}
};
-template <class T>
-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 <class U>
- TEST_CONSTEXPR_CXX20 explicit unaligned_allocator(unaligned_allocator<U>) TEST_NOEXCEPT {}
-
- TEST_CONSTEXPR_CXX20 T* allocate(std::size_t n) { return static_cast<T*>(std::allocator<T>().allocate(n + 1)) + 1; }
-
- TEST_CONSTEXPR_CXX20 void deallocate(T* p, std::size_t n) { std::allocator<T>().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
More information about the libcxx-commits
mailing list