From 835a545b85e0579776ac4fc8b7a3d38093bf6c36 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Sun, 13 Aug 2023 00:21:04 +0800 Subject: [PATCH 1/8] ASAN fix --- stl/inc/xstring | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index bfd9d09b3a..bf2dbfc5d1 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -3371,11 +3371,9 @@ public: _In_reads_(_Count) const _Elem* const _Ptr, _CRT_GUARDOVERFLOW const size_type _Count) { // assign [_Ptr, _Ptr + _Count) if (_Count <= _Mypair._Myval2._Myres) { - _ASAN_STRING_MODIFY(*this, _Mypair._Myval2._Mysize, _Count); _Elem* const _Old_ptr = _Mypair._Myval2._Myptr(); - _Mypair._Myval2._Mysize = _Count; _Traits::move(_Old_ptr, _Ptr, _Count); - _Traits::assign(_Old_ptr[_Count], _Elem()); + _Eos(_Count); return *this; } From 96cddc467f3d384025e182611ae5b435daeb94a5 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Sun, 13 Aug 2023 00:26:02 +0800 Subject: [PATCH 2/8] update test --- tests/std/tests/GH_002030_asan_annotate_string/test.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) 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 ae8fd17f33..ed680978e6 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -1920,6 +1920,13 @@ void test_gh_3883() { assert(t == "AAAAAAA"); } +void test_gh_3955() { + // GH-3955 : ASAN report container-overflow in a legal case + std::string s(19, '0'); + s = &s[3]; + assert(s == string(16, '0')); +} + int main() { run_allocator_matrix(); #ifdef __cpp_char8_t @@ -1932,4 +1939,5 @@ int main() { test_DevCom_10116361(); test_DevCom_10109507(); test_gh_3883(); + test_gh_3955(); } From c70417935a58496f519b67bf2b8d46c0c02ff16d Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Sun, 13 Aug 2023 00:27:30 +0800 Subject: [PATCH 3/8] formatting --- stl/inc/xstring | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index bf2dbfc5d1..341b6b3ee1 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -3371,7 +3371,7 @@ public: _In_reads_(_Count) const _Elem* const _Ptr, _CRT_GUARDOVERFLOW const size_type _Count) { // assign [_Ptr, _Ptr + _Count) if (_Count <= _Mypair._Myval2._Myres) { - _Elem* const _Old_ptr = _Mypair._Myval2._Myptr(); + _Elem* const _Old_ptr = _Mypair._Myval2._Myptr(); _Traits::move(_Old_ptr, _Ptr, _Count); _Eos(_Count); return *this; From c367a869163b4ad28e3bb83d0416f73d0d70ad2d Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Sun, 13 Aug 2023 00:46:12 +0800 Subject: [PATCH 4/8] remove std --- 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 ed680978e6..d2b4f72775 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -1922,7 +1922,7 @@ void test_gh_3883() { void test_gh_3955() { // GH-3955 : ASAN report container-overflow in a legal case - std::string s(19, '0'); + string s(19, '0'); s = &s[3]; assert(s == string(16, '0')); } From dd3ca8db1ed858501411447580adc6af277adee2 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Tue, 15 Aug 2023 14:06:55 -0700 Subject: [PATCH 5/8] ASan fix in `basic_string::assign(pointer, size_type)` The argument may alias `*this`, so don't shrink until after we copy the characters. --- stl/inc/xstring | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 341b6b3ee1..c72969f866 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -3371,9 +3371,12 @@ public: _In_reads_(_Count) const _Elem* const _Ptr, _CRT_GUARDOVERFLOW const size_type _Count) { // assign [_Ptr, _Ptr + _Count) if (_Count <= _Mypair._Myval2._Myres) { - _Elem* const _Old_ptr = _Mypair._Myval2._Myptr(); + _ASAN_STRING_REMOVE(*this); + _Elem* const _Old_ptr = _Mypair._Myval2._Myptr(); + _Mypair._Myval2._Mysize = _Count; _Traits::move(_Old_ptr, _Ptr, _Count); - _Eos(_Count); + _Traits::assign(_Old_ptr[_Count], _Elem()); + _ASAN_STRING_CREATE(*this); return *this; } From 12438544921e702ba0e3d12227ae9348e06c39db Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Tue, 15 Aug 2023 14:08:15 -0700 Subject: [PATCH 6/8] ASan fix for `basic_string::resize_and_overwrite` We need to update the buffer info even if we do not allocate. --- stl/inc/xstring | 3 +++ 1 file changed, 3 insertions(+) diff --git a/stl/inc/xstring b/stl/inc/xstring index c72969f866..6a772fb53f 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -4157,6 +4157,9 @@ public: [](_Elem* const _New_ptr, const _Elem* const _Old_ptr, const size_type _Old_size) { _Traits::copy(_New_ptr, _Old_ptr, _Old_size + 1); }); + } else { + _ASAN_STRING_MODIFY(*this, _Mypair._Myval2._Mysize, _New_size); + _Mypair._Myval2._Mysize = _New_size; } auto _Arg_ptr = _Mypair._Myval2._Myptr(); From 35de6c54a4e6bd9203d10bb192abcc36d0d96348 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Tue, 15 Aug 2023 14:09:50 -0700 Subject: [PATCH 7/8] `basic_string::resize_and_overwrite` drive-by cleanup Compare the converted "result size" to the "new size" so we don't need to suppress C4108. Improves readability and throughput a teeny bit. --- stl/inc/xstring | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 6a772fb53f..ae566f24af 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -4142,8 +4142,6 @@ public: } } -#pragma warning(push) -#pragma warning(disable : 4018) // '<=': signed/unsigned mismatch (we compare to 0 before _New_size below, so it's safe) template constexpr void #if _HAS_CXX23 @@ -4162,16 +4160,16 @@ public: _Mypair._Myval2._Mysize = _New_size; } - auto _Arg_ptr = _Mypair._Myval2._Myptr(); - auto _Arg_size = _New_size; - const auto _Result_size = _STD move(_Op)(_Arg_ptr, _Arg_size); + auto _Arg_ptr = _Mypair._Myval2._Myptr(); + auto _Arg_size = _New_size; + const auto _Result_size = _STD move(_Op)(_Arg_ptr, _Arg_size); + const auto _Result_as_size_type = static_cast(_Result_size); #if _CONTAINER_DEBUG_LEVEL > 0 _STL_VERIFY(_Result_size >= 0, "the returned size can't be smaller than 0"); - _STL_VERIFY(_Result_size <= _New_size, "the returned size can't be greater than the passed size"); + _STL_VERIFY(_Result_as_size_type <= _New_size, "the returned size can't be greater than the passed size"); #endif // _CONTAINER_DEBUG_LEVEL > 0 - _Eos(static_cast(_Result_size)); + _Eos(_Result_as_size_type); } -#pragma warning(pop) #if _HAS_CXX23 template From 4915243ab94ee7d0854bba1b1d9cf2b9745af803 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Wed, 16 Aug 2023 10:29:20 +0800 Subject: [PATCH 8/8] add test for the fix to my "fix" --- tests/std/tests/GH_002030_asan_annotate_string/test.cpp | 4 ++++ 1 file changed, 4 insertions(+) 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 d2b4f72775..eb1d4127c1 100644 --- a/tests/std/tests/GH_002030_asan_annotate_string/test.cpp +++ b/tests/std/tests/GH_002030_asan_annotate_string/test.cpp @@ -1925,6 +1925,10 @@ void test_gh_3955() { string s(19, '0'); s = &s[3]; assert(s == string(16, '0')); + + string t(19, '0'); + s = &t[0]; + assert(s == t); } int main() {