From 7d62aaf32d8a871504d2f7b47cddf8d2ae048806 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Mon, 6 Jun 2022 22:42:22 +0700 Subject: [PATCH 1/7] fix leaks --- stl/inc/syncstream | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/stl/inc/syncstream b/stl/inc/syncstream index 1437ca8ac1..2d7b7f3332 100644 --- a/stl/inc/syncstream +++ b/stl/inc/syncstream @@ -218,11 +218,12 @@ protected: } const _Size_type _New_capacity = _Calculate_growth(_Buf_size, _Buf_size + 1, _Max_allocation); - const _Elem* const _Old_ptr = streambuf_type::pbase(); + _Elem* const _Old_ptr = streambuf_type::pbase(); const _Size_type _Old_data_size = _Get_data_size(); _Elem* const _New_ptr = _Unfancy(_Al.allocate(_New_capacity)); _Traits::copy(_New_ptr, _Old_ptr, _Old_data_size); + _Getal().deallocate(_Refancy<_Pointer>(_Old_ptr), _Old_data_size); streambuf_type::setp(_New_ptr, _New_ptr + _Old_data_size, _New_ptr + _New_capacity); streambuf_type::sputc(_Traits::to_char_type(_Current_elem)); From da122d6186622f78b9505167c8e57bc5703a8bb6 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Mon, 6 Jun 2022 23:18:24 +0700 Subject: [PATCH 2/7] add test --- tests/std/test.lst | 1 + .../GH_002760_syncstream_memory_leak/env.lst | 20 +++++++++++++++ .../GH_002760_syncstream_memory_leak/test.cpp | 25 +++++++++++++++++++ 3 files changed, 46 insertions(+) create mode 100644 tests/std/tests/GH_002760_syncstream_memory_leak/env.lst create mode 100644 tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp diff --git a/tests/std/test.lst b/tests/std/test.lst index 03282d2299..d51f61ce63 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -204,6 +204,7 @@ tests\GH_002558_format_presetPadding tests\GH_002581_common_reference_workaround tests\GH_002655_alternate_name_broke_linker tests\GH_002711_Zc_alignedNew- +tests\GH_002760_syncstream_memory_leak tests\LWG2597_complex_branch_cut tests\LWG3018_shared_ptr_function tests\LWG3121_constrained_tuple_forwarding_ctor diff --git a/tests/std/tests/GH_002760_syncstream_memory_leak/env.lst b/tests/std/tests/GH_002760_syncstream_memory_leak/env.lst new file mode 100644 index 0000000000..acad367598 --- /dev/null +++ b/tests/std/tests/GH_002760_syncstream_memory_leak/env.lst @@ -0,0 +1,20 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +# This is usual_20_matrix.lst but only with Debug CRT + +RUNALL_INCLUDE ..\prefix.lst +RUNALL_CROSSLIST +PM_CL="/w14640 /Zc:threadSafeInit- /EHsc" +RUNALL_CROSSLIST +PM_CL="/MDd /D_ITERATOR_DEBUG_LEVEL=0 /std:c++latest /permissive- /Zc:wchar_t- /Zc:preprocessor" +PM_CL="/MDd /D_ITERATOR_DEBUG_LEVEL=1 /std:c++latest /permissive-" +PM_CL="/MDd /D_ITERATOR_DEBUG_LEVEL=2 /std:c++20 /permissive- /fp:except /Zc:preprocessor" +PM_CL="/MT /D_ITERATOR_DEBUG_LEVEL=0 /std:c++latest /permissive- /analyze:only /analyze:autolog-" +PM_CL="/MTd /D_ITERATOR_DEBUG_LEVEL=0 /std:c++latest /permissive- /fp:strict" +PM_CL="/MTd /D_ITERATOR_DEBUG_LEVEL=1 /std:c++latest /permissive-" +PM_CL="/MTd /D_ITERATOR_DEBUG_LEVEL=2 /std:c++latest /permissive" +PM_CL="/MTd /D_ITERATOR_DEBUG_LEVEL=2 /std:c++latest /permissive- /analyze:only /analyze:autolog-" +PM_CL="/BE /c /MD /std:c++20 /permissive-" +PM_CL="/BE /c /MTd /std:c++latest /permissive-" +PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing /std:c++latest /permissive- /MTd /fp:strict" diff --git a/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp b/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp new file mode 100644 index 0000000000..8515b997d0 --- /dev/null +++ b/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp @@ -0,0 +1,25 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#define _CRTDBG_MAP_ALLOC +#include +#include +#include +#include +#include +#include + +using namespace std; + +int main() { + _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF); + _CrtMemState start, end, diff; + _CrtMemCheckpoint(&start); + { + stringstream s; + osyncstream bout(s); + bout << "Hello, this line is long, LEAK incoming" << '\n'; + } + _CrtMemCheckpoint(&end); + assert(_CrtMemDifference(&diff, &start, &end) == 0); +} From db73731a677f5f843ffd1179c39994fac27d67b5 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Mon, 6 Jun 2022 23:35:46 +0700 Subject: [PATCH 3/7] reorder includes --- tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp b/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp index 8515b997d0..c3a700e161 100644 --- a/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp +++ b/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp @@ -2,10 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception #define _CRTDBG_MAP_ALLOC -#include -#include #include -#include +// +#include +// +#include #include #include From 4ede8f18099d5fe6ca7f939c8e374f7d60cd48d8 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Mon, 6 Jun 2022 23:39:41 +0700 Subject: [PATCH 4/7] add GH comment --- tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp b/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp index c3a700e161..60da4bca45 100644 --- a/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp +++ b/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp @@ -11,7 +11,7 @@ #include using namespace std; - +// GH-2760, : std::osyncstream memory leak int main() { _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF); _CrtMemState start, end, diff; From 4077243caca1e2407c045f3a50a0d8f2191f89f7 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Tue, 7 Jun 2022 02:01:32 +0700 Subject: [PATCH 5/7] add one more comment --- tests/std/tests/usual_20_matrix.lst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/std/tests/usual_20_matrix.lst b/tests/std/tests/usual_20_matrix.lst index 38eef3ee0c..9a24d4355d 100644 --- a/tests/std/tests/usual_20_matrix.lst +++ b/tests/std/tests/usual_20_matrix.lst @@ -1,6 +1,8 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +# When updating this file, also update tests\GH_002760_syncstream_memory_leak\env.lst to match + RUNALL_INCLUDE .\prefix.lst RUNALL_CROSSLIST PM_CL="/w14640 /Zc:threadSafeInit- /EHsc" From d62b7dd68a9ccfc5188d18c75d1b64a983eeee16 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 6 Jun 2022 18:26:32 -0700 Subject: [PATCH 6/7] Code review feedback. --- stl/inc/syncstream | 2 +- .../GH_002760_syncstream_memory_leak/env.lst | 18 +----------------- .../GH_002760_syncstream_memory_leak/test.cpp | 16 ++++++++-------- tests/std/tests/usual_20_matrix.lst | 2 -- 4 files changed, 10 insertions(+), 28 deletions(-) diff --git a/stl/inc/syncstream b/stl/inc/syncstream index 2d7b7f3332..62e9300f9a 100644 --- a/stl/inc/syncstream +++ b/stl/inc/syncstream @@ -223,7 +223,7 @@ protected: _Elem* const _New_ptr = _Unfancy(_Al.allocate(_New_capacity)); _Traits::copy(_New_ptr, _Old_ptr, _Old_data_size); - _Getal().deallocate(_Refancy<_Pointer>(_Old_ptr), _Old_data_size); + _Al.deallocate(_Refancy<_Pointer>(_Old_ptr), _Old_data_size); streambuf_type::setp(_New_ptr, _New_ptr + _Old_data_size, _New_ptr + _New_capacity); streambuf_type::sputc(_Traits::to_char_type(_Current_elem)); diff --git a/tests/std/tests/GH_002760_syncstream_memory_leak/env.lst b/tests/std/tests/GH_002760_syncstream_memory_leak/env.lst index acad367598..351a8293d9 100644 --- a/tests/std/tests/GH_002760_syncstream_memory_leak/env.lst +++ b/tests/std/tests/GH_002760_syncstream_memory_leak/env.lst @@ -1,20 +1,4 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -# This is usual_20_matrix.lst but only with Debug CRT - -RUNALL_INCLUDE ..\prefix.lst -RUNALL_CROSSLIST -PM_CL="/w14640 /Zc:threadSafeInit- /EHsc" -RUNALL_CROSSLIST -PM_CL="/MDd /D_ITERATOR_DEBUG_LEVEL=0 /std:c++latest /permissive- /Zc:wchar_t- /Zc:preprocessor" -PM_CL="/MDd /D_ITERATOR_DEBUG_LEVEL=1 /std:c++latest /permissive-" -PM_CL="/MDd /D_ITERATOR_DEBUG_LEVEL=2 /std:c++20 /permissive- /fp:except /Zc:preprocessor" -PM_CL="/MT /D_ITERATOR_DEBUG_LEVEL=0 /std:c++latest /permissive- /analyze:only /analyze:autolog-" -PM_CL="/MTd /D_ITERATOR_DEBUG_LEVEL=0 /std:c++latest /permissive- /fp:strict" -PM_CL="/MTd /D_ITERATOR_DEBUG_LEVEL=1 /std:c++latest /permissive-" -PM_CL="/MTd /D_ITERATOR_DEBUG_LEVEL=2 /std:c++latest /permissive" -PM_CL="/MTd /D_ITERATOR_DEBUG_LEVEL=2 /std:c++latest /permissive- /analyze:only /analyze:autolog-" -PM_CL="/BE /c /MD /std:c++20 /permissive-" -PM_CL="/BE /c /MTd /std:c++latest /permissive-" -PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing /std:c++latest /permissive- /MTd /fp:strict" +RUNALL_INCLUDE ..\usual_20_matrix.lst diff --git a/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp b/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp index 60da4bca45..bb8200aace 100644 --- a/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp +++ b/tests/std/tests/GH_002760_syncstream_memory_leak/test.cpp @@ -1,25 +1,25 @@ // Copyright (c) Microsoft Corporation. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -#define _CRTDBG_MAP_ALLOC -#include -// -#include -// +// REQUIRES: debug_CRT + #include +#include #include #include - using namespace std; + // GH-2760, : std::osyncstream memory leak int main() { _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF); - _CrtMemState start, end, diff; + [[maybe_unused]] _CrtMemState start; + [[maybe_unused]] _CrtMemState end; + [[maybe_unused]] _CrtMemState diff; _CrtMemCheckpoint(&start); { stringstream s; osyncstream bout(s); - bout << "Hello, this line is long, LEAK incoming" << '\n'; + bout << "Hello, this line is long, which previously leaked memory" << '\n'; } _CrtMemCheckpoint(&end); assert(_CrtMemDifference(&diff, &start, &end) == 0); diff --git a/tests/std/tests/usual_20_matrix.lst b/tests/std/tests/usual_20_matrix.lst index 9a24d4355d..38eef3ee0c 100644 --- a/tests/std/tests/usual_20_matrix.lst +++ b/tests/std/tests/usual_20_matrix.lst @@ -1,8 +1,6 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -# When updating this file, also update tests\GH_002760_syncstream_memory_leak\env.lst to match - RUNALL_INCLUDE .\prefix.lst RUNALL_CROSSLIST PM_CL="/w14640 /Zc:threadSafeInit- /EHsc" From 50cd427f8a142064ab62fc8d83785d3068c275ba Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 6 Jun 2022 19:06:29 -0700 Subject: [PATCH 7/7] Use _Buf_size following what _Tidy() does. --- stl/inc/syncstream | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/stl/inc/syncstream b/stl/inc/syncstream index 62e9300f9a..3e41b9925c 100644 --- a/stl/inc/syncstream +++ b/stl/inc/syncstream @@ -223,7 +223,9 @@ protected: _Elem* const _New_ptr = _Unfancy(_Al.allocate(_New_capacity)); _Traits::copy(_New_ptr, _Old_ptr, _Old_data_size); - _Al.deallocate(_Refancy<_Pointer>(_Old_ptr), _Old_data_size); + if (0 < _Buf_size) { + _Al.deallocate(_Refancy<_Pointer>(_Old_ptr), _Buf_size); + } streambuf_type::setp(_New_ptr, _New_ptr + _Old_data_size, _New_ptr + _New_capacity); streambuf_type::sputc(_Traits::to_char_type(_Current_elem));