From e7e4d3c36439d219764d2b5f5f146720a6e10748 Mon Sep 17 00:00:00 2001 From: Advenam Tacet Date: Mon, 18 Dec 2023 23:30:29 +0100 Subject: [PATCH 1/3] [ASan][libc++] Turn on ASan annotations for short strings This commit turns on ASan annotations in `std::basic_string` for short stings (SSO case). Originally suggested here: https://reviews.llvm.org/D147680 String annotations added here: https://github.com/llvm/llvm-project/pull/72677 Annotating `std::basic_string` with default allocator is implemented in https://github.com/llvm/llvm-project/pull/72677 but annotations for short strings (SSO - Short String Optimization) are turned off there. This commit turns them on. This also removes `_LIBCPP_SHORT_STRING_ANNOTATIONS_ALLOWED`, because we do not plan to support turning on and off short string annotations. Support in ASan API exists since https://github.com/llvm/llvm-project/commit/dd1b7b797a116eed588fd752fbe61d34deeb24e4. You can turn off annotations for a specific allocator based on changes from https://github.com/llvm/llvm-project/commit/2fa1bec7a20bb23f2e6620085adb257dafaa3be0. This PR 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` and `std::deque` collections. These enhancements empower ASan to effectively detect instances where the instrumented program attempts to access memory within a collection's internal allocation that remains unused. This includes cases where access occurs before or after the stored elements in `std::deque`, or between the `std::basic_string`'s size (including the null terminator) and capacity bounds. The introduction of these annotations was spurred by a real-world software bug discovered by Trail of Bits, involving an out-of-bounds memory access during the comparison of two strings using the `std::equals` function. This function was taking iterators (`iter1_begin`, `iter1_end`, `iter2_begin`) to perform the comparison, using a custom comparison function. When the `iter1` object exceeded the length of `iter2`, an out-of-bounds read could occur on the `iter2` object. Container sanitization, upon enabling these annotations, would effectively identify and flag this potential vulnerability. If you have any questions, please email: advenam.tacet@trailofbits.com disconnect3d@trailofbits.com --- libcxx/include/string | 14 +- .../asan_deque_integration.pass.cpp | 183 ++++++++++++++++++ .../strings/basic.string/asan_short.pass.cpp | 56 ++++++ .../asan_vector_integration.pass.cpp | 183 ++++++++++++++++++ libcxx/test/support/asan_testing.h | 16 +- 5 files changed, 431 insertions(+), 21 deletions(-) create mode 100644 libcxx/test/libcxx/containers/strings/basic.string/asan_deque_integration.pass.cpp create mode 100644 libcxx/test/libcxx/containers/strings/basic.string/asan_short.pass.cpp create mode 100644 libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp diff --git a/libcxx/include/string b/libcxx/include/string index e97139206d4fa..4116f350a8047 100644 --- a/libcxx/include/string +++ b/libcxx/include/string @@ -659,7 +659,6 @@ _LIBCPP_PUSH_MACROS #else # define _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS #endif -#define _LIBCPP_SHORT_STRING_ANNOTATIONS_ALLOWED false _LIBCPP_BEGIN_NAMESPACE_STD @@ -1896,22 +1895,17 @@ private: #endif } - // ASan: short string is poisoned if and only if this function returns true. - _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 bool __asan_short_string_is_annotated() const _NOEXCEPT { - return _LIBCPP_SHORT_STRING_ANNOTATIONS_ALLOWED && !__libcpp_is_constant_evaluated(); - } - _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __annotate_new(size_type __current_size) const _NOEXCEPT { (void) __current_size; #if !defined(_LIBCPP_HAS_NO_ASAN) && defined(_LIBCPP_INSTRUMENTED_WITH_ASAN) - if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long())) + if (!__libcpp_is_constant_evaluated()) __annotate_contiguous_container(data() + capacity() + 1, data() + __current_size + 1); #endif } _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __annotate_delete() const _NOEXCEPT { #if !defined(_LIBCPP_HAS_NO_ASAN) && defined(_LIBCPP_INSTRUMENTED_WITH_ASAN) - if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long())) + if (!__libcpp_is_constant_evaluated()) __annotate_contiguous_container(data() + size() + 1, data() + capacity() + 1); #endif } @@ -1919,7 +1913,7 @@ private: _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __annotate_increase(size_type __n) const _NOEXCEPT { (void) __n; #if !defined(_LIBCPP_HAS_NO_ASAN) && defined(_LIBCPP_INSTRUMENTED_WITH_ASAN) - if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long())) + if (!__libcpp_is_constant_evaluated()) __annotate_contiguous_container(data() + size() + 1, data() + size() + 1 + __n); #endif } @@ -1927,7 +1921,7 @@ private: _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __annotate_shrink(size_type __old_size) const _NOEXCEPT { (void) __old_size; #if !defined(_LIBCPP_HAS_NO_ASAN) && defined(_LIBCPP_INSTRUMENTED_WITH_ASAN) - if (!__libcpp_is_constant_evaluated() && (__asan_short_string_is_annotated() || __is_long())) + if (!__libcpp_is_constant_evaluated()) __annotate_contiguous_container(data() + __old_size + 1, data() + size() + 1); #endif } diff --git a/libcxx/test/libcxx/containers/strings/basic.string/asan_deque_integration.pass.cpp b/libcxx/test/libcxx/containers/strings/basic.string/asan_deque_integration.pass.cpp new file mode 100644 index 0000000000000..e19410c5409b1 --- /dev/null +++ b/libcxx/test/libcxx/containers/strings/basic.string/asan_deque_integration.pass.cpp @@ -0,0 +1,183 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +// REQUIRES: asan +// UNSUPPORTED: c++03 + +#include +#include +#include "test_macros.h" +#include "asan_testing.h" // includes deque and string - don't do it before +#include "min_allocator.h" + +// This tests exists to check if strings work well with deque, as those +// may be partialy annotated, we cannot simply call +// is_double_ended_contiguous_container_asan_correct, as it assumes that +// object memory inside is not annotated, so we check everything in a more careful way. + +template +bool verify_inside(D const& d) { + for (size_t i = 0; i < d.size(); ++i) { + if (!is_string_asan_correct(d[i])) + return false; + } + + return true; +} + +template +S get_s(char c) { + S s; + for (size_t i = 0; i < N; ++i) + s.push_back(c); + + return s; +} + +template +void test_string() { + size_t const N = sizeof(S) < 256 ? (4096 / sizeof(S)) : 16; + + { + C d1a(1), d1b(N), d1c(N + 1), d1d(32 * N); + assert(verify_inside(d1a)); + assert(verify_inside(d1b)); + assert(verify_inside(d1c)); + assert(verify_inside(d1d)); + } + { + C d2; + for (size_t i = 0; i < 16 * N; ++i) { + d2.push_back(get_s(i % 10 + 'a')); + assert(verify_inside(d2)); + d2.push_back(get_s(i % 10 + 'b')); + assert(verify_inside(d2)); + + d2.pop_front(); + assert(verify_inside(d2)); + } + } + { + C d3; + for (size_t i = 0; i < 16 * N; ++i) { + d3.push_front(get_s(i % 10 + 'a')); + assert(verify_inside(d3)); + d3.push_front(get_s(i % 10 + 'b')); + assert(verify_inside(d3)); + + d3.pop_back(); + assert(verify_inside(d3)); + } + } + { + C d4; + for (size_t i = 0; i < 16 * N; ++i) { + // When there is no SSO, all elements inside should not be poisoned, + // so we can verify deque poisoning. + d4.push_front(get_s(i % 10 + 'a')); + assert(verify_inside(d4)); + assert(is_double_ended_contiguous_container_asan_correct(d4)); + d4.push_back(get_s(i % 10 + 'b')); + assert(verify_inside(d4)); + assert(is_double_ended_contiguous_container_asan_correct(d4)); + } + } + { + C d5; + for (size_t i = 0; i < 5 * N; ++i) { + // In d4 we never had poisoned memory inside deque. + // Here we start with SSO, so part of the inside of the container, + // will be poisoned. + d5.push_front(S()); + assert(verify_inside(d5)); + } + for (size_t i = 0; i < d5.size(); ++i) { + // We change the size to have long string. + // Memory owne by deque should not be poisoned by string. + d5[i].resize(1000); + assert(verify_inside(d5)); + } + + assert(is_double_ended_contiguous_container_asan_correct(d5)); + + d5.erase(d5.begin() + 2); + assert(verify_inside(d5)); + + d5.erase(d5.end() - 2); + assert(verify_inside(d5)); + + assert(is_double_ended_contiguous_container_asan_correct(d5)); + } + { + C d6a; + assert(is_double_ended_contiguous_container_asan_correct(d6a)); + + C d6b(N + 2, get_s('a')); + d6b.push_front(get_s('b')); + while (!d6b.empty()) { + d6b.pop_back(); + assert(is_double_ended_contiguous_container_asan_correct(d6b)); + } + + C d6c(N + 2, get_s('c')); + while (!d6c.empty()) { + d6c.pop_back(); + assert(is_double_ended_contiguous_container_asan_correct(d6c)); + } + } + { + C d7(9 * N + 2); + + d7.insert(d7.begin() + 1, S()); + assert(verify_inside(d7)); + + d7.insert(d7.end() - 3, S()); + assert(verify_inside(d7)); + + d7.insert(d7.begin() + 2 * N, get_s('a')); + assert(verify_inside(d7)); + + d7.insert(d7.end() - 2 * N, get_s('b')); + assert(verify_inside(d7)); + + d7.insert(d7.begin() + 2 * N, 3 * N, get_s('c')); + assert(verify_inside(d7)); + + // It may not be short for big element types, but it will be checked correctly: + d7.insert(d7.end() - 2 * N, 3 * N, get_s('d')); + assert(verify_inside(d7)); + + d7.erase(d7.begin() + 2); + assert(verify_inside(d7)); + + d7.erase(d7.end() - 2); + assert(verify_inside(d7)); + } +} + +template +void test_container() { + test_string>, S>(); + test_string>, S>(); + test_string>, S>(); +} + +int main(int, char**) { + // Those tests support only types based on std::basic_string. + test_container(); + test_container(); +#if TEST_STD_VER >= 11 + test_container(); + test_container(); +#endif +#if TEST_STD_VER >= 20 + test_container(); +#endif + + return 0; +} diff --git a/libcxx/test/libcxx/containers/strings/basic.string/asan_short.pass.cpp b/libcxx/test/libcxx/containers/strings/basic.string/asan_short.pass.cpp new file mode 100644 index 0000000000000..53c70bed189b5 --- /dev/null +++ b/libcxx/test/libcxx/containers/strings/basic.string/asan_short.pass.cpp @@ -0,0 +1,56 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +// REQUIRES: asan +// UNSUPPORTED: c++03 + +// + +// Basic test if ASan annotations work for short strings. + +#include +#include +#include + +#include "asan_testing.h" +#include "min_allocator.h" +#include "test_iterators.h" +#include "test_macros.h" + +extern "C" void __sanitizer_set_death_callback(void (*callback)(void)); + +void do_exit() { exit(0); } + +int main(int, char**) { + { + typedef cpp17_input_iterator MyInputIter; + // Should not trigger ASan. + std::basic_string, safe_allocator> v; + char i[] = {'a', 'b', 'c', 'd'}; + + v.insert(v.begin(), MyInputIter(i), MyInputIter(i + 4)); + assert(v[0] == 'a'); + assert(is_string_asan_correct(v)); + } + + __sanitizer_set_death_callback(do_exit); + { + using T = char; + using C = std::basic_string, safe_allocator>; + const T t[] = {'a', 'b', 'c', 'd', 'e', 'f', 'g'}; + C c(std::begin(t), std::end(t)); + assert(is_string_asan_correct(c)); + assert(__sanitizer_verify_contiguous_container(c.data(), c.data() + c.size() + 1, c.data() + c.capacity() + 1) != + 0); + volatile T foo = c[c.size() + 1]; // should trigger ASAN. Use volatile to prevent being optimized away. + assert(false); // if we got here, ASAN didn't trigger + ((void)foo); + } + + return 0; +} diff --git a/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp b/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp new file mode 100644 index 0000000000000..3e31c19b3c002 --- /dev/null +++ b/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp @@ -0,0 +1,183 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +// REQUIRES: asan +// UNSUPPORTED: c++03 + +#include +#include +#include "test_macros.h" +#include "asan_testing.h" // includes vector and string - don't do it before +#include "min_allocator.h" + +// This tests exists to check if strings work well with vector, as those +// may be partialy annotated, we cannot simply call +// is_contiguous_container_asan_correct, as it assumes that +// object memory inside is not annotated, so we check everything in a more careful way. + +template +bool verify_inside(D const& d) { + for (size_t i = 0; i < d.size(); ++i) { + if (!is_string_asan_correct(d[i])) + return false; + } + + return true; +} + +template +S get_s(char c) { + S s; + for (size_t i = 0; i < N; ++i) + s.push_back(c); + + return s; +} + +template +void test_string() { + size_t const N = sizeof(S) < 256 ? (4096 / sizeof(S)) : 16; + + { + C d1a(1), d1b(N), d1c(N + 1), d1d(32 * N); + assert(verify_inside(d1a)); + assert(verify_inside(d1b)); + assert(verify_inside(d1c)); + assert(verify_inside(d1d)); + } + { + C d2; + for (size_t i = 0; i < 16 * N; ++i) { + d2.push_back(get_s(i % 10 + 'a')); + assert(verify_inside(d2)); + d2.push_back(get_s(i % 10 + 'b')); + assert(verify_inside(d2)); + + d2.erase(d2.cbegin()); + assert(verify_inside(d2)); + } + } + { + C d3; + for (size_t i = 0; i < 16 * N; ++i) { + d3.push_back(get_s(i % 10 + 'a')); + assert(verify_inside(d3)); + d3.push_back(get_s(i % 10 + 'b')); + assert(verify_inside(d3)); + + d3.pop_back(); + assert(verify_inside(d3)); + } + } + { + C d4; + for (size_t i = 0; i < 16 * N; ++i) { + // When there is no SSO, all elements inside should not be poisoned, + // so we can verify vector poisoning. + d4.push_back(get_s(i % 10 + 'a')); + assert(verify_inside(d4)); + assert(is_contiguous_container_asan_correct(d4)); + d4.push_back(get_s(i % 10 + 'b')); + assert(verify_inside(d4)); + assert(is_contiguous_container_asan_correct(d4)); + } + } + { + C d5; + for (size_t i = 0; i < 5 * N; ++i) { + // In d4 we never had poisoned memory inside vector. + // Here we start with SSO, so part of the inside of the container, + // will be poisoned. + d5.push_back(S()); + assert(verify_inside(d5)); + } + for (size_t i = 0; i < d5.size(); ++i) { + // We change the size to have long string. + // Memory owne by vector should not be poisoned by string. + d5[i].resize(1000); + assert(verify_inside(d5)); + } + + assert(is_contiguous_container_asan_correct(d5)); + + d5.erase(d5.begin() + 2); + assert(verify_inside(d5)); + + d5.erase(d5.end() - 2); + assert(verify_inside(d5)); + + assert(is_contiguous_container_asan_correct(d5)); + } + { + C d6a; + assert(is_contiguous_container_asan_correct(d6a)); + + C d6b(N + 2, get_s('a')); + d6b.push_back(get_s('b')); + while (!d6b.empty()) { + d6b.pop_back(); + assert(is_contiguous_container_asan_correct(d6b)); + } + + C d6c(N + 2, get_s('c')); + while (!d6c.empty()) { + d6c.pop_back(); + assert(is_contiguous_container_asan_correct(d6c)); + } + } + { + C d7(9 * N + 2); + + d7.insert(d7.begin() + 1, S()); + assert(verify_inside(d7)); + + d7.insert(d7.end() - 3, S()); + assert(verify_inside(d7)); + + d7.insert(d7.begin() + 2 * N, get_s('a')); + assert(verify_inside(d7)); + + d7.insert(d7.end() - 2 * N, get_s('b')); + assert(verify_inside(d7)); + + d7.insert(d7.begin() + 2 * N, 3 * N, get_s('c')); + assert(verify_inside(d7)); + + // It may not be short for big element types, but it will be checked correctly: + d7.insert(d7.end() - 2 * N, 3 * N, get_s('d')); + assert(verify_inside(d7)); + + d7.erase(d7.begin() + 2); + assert(verify_inside(d7)); + + d7.erase(d7.end() - 2); + assert(verify_inside(d7)); + } +} + +template +void test_container() { + test_string>, S>(); + test_string>, S>(); + test_string>, S>(); +} + +int main(int, char**) { + // Those tests support only types based on std::basic_string. + test_container(); + test_container(); +#if TEST_STD_VER >= 11 + test_container(); + test_container(); +#endif +#if TEST_STD_VER >= 20 + test_container(); +#endif + + return 0; +} diff --git a/libcxx/test/support/asan_testing.h b/libcxx/test/support/asan_testing.h index 6bfc8280a4ead..8d240ff20d16c 100644 --- a/libcxx/test/support/asan_testing.h +++ b/libcxx/test/support/asan_testing.h @@ -74,17 +74,11 @@ TEST_CONSTEXPR bool is_string_asan_correct(const std::basic_string::value) - return __sanitizer_verify_contiguous_container(c.data(), c.data() + c.size() + 1, c.data() + c.capacity() + 1) != - 0; - else - return __sanitizer_verify_contiguous_container( - c.data(), c.data() + c.capacity() + 1, c.data() + c.capacity() + 1) != 0; - } else { - return __sanitizer_verify_contiguous_container(std::addressof(c), std::addressof(c) + 1, std::addressof(c) + 1) != - 0; - } + if (std::__asan_annotate_container_with_allocator::value) + return __sanitizer_verify_contiguous_container(c.data(), c.data() + c.size() + 1, c.data() + c.capacity() + 1) != 0; + else + return __sanitizer_verify_contiguous_container( + c.data(), c.data() + c.capacity() + 1, c.data() + c.capacity() + 1) != 0; } #else # include From aa55b258644c3b5fc194757759e2d17161765e90 Mon Sep 17 00:00:00 2001 From: Advenam Tacet Date: Tue, 16 Jan 2024 22:44:04 +0100 Subject: [PATCH 2/3] Fixes based on a code review --- .../asan_vector_integration.pass.cpp | 55 +++++++++---------- libcxx/test/support/asan_testing.h | 13 ----- 2 files changed, 26 insertions(+), 42 deletions(-) diff --git a/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp b/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp index 3e31c19b3c002..93ccd2f57e03f 100644 --- a/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp +++ b/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp @@ -21,13 +21,10 @@ // object memory inside is not annotated, so we check everything in a more careful way. template -bool verify_inside(D const& d) { +void verify_inside(D const& d) { for (size_t i = 0; i < d.size(); ++i) { - if (!is_string_asan_correct(d[i])) - return false; + assert(is_string_asan_correct(d[i])); } - - return true; } template @@ -45,33 +42,33 @@ void test_string() { { C d1a(1), d1b(N), d1c(N + 1), d1d(32 * N); - assert(verify_inside(d1a)); - assert(verify_inside(d1b)); - assert(verify_inside(d1c)); - assert(verify_inside(d1d)); + verify_inside(d1a); + verify_inside(d1b); + verify_inside(d1c); + verify_inside(d1d); } { C d2; for (size_t i = 0; i < 16 * N; ++i) { d2.push_back(get_s(i % 10 + 'a')); - assert(verify_inside(d2)); + verify_inside(d2); d2.push_back(get_s(i % 10 + 'b')); - assert(verify_inside(d2)); + verify_inside(d2); d2.erase(d2.cbegin()); - assert(verify_inside(d2)); + verify_inside(d2); } } { C d3; for (size_t i = 0; i < 16 * N; ++i) { d3.push_back(get_s(i % 10 + 'a')); - assert(verify_inside(d3)); + verify_inside(d3); d3.push_back(get_s(i % 10 + 'b')); - assert(verify_inside(d3)); + verify_inside(d3); d3.pop_back(); - assert(verify_inside(d3)); + verify_inside(d3); } } { @@ -80,10 +77,10 @@ void test_string() { // When there is no SSO, all elements inside should not be poisoned, // so we can verify vector poisoning. d4.push_back(get_s(i % 10 + 'a')); - assert(verify_inside(d4)); + verify_inside(d4); assert(is_contiguous_container_asan_correct(d4)); d4.push_back(get_s(i % 10 + 'b')); - assert(verify_inside(d4)); + verify_inside(d4); assert(is_contiguous_container_asan_correct(d4)); } } @@ -94,22 +91,22 @@ void test_string() { // Here we start with SSO, so part of the inside of the container, // will be poisoned. d5.push_back(S()); - assert(verify_inside(d5)); + verify_inside(d5); } for (size_t i = 0; i < d5.size(); ++i) { // We change the size to have long string. // Memory owne by vector should not be poisoned by string. d5[i].resize(1000); - assert(verify_inside(d5)); + verify_inside(d5); } assert(is_contiguous_container_asan_correct(d5)); d5.erase(d5.begin() + 2); - assert(verify_inside(d5)); + verify_inside(d5); d5.erase(d5.end() - 2); - assert(verify_inside(d5)); + verify_inside(d5); assert(is_contiguous_container_asan_correct(d5)); } @@ -134,29 +131,29 @@ void test_string() { C d7(9 * N + 2); d7.insert(d7.begin() + 1, S()); - assert(verify_inside(d7)); + verify_inside(d7); d7.insert(d7.end() - 3, S()); - assert(verify_inside(d7)); + verify_inside(d7); d7.insert(d7.begin() + 2 * N, get_s('a')); - assert(verify_inside(d7)); + verify_inside(d7); d7.insert(d7.end() - 2 * N, get_s('b')); - assert(verify_inside(d7)); + verify_inside(d7); d7.insert(d7.begin() + 2 * N, 3 * N, get_s('c')); - assert(verify_inside(d7)); + verify_inside(d7); // It may not be short for big element types, but it will be checked correctly: d7.insert(d7.end() - 2 * N, 3 * N, get_s('d')); - assert(verify_inside(d7)); + verify_inside(d7); d7.erase(d7.begin() + 2); - assert(verify_inside(d7)); + verify_inside(d7); d7.erase(d7.end() - 2); - assert(verify_inside(d7)); + verify_inside(d7); } } diff --git a/libcxx/test/support/asan_testing.h b/libcxx/test/support/asan_testing.h index 8d240ff20d16c..3785c1f9c20de 100644 --- a/libcxx/test/support/asan_testing.h +++ b/libcxx/test/support/asan_testing.h @@ -56,19 +56,6 @@ TEST_CONSTEXPR bool is_double_ended_contiguous_container_asan_correct(const std: #endif #if TEST_HAS_FEATURE(address_sanitizer) -template -bool is_string_short(S const& s) { - // We do not have access to __is_long(), but we can check if strings - // buffer is inside strings memory. If strings memory contains its content, - // SSO is in use. To check it, we can just confirm that the beginning is in - // the string object memory block. - // &s - beginning of objects memory - // &s[0] - beginning of the buffer - // (&s+1) - end of objects memory - return (void*)std::addressof(s) <= (void*)std::addressof(s[0]) && - (void*)std::addressof(s[0]) < (void*)(std::addressof(s) + 1); -} - template TEST_CONSTEXPR bool is_string_asan_correct(const std::basic_string& c) { if (TEST_IS_CONSTANT_EVALUATED) From 3b4dfc2aa2251ff427fc7bd5b067485c111ea28a Mon Sep 17 00:00:00 2001 From: Advenam Tacet Date: Tue, 16 Jan 2024 23:33:09 +0100 Subject: [PATCH 3/3] Code review fixes --- .../asan_deque_integration.pass.cpp | 59 +++++++++---------- .../asan_vector_integration.pass.cpp | 4 +- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/libcxx/test/libcxx/containers/strings/basic.string/asan_deque_integration.pass.cpp b/libcxx/test/libcxx/containers/strings/basic.string/asan_deque_integration.pass.cpp index e19410c5409b1..b914609f35ddf 100644 --- a/libcxx/test/libcxx/containers/strings/basic.string/asan_deque_integration.pass.cpp +++ b/libcxx/test/libcxx/containers/strings/basic.string/asan_deque_integration.pass.cpp @@ -10,9 +10,11 @@ // UNSUPPORTED: c++03 #include +#include #include +#include #include "test_macros.h" -#include "asan_testing.h" // includes deque and string - don't do it before +#include "asan_testing.h" #include "min_allocator.h" // This tests exists to check if strings work well with deque, as those @@ -21,13 +23,10 @@ // object memory inside is not annotated, so we check everything in a more careful way. template -bool verify_inside(D const& d) { +void verify_inside(D const& d) { for (size_t i = 0; i < d.size(); ++i) { - if (!is_string_asan_correct(d[i])) - return false; + assert(is_string_asan_correct(d[i])); } - - return true; } template @@ -45,33 +44,33 @@ void test_string() { { C d1a(1), d1b(N), d1c(N + 1), d1d(32 * N); - assert(verify_inside(d1a)); - assert(verify_inside(d1b)); - assert(verify_inside(d1c)); - assert(verify_inside(d1d)); + verify_inside(d1a); + verify_inside(d1b); + verify_inside(d1c); + verify_inside(d1d); } { C d2; for (size_t i = 0; i < 16 * N; ++i) { d2.push_back(get_s(i % 10 + 'a')); - assert(verify_inside(d2)); + verify_inside(d2); d2.push_back(get_s(i % 10 + 'b')); - assert(verify_inside(d2)); + verify_inside(d2); d2.pop_front(); - assert(verify_inside(d2)); + verify_inside(d2); } } { C d3; for (size_t i = 0; i < 16 * N; ++i) { d3.push_front(get_s(i % 10 + 'a')); - assert(verify_inside(d3)); + verify_inside(d3); d3.push_front(get_s(i % 10 + 'b')); - assert(verify_inside(d3)); + verify_inside(d3); d3.pop_back(); - assert(verify_inside(d3)); + verify_inside(d3); } } { @@ -80,10 +79,10 @@ void test_string() { // When there is no SSO, all elements inside should not be poisoned, // so we can verify deque poisoning. d4.push_front(get_s(i % 10 + 'a')); - assert(verify_inside(d4)); + verify_inside(d4); assert(is_double_ended_contiguous_container_asan_correct(d4)); d4.push_back(get_s(i % 10 + 'b')); - assert(verify_inside(d4)); + verify_inside(d4); assert(is_double_ended_contiguous_container_asan_correct(d4)); } } @@ -94,22 +93,22 @@ void test_string() { // Here we start with SSO, so part of the inside of the container, // will be poisoned. d5.push_front(S()); - assert(verify_inside(d5)); + verify_inside(d5); } for (size_t i = 0; i < d5.size(); ++i) { // We change the size to have long string. // Memory owne by deque should not be poisoned by string. d5[i].resize(1000); - assert(verify_inside(d5)); + verify_inside(d5); } assert(is_double_ended_contiguous_container_asan_correct(d5)); d5.erase(d5.begin() + 2); - assert(verify_inside(d5)); + verify_inside(d5); d5.erase(d5.end() - 2); - assert(verify_inside(d5)); + verify_inside(d5); assert(is_double_ended_contiguous_container_asan_correct(d5)); } @@ -134,29 +133,29 @@ void test_string() { C d7(9 * N + 2); d7.insert(d7.begin() + 1, S()); - assert(verify_inside(d7)); + verify_inside(d7); d7.insert(d7.end() - 3, S()); - assert(verify_inside(d7)); + verify_inside(d7); d7.insert(d7.begin() + 2 * N, get_s('a')); - assert(verify_inside(d7)); + verify_inside(d7); d7.insert(d7.end() - 2 * N, get_s('b')); - assert(verify_inside(d7)); + verify_inside(d7); d7.insert(d7.begin() + 2 * N, 3 * N, get_s('c')); - assert(verify_inside(d7)); + verify_inside(d7); // It may not be short for big element types, but it will be checked correctly: d7.insert(d7.end() - 2 * N, 3 * N, get_s('d')); - assert(verify_inside(d7)); + verify_inside(d7); d7.erase(d7.begin() + 2); - assert(verify_inside(d7)); + verify_inside(d7); d7.erase(d7.end() - 2); - assert(verify_inside(d7)); + verify_inside(d7); } } diff --git a/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp b/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp index 93ccd2f57e03f..5b1900fb00d5b 100644 --- a/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp +++ b/libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp @@ -10,9 +10,11 @@ // UNSUPPORTED: c++03 #include +#include +#include #include #include "test_macros.h" -#include "asan_testing.h" // includes vector and string - don't do it before +#include "asan_testing.h" #include "min_allocator.h" // This tests exists to check if strings work well with vector, as those