[libcxx-commits] [libcxx] [llvm] [libcxx] Remove ASan container overflow checks for SSO strings (PR #194208)
via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Apr 25 23:23:38 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Aiden Grossman (boomanaiden154)
<details>
<summary>Changes</summary>
Essentially a revert of #<!-- -->79536, although manual given how much the code has changed.
Doing container overflow checks on SSO strings interacts very poorly with a lot of compiler optimizations, which assume that the entire object is dereferencable when it is live. LLVM can sometimes insert speculative loads that will create false positives when they try and load a partially filled SSO buffer. This was previously prevented using inline asm barriers within std::string itself. However, this can also manifest in other ways, with the motivating example for the revert being GlobalOpt inserting a speculative load of a std::variant containing a SSO std::string, causing an ASan failure.
---
Full diff: https://github.com/llvm/llvm-project/pull/194208.diff
6 Files Affected:
- (modified) libcxx/include/string (-30)
- (removed) libcxx/test/libcxx/strings/basic.string/asan_deque_integration.pass.cpp (-182)
- (removed) libcxx/test/libcxx/strings/basic.string/asan_short.pass.cpp (-56)
- (removed) libcxx/test/libcxx/strings/basic.string/asan_vector_integration.pass.cpp (-182)
- (modified) libcxx/test/support/asan_testing.h (+24-5)
- (removed) llvm/unittests/DebugInfo/LogicalView/Inputs/test-dwarf-clang-unspec-params.elf ()
``````````diff
diff --git a/libcxx/include/string b/libcxx/include/string
index 0c8767df2cdd2..d2551beb12045 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -759,20 +759,10 @@ public:
//
// This string implementation doesn't contain any references into itself. It only contains a bit that says whether
// it is in small or large string mode, so the entire structure is trivially relocatable if its members are.
-# if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS_FOR_STRING
- // When compiling with AddressSanitizer (ASan), basic_string cannot be trivially
- // relocatable. Because the object's memory might be poisoned when its content
- // is kept inside objects memory (short string optimization), instead of in allocated
- // external memory. In such cases, the destructor is responsible for unpoisoning
- // the memory to avoid triggering false positives.
- // Therefore it's crucial to ensure the destructor is called.
- using __trivially_relocatable _LIBCPP_NODEBUG = void;
-# else
using __trivially_relocatable _LIBCPP_NODEBUG = __conditional_t<
__libcpp_is_trivially_relocatable<allocator_type>::value && __libcpp_is_trivially_relocatable<pointer>::value,
basic_string,
void>;
-# endif
static_assert(!is_array<value_type>::value, "Character type of basic_string must not be an array");
static_assert(is_standard_layout<value_type>::value, "Character type of basic_string must be standard-layout");
@@ -2185,7 +2175,6 @@ private:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS size_type
__get_short_size() const _NOEXCEPT {
_LIBCPP_ASSERT_INTERNAL(!__rep_.__s.__is_long_, "String has to be short when trying to get the short size");
- __annotate_memory_barrier();
return __rep_.__s.__size_;
}
@@ -2195,7 +2184,6 @@ private:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type __get_long_size() const _NOEXCEPT {
_LIBCPP_ASSERT_INTERNAL(__rep_.__l.__is_long_, "String has to be long when trying to get the long size");
- __annotate_memory_barrier();
return __rep_.__l.__size_;
}
@@ -2208,31 +2196,26 @@ private:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type __get_long_cap() const _NOEXCEPT {
_LIBCPP_ASSERT_INTERNAL(__rep_.__l.__is_long_, "String has to be long when trying to get the long capacity");
- __annotate_memory_barrier();
return __rep_.__l.__cap_ * __endian_factor;
}
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pointer __get_long_pointer() _NOEXCEPT {
_LIBCPP_ASSERT_INTERNAL(__rep_.__l.__is_long_, "String has to be long when trying to get the long pointer");
- __annotate_memory_barrier();
return __rep_.__l.__data_;
}
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 const_pointer __get_long_pointer() const _NOEXCEPT {
_LIBCPP_ASSERT_INTERNAL(__rep_.__l.__is_long_, "String has to be long when trying to get the long pointer");
- __annotate_memory_barrier();
return __rep_.__l.__data_;
}
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS pointer
__get_short_pointer() _NOEXCEPT {
- __annotate_memory_barrier();
return pointer_traits<pointer>::pointer_to(__rep_.__s.__data_[0]);
}
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS const_pointer
__get_short_pointer() const _NOEXCEPT {
- __annotate_memory_barrier();
return pointer_traits<const_pointer>::pointer_to(__rep_.__s.__data_[0]);
}
@@ -2308,25 +2291,12 @@ private:
(void)__old_mid;
(void)__new_mid;
# if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS_FOR_STRING
-# if defined(__APPLE__)
- // TODO: remove after addressing issue #96099 (https://llvm.org/PR96099)
if (!__is_long())
return;
-# endif
std::__annotate_contiguous_container<_Allocator>(data(), data() + capacity() + 1, __old_mid, __new_mid);
# endif
}
- // Prevents speculative load before short/long state is determined.
- // The following functions are no-ops outside of AddressSanitizer mode.
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __annotate_memory_barrier() const {
-# if _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS_FOR_STRING
- if (__libcpp_is_constant_evaluated())
- return;
- asm volatile("" ::: "memory");
-# endif
- }
-
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __annotate_new(size_type __current_size) const _NOEXCEPT {
__annotate_contiguous_container(data() + capacity() + 1, data() + __current_size + 1);
}
diff --git a/libcxx/test/libcxx/strings/basic.string/asan_deque_integration.pass.cpp b/libcxx/test/libcxx/strings/basic.string/asan_deque_integration.pass.cpp
deleted file mode 100644
index 1205190b3a6e1..0000000000000
--- a/libcxx/test/libcxx/strings/basic.string/asan_deque_integration.pass.cpp
+++ /dev/null
@@ -1,182 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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 <cassert>
-#include <string>
-#include <array>
-#include <deque>
-#include "test_macros.h"
-#include "asan_testing.h"
-#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 <typename D>
-void verify_inside(D const& d) {
- for (size_t i = 0; i < d.size(); ++i) {
- assert(is_string_asan_correct(d[i]));
- }
-}
-
-template <typename S, size_t N>
-S get_s(char c) {
- S s;
- for (size_t i = 0; i < N; ++i)
- s.push_back(c);
-
- return s;
-}
-
-template <class C, class S>
-void test_string() {
- size_t const N = sizeof(S) < 256 ? (4096 / sizeof(S)) : 16;
-
- {
- C d1a(1), d1b(N), d1c(N + 1), d1d(5 * N);
- verify_inside(d1a);
- verify_inside(d1b);
- verify_inside(d1c);
- verify_inside(d1d);
- }
- {
- C d2;
- for (size_t i = 0; i < 3 * N + 2; ++i) {
- d2.push_back(get_s<S, 1>(i % 10 + 'a'));
- verify_inside(d2);
- d2.push_back(get_s<S, 22>(i % 10 + 'b'));
- verify_inside(d2);
-
- d2.pop_front();
- verify_inside(d2);
- }
- }
- {
- C d3;
- for (size_t i = 0; i < 3 * N + 2; ++i) {
- d3.push_front(get_s<S, 1>(i % 10 + 'a'));
- verify_inside(d3);
- d3.push_front(get_s<S, 28>(i % 10 + 'b'));
- verify_inside(d3);
-
- d3.pop_back();
- verify_inside(d3);
- }
- }
- {
- C d4;
- for (size_t i = 0; i < 3 * N + 2; ++i) {
- // When there is no SSO, all elements inside should not be poisoned,
- // so we can verify deque poisoning.
- d4.push_front(get_s<S, 33>(i % 10 + 'a'));
- verify_inside(d4);
- assert(is_double_ended_contiguous_container_asan_correct(d4));
- d4.push_back(get_s<S, 28>(i % 10 + 'b'));
- verify_inside(d4);
- assert(is_double_ended_contiguous_container_asan_correct(d4));
- }
- }
- {
- C d5;
- for (size_t i = 0; i < 3 * N + 2; ++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());
- 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(100);
- verify_inside(d5);
- }
-
- assert(is_double_ended_contiguous_container_asan_correct(d5));
-
- d5.erase(d5.begin() + 2);
- verify_inside(d5);
-
- d5.erase(d5.end() - 2);
- 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<S, 100>('a'));
- d6b.push_front(get_s<S, 101>('b'));
- while (!d6b.empty()) {
- d6b.pop_back();
- assert(is_double_ended_contiguous_container_asan_correct(d6b));
- }
-
- C d6c(N + 2, get_s<S, 102>('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());
- verify_inside(d7);
-
- d7.insert(d7.end() - 3, S());
- verify_inside(d7);
-
- d7.insert(d7.begin() + 2 * N, get_s<S, 1>('a'));
- verify_inside(d7);
-
- d7.insert(d7.end() - 2 * N, get_s<S, 1>('b'));
- verify_inside(d7);
-
- d7.insert(d7.begin() + 2 * N, 3 * N, get_s<S, 1>('c'));
- 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<S, 2>('d'));
- verify_inside(d7);
-
- d7.erase(d7.begin() + 2);
- verify_inside(d7);
-
- d7.erase(d7.end() - 2);
- verify_inside(d7);
- }
-}
-
-template <class S>
-void test_container() {
- test_string<std::deque<S, std::allocator<S>>, S>();
- test_string<std::deque<S, min_allocator<S>>, S>();
- test_string<std::deque<S, safe_allocator<S>>, S>();
-}
-
-int main(int, char**) {
- // Those tests support only types based on std::basic_string.
- test_container<std::string>();
- test_container<std::wstring>();
-#if TEST_STD_VER >= 11
- test_container<std::u16string>();
- test_container<std::u32string>();
-#endif
-#if TEST_STD_VER >= 20
- test_container<std::u8string>();
-#endif
-
- return 0;
-}
diff --git a/libcxx/test/libcxx/strings/basic.string/asan_short.pass.cpp b/libcxx/test/libcxx/strings/basic.string/asan_short.pass.cpp
deleted file mode 100644
index 5140f429216fc..0000000000000
--- a/libcxx/test/libcxx/strings/basic.string/asan_short.pass.cpp
+++ /dev/null
@@ -1,56 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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: libcpp-instrumented-with-asan
-// UNSUPPORTED: c++03
-
-// <string>
-
-// Basic test if ASan annotations work for short strings.
-
-#include <string>
-#include <cassert>
-#include <cstdlib>
-
-#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<char*> MyInputIter;
- // Should not trigger ASan.
- std::basic_string<char, std::char_traits<char>, safe_allocator<char>> 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<T, std::char_traits<T>, safe_allocator<T>>;
- 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/strings/basic.string/asan_vector_integration.pass.cpp b/libcxx/test/libcxx/strings/basic.string/asan_vector_integration.pass.cpp
deleted file mode 100644
index b7d95b7069083..0000000000000
--- a/libcxx/test/libcxx/strings/basic.string/asan_vector_integration.pass.cpp
+++ /dev/null
@@ -1,182 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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 <cassert>
-#include <string>
-#include <vector>
-#include <array>
-#include "test_macros.h"
-#include "asan_testing.h"
-#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 <typename D>
-void verify_inside(D const& d) {
- for (size_t i = 0; i < d.size(); ++i) {
- assert(is_string_asan_correct(d[i]));
- }
-}
-
-template <typename S, size_t N>
-S get_s(char c) {
- S s;
- for (size_t i = 0; i < N; ++i)
- s.push_back(c);
-
- return s;
-}
-
-template <class C, class S>
-void test_string() {
- size_t const N = sizeof(S) < 256 ? (4096 / sizeof(S)) : 16;
-
- {
- C d1a(1), d1b(N), d1c(N + 1), d1d(5 * N);
- verify_inside(d1a);
- verify_inside(d1b);
- verify_inside(d1c);
- verify_inside(d1d);
- }
- {
- C d2;
- for (size_t i = 0; i < 3 * N + 2; ++i) {
- d2.push_back(get_s<S, 1>(i % 10 + 'a'));
- verify_inside(d2);
- d2.push_back(get_s<S, 28>(i % 10 + 'b'));
- verify_inside(d2);
-
- d2.erase(d2.cbegin());
- verify_inside(d2);
- }
- }
- {
- C d3;
- for (size_t i = 0; i < 3 * N + 2; ++i) {
- d3.push_back(get_s<S, 1>(i % 10 + 'a'));
- verify_inside(d3);
- d3.push_back(get_s<S, 28>(i % 10 + 'b'));
- verify_inside(d3);
-
- d3.pop_back();
- verify_inside(d3);
- }
- }
- {
- C d4;
- for (size_t i = 0; i < 3 * N + 2; ++i) {
- // When there is no SSO, all elements inside should not be poisoned,
- // so we can verify vector poisoning.
- d4.push_back(get_s<S, 33>(i % 10 + 'a'));
- verify_inside(d4);
- assert(is_contiguous_container_asan_correct(d4));
- d4.push_back(get_s<S, 28>(i % 10 + 'b'));
- verify_inside(d4);
- assert(is_contiguous_container_asan_correct(d4));
- }
- }
- {
- C d5;
- for (size_t i = 0; i < 3 * N + 2; ++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());
- 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(100);
- verify_inside(d5);
- }
-
- assert(is_contiguous_container_asan_correct(d5));
-
- d5.erase(d5.begin() + 2);
- verify_inside(d5);
-
- d5.erase(d5.end() - 2);
- 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<S, 100>('a'));
- d6b.push_back(get_s<S, 101>('b'));
- while (!d6b.empty()) {
- d6b.pop_back();
- assert(is_contiguous_container_asan_correct(d6b));
- }
-
- C d6c(N + 2, get_s<S, 102>('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());
- verify_inside(d7);
-
- d7.insert(d7.end() - 3, S());
- verify_inside(d7);
-
- d7.insert(d7.begin() + 2 * N, get_s<S, 1>('a'));
- verify_inside(d7);
-
- d7.insert(d7.end() - 2 * N, get_s<S, 1>('b'));
- verify_inside(d7);
-
- d7.insert(d7.begin() + 2 * N, 3 * N, get_s<S, 1>('c'));
- 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<S, 2>('d'));
- verify_inside(d7);
-
- d7.erase(d7.begin() + 2);
- verify_inside(d7);
-
- d7.erase(d7.end() - 2);
- verify_inside(d7);
- }
-}
-
-template <class S>
-void test_container() {
- test_string<std::vector<S, std::allocator<S>>, S>();
- test_string<std::vector<S, min_allocator<S>>, S>();
- test_string<std::vector<S, safe_allocator<S>>, S>();
-}
-
-int main(int, char**) {
- // Those tests support only types based on std::basic_string.
- test_container<std::string>();
- test_container<std::wstring>();
-#if TEST_STD_VER >= 11
- test_container<std::u16string>();
- test_container<std::u32string>();
-#endif
-#if TEST_STD_VER >= 20
- test_container<std::u8string>();
-#endif
-
- return 0;
-}
diff --git a/libcxx/test/support/asan_testing.h b/libcxx/test/support/asan_testing.h
index 2400befe99bde..ebddafa5035a3 100644
--- a/libcxx/test/support/asan_testing.h
+++ b/libcxx/test/support/asan_testing.h
@@ -56,16 +56,35 @@ TEST_CONSTEXPR bool is_double_ended_contiguous_container_asan_correct(const std:
#endif
#if TEST_HAS_FEATURE(address_sanitizer) && _LIBCPP_ENABLE_ASAN_CONTAINER_CHECKS_FOR_STRING
+template <typename S>
+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 <typename ChrT, typename TraitsT, typename Alloc>
TEST_CONSTEXPR bool is_string_asan_correct(const std::basic_string<ChrT, TraitsT, Alloc>& c) {
if (TEST_IS_CONSTANT_EVALUATED)
return true;
- if (std::__asan_annotate_container_with_allocator<Alloc>::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;
+ if (!is_string_short(c)) {
+ if (std::__asan_annotate_container_with_allocator<Alloc>::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;
+ }
}
#else
# include <string>
diff --git a/llvm/unittests/DebugInfo/LogicalView/Inputs/test-dwarf-clang-unspec-params.elf b/llvm/unittests/DebugInfo/LogicalView/Inputs/test-dwarf-clang-unspec-params.elf
deleted file mode 100755
index 67c6e71fbf7b9..0000000000000
Binary files a/llvm/unittests/DebugInfo/LogicalView/Inputs/test-dwarf-clang-unspec-params.elf and /dev/null differ
``````````
</details>
https://github.com/llvm/llvm-project/pull/194208
More information about the libcxx-commits
mailing list