diff --git a/stl/inc/vector b/stl/inc/vector index 4a38d29e03..ecc8c430bb 100644 --- a/stl/inc/vector +++ b/stl/inc/vector @@ -529,15 +529,29 @@ private: 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) { + // If we realign the _End forward to maximize coverage, we need to keep other significant points + // aligned with our concept of the end of the buffer, otherwise we may leave shadow bytes behind when + // cleaning up. + // old state: - // [_First, _Old_last) valid - // [_Old_last, _End) poison + // [_First, _Old_last_aligned) valid + // [_Old_last_aligned, asan_aligned_after(_End)) poison // new state: - // [_First, _New_last) valid - // [_New_last, asan_aligned_after(_End)) poison + // [_First, _New_last_aligned) valid + // [_New_last_aligned, asan_aligned_after(_End)) poison + + const void* const _New_end_aligned = _STD _Get_asan_aligned_after(_End); + const void* const _Old_last_aligned = (_Old_last == _End) ? _New_end_aligned : _Old_last; + const void* const _New_last_aligned = (_New_last == _End) ? _New_end_aligned : _New_last; + _CSTD __sanitizer_annotate_contiguous_container( - _First, _STD _Get_asan_aligned_after(_End), _Old_last, _New_last); + _First, _New_end_aligned, _Old_last_aligned, _New_last_aligned); } else { + + // Allocation is potentially unaligned, so we cannot annotate whole buffer since we might be + // entering memory owned by someone else. Therefore, pull back where the annotations end on the buffer, + // but may miss some coverage near end of buffer. + 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. diff --git a/stl/inc/xstring b/stl/inc/xstring index b371e31d6b..83a590d3c7 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -714,22 +714,34 @@ private: // 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); + // If we realign the _End forward to maximize coverage, we need to keep other significant points + // aligned with our concept of the end of the buffer, otherwise we may leave shadow bytes behind when + // cleaning up. + + // --- always aligned case --- + // old state: + // [_First, _Old_last_aligned) valid + // [_Old_last_aligned, asan_aligned_after(_End)) poison + // new state: + // [_First, _New_last_aligned) valid + // [_New_last_aligned, asan_aligned_after(_End)) poison + + const void* const _New_end_aligned = _STD _Get_asan_aligned_after(_End); + const void* const _Old_last_aligned = (_Old_last == _End) ? _New_end_aligned : _Old_last; + const void* const _New_last_aligned = (_New_last == _End) ? _New_end_aligned : _New_last; + + _CSTD __sanitizer_annotate_contiguous_container( + _First, _New_end_aligned, _Old_last_aligned, _New_last_aligned); + return; } - 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 + // Allocation is potentially unaligned, so we cannot annotate whole buffer since we might be + // entering memory owned by someone else. Therefore, pull back where the annotations end on the buffer, + // but may miss some coverage near end of buffer. + const _Asan_aligned_pointers _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); // --- sometimes non-aligned case --- // old state: diff --git a/tests/std/test.lst b/tests/std/test.lst index e034874360..8c4f26db24 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -281,6 +281,7 @@ tests\GH_005800_stable_sort_large_alignment tests\GH_005816_numeric_limits_traps tests\GH_005968_headers_provide_begin_end tests\GH_005974_asan_annotate_optional +tests\GH_006276_annotation_poison_cleanup tests\LWG2381_num_get_floating_point tests\LWG2510_tag_classes tests\LWG2597_complex_branch_cut diff --git a/tests/std/tests/GH_006276_annotation_poison_cleanup/env.lst b/tests/std/tests/GH_006276_annotation_poison_cleanup/env.lst new file mode 100644 index 0000000000..87996b3e1f --- /dev/null +++ b/tests/std/tests/GH_006276_annotation_poison_cleanup/env.lst @@ -0,0 +1,34 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +# This test matrix is the usual test matrix, with all currently unsupported options removed, crossed with the ASan flags. + +# TRANSITION, google/sanitizers#328: clang-cl does not support /MDd or /MTd with ASan +RUNALL_INCLUDE ..\prefix.lst +RUNALL_CROSSLIST +PM_CL="/Zi /wd4611 /w14640 /Zc:threadSafeInit-" PM_LINK="/debug" +RUNALL_CROSSLIST +PM_CL="-fsanitize=address /BE /c /EHsc /MD /std:c++14" +PM_CL="-fsanitize=address /BE /c /EHsc /MDd /std:c++17 /permissive-" +PM_CL="-fsanitize=address /BE /c /EHsc /MT /std:c++20 /permissive-" +PM_CL="-fsanitize=address /BE /c /EHsc /MTd /std:c++latest /permissive-" +PM_CL="-fsanitize=address /EHsc /MD /std:c++14" +PM_CL="-fsanitize=address /EHsc /MD /std:c++17" +PM_CL="-fsanitize=address /EHsc /MD /std:c++20" +PM_CL="-fsanitize=address /EHsc /MD /std:c++latest /permissive- /Zc:char8_t- /Zc:preprocessor" +PM_CL="-fsanitize=address /EHsc /MD /std:c++latest /permissive- /Zc:noexceptTypes-" +PM_CL="-fsanitize=address /EHsc /MDd /std:c++14 /fp:except /Zc:preprocessor" +PM_CL="-fsanitize=address /EHsc /MDd /std:c++17 /permissive-" +PM_CL="-fsanitize=address /EHsc /MDd /std:c++20 /permissive-" +PM_CL="-fsanitize=address /EHsc /MDd /std:c++latest /permissive- /Zc:wchar_t-" +PM_CL="-fsanitize=address /EHsc /MDd /std:c++latest /permissive-" +PM_CL="-fsanitize=address /EHsc /MT /std:c++latest /permissive- /analyze:only /analyze:autolog-" +PM_CL="-fsanitize=address /EHsc /MT /std:c++latest /permissive-" +PM_CL="-fsanitize=address /EHsc /MTd /std:c++latest /permissive" +PM_CL="-fsanitize=address /EHsc /MTd /std:c++latest /permissive- /analyze:only /analyze:autolog-" +PM_CL="-fsanitize=address /EHsc /MTd /std:c++latest /permissive- /fp:strict" +PM_CL="-fsanitize=address /EHsc /MTd /std:c++latest /permissive-" +PM_COMPILER="clang-cl" PM_CL="-fsanitize=address -fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /MD /std:c++14" +PM_COMPILER="clang-cl" PM_CL="-fsanitize=address -fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /MD /std:c++17" +PM_COMPILER="clang-cl" PM_CL="-fsanitize=address -fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /MT /std:c++20 /permissive-" +PM_COMPILER="clang-cl" PM_CL="-fsanitize=address -fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /MT /std:c++latest /permissive- /fp:strict" diff --git a/tests/std/tests/GH_006276_annotation_poison_cleanup/test.cpp b/tests/std/tests/GH_006276_annotation_poison_cleanup/test.cpp new file mode 100644 index 0000000000..4227f5104c --- /dev/null +++ b/tests/std/tests/GH_006276_annotation_poison_cleanup/test.cpp @@ -0,0 +1,179 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +// REQUIRES: x64 || x86 || arm64 + +#if defined(__clang__) && defined(_M_ARM64) // TRANSITION, LLVM-184902, fixed in Clang 23 +#pragma comment(linker, "/INFERASANLIBS") +int main() {} +#else // ^^^ workaround / no workaround vvv + +#pragma warning(disable : 4984) // 'if constexpr' is a C++17 language extension +#pragma warning(disable : 4324) // '%s': structure was padded due to alignment specifier +#pragma warning(disable : 4365) // '%s': conversion from '%s' to '%s', signed/unsigned mismatch + +#ifdef __clang__ +#pragma clang diagnostic ignored "-Wc++17-extensions" // constexpr if is a C++17 extension +#define NO_SANITIZER_ADDRESS __attribute__((no_sanitize_address)) +#else // ^^^ clang / msvc vvv +#define NO_SANITIZER_ADDRESS __declspec(no_sanitize_address) +#endif // __clang__ + +#include +#include +#include +#include + +using namespace std; + +extern "C" uintptr_t __asan_shadow_memory_dynamic_address; + +NO_SANITIZER_ADDRESS unsigned char* shadow_addr_of(const void* addr) { + return (unsigned char*) (((uintptr_t) addr >> 3) + __asan_shadow_memory_dynamic_address); +} + +NO_SANITIZER_ADDRESS unsigned char shadow_byte_of(const void* addr) { + return *shadow_addr_of(addr); +} + +NO_SANITIZER_ADDRESS void print_shadow_bytes(const void* addr, const size_t string_size) { + for (size_t i = 0; i < string_size; i += 8) { + printf("%02x ", shadow_byte_of(reinterpret_cast(addr) + i)); + } + printf("\n"); +} + +template +class asan_unaware_arena { + // In practice, custom memory pools should also be annotated for ASan. + // For simplicity, we aren't doing that so we can directly see the + // effects from only the container poisoning. +public: + asan_unaware_arena() noexcept { + fprintf(stderr, "Creating arena at %p with shadow at %p\n", _alloc_buffer, shadow_addr_of(_alloc_buffer)); + } + + ~asan_unaware_arena() noexcept { + fprintf(stderr, "Shadow during destruction (should be all 00):\t"); + print_shadow(); + + // Shadow should be cleared. If it isn't, this memset will trigger an ASan container-overflow error. + // This will likely appear as unknown-error since partial poisoning relies on nearby + // shadow bytes to determine error type. + memset(_alloc_buffer, 0, ArenaSize); + } + + asan_unaware_arena(const asan_unaware_arena&) = delete; + asan_unaware_arena& operator=(const asan_unaware_arena&) = delete; + + asan_unaware_arena(asan_unaware_arena&&) = delete; + asan_unaware_arena& operator=(asan_unaware_arena&&) = delete; + + char* allocate(size_t num_bytes) { + if (_next_alloc + num_bytes > _alloc_buffer + ArenaSize) { + throw bad_alloc(); + } + + char* result = _next_alloc; + _next_alloc += num_bytes; + _next_alloc = reinterpret_cast((reinterpret_cast(_next_alloc) + (AllocationAlignment - 1)) + & ~(AllocationAlignment - 1)); // align the next allocation pointer. + + fprintf(stderr, "Allocated %p -> %p (%zu bytes) from arena, %p -> %p (%zu bytes) in shadow\n", result, + _next_alloc, num_bytes, shadow_addr_of(result), shadow_addr_of(_next_alloc), + shadow_addr_of(_next_alloc) - shadow_addr_of(result)); + + return result; + } + + void print_shadow() const noexcept { + print_shadow_bytes(_alloc_buffer, ArenaSize); + } + +private: + alignas(AllocationAlignment) char _alloc_buffer[ArenaSize]{}; + char* _next_alloc = _alloc_buffer; +}; + +template +struct arena_reuse_allocator { + using value_type = T; + static constexpr size_t Size = AllocSize; + static constexpr size_t _Minimum_asan_allocation_alignment = Alignment; + + template + struct rebind { + using other = arena_reuse_allocator; + }; + + arena_reuse_allocator(asan_unaware_arena* a) noexcept : _arena(a) {} + + template + arena_reuse_allocator(const arena_reuse_allocator& rhs) noexcept + : _arena(rhs._arena) {} + + T* allocate(size_t n) { + return reinterpret_cast(_arena->allocate(n * sizeof(T))); + } + + void deallocate(T*, size_t) noexcept {} + + asan_unaware_arena* _arena; +}; + +const size_t arena_size = 256; + +template +void test_string_poisoning() { + fprintf(stderr, "\nTesting string with allocator alignment of %zu\n", Alignment); + + asan_unaware_arena test_arena; + arena_reuse_allocator alloc(&test_arena); + + basic_string, decltype(alloc)> test_string(L"a 24 length string repr", alloc); + fprintf(stderr, "Shadow after string constructor:\t\t"); + test_arena.print_shadow(); + + test_string.append(L"o"); // add any character to trigger resize + fprintf(stderr, "Shadow after string resize:\t\t\t"); + test_arena.print_shadow(); +} + +template +void test_vector_poisoning() { + fprintf(stderr, "\nTesting vector with allocator alignment of %zu\n", Alignment); + + asan_unaware_arena test_arena; + arena_reuse_allocator alloc(&test_arena); + + vector test_vector(23, L'a', alloc); + fprintf(stderr, "Shadow after vector constructor:\t\t"); + test_arena.print_shadow(); + + test_vector.push_back(L'o'); // trigger resize + fprintf(stderr, "Shadow after vector resize:\t\t\t"); + test_arena.print_shadow(); +} + +int main() { + // Annotations for std::string and std::vector follow different + // paths based off allocation alignment, so test with both. + + // Alignment >= 8 is aligned with shadow bytes' 8-byte granularity, so string/vector + // annotations try to maximize coverage by poisoning past the allocation, since the allocator + // should not hand out that memory anyway. + + // Alignment < 8 is not aligned with shadow bytes, so string/vector annotations + // are more conservative and only poison the memory that was handed out by the + // allocator, leaving some at the end unpoisoned. + + test_string_poisoning<2>(); // under poisoned code path + test_string_poisoning<8>(); // over poisoned code path + + test_vector_poisoning<2>(); // under poisoned code path + test_vector_poisoning<8>(); // over poisoned code path + + return 0; +} + +#endif // ^^^ no workaround ^^^