From 2a921c0b1311e3ca1be54413d8b64bdc6157e156 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 3 Mar 2021 12:25:40 +0100 Subject: [PATCH 1/4] Fix uninitialized_meow again Fixes #1712 --- stl/inc/memory | 2 +- stl/inc/xmemory | 2 +- .../test.cpp | 26 ++++++++++++++++++- .../test.cpp | 26 ++++++++++++++++++- 4 files changed, 52 insertions(+), 4 deletions(-) diff --git a/stl/inc/memory b/stl/inc/memory index 17aa1aee70a..e52aa0b932f 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -78,7 +78,7 @@ namespace ranges { _STL_INTERNAL_STATIC_ASSERT(_No_throw_sentinel_for<_OSe, _Out>); _STL_INTERNAL_STATIC_ASSERT(constructible_from, iter_reference_t<_It>>); - if constexpr (is_same_v<_Se, _It> && _Ptr_copy_cat<_It, _Out>::_Really_trivial) { + if constexpr (is_same_v<_Se, _It> && is_same_v<_OSe, _Out> && _Ptr_copy_cat<_It, _Out>::_Really_trivial) { return _Copy_memcpy_common(_IFirst, _ILast, _OFirst, _OLast); } else { _Uninitialized_backout _Backout{_STD move(_OFirst)}; diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 95b331f12f0..ca3bb9bc9bf 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1582,7 +1582,7 @@ namespace ranges { uninitialized_move_result<_It, _Out> _Uninitialized_move_unchecked( _It _IFirst, const _Se _ILast, _Out _OFirst, const _OSe _OLast) { // clang-format on - if constexpr (is_same_v<_Se, _It> && _Ptr_move_cat<_It, _Out>::_Really_trivial) { + if constexpr (is_same_v<_Se, _It> && is_same_v<_OSe, _Out> && _Ptr_move_cat<_It, _Out>::_Really_trivial) { return _Copy_memcpy_common(_IFirst, _ILast, _OFirst, _OLast); } else { _Uninitialized_backout _Backout{_STD move(_OFirst)}; diff --git a/tests/std/tests/P0896R4_ranges_alg_uninitialized_copy/test.cpp b/tests/std/tests/P0896R4_ranges_alg_uninitialized_copy/test.cpp index 38c0d687d54..73485413b37 100644 --- a/tests/std/tests/P0896R4_ranges_alg_uninitialized_copy/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_uninitialized_copy/test.cpp @@ -203,7 +203,7 @@ struct memcpy_test { static constexpr int expected_input_long[] = {13, 55, 12345, 42}; static void call() { - { // Validate only range overload + { // Validate matching ranges int input[] = {13, 55, 12345}; int output[] = {-1, -1, -1}; @@ -235,6 +235,30 @@ struct memcpy_test { assert(ranges::equal(input, expected_input_long)); assert(ranges::equal(output, expected_output)); } + + { // Validate no common input range + int input[] = {13, 55, 12345}; + int output[] = {-1, -1, -1}; + + const auto result = + ranges::uninitialized_copy(begin(input), unreachable_sentinel, begin(output), end(output)); + assert(result.in == end(input)); + assert(result.out == end(output)); + assert(ranges::equal(input, expected_input)); + assert(ranges::equal(output, expected_output)); + } + + { // Validate no common output range + int input[] = {13, 55, 12345}; + int output[] = {-1, -1, -1}; + + const auto result = + ranges::uninitialized_copy(begin(input), end(input), begin(output), unreachable_sentinel); + assert(result.in == end(input)); + assert(result.out == end(output)); + assert(ranges::equal(input, expected_input)); + assert(ranges::equal(output, expected_output)); + } } }; diff --git a/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp b/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp index 865dcffcccf..84cd3bbae72 100644 --- a/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp @@ -204,7 +204,7 @@ struct memcpy_test { static constexpr int expected_input_long[] = {13, 55, 12345, 42}; static void call() { - { // Validate range + { // Validate matching ranges int input[] = {13, 55, 12345}; int output[] = {-1, -1, -1}; @@ -234,6 +234,30 @@ struct memcpy_test { assert(ranges::equal(input, expected_input_long)); assert(ranges::equal(output, expected_output)); } + + { // Validate no common input range + int input[] = {13, 55, 12345}; + int output[] = {-1, -1, -1}; + + const auto result = + ranges::uninitialized_move(begin(input), unreachable_sentinel, begin(output), end(output)); + assert(result.in == end(input)); + assert(result.out == end(output)); + assert(ranges::equal(input, expected_input)); + assert(ranges::equal(output, expected_output)); + } + + { // Validate no common output range + int input[] = {13, 55, 12345}; + int output[] = {-1, -1, -1}; + + const auto result = + ranges::uninitialized_move(begin(input), end(input), begin(output), unreachable_sentinel); + assert(result.in == end(input)); + assert(result.out == end(output)); + assert(ranges::equal(input, expected_input)); + assert(ranges::equal(output, expected_output)); + } } }; From 61cef4af2b4dfbd4d3449cd5bfbfe67e7c7935d2 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 3 Mar 2021 12:31:36 +0100 Subject: [PATCH 2/4] Be consistent with the tests --- .../std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp b/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp index 84cd3bbae72..275161c4878 100644 --- a/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp @@ -208,7 +208,9 @@ struct memcpy_test { int input[] = {13, 55, 12345}; int output[] = {-1, -1, -1}; - ranges::uninitialized_move(input, output); + const auto result = ranges::uninitialized_move(input, output); + assert(result.in == end(input)); + assert(result.out == end(output)); assert(ranges::equal(input, expected_input)); assert(ranges::equal(output, expected_output)); } From 300e73f9dea7dd5c538cf13f86a9bcc79e54cda8 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 3 Mar 2021 19:30:46 +0100 Subject: [PATCH 3/4] Review comments and actually have uninitialized ranges for memcopy test --- .../test.cpp | 25 ++++++++++--------- .../test.cpp | 25 ++++++++++--------- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/tests/std/tests/P0896R4_ranges_alg_uninitialized_copy/test.cpp b/tests/std/tests/P0896R4_ranges_alg_uninitialized_copy/test.cpp index 73485413b37..c2a8b28d545 100644 --- a/tests/std/tests/P0896R4_ranges_alg_uninitialized_copy/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_uninitialized_copy/test.cpp @@ -204,8 +204,8 @@ struct memcpy_test { static void call() { { // Validate matching ranges - int input[] = {13, 55, 12345}; - int output[] = {-1, -1, -1}; + int input[] = {13, 55, 12345}; + int output[3]; const auto result = ranges::uninitialized_copy(input, output); assert(result.in == end(input)); @@ -215,10 +215,11 @@ struct memcpy_test { } { // Validate input shorter - int input[] = {13, 55, 12345}; - int output[] = {-1, -1, -1, -1}; + int input[] = {13, 55, 12345}; + int output[4]; auto result = ranges::uninitialized_copy(input, output); + construct_at(addressof(*result.out), -1); // Need to construct non written element for comparison assert(result.in == end(input)); assert(++result.out == end(output)); assert(ranges::equal(input, expected_input)); @@ -226,8 +227,8 @@ struct memcpy_test { } { // Validate output shorter - int input[] = {13, 55, 12345, 42}; - int output[] = {-1, -1, -1}; + int input[] = {13, 55, 12345, 42}; + int output[3]; auto result = ranges::uninitialized_copy(input, output); assert(++result.in == end(input)); @@ -236,9 +237,9 @@ struct memcpy_test { assert(ranges::equal(output, expected_output)); } - { // Validate no common input range - int input[] = {13, 55, 12345}; - int output[] = {-1, -1, -1}; + { // Validate non-common input range + int input[] = {13, 55, 12345}; + int output[3]; const auto result = ranges::uninitialized_copy(begin(input), unreachable_sentinel, begin(output), end(output)); @@ -248,9 +249,9 @@ struct memcpy_test { assert(ranges::equal(output, expected_output)); } - { // Validate no common output range - int input[] = {13, 55, 12345}; - int output[] = {-1, -1, -1}; + { // Validate non-common output range + int input[] = {13, 55, 12345}; + int output[3]; const auto result = ranges::uninitialized_copy(begin(input), end(input), begin(output), unreachable_sentinel); diff --git a/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp b/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp index 275161c4878..77249b079f4 100644 --- a/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp @@ -205,8 +205,8 @@ struct memcpy_test { static void call() { { // Validate matching ranges - int input[] = {13, 55, 12345}; - int output[] = {-1, -1, -1}; + int input[] = {13, 55, 12345}; + int output[3]; const auto result = ranges::uninitialized_move(input, output); assert(result.in == end(input)); @@ -216,10 +216,11 @@ struct memcpy_test { } { // Validate input shorter - int input[] = {13, 55, 12345}; - int output[] = {-1, -1, -1, -1}; + int input[] = {13, 55, 12345}; + int output[4]; auto result = ranges::uninitialized_move(input, output); + construct_at(addressof(*result.out), -1); // Need to construct non written element for comparison assert(result.in == end(input)); assert(++result.out == end(output)); assert(ranges::equal(input, expected_input)); @@ -227,8 +228,8 @@ struct memcpy_test { } { // Validate output shorter - int input[] = {13, 55, 12345, 42}; - int output[] = {-1, -1, -1}; + int input[] = {13, 55, 12345, 42}; + int output[3]; auto result = ranges::uninitialized_move(input, output); assert(++result.in == end(input)); @@ -237,9 +238,9 @@ struct memcpy_test { assert(ranges::equal(output, expected_output)); } - { // Validate no common input range - int input[] = {13, 55, 12345}; - int output[] = {-1, -1, -1}; + { // Validate non-common input range + int input[] = {13, 55, 12345}; + int output[3]; const auto result = ranges::uninitialized_move(begin(input), unreachable_sentinel, begin(output), end(output)); @@ -249,9 +250,9 @@ struct memcpy_test { assert(ranges::equal(output, expected_output)); } - { // Validate no common output range - int input[] = {13, 55, 12345}; - int output[] = {-1, -1, -1}; + { // Validate non-common output range + int input[] = {13, 55, 12345}; + int output[3]; const auto result = ranges::uninitialized_move(begin(input), end(input), begin(output), unreachable_sentinel); From b8a2a8b1172c93a3bdb08c20e397e24c7f9a7a7f Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 3 Mar 2021 22:05:12 +0100 Subject: [PATCH 4/4] No chance for UB --- .../test.cpp | 21 +++++++++---------- .../test.cpp | 21 +++++++++---------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/tests/std/tests/P0896R4_ranges_alg_uninitialized_copy/test.cpp b/tests/std/tests/P0896R4_ranges_alg_uninitialized_copy/test.cpp index c2a8b28d545..0199745aa45 100644 --- a/tests/std/tests/P0896R4_ranges_alg_uninitialized_copy/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_uninitialized_copy/test.cpp @@ -204,8 +204,8 @@ struct memcpy_test { static void call() { { // Validate matching ranges - int input[] = {13, 55, 12345}; - int output[3]; + int input[] = {13, 55, 12345}; + int output[] = {-1, -1, -1}; const auto result = ranges::uninitialized_copy(input, output); assert(result.in == end(input)); @@ -215,11 +215,10 @@ struct memcpy_test { } { // Validate input shorter - int input[] = {13, 55, 12345}; - int output[4]; + int input[] = {13, 55, 12345}; + int output[] = {-1, -1, -1, -1}; auto result = ranges::uninitialized_copy(input, output); - construct_at(addressof(*result.out), -1); // Need to construct non written element for comparison assert(result.in == end(input)); assert(++result.out == end(output)); assert(ranges::equal(input, expected_input)); @@ -227,8 +226,8 @@ struct memcpy_test { } { // Validate output shorter - int input[] = {13, 55, 12345, 42}; - int output[3]; + int input[] = {13, 55, 12345, 42}; + int output[] = {-1, -1, -1}; auto result = ranges::uninitialized_copy(input, output); assert(++result.in == end(input)); @@ -238,8 +237,8 @@ struct memcpy_test { } { // Validate non-common input range - int input[] = {13, 55, 12345}; - int output[3]; + int input[] = {13, 55, 12345}; + int output[] = {-1, -1, -1}; const auto result = ranges::uninitialized_copy(begin(input), unreachable_sentinel, begin(output), end(output)); @@ -250,8 +249,8 @@ struct memcpy_test { } { // Validate non-common output range - int input[] = {13, 55, 12345}; - int output[3]; + int input[] = {13, 55, 12345}; + int output[] = {-1, -1, -1}; const auto result = ranges::uninitialized_copy(begin(input), end(input), begin(output), unreachable_sentinel); diff --git a/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp b/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp index 77249b079f4..45adaeecf7e 100644 --- a/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_uninitialized_move/test.cpp @@ -205,8 +205,8 @@ struct memcpy_test { static void call() { { // Validate matching ranges - int input[] = {13, 55, 12345}; - int output[3]; + int input[] = {13, 55, 12345}; + int output[] = {-1, -1, -1}; const auto result = ranges::uninitialized_move(input, output); assert(result.in == end(input)); @@ -216,11 +216,10 @@ struct memcpy_test { } { // Validate input shorter - int input[] = {13, 55, 12345}; - int output[4]; + int input[] = {13, 55, 12345}; + int output[] = {-1, -1, -1, -1}; auto result = ranges::uninitialized_move(input, output); - construct_at(addressof(*result.out), -1); // Need to construct non written element for comparison assert(result.in == end(input)); assert(++result.out == end(output)); assert(ranges::equal(input, expected_input)); @@ -228,8 +227,8 @@ struct memcpy_test { } { // Validate output shorter - int input[] = {13, 55, 12345, 42}; - int output[3]; + int input[] = {13, 55, 12345, 42}; + int output[] = {-1, -1, -1}; auto result = ranges::uninitialized_move(input, output); assert(++result.in == end(input)); @@ -239,8 +238,8 @@ struct memcpy_test { } { // Validate non-common input range - int input[] = {13, 55, 12345}; - int output[3]; + int input[] = {13, 55, 12345}; + int output[] = {-1, -1, -1}; const auto result = ranges::uninitialized_move(begin(input), unreachable_sentinel, begin(output), end(output)); @@ -251,8 +250,8 @@ struct memcpy_test { } { // Validate non-common output range - int input[] = {13, 55, 12345}; - int output[3]; + int input[] = {13, 55, 12345}; + int output[] = {-1, -1, -1}; const auto result = ranges::uninitialized_move(begin(input), end(input), begin(output), unreachable_sentinel);