From 3428840844dac39b46ccff05f74775631d51a799 Mon Sep 17 00:00:00 2001 From: David Justo Date: Wed, 15 Jan 2025 19:38:10 -0800 Subject: [PATCH 01/25] add naive first implementation of per-allocator disablement of ASan --- stl/inc/vector | 4 ++++ stl/inc/xmemory | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/stl/inc/vector b/stl/inc/vector index 4f79f4986c..6704a68a23 100644 --- a/stl/inc/vector +++ b/stl/inc/vector @@ -493,6 +493,10 @@ private: return; } + if (!is_ASan_enabled_for_allocator_v) { + return; + } + const void* const _First = _STD _Unfancy(_First_); const void* const _End = _STD _Unfancy(_End_); const void* const _Old_last = _STD _Unfancy(_Old_last_); diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 462a7465fc..11574bc068 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -805,6 +805,17 @@ struct _Simple_types { // wraps types from allocators with simple addressing for _INLINE_VAR constexpr size_t _Asan_granularity = 8; _INLINE_VAR constexpr size_t _Asan_granularity_mask = _Asan_granularity - 1; +// Represents whether ASan is enabled for allocator type 'T'. +// By default, ASan is enabled. +template +struct is_ASan_enabled_for_allocator { + static constexpr bool value = true; +}; + +// Convenience helper to determine if ASan is enabled for a given allocator type 'T' +template +constexpr bool is_ASan_enabled_for_allocator_v = is_ASan_enabled_for_allocator::value; + struct _Asan_aligned_pointers { const void* _First; const void* _End; From 790ac7e9acdb24cd4a7cc2f8d1e0547ddcfba102 Mon Sep 17 00:00:00 2001 From: David Justo Date: Thu, 16 Jan 2025 13:54:43 -0800 Subject: [PATCH 02/25] improve comments Co-authored-by: Casey Carter --- stl/inc/xmemory | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 11574bc068..436556bc05 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -805,7 +805,7 @@ struct _Simple_types { // wraps types from allocators with simple addressing for _INLINE_VAR constexpr size_t _Asan_granularity = 8; _INLINE_VAR constexpr size_t _Asan_granularity_mask = _Asan_granularity - 1; -// Represents whether ASan is enabled for allocator type 'T'. +// Represents whether ASan container annotations are enabled for allocator type 'T'. // By default, ASan is enabled. template struct is_ASan_enabled_for_allocator { From 846061c93f295f69bd28dfc1544610c94b9442c6 Mon Sep 17 00:00:00 2001 From: David Justo Date: Thu, 16 Jan 2025 14:00:48 -0800 Subject: [PATCH 03/25] Simplify implementation Co-authored-by: Casey Carter --- stl/inc/xmemory | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 436556bc05..b48d9aa653 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -806,15 +806,8 @@ _INLINE_VAR constexpr size_t _Asan_granularity = 8; _INLINE_VAR constexpr size_t _Asan_granularity_mask = _Asan_granularity - 1; // Represents whether ASan container annotations are enabled for allocator type 'T'. -// By default, ASan is enabled. -template -struct is_ASan_enabled_for_allocator { - static constexpr bool value = true; -}; - -// Convenience helper to determine if ASan is enabled for a given allocator type 'T' -template -constexpr bool is_ASan_enabled_for_allocator_v = is_ASan_enabled_for_allocator::value; +template +constexpr bool _Is_ASan_enabled_for_allocator = true; struct _Asan_aligned_pointers { const void* _First; From 71c7716a8a53bc36d97bad260f59662c3b1dc725 Mon Sep 17 00:00:00 2001 From: David Justo Date: Thu, 16 Jan 2025 14:06:51 -0800 Subject: [PATCH 04/25] constexpr if-statement and rename variable template --- stl/inc/vector | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/vector b/stl/inc/vector index 6704a68a23..47bbd08164 100644 --- a/stl/inc/vector +++ b/stl/inc/vector @@ -493,7 +493,7 @@ private: return; } - if (!is_ASan_enabled_for_allocator_v) { + if constexpr (!_Is_ASan_enabled_for_allocator) { return; } From 0cfbb2faf8b770f53fd556aee3f70747303714e1 Mon Sep 17 00:00:00 2001 From: David Justo Date: Thu, 16 Jan 2025 15:32:45 -0800 Subject: [PATCH 05/25] checkpoint progress --- stl/inc/xmemory | 2 +- .../GH_002030_asan_annotate_vector/test.cpp | 45 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index b48d9aa653..6ebd57fb55 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -805,7 +805,7 @@ struct _Simple_types { // wraps types from allocators with simple addressing for _INLINE_VAR constexpr size_t _Asan_granularity = 8; _INLINE_VAR constexpr size_t _Asan_granularity_mask = _Asan_granularity - 1; -// Represents whether ASan container annotations are enabled for allocator type 'T'. +// Represents whether ASan container annotations are enabled a given allocator. template constexpr bool _Is_ASan_enabled_for_allocator = true; diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index 93ac83e275..8c0e5cfbdc 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -274,6 +274,44 @@ struct implicit_allocator : custom_test_allocator { STATIC_ASSERT(_Container_allocation_minimum_asan_alignment>> == 1); STATIC_ASSERT(_Container_allocation_minimum_asan_alignment>> == 2); +template +struct arena_allocator : custom_test_allocator { + arena_allocator() : arenaSize(arenaSize), arena(new char[arenaSize]), offset(0) { + } + + T* allocate(size_t size) { + if (offset + size > arenaSize) { + // ran out of space in the arena + throw std::bad_alloc(); + } + void* ptr = arena + offset; + offset += size; + return ptr; + } + + void deallocate(T* pointer, size_t) noexcept { + // no-op - arena allocators do not deallocate individual items, they deallocate the arena all at once. + } + + void reset() { + // reset offset to zero, and memset memory back to zero for re-use + offset = 0; + memset(arena, 0, arenaSize); + } + + ~arena_allocator() { + delete[] arena; + } + + const size_t arenaSize = 100; // chosen arbitrarily + char* arena; + size_t offset; +}; + +// Arena allocators don't play well with ASan, so we disable them +template +constexpr bool _Is_ASan_enabled_for_allocator = false; + template void test_push_pop() { using T = typename Alloc::value_type; @@ -1002,12 +1040,19 @@ void run_custom_allocator_matrix() { run_tests>(); } +void run_arena_allocator_test() { + // TODO: add test where an allocator's arena is filled in, then reset, + // then continues to be used. ASan should not fire. +} + template void run_allocator_matrix() { run_tests>(); run_custom_allocator_matrix(); run_custom_allocator_matrix(); run_custom_allocator_matrix(); + run_custom_allocator_matrix(); + run_arena_allocator_test(); } int main() { From a040f8c481cb76ef9682833c67580d9f43561208 Mon Sep 17 00:00:00 2001 From: David Justo Date: Fri, 17 Jan 2025 17:46:13 -0800 Subject: [PATCH 06/25] checkpoint progress: allocator is non-conforming still --- .../GH_002030_asan_annotate_vector/test.cpp | 64 +++++++++++-------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index 8c0e5cfbdc..8fec1690b4 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -274,43 +274,45 @@ struct implicit_allocator : custom_test_allocator { STATIC_ASSERT(_Container_allocation_minimum_asan_alignment>> == 1); STATIC_ASSERT(_Container_allocation_minimum_asan_alignment>> == 2); -template -struct arena_allocator : custom_test_allocator { - arena_allocator() : arenaSize(arenaSize), arena(new char[arenaSize]), offset(0) { - } +template +class arena_allocator { + public: + using value_type = T; - T* allocate(size_t size) { - if (offset + size > arenaSize) { - // ran out of space in the arena - throw std::bad_alloc(); - } - void* ptr = arena + offset; - offset += size; - return ptr; + arena_allocator() : size_(10000), offset_(0) { + data_ = new T[size_]; + }; + template + constexpr arena_allocator(const arena_allocator& other) : size_(other.size_), offset_(other.offset_) { + data_ = std::copy(other.data_, other.data_ + other.size_, data_); } - void deallocate(T* pointer, size_t) noexcept { - // no-op - arena allocators do not deallocate individual items, they deallocate the arena all at once. + T* allocate(size_t n) { + assert(offset_ + n <= size_ && "Arena out of memory"); + T* result = data_ + offset_; + offset_ += n; + return result; } - void reset() { - // reset offset to zero, and memset memory back to zero for re-use - offset = 0; - memset(arena, 0, arenaSize); + void deallocate(T*, size_t) noexcept { + // no-op. Memory is deallocated when the arena is reset. } - ~arena_allocator() { - delete[] arena; + void reset() noexcept { + // reset arena, set memory to zero + // the `memset` would normally trigger an ASan AV, + // but we'll have the arena_allocator opt out of ASan analysis + offset_ = 0; + memset(data_, 0, size_); } - const size_t arenaSize = 100; // chosen arbitrarily - char* arena; - size_t offset; + T* data_; + size_t size_; + size_t offset_; }; -// Arena allocators don't play well with ASan, so we disable them -template -constexpr bool _Is_ASan_enabled_for_allocator = false; +template +constexpr bool _Is_ASan_enabled_for_allocator> = false; template void test_push_pop() { @@ -1043,6 +1045,15 @@ void run_custom_allocator_matrix() { void run_arena_allocator_test() { // TODO: add test where an allocator's arena is filled in, then reset, // then continues to be used. ASan should not fire. + arena_allocator allocator; + std::vector> vec(allocator); + //vec.reserve(100); + + // vec.push_back(1); + // vec.push_back(2); + // vec.push_back(3); + + //allocator.reset(); // should not trigger ASan AV } template @@ -1051,7 +1062,6 @@ void run_allocator_matrix() { run_custom_allocator_matrix(); run_custom_allocator_matrix(); run_custom_allocator_matrix(); - run_custom_allocator_matrix(); run_arena_allocator_test(); } From 7bec17aca50a300a67b5ecb7b4a6ca76d6324b03 Mon Sep 17 00:00:00 2001 From: David Justo Date: Wed, 22 Jan 2025 11:38:19 -0800 Subject: [PATCH 07/25] add seemingly compliant arena allocator. some TODOs and FIXMEs remain --- .../GH_002030_asan_annotate_vector/test.cpp | 131 +++++++++++++----- 1 file changed, 96 insertions(+), 35 deletions(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index 8fec1690b4..a48144538a 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -274,45 +274,93 @@ struct implicit_allocator : custom_test_allocator { STATIC_ASSERT(_Container_allocation_minimum_asan_alignment>> == 1); STATIC_ASSERT(_Container_allocation_minimum_asan_alignment>> == 2); -template -class arena_allocator { - public: +// Helper class for `ArenaAllocator`, where data is linearly allocated in a finite buffer. +class Arena { +public: + Arena(size_t allocation_size, size_t capacity): + data(new char[capacity* allocation_size]), + allocation_size(allocation_size), + capacity(capacity), + offset(0) {} + + Arena(const Arena& other): + data(other.data), + allocation_size(other.allocation_size), + capacity(other.capacity), + offset(other.offset) {} + + void* allocate(size_t n) { + assert(offset + n <= capacity && "Allocation failed: the arena is out of memory. You may call `reset` to free up space."); + void* result = data + (offset * allocation_size); + offset += n; + return result; + } + + // Deallocates memory in the arena, and `memset`s it all to zero + void reset() noexcept { + offset = 0; + memset(data, 0, capacity*allocation_size); + } + + char* const data; + const size_t allocation_size; + const size_t capacity; + size_t offset; +}; + +// An allocator that allocates memory in a pre-allocated buffer (the arena). +template +class ArenaAllocator { +public: + using value_type = T; + Arena* arena; + + ArenaAllocator(size_t alloc_size, size_t capacity) { + assert(sizeof(T) <= alloc_size && "Constructor failed: allocation size is too small for target type."); + arena = new Arena(alloc_size, capacity); + } - arena_allocator() : size_(10000), offset_(0) { - data_ = new T[size_]; - }; template - constexpr arena_allocator(const arena_allocator& other) : size_(other.size_), offset_(other.offset_) { - data_ = std::copy(other.data_, other.data_ + other.size_, data_); + ArenaAllocator(const ArenaAllocator& other) { + assert(sizeof(U) <= arena->allocation_size && "Constructor failed: allocation size is too small for target type."); + arena = other.arena; } T* allocate(size_t n) { - assert(offset_ + n <= size_ && "Arena out of memory"); - T* result = data_ + offset_; - offset_ += n; - return result; + void* ptr = arena->allocate(n); + return reinterpret_cast(ptr); } - void deallocate(T*, size_t) noexcept { - // no-op. Memory is deallocated when the arena is reset. - } + // no-op. Memory is deallocated in the `reset` method + void deallocate(value_type*, size_t) noexcept {} + + // Deallocates memory in the arena, and `memset`s it all to zero. + // the `memset` would normally trigger an ASan AV, + // but we'll have the ArenaAllocator opt out of ASan analysis void reset() noexcept { - // reset arena, set memory to zero - // the `memset` would normally trigger an ASan AV, - // but we'll have the arena_allocator opt out of ASan analysis - offset_ = 0; - memset(data_, 0, size_); + arena->reset(); + } + + template + bool operator==(ArenaAllocator const& other) + { + return this->arena == other.arena; } - T* data_; - size_t size_; - size_t offset_; + template + bool operator!=(ArenaAllocator const other) + { + return !(this == other); + } }; +// Opt out of ASan analysis for the ArenaAllocator +// FIXME: IntelliSense claims '_Is_ASan_enabled_for_allocator is not a templateC/C++(864)', +// but it works somehow. template -constexpr bool _Is_ASan_enabled_for_allocator> = false; +constexpr bool _Is_ASan_enabled_for_allocator> = false; template void test_push_pop() { @@ -1042,18 +1090,31 @@ void run_custom_allocator_matrix() { run_tests>(); } -void run_arena_allocator_test() { - // TODO: add test where an allocator's arena is filled in, then reset, - // then continues to be used. ASan should not fire. - arena_allocator allocator; - std::vector> vec(allocator); - //vec.reserve(100); +// Tests that ASan analysis can be disabled for a vector with an arena allocator. +void run_asan_disablement_test() { + + // The arena allocator stores integers in 32-bit alignment. + // It can hold up to 100 such allocations. + // The 32-bit alignment is a bit excessive for `int`s, but it ensures the allocator + // can be rebound to allocate larger types as well (up to 32-bit types). + const int size = 100; + const int alloc_size = 32; + ArenaAllocator allocator(alloc_size, size); + + // We'll give the vector capacity 1, and allocate a single integer (99). + std::vector> vec(allocator); + vec.reserve(1); + vec.push_back(99); + + // When calling reset, the arena would memset all 100 entries of it's buffer to zero. + // If the allocator was naively annotated by ASan, this would trigger an AV, because + // the arena is accessing memory not tracked by the vector's ASan annotations. - // vec.push_back(1); - // vec.push_back(2); - // vec.push_back(3); + // However, the allocator is annotated to opt out of ASan analysis through, + // `_Is_ASan_enabled_for_allocator`, so this should not trigger an AV. + allocator.reset(); - //allocator.reset(); // should not trigger ASan AV + // TODO: is it possible to add a 'negative' test case here? One where ASan expectedly fails? } template @@ -1062,7 +1123,7 @@ void run_allocator_matrix() { run_custom_allocator_matrix(); run_custom_allocator_matrix(); run_custom_allocator_matrix(); - run_arena_allocator_test(); + run_asan_disablement_test(); } int main() { From 76e0723323ca9cbd2a90d5000987deb135e03331 Mon Sep 17 00:00:00 2001 From: David Justo Date: Wed, 22 Jan 2025 11:39:53 -0800 Subject: [PATCH 08/25] remove whitespace --- tests/std/tests/GH_002030_asan_annotate_vector/test.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index a48144538a..11fbb05894 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -332,7 +332,6 @@ class ArenaAllocator { return reinterpret_cast(ptr); } - // no-op. Memory is deallocated in the `reset` method void deallocate(value_type*, size_t) noexcept {} From 516d2c04b0a9e42866a585cd1be749e2438a1cbf Mon Sep 17 00:00:00 2001 From: David Justo Date: Wed, 22 Jan 2025 17:01:58 -0800 Subject: [PATCH 09/25] add basic_string test and implementation --- stl/inc/xstring | 4 + .../GH_002030_asan_annotate_string/test.cpp | 115 ++++++++++++++++++ .../GH_002030_asan_annotate_vector/test.cpp | 2 +- 3 files changed, 120 insertions(+), 1 deletion(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 55b5a67de1..91734bfa15 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -632,6 +632,10 @@ private: return; } + if constexpr (!_Is_ASan_enabled_for_allocator) { + return; + } + // Note that `_Capacity`, `_Old_size`, and `_New_size` do not include the null terminator const void* const _End = _First + _Capacity + 1; const void* const _Old_last = _First + _Old_size + 1; diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp index 5d86988417..60ab976896 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -287,6 +287,94 @@ STATIC_ASSERT(_Container_allocation_minimum_asan_alignment< basic_string, implicit_allocator>> == 2); +// Helper class for `ArenaAllocator`, where data is linearly allocated in a finite buffer. +class Arena { +public: + Arena(size_t allocation_size, size_t capacity): + data(new char[capacity* allocation_size]), + allocation_size(allocation_size), + capacity(capacity), + offset(0) {} + + Arena(const Arena& other): + data(other.data), + allocation_size(other.allocation_size), + capacity(other.capacity), + offset(other.offset) {} + + void* allocate(size_t n) { + assert(offset + n <= capacity && "Allocation failed: the arena is out of memory. You may call `reset` to free up space."); + void* result = data + (offset * allocation_size); + offset += n; + return result; + } + + // Deallocates memory in the arena, and `memset`s it all to zero + void reset() noexcept { + offset = 0; + memset(data, 0, capacity*allocation_size); + } + + char* const data; + const size_t allocation_size; + const size_t capacity; + size_t offset; +}; + +// FIXME: refactor this class into a helper class. +// An allocator that allocates memory in a pre-allocated buffer (the arena). +template +class ArenaAllocator { +public: + + using value_type = T; + Arena* arena; + + ArenaAllocator(size_t alloc_size, size_t capacity) { + assert(sizeof(T) <= alloc_size && "Constructor failed: allocation size is too small for target type."); + arena = new Arena(alloc_size, capacity); + } + + template + ArenaAllocator(const ArenaAllocator& other) { + assert(sizeof(T) <= other.arena->allocation_size && "Constructor failed: allocation size is too small for target type."); + arena = other.arena; + } + + T* allocate(size_t n) { + void* ptr = arena->allocate(n); + return reinterpret_cast(ptr); + } + + // no-op. Memory is deallocated in the `reset` method + void deallocate(value_type*, size_t) noexcept {} + + // Deallocates memory in the arena, and `memset`s it all to zero. + // the `memset` would normally trigger an ASan AV, + // but we'll have the ArenaAllocator opt out of ASan analysis + void reset() noexcept { + arena->reset(); + } + + template + bool operator==(ArenaAllocator const& other) + { + return this->arena == other.arena; + } + + template + bool operator!=(ArenaAllocator const other) + { + return !(this == other); + } +}; + +// Opt out of ASan analysis for the ArenaAllocator +// FIXME: IntelliSense claims '_Is_ASan_enabled_for_allocator is not a templateC/C++(864)', +// but it works somehow. +template +constexpr bool _Is_ASan_enabled_for_allocator> = false; + template void test_construction() { using CharType = typename Alloc::value_type; @@ -1853,6 +1941,32 @@ void run_tests() { #endif // ^^^ no workaround ^^^ } +// Tests that ASan analysis can be disabled for a vector with an arena allocator. +void run_asan_disablement_test() { + + // The arena allocator stores integers in 32-bit alignment. + // It can hold up to 100 such allocations. + // The 32-bit alignment is a bit excessive for `char`s, but it ensures the allocator + // can be rebound to allocate larger types as well (up to 32-bit types). + const int size = 100; + const int alloc_size = 32; + ArenaAllocator allocator(alloc_size, size); + + std::basic_string, ArenaAllocator> myString(allocator); + myString.reserve(50); + myString.push_back('A'); + + // When calling reset, the arena would memset all 100 entries of it's buffer to zero. + // If the allocator was naively annotated by ASan, this would trigger an AV, because + // the arena is accessing memory not tracked by the vector's ASan annotations. + + // However, the allocator is annotated to opt out of ASan analysis through, + // `_Is_ASan_enabled_for_allocator`, so this should not trigger an AV. + allocator.reset(); + + // TODO: is it possible to add a 'negative' test case here? One where ASan expectedly fails? +} + template class Alloc> void run_custom_allocator_matrix() { run_tests>(); @@ -1867,6 +1981,7 @@ void run_allocator_matrix() { run_custom_allocator_matrix(); run_custom_allocator_matrix(); run_custom_allocator_matrix(); + run_asan_disablement_test(); } void test_DevCom_10116361() { diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index 11fbb05894..e2d6f9f6ce 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -323,7 +323,7 @@ class ArenaAllocator { template ArenaAllocator(const ArenaAllocator& other) { - assert(sizeof(U) <= arena->allocation_size && "Constructor failed: allocation size is too small for target type."); + assert(sizeof(T) <= other.arena->allocation_size && "Constructor failed: allocation size is too small for target type."); arena = other.arena; } From 6b0425452438bb911546e8a5d43f79b41bd831c3 Mon Sep 17 00:00:00 2001 From: David Justo Date: Wed, 22 Jan 2025 17:04:13 -0800 Subject: [PATCH 10/25] fix indentation --- tests/std/tests/GH_002030_asan_annotate_string/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp index 60ab976896..eeddaaffc4 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -1952,7 +1952,7 @@ void run_asan_disablement_test() { const int alloc_size = 32; ArenaAllocator allocator(alloc_size, size); - std::basic_string, ArenaAllocator> myString(allocator); + std::basic_string, ArenaAllocator> myString(allocator); myString.reserve(50); myString.push_back('A'); From a5de2914303634eda5f8c7c6a0525a89a1c82467 Mon Sep 17 00:00:00 2001 From: David Justo Date: Wed, 22 Jan 2025 17:20:28 -0800 Subject: [PATCH 11/25] template new test --- .../std/tests/GH_002030_asan_annotate_string/test.cpp | 8 ++++---- .../std/tests/GH_002030_asan_annotate_vector/test.cpp | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp index eeddaaffc4..14e35d4e5a 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -1942,6 +1942,7 @@ void run_tests() { } // Tests that ASan analysis can be disabled for a vector with an arena allocator. +template void run_asan_disablement_test() { // The arena allocator stores integers in 32-bit alignment. @@ -1950,11 +1951,10 @@ void run_asan_disablement_test() { // can be rebound to allocate larger types as well (up to 32-bit types). const int size = 100; const int alloc_size = 32; - ArenaAllocator allocator(alloc_size, size); + ArenaAllocator allocator(alloc_size, size); - std::basic_string, ArenaAllocator> myString(allocator); + std::basic_string, ArenaAllocator> myString(allocator); myString.reserve(50); - myString.push_back('A'); // When calling reset, the arena would memset all 100 entries of it's buffer to zero. // If the allocator was naively annotated by ASan, this would trigger an AV, because @@ -1981,7 +1981,7 @@ void run_allocator_matrix() { run_custom_allocator_matrix(); run_custom_allocator_matrix(); run_custom_allocator_matrix(); - run_asan_disablement_test(); + run_asan_disablement_test(); } void test_DevCom_10116361() { diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index e2d6f9f6ce..44c05414c8 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -1090,6 +1090,7 @@ void run_custom_allocator_matrix() { } // Tests that ASan analysis can be disabled for a vector with an arena allocator. +template void run_asan_disablement_test() { // The arena allocator stores integers in 32-bit alignment. @@ -1098,12 +1099,11 @@ void run_asan_disablement_test() { // can be rebound to allocate larger types as well (up to 32-bit types). const int size = 100; const int alloc_size = 32; - ArenaAllocator allocator(alloc_size, size); + ArenaAllocator allocator(alloc_size, size); - // We'll give the vector capacity 1, and allocate a single integer (99). - std::vector> vec(allocator); + // We'll give the vector capacity 1 + std::vector> vec(allocator); vec.reserve(1); - vec.push_back(99); // When calling reset, the arena would memset all 100 entries of it's buffer to zero. // If the allocator was naively annotated by ASan, this would trigger an AV, because @@ -1122,7 +1122,7 @@ void run_allocator_matrix() { run_custom_allocator_matrix(); run_custom_allocator_matrix(); run_custom_allocator_matrix(); - run_asan_disablement_test(); + run_asan_disablement_test(); } int main() { From 4cc951ba058e9864adb35c775b51f2c4d0ae9abe Mon Sep 17 00:00:00 2001 From: David Justo Date: Thu, 23 Jan 2025 11:06:11 -0800 Subject: [PATCH 12/25] remove use of arena allocator to simplify tests, basic_string test still WIP --- .../GH_002030_asan_annotate_string/test.cpp | 115 +++--------------- .../GH_002030_asan_annotate_vector/test.cpp | 115 +++--------------- 2 files changed, 38 insertions(+), 192 deletions(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp index 14e35d4e5a..3ba1adc5df 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -287,93 +287,25 @@ STATIC_ASSERT(_Container_allocation_minimum_asan_alignment< basic_string, implicit_allocator>> == 2); -// Helper class for `ArenaAllocator`, where data is linearly allocated in a finite buffer. -class Arena { -public: - Arena(size_t allocation_size, size_t capacity): - data(new char[capacity* allocation_size]), - allocation_size(allocation_size), - capacity(capacity), - offset(0) {} - - Arena(const Arena& other): - data(other.data), - allocation_size(other.allocation_size), - capacity(other.capacity), - offset(other.offset) {} - - void* allocate(size_t n) { - assert(offset + n <= capacity && "Allocation failed: the arena is out of memory. You may call `reset` to free up space."); - void* result = data + (offset * allocation_size); - offset += n; - return result; - } - - // Deallocates memory in the arena, and `memset`s it all to zero - void reset() noexcept { - offset = 0; - memset(data, 0, capacity*allocation_size); - } - - char* const data; - const size_t allocation_size; - const size_t capacity; - size_t offset; -}; - -// FIXME: refactor this class into a helper class. -// An allocator that allocates memory in a pre-allocated buffer (the arena). -template -class ArenaAllocator { -public: - - using value_type = T; - Arena* arena; - - ArenaAllocator(size_t alloc_size, size_t capacity) { - assert(sizeof(T) <= alloc_size && "Constructor failed: allocation size is too small for target type."); - arena = new Arena(alloc_size, capacity); - } - +// Simple implicit allocator that opts out of ASan annotations (via `_Is_ASan_enabled_for_allocator`) +template +struct implicit_allocator_no_asan_annotations : implicit_allocator { + implicit_allocator_no_asan_annotations() = default; template - ArenaAllocator(const ArenaAllocator& other) { - assert(sizeof(T) <= other.arena->allocation_size && "Constructor failed: allocation size is too small for target type."); - arena = other.arena; - } + constexpr implicit_allocator_no_asan_annotations(const implicit_allocator_no_asan_annotations&) noexcept {} T* allocate(size_t n) { - void* ptr = arena->allocate(n); - return reinterpret_cast(ptr); - } - - // no-op. Memory is deallocated in the `reset` method - void deallocate(value_type*, size_t) noexcept {} - - // Deallocates memory in the arena, and `memset`s it all to zero. - // the `memset` would normally trigger an ASan AV, - // but we'll have the ArenaAllocator opt out of ASan analysis - void reset() noexcept { - arena->reset(); - } - - template - bool operator==(ArenaAllocator const& other) - { - return this->arena == other.arena; + T* mem = new T[n + 1]; + return mem + 1; } - template - bool operator!=(ArenaAllocator const other) - { - return !(this == other); + void deallocate(T* p, size_t) noexcept { + delete[] (p - 1); } }; -// Opt out of ASan analysis for the ArenaAllocator -// FIXME: IntelliSense claims '_Is_ASan_enabled_for_allocator is not a templateC/C++(864)', -// but it works somehow. template -constexpr bool _Is_ASan_enabled_for_allocator> = false; +constexpr bool _Is_ASan_enabled_for_allocator> = false; template void test_construction() { @@ -1942,27 +1874,18 @@ void run_tests() { } // Tests that ASan analysis can be disabled for a vector with an arena allocator. -template +template void run_asan_disablement_test() { - // The arena allocator stores integers in 32-bit alignment. - // It can hold up to 100 such allocations. - // The 32-bit alignment is a bit excessive for `char`s, but it ensures the allocator - // can be rebound to allocate larger types as well (up to 32-bit types). - const int size = 100; - const int alloc_size = 32; - ArenaAllocator allocator(alloc_size, size); - - std::basic_string, ArenaAllocator> myString(allocator); - myString.reserve(50); - - // When calling reset, the arena would memset all 100 entries of it's buffer to zero. - // If the allocator was naively annotated by ASan, this would trigger an AV, because - // the arena is accessing memory not tracked by the vector's ASan annotations. + // We'll give the vector capacity 100 + std::basic_string, implicit_allocator_no_asan_annotations> myString; + myString.reserve(100); - // However, the allocator is annotated to opt out of ASan analysis through, - // `_Is_ASan_enabled_for_allocator`, so this should not trigger an AV. - allocator.reset(); + // We access position (50) of the string, which is within the capacity of the vector + // but uninitialized. This would normally trigger an AV because of ASan annotations. + // However, the allocator has opted out of ASan analysis through, + // `_Is_ASan_enabled_for_allocator`, so this should succeed. + // myString.data()[50] = CharType{'A'}; // TODO< this does not compile. Need to investigate // TODO: is it possible to add a 'negative' test case here? One where ASan expectedly fails? } diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index 44c05414c8..7fd2279c58 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -274,92 +274,25 @@ struct implicit_allocator : custom_test_allocator { STATIC_ASSERT(_Container_allocation_minimum_asan_alignment>> == 1); STATIC_ASSERT(_Container_allocation_minimum_asan_alignment>> == 2); -// Helper class for `ArenaAllocator`, where data is linearly allocated in a finite buffer. -class Arena { -public: - Arena(size_t allocation_size, size_t capacity): - data(new char[capacity* allocation_size]), - allocation_size(allocation_size), - capacity(capacity), - offset(0) {} - - Arena(const Arena& other): - data(other.data), - allocation_size(other.allocation_size), - capacity(other.capacity), - offset(other.offset) {} - - void* allocate(size_t n) { - assert(offset + n <= capacity && "Allocation failed: the arena is out of memory. You may call `reset` to free up space."); - void* result = data + (offset * allocation_size); - offset += n; - return result; - } - - // Deallocates memory in the arena, and `memset`s it all to zero - void reset() noexcept { - offset = 0; - memset(data, 0, capacity*allocation_size); - } - - char* const data; - const size_t allocation_size; - const size_t capacity; - size_t offset; -}; - -// An allocator that allocates memory in a pre-allocated buffer (the arena). -template -class ArenaAllocator { -public: - - using value_type = T; - Arena* arena; - - ArenaAllocator(size_t alloc_size, size_t capacity) { - assert(sizeof(T) <= alloc_size && "Constructor failed: allocation size is too small for target type."); - arena = new Arena(alloc_size, capacity); - } - +// Simple implicit allocator that opts out of ASan annotations (via `_Is_ASan_enabled_for_allocator`) +template +struct implicit_allocator_no_asan_annotations : implicit_allocator { + implicit_allocator_no_asan_annotations() = default; template - ArenaAllocator(const ArenaAllocator& other) { - assert(sizeof(T) <= other.arena->allocation_size && "Constructor failed: allocation size is too small for target type."); - arena = other.arena; - } + constexpr implicit_allocator_no_asan_annotations(const implicit_allocator_no_asan_annotations&) noexcept {} T* allocate(size_t n) { - void* ptr = arena->allocate(n); - return reinterpret_cast(ptr); - } - - // no-op. Memory is deallocated in the `reset` method - void deallocate(value_type*, size_t) noexcept {} - - // Deallocates memory in the arena, and `memset`s it all to zero. - // the `memset` would normally trigger an ASan AV, - // but we'll have the ArenaAllocator opt out of ASan analysis - void reset() noexcept { - arena->reset(); - } - - template - bool operator==(ArenaAllocator const& other) - { - return this->arena == other.arena; + T* mem = new T[n + 1]; + return mem + 1; } - template - bool operator!=(ArenaAllocator const other) - { - return !(this == other); + void deallocate(T* p, size_t) noexcept { + delete[] (p - 1); } }; -// Opt out of ASan analysis for the ArenaAllocator -// FIXME: IntelliSense claims '_Is_ASan_enabled_for_allocator is not a templateC/C++(864)', -// but it works somehow. template -constexpr bool _Is_ASan_enabled_for_allocator> = false; +constexpr bool _Is_ASan_enabled_for_allocator> = false; template void test_push_pop() { @@ -1093,25 +1026,15 @@ void run_custom_allocator_matrix() { template void run_asan_disablement_test() { - // The arena allocator stores integers in 32-bit alignment. - // It can hold up to 100 such allocations. - // The 32-bit alignment is a bit excessive for `int`s, but it ensures the allocator - // can be rebound to allocate larger types as well (up to 32-bit types). - const int size = 100; - const int alloc_size = 32; - ArenaAllocator allocator(alloc_size, size); - - // We'll give the vector capacity 1 - std::vector> vec(allocator); - vec.reserve(1); - - // When calling reset, the arena would memset all 100 entries of it's buffer to zero. - // If the allocator was naively annotated by ASan, this would trigger an AV, because - // the arena is accessing memory not tracked by the vector's ASan annotations. - - // However, the allocator is annotated to opt out of ASan analysis through, - // `_Is_ASan_enabled_for_allocator`, so this should not trigger an AV. - allocator.reset(); + // We'll give the vector capacity 100 + std::vector> vec; + vec.reserve(100); + + // We access position (50) of the vector, which is within the capacity of the vector + // but uninitialized. This would normally trigger an AV because of ASan annotations. + // However, the allocator has opted out of ASan analysis through, + // `_Is_ASan_enabled_for_allocator`, so this should succeed. + vec.data()[50] = T(); // TODO: is it possible to add a 'negative' test case here? One where ASan expectedly fails? } From 9823cb464b95b119c264cbd6ba3777bc63b06b50 Mon Sep 17 00:00:00 2001 From: David Justo Date: Fri, 24 Jan 2025 09:40:56 -0800 Subject: [PATCH 13/25] edit string test --- .../std/tests/GH_002030_asan_annotate_string/test.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp index 3ba1adc5df..5fea57885e 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -305,7 +305,7 @@ struct implicit_allocator_no_asan_annotations : implicit_allocator -constexpr bool _Is_ASan_enabled_for_allocator> = false; +constexpr bool _Is_ASan_enabled_for_allocator> = true; template void test_construction() { @@ -1877,15 +1877,12 @@ void run_tests() { template void run_asan_disablement_test() { - // We'll give the vector capacity 100 + // We'll give the string capacity 100 std::basic_string, implicit_allocator_no_asan_annotations> myString; myString.reserve(100); - // We access position (50) of the string, which is within the capacity of the vector - // but uninitialized. This would normally trigger an AV because of ASan annotations. - // However, the allocator has opted out of ASan analysis through, - // `_Is_ASan_enabled_for_allocator`, so this should succeed. - // myString.data()[50] = CharType{'A'}; // TODO< this does not compile. Need to investigate + CharType* data = &myString[0]; // Get a mutable pointer to the string's data + data[50] = CharType{'A'}; // TODO: this isn't failing in ASAn due to a newly found bug. Merge that bug fix first. // TODO: is it possible to add a 'negative' test case here? One where ASan expectedly fails? } From 618804105c07c66b4514213860aaedf0ea356c07 Mon Sep 17 00:00:00 2001 From: David Justo Date: Fri, 24 Jan 2025 09:59:56 -0800 Subject: [PATCH 14/25] reference new GH bug in PR --- tests/std/tests/GH_002030_asan_annotate_string/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp index 5fea57885e..6644025eb3 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -1882,7 +1882,7 @@ void run_asan_disablement_test() { myString.reserve(100); CharType* data = &myString[0]; // Get a mutable pointer to the string's data - data[50] = CharType{'A'}; // TODO: this isn't failing in ASAn due to a newly found bug. Merge that bug fix first. + data[50] = CharType{'A'}; // TODO: this isn't failing due to bug: https://github.com/microsoft/STL/issues/5251 // TODO: is it possible to add a 'negative' test case here? One where ASan expectedly fails? } From 57e7affd7f3f5d6f32c255c0a7b822c7006f7551 Mon Sep 17 00:00:00 2001 From: David Justo Date: Mon, 27 Jan 2025 18:04:31 -0800 Subject: [PATCH 15/25] rename template variable as per feedback --- stl/inc/vector | 2 +- stl/inc/xmemory | 2 +- stl/inc/xstring | 2 +- tests/std/tests/GH_002030_asan_annotate_string/test.cpp | 4 ++-- tests/std/tests/GH_002030_asan_annotate_vector/test.cpp | 6 +++--- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/stl/inc/vector b/stl/inc/vector index 47bbd08164..0358b1f8da 100644 --- a/stl/inc/vector +++ b/stl/inc/vector @@ -493,7 +493,7 @@ private: return; } - if constexpr (!_Is_ASan_enabled_for_allocator) { + if constexpr (_Disable_ASan_container_annotations_for_allocator) { return; } diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 10b30349d2..7bbf58e353 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -807,7 +807,7 @@ _INLINE_VAR constexpr size_t _Asan_granularity_mask = _Asan_granularity - 1; // Represents whether ASan container annotations are enabled a given allocator. template -constexpr bool _Is_ASan_enabled_for_allocator = true; +constexpr bool _Disable_ASan_container_annotations_for_allocator = false; struct _Asan_aligned_pointers { const void* _First; diff --git a/stl/inc/xstring b/stl/inc/xstring index afcfeec442..b12b13428c 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -632,7 +632,7 @@ private: return; } - if constexpr (!_Is_ASan_enabled_for_allocator) { + if constexpr (_Disable_ASan_container_annotations_for_allocator) { return; } diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp index 6644025eb3..4c139a8b69 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -287,7 +287,7 @@ STATIC_ASSERT(_Container_allocation_minimum_asan_alignment< basic_string, implicit_allocator>> == 2); -// Simple implicit allocator that opts out of ASan annotations (via `_Is_ASan_enabled_for_allocator`) +// Simple implicit allocator that opts out of ASan annotations (via `_Disable_ASan_container_annotations_for_allocator`) template struct implicit_allocator_no_asan_annotations : implicit_allocator { implicit_allocator_no_asan_annotations() = default; @@ -305,7 +305,7 @@ struct implicit_allocator_no_asan_annotations : implicit_allocator -constexpr bool _Is_ASan_enabled_for_allocator> = true; +constexpr bool _Disable_ASan_container_annotations_for_allocator> = true; template void test_construction() { diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index 7fd2279c58..73785a873e 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -274,7 +274,7 @@ struct implicit_allocator : custom_test_allocator { STATIC_ASSERT(_Container_allocation_minimum_asan_alignment>> == 1); STATIC_ASSERT(_Container_allocation_minimum_asan_alignment>> == 2); -// Simple implicit allocator that opts out of ASan annotations (via `_Is_ASan_enabled_for_allocator`) +// Simple implicit allocator that opts out of ASan annotations (via `_Disable_ASan_container_annotations_for_allocator`) template struct implicit_allocator_no_asan_annotations : implicit_allocator { implicit_allocator_no_asan_annotations() = default; @@ -292,7 +292,7 @@ struct implicit_allocator_no_asan_annotations : implicit_allocator -constexpr bool _Is_ASan_enabled_for_allocator> = false; +constexpr bool _Disable_ASan_container_annotations_for_allocator> = true; template void test_push_pop() { @@ -1033,7 +1033,7 @@ void run_asan_disablement_test() { // We access position (50) of the vector, which is within the capacity of the vector // but uninitialized. This would normally trigger an AV because of ASan annotations. // However, the allocator has opted out of ASan analysis through, - // `_Is_ASan_enabled_for_allocator`, so this should succeed. + // `_Disable_ASan_container_annotations_for_allocator`, so this should succeed. vec.data()[50] = T(); // TODO: is it possible to add a 'negative' test case here? One where ASan expectedly fails? From 6d0b7b8a55603a6965b18417785d5053369b63e6 Mon Sep 17 00:00:00 2001 From: David Justo Date: Wed, 29 Jan 2025 17:02:24 -0800 Subject: [PATCH 16/25] fixup tests --- stl/inc/xmemory | 2 +- .../GH_002030_asan_annotate_string/test.cpp | 51 +++++++----- .../GH_002030_asan_annotate_vector/test.cpp | 77 ++++++++++++------- 3 files changed, 80 insertions(+), 50 deletions(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 7bbf58e353..31d0bed8c9 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -805,7 +805,7 @@ struct _Simple_types { // wraps types from allocators with simple addressing for _INLINE_VAR constexpr size_t _Asan_granularity = 8; _INLINE_VAR constexpr size_t _Asan_granularity_mask = _Asan_granularity - 1; -// Represents whether ASan container annotations are enabled a given allocator. +// Controls whether ASan `container-overflow` errors are reported for this allocator. template constexpr bool _Disable_ASan_container_annotations_for_allocator = false; diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp index 36aae83195..05eff15efa 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -289,12 +289,14 @@ STATIC_ASSERT(_Container_allocation_minimum_asan_alignment< basic_string, implicit_allocator>> == 2); -// Simple implicit allocator that opts out of ASan annotations (via `_Disable_ASan_container_annotations_for_allocator`) +// Simple implicit allocator that opts out of ASan containers annotations (via +// `_Disable_ASan_container_annotations_for_allocator`) template struct implicit_allocator_no_asan_annotations : implicit_allocator { implicit_allocator_no_asan_annotations() = default; template - constexpr implicit_allocator_no_asan_annotations(const implicit_allocator_no_asan_annotations&) noexcept {} + constexpr implicit_allocator_no_asan_annotations( + const implicit_allocator_no_asan_annotations&) noexcept {} T* allocate(size_t n) { T* mem = new T[n + 1]; @@ -1875,18 +1877,26 @@ void run_tests() { #endif // ^^^ no workaround ^^^ } -// Tests that ASan analysis can be disabled for a vector with an arena allocator. -template -void run_asan_disablement_test() { +// Test that writing to un-initialized memory in a string triggers ASan container-overflow checks. +template > +void run_asan_container_overflow_death_test() { - // We'll give the string capacity 100 - std::basic_string, implicit_allocator_no_asan_annotations> myString; + // We'll give the string capacity 100 (all uninitialized memory). + std::basic_string, Alloc> myString; myString.reserve(100); - CharType* data = &myString[0]; // Get a mutable pointer to the string's data - data[50] = CharType{'A'}; // TODO: this isn't failing due to bug: https://github.com/microsoft/STL/issues/5251 + // Write to 50th element to trigger ASan container-overflow check. + CharType* myData = &myString[0]; + myData[50] = CharType{'A'}; +} - // TODO: is it possible to add a 'negative' test case here? One where ASan expectedly fails? +// Test that ASan `container-overflow` checks can be disabled for a custom allocator. +template +void run_asan_annotations_disablement_test() { + + // Test that ASan annotations are disabled for the `implicit_allocator_no_asan_annotations` allocator, + // which should make the container-overflow 'death test' pass. + run_asan_container_overflow_death_test>(); } template class Alloc> @@ -1903,7 +1913,11 @@ void run_allocator_matrix() { run_custom_allocator_matrix(); run_custom_allocator_matrix(); run_custom_allocator_matrix(); - run_asan_disablement_test(); + + // To test ASan annotation disablement, we use an ad-hoc allocator type to avoid disrupting other + // tests that depend on annotations being enabled. Therefore, unlike the prior tests, + // this test is not parametrized by the allocator type. + run_asan_annotations_disablement_test(); } void test_DevCom_10116361() { @@ -1954,15 +1968,6 @@ void test_gh_3955() { assert(s == t); } -void test_gh_5251() { - // GH-5251 : ASan annotations do not prevent writing to allocated - // but uninitialized basic_string memory - string myString; - myString.reserve(100); - char* myData = &myString[0]; - myData[50] = 'A'; // ASan should fire! -} - int main(int argc, char* argv[]) { std_testing::death_test_executive exec([] { run_allocator_matrix(); @@ -1979,7 +1984,11 @@ int main(int argc, char* argv[]) { test_gh_3955(); }); #ifdef __SANITIZE_ADDRESS__ - exec.add_death_tests({test_gh_5251}); +#ifdef __cpp_char8_t + exec.add_death_tests({run_asan_container_overflow_death_test}); +#endif // __cpp_char8_t + exec.add_death_tests({run_asan_container_overflow_death_test, + run_asan_container_overflow_death_test, run_asan_container_overflow_death_test}); #endif // ASan instrumentation enabled return exec.run(argc, argv); } diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index 73785a873e..bc94305aa4 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -12,6 +12,8 @@ #include #include +#include + #pragma warning(disable : 4984) // 'if constexpr' is a C++17 language extension #ifdef __clang__ @@ -53,6 +55,7 @@ struct non_trivial_can_throw { } non_trivial_can_throw& operator=(const non_trivial_can_throw&) { + ++i; return *this; } @@ -279,7 +282,8 @@ template struct implicit_allocator_no_asan_annotations : implicit_allocator { implicit_allocator_no_asan_annotations() = default; template - constexpr implicit_allocator_no_asan_annotations(const implicit_allocator_no_asan_annotations&) noexcept {} + constexpr implicit_allocator_no_asan_annotations( + const implicit_allocator_no_asan_annotations&) noexcept {} T* allocate(size_t n) { T* mem = new T[n + 1]; @@ -1022,21 +1026,24 @@ void run_custom_allocator_matrix() { run_tests>(); } -// Tests that ASan analysis can be disabled for a vector with an arena allocator. -template -void run_asan_disablement_test() { +// Test that writing to un-initialized memory in a string triggers ASan container-overflow checks. +template > +void run_asan_container_overflow_death_test() { + // We'll give the vector capacity 100 (all uninitialized memory). + std::vector vector; + vector.reserve(100); - // We'll give the vector capacity 100 - std::vector> vec; - vec.reserve(100); + // Write to 50th element to trigger ASan container-overflow check. + vector.data()[50] = T(); +} - // We access position (50) of the vector, which is within the capacity of the vector - // but uninitialized. This would normally trigger an AV because of ASan annotations. - // However, the allocator has opted out of ASan analysis through, - // `_Disable_ASan_container_annotations_for_allocator`, so this should succeed. - vec.data()[50] = T(); +// Test that ASan `container-overflow` checks can be disabled for a custom allocator. +template +void run_asan_annotations_disablement_test() { - // TODO: is it possible to add a 'negative' test case here? One where ASan expectedly fails? + // Test that ASan annotations are disabled for the `implicit_allocator_no_asan_annotations` allocator, + // which should make the container-overflow 'death test' pass. + run_asan_container_overflow_death_test>(); } template @@ -1045,26 +1052,40 @@ void run_allocator_matrix() { run_custom_allocator_matrix(); run_custom_allocator_matrix(); run_custom_allocator_matrix(); - run_asan_disablement_test(); + + // To test ASan annotation disablement, we use an ad-hoc allocator type to avoid disrupting other + // tests that depend on annotations being enabled. Therefore, unlike the prior tests, + // this test is not parametrized by the allocator type. + run_asan_annotations_disablement_test(); } -int main() { - // Do some work even when we aren't instrumented - run_allocator_matrix(); +int main(int argc, char* argv[]) { + std_testing::death_test_executive exec([] { + // Do some work even when we aren't instrumented + run_allocator_matrix(); #ifdef __SANITIZE_ADDRESS__ - run_allocator_matrix(); - run_allocator_matrix(); - run_allocator_matrix(); - run_allocator_matrix(); + run_allocator_matrix(); + run_allocator_matrix(); + run_allocator_matrix(); + run_allocator_matrix(); #ifndef __clang__ // TRANSITION, LLVM-35365 - test_push_back_throw(); - test_emplace_back_throw(); - test_insert_range_throw(); - test_insert_throw(); - test_emplace_throw(); - test_resize_throw(); - test_insert_n_throw(); + test_push_back_throw(); + test_emplace_back_throw(); + test_insert_range_throw(); + test_insert_throw(); + test_emplace_throw(); + test_resize_throw(); + test_insert_n_throw(); #endif // ^^^ no workaround ^^^ #endif // ASan instrumentation enabled + }); + +#ifdef __SANITIZE_ADDRESS__ + exec.add_death_tests({run_asan_container_overflow_death_test, run_asan_container_overflow_death_test, + run_asan_container_overflow_death_test, + run_asan_container_overflow_death_test}); +#endif // ASan instrumentation enabled + + return exec.run(argc, argv); } From cbb714d9eee52ae8d20f816ba7e0857853c20073 Mon Sep 17 00:00:00 2001 From: David Justo Date: Wed, 29 Jan 2025 17:08:24 -0800 Subject: [PATCH 17/25] clean up comments --- .../std/tests/GH_002030_asan_annotate_string/test.cpp | 11 +++++------ .../std/tests/GH_002030_asan_annotate_vector/test.cpp | 10 +++++----- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp index 05eff15efa..a6963dabea 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -289,8 +289,7 @@ STATIC_ASSERT(_Container_allocation_minimum_asan_alignment< basic_string, implicit_allocator>> == 2); -// Simple implicit allocator that opts out of ASan containers annotations (via -// `_Disable_ASan_container_annotations_for_allocator`) +// Simple allocator that opts out of ASan annotations (via `_Disable_ASan_container_annotations_for_allocator`) template struct implicit_allocator_no_asan_annotations : implicit_allocator { implicit_allocator_no_asan_annotations() = default; @@ -1877,15 +1876,15 @@ void run_tests() { #endif // ^^^ no workaround ^^^ } -// Test that writing to un-initialized memory in a string triggers ASan container-overflow checks. +// Test that writing to un-initialized memory in a string triggers ASan container-overflow error. template > void run_asan_container_overflow_death_test() { - // We'll give the string capacity 100 (all uninitialized memory). + // We'll give the string capacity 100 (all un-initialized memory). std::basic_string, Alloc> myString; myString.reserve(100); - // Write to 50th element to trigger ASan container-overflow check. + // Write to the 50th element to trigger ASan container-overflow check. CharType* myData = &myString[0]; myData[50] = CharType{'A'}; } @@ -1894,7 +1893,7 @@ void run_asan_container_overflow_death_test() { template void run_asan_annotations_disablement_test() { - // Test that ASan annotations are disabled for the `implicit_allocator_no_asan_annotations` allocator, + // ASan annotations are disabled for the `implicit_allocator_no_asan_annotations` allocator, // which should make the container-overflow 'death test' pass. run_asan_container_overflow_death_test>(); } diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index bc94305aa4..710c6dcb47 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -277,7 +277,7 @@ struct implicit_allocator : custom_test_allocator { STATIC_ASSERT(_Container_allocation_minimum_asan_alignment>> == 1); STATIC_ASSERT(_Container_allocation_minimum_asan_alignment>> == 2); -// Simple implicit allocator that opts out of ASan annotations (via `_Disable_ASan_container_annotations_for_allocator`) +// Simple allocator that opts out of ASan annotations (via `_Disable_ASan_container_annotations_for_allocator`) template struct implicit_allocator_no_asan_annotations : implicit_allocator { implicit_allocator_no_asan_annotations() = default; @@ -1026,14 +1026,14 @@ void run_custom_allocator_matrix() { run_tests>(); } -// Test that writing to un-initialized memory in a string triggers ASan container-overflow checks. +// Test that writing to un-initialized memory in a string triggers ASan container-overflow error. template > void run_asan_container_overflow_death_test() { - // We'll give the vector capacity 100 (all uninitialized memory). + // We'll give the vector capacity 100 (all un0initialized memory). std::vector vector; vector.reserve(100); - // Write to 50th element to trigger ASan container-overflow check. + // Write to the 50th element to trigger ASan container-overflow check. vector.data()[50] = T(); } @@ -1041,7 +1041,7 @@ void run_asan_container_overflow_death_test() { template void run_asan_annotations_disablement_test() { - // Test that ASan annotations are disabled for the `implicit_allocator_no_asan_annotations` allocator, + // ASan annotations are disabled for the `implicit_allocator_no_asan_annotations` allocator, // which should make the container-overflow 'death test' pass. run_asan_container_overflow_death_test>(); } From 59fa3c9ee6d5c236b0f1a9dff014e8f753d6da36 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 30 Jan 2025 12:46:17 -0800 Subject: [PATCH 18/25] Follow the "can_throw" pattern in copy assignment. --- tests/std/tests/GH_002030_asan_annotate_vector/test.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index 710c6dcb47..4e92800a11 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -56,6 +56,9 @@ struct non_trivial_can_throw { non_trivial_can_throw& operator=(const non_trivial_can_throw&) { ++i; + if (i == 0) { + throw i; + } return *this; } From b6c5dcaeb927f87688eed9153ad6343155b756d8 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 30 Jan 2025 13:09:12 -0800 Subject: [PATCH 19/25] Guard vector/string logic with `if constexpr (!disable)` to avoid dead code. --- stl/inc/vector | 78 +++++++++++++++++++++---------------------- stl/inc/xstring | 89 ++++++++++++++++++++++++------------------------- 2 files changed, 82 insertions(+), 85 deletions(-) diff --git a/stl/inc/vector b/stl/inc/vector index 0358b1f8da..27f3ad281d 100644 --- a/stl/inc/vector +++ b/stl/inc/vector @@ -483,52 +483,50 @@ private: _STL_INTERNAL_CHECK(_Old_last_ != nullptr); _STL_INTERNAL_CHECK(_New_last_ != nullptr); + if constexpr (!_Disable_ASan_container_annotations_for_allocator) { #if _HAS_CXX20 - if (_STD is_constant_evaluated()) { - return; - } + if (_STD is_constant_evaluated()) { + return; + } #endif // _HAS_CXX20 - if (!_Asan_vector_should_annotate) { - return; - } - - if constexpr (_Disable_ASan_container_annotations_for_allocator) { - return; - } - - const void* const _First = _STD _Unfancy(_First_); - const void* const _End = _STD _Unfancy(_End_); - const void* const _Old_last = _STD _Unfancy(_Old_last_); - const void* const _New_last = _STD _Unfancy(_New_last_); - if constexpr ((_Container_allocation_minimum_asan_alignment) >= _Asan_granularity) { - // old state: - // [_First, _Old_last) valid - // [_Old_last, _End) poison - // new state: - // [_First, _New_last) valid - // [_New_last, asan_aligned_after(_End)) poison - _CSTD __sanitizer_annotate_contiguous_container( - _First, _STD _Get_asan_aligned_after(_End), _Old_last, _New_last); - } else { - const auto _Aligned = _STD _Get_asan_aligned_first_end(_First, _End); - if (_Aligned._First == _Aligned._End) { - // The buffer does not end at least one shadow memory section; nothing to do. + if (!_Asan_vector_should_annotate) { return; } - const void* const _Old_fixed = _Aligned._Clamp_to_end(_Old_last); - const void* const _New_fixed = _Aligned._Clamp_to_end(_New_last); - - // old state: - // [_Aligned._First, _Old_fixed) valid - // [_Old_fixed, _Aligned._End) poison - // [_Aligned._End, _End) valid - // new state: - // [_Aligned._First, _New_fixed) valid - // [_New_fixed, _Aligned._End) poison - // [_Aligned._End, _End) valid - _CSTD __sanitizer_annotate_contiguous_container(_Aligned._First, _Aligned._End, _Old_fixed, _New_fixed); + const void* const _First = _STD _Unfancy(_First_); + const void* const _End = _STD _Unfancy(_End_); + const void* const _Old_last = _STD _Unfancy(_Old_last_); + const void* const _New_last = _STD _Unfancy(_New_last_); + if constexpr ((_Container_allocation_minimum_asan_alignment) >= _Asan_granularity) { + // old state: + // [_First, _Old_last) valid + // [_Old_last, _End) poison + // new state: + // [_First, _New_last) valid + // [_New_last, asan_aligned_after(_End)) poison + _CSTD __sanitizer_annotate_contiguous_container( + _First, _STD _Get_asan_aligned_after(_End), _Old_last, _New_last); + } else { + const auto _Aligned = _STD _Get_asan_aligned_first_end(_First, _End); + if (_Aligned._First == _Aligned._End) { + // The buffer does not end at least one shadow memory section; nothing to do. + return; + } + + const void* const _Old_fixed = _Aligned._Clamp_to_end(_Old_last); + const void* const _New_fixed = _Aligned._Clamp_to_end(_New_last); + + // old state: + // [_Aligned._First, _Old_fixed) valid + // [_Old_fixed, _Aligned._End) poison + // [_Aligned._End, _End) valid + // new state: + // [_Aligned._First, _New_fixed) valid + // [_New_fixed, _Aligned._End) poison + // [_Aligned._End, _End) valid + _CSTD __sanitizer_annotate_contiguous_container(_Aligned._First, _Aligned._End, _Old_fixed, _New_fixed); + } } } diff --git a/stl/inc/xstring b/stl/inc/xstring index 738dbee4c2..8bf2a0a5c0 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -622,58 +622,57 @@ private: static _CONSTEXPR20 void _Apply_annotation(const value_type* const _First, const size_type _Capacity, const size_type _Old_size, const size_type _New_size) noexcept { + if constexpr (!_Disable_ASan_container_annotations_for_allocator) { #if _HAS_CXX20 - if (_STD is_constant_evaluated()) { - return; - } + if (_STD is_constant_evaluated()) { + return; + } #endif // _HAS_CXX20 - // Don't annotate small strings; only annotate on the heap. - if (_Capacity <= _Small_string_capacity || !_Asan_string_should_annotate) { - return; - } - if constexpr (_Disable_ASan_container_annotations_for_allocator) { - return; - } + // Don't annotate small strings; only annotate on the heap. + if (_Capacity <= _Small_string_capacity || !_Asan_string_should_annotate) { + return; + } - // Note that `_Capacity`, `_Old_size`, and `_New_size` do not include the null terminator - const void* const _End = _First + _Capacity + 1; - const void* const _Old_last = _First + _Old_size + 1; - const void* const _New_last = _First + _New_size + 1; + // Note that `_Capacity`, `_Old_size`, and `_New_size` do not include the null terminator + const void* const _End = _First + _Capacity + 1; + const void* const _Old_last = _First + _Old_size + 1; + const void* const _New_last = _First + _New_size + 1; - constexpr bool _Large_string_always_asan_aligned = - (_Container_allocation_minimum_asan_alignment) >= _Asan_granularity; + constexpr bool _Large_string_always_asan_aligned = + (_Container_allocation_minimum_asan_alignment) >= _Asan_granularity; - // for the non-aligned buffer options, the buffer must always have size >= 9 bytes, - // so it will always end at least one shadow memory section. + // for the non-aligned buffer options, the buffer must always have size >= 9 bytes, + // so it will always end at least one shadow memory section. - _Asan_aligned_pointers _Aligned; - if constexpr (_Large_string_always_asan_aligned) { - _Aligned = {_First, _STD _Get_asan_aligned_after(_End)}; - } else { - _Aligned = _STD _Get_asan_aligned_first_end(_First, _End); - } - const void* const _Old_fixed = _Aligned._Clamp_to_end(_Old_last); - const void* const _New_fixed = _Aligned._Clamp_to_end(_New_last); - - // --- always aligned case --- - // old state: - // [_First, _Old_last) valid - // [_Old_last, asan_aligned_after(_End)) poison - // new state: - // [_First, _New_last) valid - // [_New_last, asan_aligned_after(_End)) poison - - // --- sometimes non-aligned case --- - // old state: - // [_Aligned._First, _Old_fixed) valid - // [_Old_fixed, _Aligned._End) poison - // [_Aligned._End, _End) valid - // new state: - // [_Aligned._First, _New_fixed) valid - // [_New_fixed, _Aligned._End) poison - // [_Aligned._End, _End) valid - _CSTD __sanitizer_annotate_contiguous_container(_Aligned._First, _Aligned._End, _Old_fixed, _New_fixed); + _Asan_aligned_pointers _Aligned; + if constexpr (_Large_string_always_asan_aligned) { + _Aligned = {_First, _STD _Get_asan_aligned_after(_End)}; + } else { + _Aligned = _STD _Get_asan_aligned_first_end(_First, _End); + } + const void* const _Old_fixed = _Aligned._Clamp_to_end(_Old_last); + const void* const _New_fixed = _Aligned._Clamp_to_end(_New_last); + + // --- always aligned case --- + // old state: + // [_First, _Old_last) valid + // [_Old_last, asan_aligned_after(_End)) poison + // new state: + // [_First, _New_last) valid + // [_New_last, asan_aligned_after(_End)) poison + + // --- sometimes non-aligned case --- + // old state: + // [_Aligned._First, _Old_fixed) valid + // [_Old_fixed, _Aligned._End) poison + // [_Aligned._End, _End) valid + // new state: + // [_Aligned._First, _New_fixed) valid + // [_New_fixed, _Aligned._End) poison + // [_Aligned._End, _End) valid + _CSTD __sanitizer_annotate_contiguous_container(_Aligned._First, _Aligned._End, _Old_fixed, _New_fixed); + } } #define _ASAN_STRING_REMOVE(_Str) (_Str)._Remove_annotation() From 9f68b179e8f5c73907a8afc4de20c78a45e4143f Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 30 Jan 2025 13:42:20 -0800 Subject: [PATCH 20/25] Fix comments. --- tests/std/tests/GH_002030_asan_annotate_string/test.cpp | 8 ++++---- tests/std/tests/GH_002030_asan_annotate_vector/test.cpp | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp index a6963dabea..c28df9a2cd 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -1876,15 +1876,15 @@ void run_tests() { #endif // ^^^ no workaround ^^^ } -// Test that writing to un-initialized memory in a string triggers ASan container-overflow error. +// Test that writing to uninitialized memory in a string triggers an ASan container-overflow error. (See GH-5251.) template > void run_asan_container_overflow_death_test() { - // We'll give the string capacity 100 (all un-initialized memory). + // We'll give the string capacity 100 (all uninitialized memory, except for the null terminator). std::basic_string, Alloc> myString; myString.reserve(100); - // Write to the 50th element to trigger ASan container-overflow check. + // Write to the element at index 50 to trigger an ASan container-overflow check. CharType* myData = &myString[0]; myData[50] = CharType{'A'}; } @@ -1915,7 +1915,7 @@ void run_allocator_matrix() { // To test ASan annotation disablement, we use an ad-hoc allocator type to avoid disrupting other // tests that depend on annotations being enabled. Therefore, unlike the prior tests, - // this test is not parametrized by the allocator type. + // this test is not parameterized by the allocator type. run_asan_annotations_disablement_test(); } diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index 4e92800a11..ace13b4481 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -1029,14 +1029,14 @@ void run_custom_allocator_matrix() { run_tests>(); } -// Test that writing to un-initialized memory in a string triggers ASan container-overflow error. +// Test that writing to uninitialized memory in a vector triggers an ASan container-overflow error. template > void run_asan_container_overflow_death_test() { - // We'll give the vector capacity 100 (all un0initialized memory). + // We'll give the vector capacity 100 (all uninitialized memory). std::vector vector; vector.reserve(100); - // Write to the 50th element to trigger ASan container-overflow check. + // Write to the element at index 50 to trigger an ASan container-overflow check. vector.data()[50] = T(); } @@ -1058,7 +1058,7 @@ void run_allocator_matrix() { // To test ASan annotation disablement, we use an ad-hoc allocator type to avoid disrupting other // tests that depend on annotations being enabled. Therefore, unlike the prior tests, - // this test is not parametrized by the allocator type. + // this test is not parameterized by the allocator type. run_asan_annotations_disablement_test(); } From 45709b936b9cc397ce38c0e37da2c2579122094b Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 30 Jan 2025 13:44:38 -0800 Subject: [PATCH 21/25] Avoid shadowing: `vector` => `v` --- tests/std/tests/GH_002030_asan_annotate_vector/test.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index ace13b4481..4daa729640 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -1033,11 +1033,11 @@ void run_custom_allocator_matrix() { template > void run_asan_container_overflow_death_test() { // We'll give the vector capacity 100 (all uninitialized memory). - std::vector vector; - vector.reserve(100); + std::vector v; + v.reserve(100); // Write to the element at index 50 to trigger an ASan container-overflow check. - vector.data()[50] = T(); + v.data()[50] = T(); } // Test that ASan `container-overflow` checks can be disabled for a custom allocator. From af4fc604b434a86c6e6855735d164071a1608895 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 30 Jan 2025 13:49:28 -0800 Subject: [PATCH 22/25] Drop unnecessary `std::`. --- tests/std/tests/GH_002030_asan_annotate_string/test.cpp | 4 ++-- tests/std/tests/GH_002030_asan_annotate_vector/test.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp index c28df9a2cd..a58cc7e323 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -1877,11 +1877,11 @@ void run_tests() { } // Test that writing to uninitialized memory in a string triggers an ASan container-overflow error. (See GH-5251.) -template > +template > void run_asan_container_overflow_death_test() { // We'll give the string capacity 100 (all uninitialized memory, except for the null terminator). - std::basic_string, Alloc> myString; + basic_string, Alloc> myString; myString.reserve(100); // Write to the element at index 50 to trigger an ASan container-overflow check. diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index 4daa729640..9dd10d79c9 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -1030,10 +1030,10 @@ void run_custom_allocator_matrix() { } // Test that writing to uninitialized memory in a vector triggers an ASan container-overflow error. -template > +template > void run_asan_container_overflow_death_test() { // We'll give the vector capacity 100 (all uninitialized memory). - std::vector v; + vector v; v.reserve(100); // Write to the element at index 50 to trigger an ASan container-overflow check. From 1b66d9220fd51ff3c6e01d39c0e60f8a19d4b987 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 30 Jan 2025 13:54:06 -0800 Subject: [PATCH 23/25] Improve add_death_tests: add char, move char8_t, add trailing commas. --- .../std/tests/GH_002030_asan_annotate_string/test.cpp | 10 +++++++--- .../std/tests/GH_002030_asan_annotate_vector/test.cpp | 7 +++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp index a58cc7e323..4c19f2d3f4 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -1983,11 +1983,15 @@ int main(int argc, char* argv[]) { test_gh_3955(); }); #ifdef __SANITIZE_ADDRESS__ + exec.add_death_tests({ + run_asan_container_overflow_death_test, #ifdef __cpp_char8_t - exec.add_death_tests({run_asan_container_overflow_death_test}); + run_asan_container_overflow_death_test, #endif // __cpp_char8_t - exec.add_death_tests({run_asan_container_overflow_death_test, - run_asan_container_overflow_death_test, run_asan_container_overflow_death_test}); + run_asan_container_overflow_death_test, + run_asan_container_overflow_death_test, + run_asan_container_overflow_death_test, + }); #endif // ASan instrumentation enabled return exec.run(argc, argv); } diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index 9dd10d79c9..2b9265a4f0 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -1085,9 +1085,12 @@ int main(int argc, char* argv[]) { }); #ifdef __SANITIZE_ADDRESS__ - exec.add_death_tests({run_asan_container_overflow_death_test, run_asan_container_overflow_death_test, + exec.add_death_tests({ + run_asan_container_overflow_death_test, + run_asan_container_overflow_death_test, run_asan_container_overflow_death_test, - run_asan_container_overflow_death_test}); + run_asan_container_overflow_death_test, + }); #endif // ASan instrumentation enabled return exec.run(argc, argv); From 12c763fa8c3ed194bbb18121734d2faadd1673df Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 30 Jan 2025 13:57:36 -0800 Subject: [PATCH 24/25] Style: `T()` => `T{}` --- tests/std/tests/GH_002030_asan_annotate_vector/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index 2b9265a4f0..65cdbb7c1d 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -1037,7 +1037,7 @@ void run_asan_container_overflow_death_test() { v.reserve(100); // Write to the element at index 50 to trigger an ASan container-overflow check. - v.data()[50] = T(); + v.data()[50] = T{}; } // Test that ASan `container-overflow` checks can be disabled for a custom allocator. From 44a336a77542773cce8d6943a1c3a8cbf8322208 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 30 Jan 2025 14:45:36 -0800 Subject: [PATCH 25/25] Disable for all `` (and `typename` => `class` nitpick). --- tests/std/tests/GH_002030_asan_annotate_string/test.cpp | 6 ++++-- tests/std/tests/GH_002030_asan_annotate_vector/test.cpp | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp index 4c19f2d3f4..ab35b14f36 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -307,8 +307,10 @@ struct implicit_allocator_no_asan_annotations : implicit_allocator -constexpr bool _Disable_ASan_container_annotations_for_allocator> = true; +template +constexpr bool + _Disable_ASan_container_annotations_for_allocator> = + true; template void test_construction() { diff --git a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp index 65cdbb7c1d..300b7daf25 100644 --- a/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_vector/test.cpp @@ -298,8 +298,10 @@ struct implicit_allocator_no_asan_annotations : implicit_allocator -constexpr bool _Disable_ASan_container_annotations_for_allocator> = true; +template +constexpr bool + _Disable_ASan_container_annotations_for_allocator> = + true; template void test_push_pop() {