From 0a1274224ef8928507b8e50c19ea25e62a7a3bf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= Date: Mon, 30 Oct 2023 19:56:03 +0100 Subject: [PATCH] [libc++] Fix UB in related to "has value" flag (#68552) (#68733) The calls to std::construct_at might overwrite the previously set __has_value_ flag in the case where the flag is overlapping with the actual value or error being stored (since we use [[no_unique_address]]). To fix this issue, this patch ensures that we initialize the __has_value_ flag after we call std::construct_at. Fixes #68552 (cherry picked from commit 134c91595568ea1335b22e559f20c1a488ea270e) --- libcxx/include/__expected/expected.h | 182 ++++++++---------- .../assign/emplace.intializer_list.pass.cpp | 8 + .../expected.expected/assign/emplace.pass.cpp | 7 + .../ctor/ctor.convert.copy.pass.cpp | 11 +- .../ctor/ctor.convert.move.pass.cpp | 11 +- .../expected.expected/ctor/ctor.copy.pass.cpp | 18 +- .../ctor/ctor.default.pass.cpp | 5 +- .../ctor/ctor.inplace.pass.cpp | 18 +- .../ctor/ctor.inplace_init_list.pass.cpp | 9 +- .../expected.expected/ctor/ctor.move.pass.cpp | 20 +- .../expected.expected/ctor/ctor.u.pass.cpp | 21 +- .../ctor/ctor.unexpect.pass.cpp | 18 +- .../ctor/ctor.unexpect_init_list.pass.cpp | 9 +- .../ctor/ctor.unexpected.copy.pass.cpp | 8 +- .../ctor/ctor.unexpected.move.pass.cpp | 8 +- .../observers/has_value.pass.cpp | 32 +++ .../expected.expected/swap/free.swap.pass.cpp | 64 +++++- .../swap/member.swap.pass.cpp | 64 +++++- .../ctor/ctor.convert.copy.pass.cpp | 11 +- .../ctor/ctor.convert.move.pass.cpp | 11 +- .../expected.void/ctor/ctor.copy.pass.cpp | 11 +- .../expected.void/ctor/ctor.move.pass.cpp | 12 +- .../expected.void/ctor/ctor.unexpect.pass.cpp | 6 +- .../ctor/ctor.unexpect_init_list.pass.cpp | 9 +- .../ctor/ctor.unexpected.copy.pass.cpp | 4 +- .../ctor/ctor.unexpected.move.pass.cpp | 4 +- .../observers/has_value.pass.cpp | 12 ++ .../expected.void/swap/free.swap.pass.cpp | 32 ++- .../expected.void/swap/member.swap.pass.cpp | 30 ++- libcxx/test/std/utilities/expected/types.h | 49 +++++ 30 files changed, 540 insertions(+), 164 deletions(-) diff --git a/libcxx/include/__expected/expected.h b/libcxx/include/__expected/expected.h index 7d57aa4db5f95..5836600312de1 100644 --- a/libcxx/include/__expected/expected.h +++ b/libcxx/include/__expected/expected.h @@ -119,9 +119,7 @@ class expected { _LIBCPP_HIDE_FROM_ABI constexpr expected() noexcept(is_nothrow_default_constructible_v<_Tp>) // strengthened requires is_default_constructible_v<_Tp> - : __has_val_(true) { - std::construct_at(std::addressof(__union_.__val_)); - } + : __union_(std::in_place), __has_val_(true) {} _LIBCPP_HIDE_FROM_ABI constexpr expected(const expected&) = delete; @@ -136,14 +134,7 @@ class expected { noexcept(is_nothrow_copy_constructible_v<_Tp> && is_nothrow_copy_constructible_v<_Err>) // strengthened requires(is_copy_constructible_v<_Tp> && is_copy_constructible_v<_Err> && !(is_trivially_copy_constructible_v<_Tp> && is_trivially_copy_constructible_v<_Err>)) - : __has_val_(__other.__has_val_) { - if (__has_val_) { - std::construct_at(std::addressof(__union_.__val_), __other.__union_.__val_); - } else { - std::construct_at(std::addressof(__union_.__unex_), __other.__union_.__unex_); - } - } - + : __union_(__other.__has_val_, __other.__union_), __has_val_(__other.__has_val_) { } _LIBCPP_HIDE_FROM_ABI constexpr expected(expected&&) requires(is_move_constructible_v<_Tp> && is_move_constructible_v<_Err> @@ -154,13 +145,7 @@ class expected { noexcept(is_nothrow_move_constructible_v<_Tp> && is_nothrow_move_constructible_v<_Err>) requires(is_move_constructible_v<_Tp> && is_move_constructible_v<_Err> && !(is_trivially_move_constructible_v<_Tp> && is_trivially_move_constructible_v<_Err>)) - : __has_val_(__other.__has_val_) { - if (__has_val_) { - std::construct_at(std::addressof(__union_.__val_), std::move(__other.__union_.__val_)); - } else { - std::construct_at(std::addressof(__union_.__unex_), std::move(__other.__union_.__unex_)); - } - } + : __union_(__other.__has_val_, std::move(__other.__union_)), __has_val_(__other.__has_val_) { } private: template @@ -198,36 +183,21 @@ class expected { expected(const expected<_Up, _OtherErr>& __other) noexcept(is_nothrow_constructible_v<_Tp, const _Up&> && is_nothrow_constructible_v<_Err, const _OtherErr&>) // strengthened - : __has_val_(__other.__has_val_) { - if (__has_val_) { - std::construct_at(std::addressof(__union_.__val_), __other.__union_.__val_); - } else { - std::construct_at(std::addressof(__union_.__unex_), __other.__union_.__unex_); - } - } + : __union_(__other.__has_val_, __other.__union_), __has_val_(__other.__has_val_) {} template requires __can_convert<_Up, _OtherErr, _Up, _OtherErr>::value _LIBCPP_HIDE_FROM_ABI constexpr explicit(!is_convertible_v<_Up, _Tp> || !is_convertible_v<_OtherErr, _Err>) expected(expected<_Up, _OtherErr>&& __other) noexcept(is_nothrow_constructible_v<_Tp, _Up> && is_nothrow_constructible_v<_Err, _OtherErr>) // strengthened - : __has_val_(__other.__has_val_) { - if (__has_val_) { - std::construct_at(std::addressof(__union_.__val_), std::move(__other.__union_.__val_)); - } else { - std::construct_at(std::addressof(__union_.__unex_), std::move(__other.__union_.__unex_)); - } - } + : __union_(__other.__has_val_, std::move(__other.__union_)), __has_val_(__other.__has_val_) {} template requires(!is_same_v, in_place_t> && !is_same_v> && !__is_std_unexpected>::value && is_constructible_v<_Tp, _Up>) _LIBCPP_HIDE_FROM_ABI constexpr explicit(!is_convertible_v<_Up, _Tp>) - expected(_Up&& __u) - noexcept(is_nothrow_constructible_v<_Tp, _Up>) // strengthened - : __has_val_(true) { - std::construct_at(std::addressof(__union_.__val_), std::forward<_Up>(__u)); - } + expected(_Up&& __u) noexcept(is_nothrow_constructible_v<_Tp, _Up>) // strengthened + : __union_(std::in_place, std::forward<_Up>(__u)), __has_val_(true) {} template @@ -235,52 +205,40 @@ class expected { _LIBCPP_HIDE_FROM_ABI constexpr explicit(!is_convertible_v) expected(const unexpected<_OtherErr>& __unex) noexcept(is_nothrow_constructible_v<_Err, const _OtherErr&>) // strengthened - : __has_val_(false) { - std::construct_at(std::addressof(__union_.__unex_), __unex.error()); - } + : __union_(std::unexpect, __unex.error()), __has_val_(false) {} template requires is_constructible_v<_Err, _OtherErr> _LIBCPP_HIDE_FROM_ABI constexpr explicit(!is_convertible_v<_OtherErr, _Err>) expected(unexpected<_OtherErr>&& __unex) noexcept(is_nothrow_constructible_v<_Err, _OtherErr>) // strengthened - : __has_val_(false) { - std::construct_at(std::addressof(__union_.__unex_), std::move(__unex.error())); - } + : __union_(std::unexpect, std::move(__unex.error())), __has_val_(false) {} template requires is_constructible_v<_Tp, _Args...> _LIBCPP_HIDE_FROM_ABI constexpr explicit expected(in_place_t, _Args&&... __args) noexcept(is_nothrow_constructible_v<_Tp, _Args...>) // strengthened - : __has_val_(true) { - std::construct_at(std::addressof(__union_.__val_), std::forward<_Args>(__args)...); - } + : __union_(std::in_place, std::forward<_Args>(__args)...), __has_val_(true) {} template requires is_constructible_v< _Tp, initializer_list<_Up>&, _Args... > _LIBCPP_HIDE_FROM_ABI constexpr explicit expected(in_place_t, initializer_list<_Up> __il, _Args&&... __args) noexcept(is_nothrow_constructible_v<_Tp, initializer_list<_Up>&, _Args...>) // strengthened - : __has_val_(true) { - std::construct_at(std::addressof(__union_.__val_), __il, std::forward<_Args>(__args)...); - } + : __union_(std::in_place, __il, std::forward<_Args>(__args)...), __has_val_(true) {} template requires is_constructible_v<_Err, _Args...> _LIBCPP_HIDE_FROM_ABI constexpr explicit expected(unexpect_t, _Args&&... __args) - noexcept(is_nothrow_constructible_v<_Err, _Args...>) // strengthened - : __has_val_(false) { - std::construct_at(std::addressof(__union_.__unex_), std::forward<_Args>(__args)...); - } + noexcept(is_nothrow_constructible_v<_Err, _Args...>) // strengthened + : __union_(std::unexpect, std::forward<_Args>(__args)...), __has_val_(false) {} template requires is_constructible_v< _Err, initializer_list<_Up>&, _Args... > _LIBCPP_HIDE_FROM_ABI constexpr explicit expected(unexpect_t, initializer_list<_Up> __il, _Args&&... __args) noexcept(is_nothrow_constructible_v<_Err, initializer_list<_Up>&, _Args...>) // strengthened - : __has_val_(false) { - std::construct_at(std::addressof(__union_.__unex_), __il, std::forward<_Args>(__args)...); - } + : __union_(std::unexpect, __il, std::forward<_Args>(__args)...), __has_val_(false) {} // [expected.object.dtor], destructor @@ -439,9 +397,10 @@ class expected { std::destroy_at(std::addressof(__union_.__val_)); } else { std::destroy_at(std::addressof(__union_.__unex_)); - __has_val_ = true; } - return *std::construct_at(std::addressof(__union_.__val_), std::forward<_Args>(__args)...); + std::construct_at(std::addressof(__union_.__val_), std::forward<_Args>(__args)...); + __has_val_ = true; + return __union_.__val_; } template @@ -451,9 +410,10 @@ class expected { std::destroy_at(std::addressof(__union_.__val_)); } else { std::destroy_at(std::addressof(__union_.__unex_)); - __has_val_ = true; } - return *std::construct_at(std::addressof(__union_.__val_), __il, std::forward<_Args>(__args)...); + std::construct_at(std::addressof(__union_.__val_), __il, std::forward<_Args>(__args)...); + __has_val_ = true; + return __union_.__val_; } @@ -892,11 +852,15 @@ class expected { } private: - struct __empty_t {}; - template union __union_t { - _LIBCPP_HIDE_FROM_ABI constexpr __union_t() {} + template + _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(std::in_place_t, _Args&&... __args) + : __val_(std::forward<_Args>(__args)...) {} + + template + _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(std::unexpect_t, _Args&&... __args) + : __unex_(std::forward<_Args>(__args)...) {} template _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t( @@ -908,6 +872,14 @@ class expected { std::__expected_construct_unexpected_from_invoke_tag, _Func&& __f, _Args&&... __args) : __unex_(std::invoke(std::forward<_Func>(__f), std::forward<_Args>(__args)...)) {} + template + _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(bool __has_val, _Union&& __other) { + if (__has_val) + std::construct_at(std::addressof(__val_), std::forward<_Union>(__other).__val_); + else + std::construct_at(std::addressof(__unex_), std::forward<_Union>(__other).__unex_); + } + _LIBCPP_HIDE_FROM_ABI constexpr ~__union_t() requires(is_trivially_destructible_v<_ValueType> && is_trivially_destructible_v<_ErrorType>) = default; @@ -926,7 +898,16 @@ class expected { template requires(is_trivially_move_constructible_v<_ValueType> && is_trivially_move_constructible_v<_ErrorType>) union __union_t<_ValueType, _ErrorType> { - _LIBCPP_HIDE_FROM_ABI constexpr __union_t() : __empty_() {} + _LIBCPP_HIDE_FROM_ABI constexpr __union_t(const __union_t&) = default; + _LIBCPP_HIDE_FROM_ABI constexpr __union_t& operator=(const __union_t&) = default; + + template + _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(std::in_place_t, _Args&&... __args) + : __val_(std::forward<_Args>(__args)...) {} + + template + _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(std::unexpect_t, _Args&&... __args) + : __unex_(std::forward<_Args>(__args)...) {} template _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t( @@ -938,6 +919,14 @@ class expected { std::__expected_construct_unexpected_from_invoke_tag, _Func&& __f, _Args&&... __args) : __unex_(std::invoke(std::forward<_Func>(__f), std::forward<_Args>(__args)...)) {} + template + _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(bool __has_val, _Union&& __other) { + if (__has_val) + std::construct_at(std::addressof(__val_), std::forward<_Union>(__other).__val_); + else + std::construct_at(std::addressof(__unex_), std::forward<_Union>(__other).__unex_); + } + _LIBCPP_HIDE_FROM_ABI constexpr ~__union_t() requires(is_trivially_destructible_v<_ValueType> && is_trivially_destructible_v<_ErrorType>) = default; @@ -947,7 +936,6 @@ class expected { requires(!is_trivially_destructible_v<_ValueType> || !is_trivially_destructible_v<_ErrorType>) {} - _LIBCPP_NO_UNIQUE_ADDRESS __empty_t __empty_; _LIBCPP_NO_UNIQUE_ADDRESS _ValueType __val_; _LIBCPP_NO_UNIQUE_ADDRESS _ErrorType __unex_; }; @@ -995,11 +983,7 @@ class expected<_Tp, _Err> { _LIBCPP_HIDE_FROM_ABI constexpr expected(const expected& __rhs) noexcept(is_nothrow_copy_constructible_v<_Err>) // strengthened requires(is_copy_constructible_v<_Err> && !is_trivially_copy_constructible_v<_Err>) - : __has_val_(__rhs.__has_val_) { - if (!__rhs.__has_val_) { - std::construct_at(std::addressof(__union_.__unex_), __rhs.__union_.__unex_); - } - } + : __union_(__rhs.__has_val_, __rhs.__union_), __has_val_(__rhs.__has_val_) {} _LIBCPP_HIDE_FROM_ABI constexpr expected(expected&&) requires(is_move_constructible_v<_Err> && is_trivially_move_constructible_v<_Err>) @@ -1008,51 +992,35 @@ class expected<_Tp, _Err> { _LIBCPP_HIDE_FROM_ABI constexpr expected(expected&& __rhs) noexcept(is_nothrow_move_constructible_v<_Err>) requires(is_move_constructible_v<_Err> && !is_trivially_move_constructible_v<_Err>) - : __has_val_(__rhs.__has_val_) { - if (!__rhs.__has_val_) { - std::construct_at(std::addressof(__union_.__unex_), std::move(__rhs.__union_.__unex_)); - } - } + : __union_(__rhs.__has_val_, std::move(__rhs.__union_)), __has_val_(__rhs.__has_val_) {} template requires __can_convert<_Up, _OtherErr, const _OtherErr&>::value _LIBCPP_HIDE_FROM_ABI constexpr explicit(!is_convertible_v) expected(const expected<_Up, _OtherErr>& __rhs) noexcept(is_nothrow_constructible_v<_Err, const _OtherErr&>) // strengthened - : __has_val_(__rhs.__has_val_) { - if (!__rhs.__has_val_) { - std::construct_at(std::addressof(__union_.__unex_), __rhs.__union_.__unex_); - } - } + : __union_(__rhs.__has_val_, __rhs.__union_), __has_val_(__rhs.__has_val_) {} template requires __can_convert<_Up, _OtherErr, _OtherErr>::value _LIBCPP_HIDE_FROM_ABI constexpr explicit(!is_convertible_v<_OtherErr, _Err>) expected(expected<_Up, _OtherErr>&& __rhs) noexcept(is_nothrow_constructible_v<_Err, _OtherErr>) // strengthened - : __has_val_(__rhs.__has_val_) { - if (!__rhs.__has_val_) { - std::construct_at(std::addressof(__union_.__unex_), std::move(__rhs.__union_.__unex_)); - } - } + : __union_(__rhs.__has_val_, std::move(__rhs.__union_)), __has_val_(__rhs.__has_val_) {} template requires is_constructible_v<_Err, const _OtherErr&> _LIBCPP_HIDE_FROM_ABI constexpr explicit(!is_convertible_v) expected(const unexpected<_OtherErr>& __unex) noexcept(is_nothrow_constructible_v<_Err, const _OtherErr&>) // strengthened - : __has_val_(false) { - std::construct_at(std::addressof(__union_.__unex_), __unex.error()); - } + : __union_(std::unexpect, __unex.error()), __has_val_(false) {} template requires is_constructible_v<_Err, _OtherErr> _LIBCPP_HIDE_FROM_ABI constexpr explicit(!is_convertible_v<_OtherErr, _Err>) expected(unexpected<_OtherErr>&& __unex) noexcept(is_nothrow_constructible_v<_Err, _OtherErr>) // strengthened - : __has_val_(false) { - std::construct_at(std::addressof(__union_.__unex_), std::move(__unex.error())); - } + : __union_(std::unexpect, std::move(__unex.error())), __has_val_(false) {} _LIBCPP_HIDE_FROM_ABI constexpr explicit expected(in_place_t) noexcept : __has_val_(true) {} @@ -1060,17 +1028,13 @@ class expected<_Tp, _Err> { requires is_constructible_v<_Err, _Args...> _LIBCPP_HIDE_FROM_ABI constexpr explicit expected(unexpect_t, _Args&&... __args) noexcept(is_nothrow_constructible_v<_Err, _Args...>) // strengthened - : __has_val_(false) { - std::construct_at(std::addressof(__union_.__unex_), std::forward<_Args>(__args)...); - } + : __union_(std::unexpect, std::forward<_Args>(__args)...), __has_val_(false) {} template requires is_constructible_v< _Err, initializer_list<_Up>&, _Args... > _LIBCPP_HIDE_FROM_ABI constexpr explicit expected(unexpect_t, initializer_list<_Up> __il, _Args&&... __args) noexcept(is_nothrow_constructible_v<_Err, initializer_list<_Up>&, _Args...>) // strengthened - : __has_val_(false) { - std::construct_at(std::addressof(__union_.__unex_), __il, std::forward<_Args>(__args)...); - } + : __union_(std::unexpect, __il, std::forward<_Args>(__args)...), __has_val_(false) {} private: template @@ -1504,11 +1468,23 @@ class expected<_Tp, _Err> { union __union_t { _LIBCPP_HIDE_FROM_ABI constexpr __union_t() : __empty_() {} + template + _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(std::unexpect_t, _Args&&... __args) + : __unex_(std::forward<_Args>(__args)...) {} + template _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t( __expected_construct_unexpected_from_invoke_tag, _Func&& __f, _Args&&... __args) : __unex_(std::invoke(std::forward<_Func>(__f), std::forward<_Args>(__args)...)) {} + template + _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(bool __has_val, _Union&& __other) { + if (__has_val) + std::construct_at(std::addressof(__empty_)); + else + std::construct_at(std::addressof(__unex_), std::forward<_Union>(__other).__unex_); + } + _LIBCPP_HIDE_FROM_ABI constexpr ~__union_t() requires(is_trivially_destructible_v<_ErrorType>) = default; @@ -1529,11 +1505,23 @@ class expected<_Tp, _Err> { union __union_t<_ErrorType> { _LIBCPP_HIDE_FROM_ABI constexpr __union_t() : __empty_() {} + template + _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(std::unexpect_t, _Args&&... __args) + : __unex_(std::forward<_Args>(__args)...) {} + template _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t( __expected_construct_unexpected_from_invoke_tag, _Func&& __f, _Args&&... __args) : __unex_(std::invoke(std::forward<_Func>(__f), std::forward<_Args>(__args)...)) {} + template + _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(bool __has_val, _Union&& __other) { + if (__has_val) + std::construct_at(std::addressof(__empty_)); + else + std::construct_at(std::addressof(__unex_), std::forward<_Union>(__other).__unex_); + } + _LIBCPP_HIDE_FROM_ABI constexpr ~__union_t() requires(is_trivially_destructible_v<_ErrorType>) = default; diff --git a/libcxx/test/std/utilities/expected/expected.expected/assign/emplace.intializer_list.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/assign/emplace.intializer_list.pass.cpp index 3cdfcde3f4d69..922200a8c0263 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/assign/emplace.intializer_list.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/assign/emplace.intializer_list.pass.cpp @@ -81,6 +81,14 @@ constexpr bool test() { assert(e.value().i == 10); } + // TailClobberer + { + std::expected, bool> e(std::unexpect); + auto list = {4, 5, 6}; + e.emplace(list); + assert(e.has_value()); + } + return true; } diff --git a/libcxx/test/std/utilities/expected/expected.expected/assign/emplace.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/assign/emplace.pass.cpp index 30392b791c5d4..520704c7ea642 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/assign/emplace.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/assign/emplace.pass.cpp @@ -76,6 +76,13 @@ constexpr bool test() { assert(e.value() == 10); } + // TailClobberer + { + std::expected, bool> e(std::unexpect); + e.emplace(); + assert(e.has_value()); + } + return true; } diff --git a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.convert.copy.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.convert.copy.pass.cpp index 9274b9a2c030e..16de28d970396 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.convert.copy.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.convert.copy.pass.cpp @@ -45,6 +45,7 @@ #include #include "test_macros.h" +#include "../../types.h" // Test Constraints: template @@ -161,13 +162,19 @@ constexpr bool test() { assert(e1.error() == 5); } + // convert TailClobberer + { + const std::expected, char> e1; + std::expected, char> e2 = e1; + assert(e2.has_value()); + assert(e1.has_value()); + } + return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct ThrowingInt { ThrowingInt(int) { throw Except{}; } }; diff --git a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.convert.move.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.convert.move.pass.cpp index 71979311bfd10..0e30ea2c7fe0b 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.convert.move.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.convert.move.pass.cpp @@ -46,6 +46,7 @@ #include "MoveOnly.h" #include "test_macros.h" +#include "../../types.h" // Test Constraints: template @@ -160,13 +161,19 @@ constexpr bool test() { assert(e1.error().get() == 0); } + // convert TailClobberer + { + std::expected, char> e1; + std::expected, char> e2 = std::move(e1); + assert(e2.has_value()); + assert(e1.has_value()); + } + return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct ThrowingInt { ThrowingInt(int) { throw Except{}; } }; diff --git a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.copy.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.copy.pass.cpp index 77d73485025ab..581df51207da2 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.copy.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.copy.pass.cpp @@ -30,6 +30,7 @@ #include #include "test_macros.h" +#include "../../types.h" struct NonCopyable { NonCopyable(const NonCopyable&) = delete; @@ -93,13 +94,26 @@ constexpr bool test() { assert(!e2.has_value()); assert(e2.error() == 5); } + + // copy TailClobberer as value + { + const std::expected, bool> e1; + auto e2 = e1; + assert(e2.has_value()); + } + + // copy TailClobberer as error + { + const std::expected> e1(std::unexpect); + auto e2 = e1; + assert(!e2.has_value()); + } + return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing() = default; Throwing(const Throwing&) { throw Except{}; } diff --git a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.default.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.default.pass.cpp index 431e604e8b692..dcd046bdd9d89 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.default.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.default.pass.cpp @@ -22,6 +22,7 @@ #include #include "test_macros.h" +#include "../../types.h" struct NoDedefaultCtor { NoDedefaultCtor() = delete; @@ -45,6 +46,7 @@ constexpr void testDefaultCtor() { template constexpr void testTypes() { + testDefaultCtor(); testDefaultCtor(); testDefaultCtor(); } @@ -52,13 +54,12 @@ constexpr void testTypes() { constexpr bool test() { testTypes(); testTypes(); + testTypes>(); return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing() { throw Except{}; }; }; diff --git a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.inplace.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.inplace.pass.cpp index 92952551711e0..88ec41939439a 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.inplace.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.inplace.pass.cpp @@ -26,6 +26,7 @@ #include "MoveOnly.h" #include "test_macros.h" +#include "../../types.h" // Test Constraints: static_assert(std::is_constructible_v, std::in_place_t>); @@ -54,24 +55,24 @@ struct CopyOnly { friend constexpr bool operator==(const CopyOnly& mi, int ii) { return mi.i == ii; } }; -template +template constexpr void testInt() { - std::expected e(std::in_place, 5); + std::expected e(std::in_place, 5); assert(e.has_value()); assert(e.value() == 5); } -template +template constexpr void testLValue() { T t(5); - std::expected e(std::in_place, t); + std::expected e(std::in_place, t); assert(e.has_value()); assert(e.value() == 5); } -template +template constexpr void testRValue() { - std::expected e(std::in_place, T(5)); + std::expected e(std::in_place, T(5)); assert(e.has_value()); assert(e.value() == 5); } @@ -80,10 +81,13 @@ constexpr bool test() { testInt(); testInt(); testInt(); + testInt, bool>(); testLValue(); testLValue(); + testLValue, bool>(); testRValue(); testRValue(); + testRValue, bool>(); // no arg { @@ -111,8 +115,6 @@ constexpr bool test() { void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing(int) { throw Except{}; }; }; diff --git a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.inplace_init_list.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.inplace_init_list.pass.cpp index 3d62967fa77ee..5f206b867a965 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.inplace_init_list.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.inplace_init_list.pass.cpp @@ -31,6 +31,7 @@ #include "MoveOnly.h" #include "test_macros.h" +#include "../../types.h" // Test Constraints: static_assert( @@ -93,13 +94,17 @@ constexpr bool test() { assert(m.get() == 0); } + // TailClobberer + { + std::expected, bool> e(std::in_place, {1, 2, 3}); + assert(e.has_value()); + } + return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing(std::initializer_list, int) { throw Except{}; }; }; diff --git a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.move.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.move.pass.cpp index 5e6749e50c16c..cd89e2445860a 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.move.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.move.pass.cpp @@ -32,6 +32,7 @@ #include #include "test_macros.h" +#include "../../types.h" struct NonMovable { NonMovable(NonMovable&&) = delete; @@ -112,13 +113,28 @@ constexpr bool test() { assert(e2.error() == 5); assert(!e1.has_value()); } + + // move TailClobbererNonTrivialMove as value + { + std::expected, bool> e1; + auto e2 = std::move(e1); + assert(e2.has_value()); + assert(e1.has_value()); + } + + // move TailClobbererNonTrivialMove as error + { + std::expected> e1(std::unexpect); + auto e2 = std::move(e1); + assert(!e2.has_value()); + assert(!e1.has_value()); + } + return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing() = default; Throwing(Throwing&&) { throw Except{}; } diff --git a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.u.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.u.pass.cpp index f05c2d15f3bbe..8ec6022850d24 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.u.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.u.pass.cpp @@ -29,6 +29,7 @@ #include "MoveOnly.h" #include "test_macros.h" +#include "../../types.h" // Test Constraints: static_assert(std::is_constructible_v, int>); @@ -67,24 +68,27 @@ struct CopyOnly { friend constexpr bool operator==(const CopyOnly& mi, int ii) { return mi.i == ii; } }; -template +struct BaseError {}; +struct DerivedError : BaseError {}; + +template constexpr void testInt() { - std::expected e(5); + std::expected e(5); assert(e.has_value()); assert(e.value() == 5); } -template +template constexpr void testLValue() { T t(5); - std::expected e(t); + std::expected e(t); assert(e.has_value()); assert(e.value() == 5); } -template +template constexpr void testRValue() { - std::expected e(T(5)); + std::expected e(T(5)); assert(e.has_value()); assert(e.value() == 5); } @@ -93,10 +97,13 @@ constexpr bool test() { testInt(); testInt(); testInt(); + testInt, bool>(); testLValue(); testLValue(); + testLValue, bool>(); testRValue(); testRValue(); + testRValue, bool>(); // Test default template argument. // Without it, the template parameter cannot be deduced from an initializer list @@ -129,8 +136,6 @@ constexpr bool test() { void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing(int) { throw Except{}; }; }; diff --git a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpect.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpect.pass.cpp index 5a78e41dfcae2..27ce97737d288 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpect.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpect.pass.cpp @@ -26,6 +26,7 @@ #include "MoveOnly.h" #include "test_macros.h" +#include "../../types.h" // Test Constraints: static_assert(std::is_constructible_v, std::unexpect_t>); @@ -54,24 +55,24 @@ struct CopyOnly { friend constexpr bool operator==(const CopyOnly& mi, int ii) { return mi.i == ii; } }; -template +template constexpr void testInt() { - std::expected e(std::unexpect, 5); + std::expected e(std::unexpect, 5); assert(!e.has_value()); assert(e.error() == 5); } -template +template constexpr void testLValue() { T t(5); - std::expected e(std::unexpect, t); + std::expected e(std::unexpect, t); assert(!e.has_value()); assert(e.error() == 5); } -template +template constexpr void testRValue() { - std::expected e(std::unexpect, T(5)); + std::expected e(std::unexpect, T(5)); assert(!e.has_value()); assert(e.error() == 5); } @@ -80,10 +81,13 @@ constexpr bool test() { testInt(); testInt(); testInt(); + testInt, bool>(); testLValue(); testLValue(); + testLValue, bool>(); testRValue(); testRValue(); + testRValue, bool>(); // no arg { @@ -111,8 +115,6 @@ constexpr bool test() { void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing(int) { throw Except{}; }; }; diff --git a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpect_init_list.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpect_init_list.pass.cpp index 09bfb535606fd..9771aa77792ad 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpect_init_list.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpect_init_list.pass.cpp @@ -31,6 +31,7 @@ #include "MoveOnly.h" #include "test_macros.h" +#include "../../types.h" // Test Constraints: static_assert( @@ -93,13 +94,17 @@ constexpr bool test() { assert(m.get() == 0); } + // TailClobberer + { + std::expected> e(std::unexpect, {1, 2, 3}); + assert(!e.has_value()); + } + return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing(std::initializer_list, int) { throw Except{}; }; }; diff --git a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpected.copy.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpected.copy.pass.cpp index 09ac91182b3b8..bbfd3048533c7 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpected.copy.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpected.copy.pass.cpp @@ -27,6 +27,7 @@ #include "MoveOnly.h" #include "test_macros.h" +#include "../../types.h" // Test Constraints static_assert(std::is_constructible_v, const std::unexpected&>); @@ -49,10 +50,10 @@ struct MyInt { friend constexpr bool operator==(const MyInt&, const MyInt&) = default; }; -template +template constexpr void testUnexpected() { const std::unexpected u(5); - std::expected e(u); + std::expected e(u); assert(!e.has_value()); assert(e.error() == 5); } @@ -60,13 +61,12 @@ constexpr void testUnexpected() { constexpr bool test() { testUnexpected(); testUnexpected(); + testUnexpected, bool>(); return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing(int) { throw Except{}; } }; diff --git a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpected.move.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpected.move.pass.cpp index 9aaaa3fe1a448..800d47bda6958 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpected.move.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.unexpected.move.pass.cpp @@ -27,6 +27,7 @@ #include "MoveOnly.h" #include "test_macros.h" +#include "../../types.h" // Test Constraints static_assert(std::is_constructible_v, std::unexpected>); @@ -49,10 +50,10 @@ struct MyInt { friend constexpr bool operator==(const MyInt&, const MyInt&) = default; }; -template +template constexpr void testInt() { std::unexpected u(5); - std::expected e(std::move(u)); + std::expected e(std::move(u)); assert(!e.has_value()); assert(e.error() == 5); } @@ -69,14 +70,13 @@ constexpr bool test() { testInt(); testInt(); testInt(); + testInt, bool>(); testMoveOnly(); return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing(int) { throw Except{}; } }; diff --git a/libcxx/test/std/utilities/expected/expected.expected/observers/has_value.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/observers/has_value.pass.cpp index 27d657a065699..2b24b0ac24ddb 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/observers/has_value.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/observers/has_value.pass.cpp @@ -12,10 +12,12 @@ #include #include #include +#include #include #include #include "test_macros.h" +#include "../../types.h" // Test noexcept template @@ -43,6 +45,36 @@ constexpr bool test() { assert(!e.has_value()); } + // The following tests check that the "has_value" flag is not overwritten + // by the constructor of the value. This could happen because the flag is + // stored in the tail padding of the value. + // + // The first test is a simplified version of the real code where this was + // first observed. + // + // The other tests use a synthetic struct that clobbers its tail padding + // on construction, making the issue easier to reproduce. + // + // See https://github.com/llvm/llvm-project/issues/68552 and the linked PR. + { + auto f1 = [] -> std::expected, long> { return 0; }; + + auto f2 = [&f1] -> std::expected, int> { + return f1().transform_error([](auto) { return 0; }); + }; + + auto e = f2(); + assert(e.has_value()); + } + { + const std::expected, bool> e = {}; + // clang-cl does not support [[no_unique_address]] yet. +#if !(defined(TEST_COMPILER_CLANG) && defined(_MSC_VER)) + LIBCPP_STATIC_ASSERT(sizeof(TailClobberer<0>) == sizeof(e)); +#endif + assert(e.has_value()); + } + return true; } diff --git a/libcxx/test/std/utilities/expected/expected.expected/swap/free.swap.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/swap/free.swap.pass.cpp index 2a004b0cd7474..4a94ee0ee597f 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/swap/free.swap.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/swap/free.swap.pass.cpp @@ -131,7 +131,7 @@ constexpr bool test() { std::expected, TrackedMove> e1(std::in_place, 5); std::expected, TrackedMove> e2(std::unexpect, 10); - e1.swap(e2); + swap(e1, e2); assert(!e1.has_value()); assert(e1.error().i == 10); @@ -182,6 +182,35 @@ constexpr bool test() { assert(!e2.error().swapCalled); } + // TailClobberer + { + // is_nothrow_move_constructible_v + { + std::expected, TailClobbererNonTrivialMove<1, true>> x(std::in_place); + std::expected, TailClobbererNonTrivialMove<1, true>> y(std::unexpect); + + swap(x, y); + + // Both of these would fail if adjusting the "has value" flags happened + // _before_ constructing the member objects inside the `swap`. + assert(!x.has_value()); + assert(y.has_value()); + } + + // !is_nothrow_move_constructible_v + { + std::expected, TailClobbererNonTrivialMove<1, false>> x(std::in_place); + std::expected, TailClobbererNonTrivialMove<1, false>> y(std::unexpect); + + swap(x, y); + + // Both of these would fail if adjusting the "has value" flags happened + // _before_ constructing the member objects inside the `swap`. + assert(!x.has_value()); + assert(y.has_value()); + } + } + return true; } @@ -212,6 +241,39 @@ void testException() { assert(*e1 == 5); } } + + // TailClobberer + { + // is_nothrow_move_constructible_v + { + std::expected, TailClobbererNonTrivialMove<1>> x(std::in_place); + std::expected, TailClobbererNonTrivialMove<1>> y(std::unexpect); + try { + swap(x, y); + assert(false); + } catch (Except) { + assert(x.has_value()); + // This would fail if `TailClobbererNonTrivialMove<1>` clobbered the + // flag when rolling back the swap. + assert(!y.has_value()); + } + } + + // !is_nothrow_move_constructible_v + { + std::expected, TailClobbererNonTrivialMove<1, false, true>> x(std::in_place); + std::expected, TailClobbererNonTrivialMove<1, false, true>> y(std::unexpect); + try { + swap(x, y); + assert(false); + } catch (Except) { + // This would fail if `TailClobbererNonTrivialMove<0>` clobbered the + // flag when rolling back the swap. + assert(x.has_value()); + assert(!y.has_value()); + } + } + } #endif // TEST_HAS_NO_EXCEPTIONS } diff --git a/libcxx/test/std/utilities/expected/expected.expected/swap/member.swap.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/swap/member.swap.pass.cpp index d2d4a09922092..b6b112cfbeb8b 100644 --- a/libcxx/test/std/utilities/expected/expected.expected/swap/member.swap.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.expected/swap/member.swap.pass.cpp @@ -70,7 +70,7 @@ static_assert(!HasMemberSwap); // Test noexcept template -concept MemberSwapNoexcept = +concept MemberSwapNoexcept = // requires(std::expected x, std::expected y) { { x.swap(y) } noexcept; }; @@ -198,6 +198,35 @@ constexpr bool test() { assert(!e2.error().swapCalled); } + // TailClobberer + { + // is_nothrow_move_constructible_v + { + std::expected, TailClobbererNonTrivialMove<1, true>> x(std::in_place); + std::expected, TailClobbererNonTrivialMove<1, true>> y(std::unexpect); + + x.swap(y); + + // Both of these would fail if adjusting the "has value" flags happened + // _before_ constructing the member objects inside the `swap`. + assert(!x.has_value()); + assert(y.has_value()); + } + + // !is_nothrow_move_constructible_v + { + std::expected, TailClobbererNonTrivialMove<1, false>> x(std::in_place); + std::expected, TailClobbererNonTrivialMove<1, false>> y(std::unexpect); + + x.swap(y); + + // Both of these would fail if adjusting the "has value" flags happened + // _before_ constructing the member objects inside the `swap`. + assert(!x.has_value()); + assert(y.has_value()); + } + } + return true; } @@ -228,6 +257,39 @@ void testException() { assert(*e1 == 5); } } + + // TailClobberer + { + // is_nothrow_move_constructible_v + { + std::expected, TailClobbererNonTrivialMove<1>> x(std::in_place); + std::expected, TailClobbererNonTrivialMove<1>> y(std::unexpect); + try { + x.swap(y); + assert(false); + } catch (Except) { + assert(x.has_value()); + // This would fail if `TailClobbererNonTrivialMove<1>` clobbered the + // flag when rolling back the swap. + assert(!y.has_value()); + } + } + + // !is_nothrow_move_constructible_v + { + std::expected, TailClobbererNonTrivialMove<1, false, true>> x(std::in_place); + std::expected, TailClobbererNonTrivialMove<1, false, true>> y(std::unexpect); + try { + x.swap(y); + assert(false); + } catch (Except) { + // This would fail if `TailClobbererNonTrivialMove<0>` clobbered the + // flag when rolling back the swap. + assert(x.has_value()); + assert(!y.has_value()); + } + } + } #endif // TEST_HAS_NO_EXCEPTIONS } diff --git a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.convert.copy.pass.cpp b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.convert.copy.pass.cpp index 40f8efa5f94bf..05f556e25eac1 100644 --- a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.convert.copy.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.convert.copy.pass.cpp @@ -33,6 +33,7 @@ #include #include "test_macros.h" +#include "../../types.h" // Test Constraints: template @@ -97,13 +98,19 @@ constexpr bool test() { assert(e1.error() == 5); } + // convert TailClobberer + { + const std::expected> e1(std::unexpect); + std::expected> e2 = e1; + assert(!e2.has_value()); + assert(!e1.has_value()); + } + return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct ThrowingInt { ThrowingInt(int) { throw Except{}; } }; diff --git a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.convert.move.pass.cpp b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.convert.move.pass.cpp index b28fc7a03bb34..a48888be53ee0 100644 --- a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.convert.move.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.convert.move.pass.cpp @@ -34,6 +34,7 @@ #include "MoveOnly.h" #include "test_macros.h" +#include "../../types.h" // Test Constraints: template @@ -98,13 +99,19 @@ constexpr bool test() { assert(e1.error().get() == 0); } + // convert TailClobberer + { + std::expected> e1(std::unexpect); + std::expected> e2 = std::move(e1); + assert(!e2.has_value()); + assert(!e1.has_value()); + } + return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct ThrowingInt { ThrowingInt(int) { throw Except{}; } }; diff --git a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.copy.pass.cpp b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.copy.pass.cpp index 689f152a3ac55..7c04a5fa9d044 100644 --- a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.copy.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.copy.pass.cpp @@ -25,6 +25,7 @@ #include #include "test_macros.h" +#include "../../types.h" struct NonCopyable { NonCopyable(const NonCopyable&) = delete; @@ -62,13 +63,19 @@ constexpr bool test() { assert(!e2.has_value()); assert(e2.error() == 5); } + + // copy TailClobberer as error + { + const std::expected> e1(std::unexpect); + auto e2 = e1; + assert(!e2.has_value()); + } + return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing() = default; Throwing(const Throwing&) { throw Except{}; } diff --git a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.move.pass.cpp b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.move.pass.cpp index 61bce2be4897f..bfb5028c9264d 100644 --- a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.move.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.move.pass.cpp @@ -25,6 +25,7 @@ #include #include "test_macros.h" +#include "../../types.h" struct NonMovable { NonMovable(NonMovable&&) = delete; @@ -76,13 +77,20 @@ constexpr bool test() { assert(e2.error() == 5); assert(!e1.has_value()); } + + // move TailClobbererNonTrivialMove as error + { + std::expected> e1(std::unexpect); + auto e2 = std::move(e1); + assert(!e2.has_value()); + assert(!e1.has_value()); + } + return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing() = default; Throwing(Throwing&&) { throw Except{}; } diff --git a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpect.pass.cpp b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpect.pass.cpp index 0a857c77d9c7a..85bc98d7f462d 100644 --- a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpect.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpect.pass.cpp @@ -26,6 +26,7 @@ #include "MoveOnly.h" #include "test_macros.h" +#include "../../types.h" // Test Constraints: static_assert(std::is_constructible_v, std::unexpect_t>); @@ -80,10 +81,13 @@ constexpr bool test() { testInt(); testInt(); testInt(); + testInt>(); testLValue(); testLValue(); + testLValue>(); testRValue(); testRValue(); + testRValue>(); // no arg { @@ -111,8 +115,6 @@ constexpr bool test() { void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing(int) { throw Except{}; }; }; diff --git a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpect_init_list.pass.cpp b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpect_init_list.pass.cpp index f116432962e09..8e4a8eaf87484 100644 --- a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpect_init_list.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpect_init_list.pass.cpp @@ -30,6 +30,7 @@ #include "MoveOnly.h" #include "test_macros.h" +#include "../../types.h" // Test Constraints: static_assert( @@ -91,13 +92,17 @@ constexpr bool test() { assert(m.get() == 0); } + // TailClobberer + { + std::expected> e(std::unexpect, {1, 2, 3}); + assert(!e.has_value()); + } + return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing(std::initializer_list, int) { throw Except{}; }; }; diff --git a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpected.copy.pass.cpp b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpected.copy.pass.cpp index 89e1c9275e3e0..ba738a3e339de 100644 --- a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpected.copy.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpected.copy.pass.cpp @@ -27,6 +27,7 @@ #include "MoveOnly.h" #include "test_macros.h" +#include "../../types.h" // Test Constraints static_assert(std::is_constructible_v, const std::unexpected&>); @@ -60,13 +61,12 @@ constexpr void testUnexpected() { constexpr bool test() { testUnexpected(); testUnexpected(); + testUnexpected>(); return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing(int) { throw Except{}; } }; diff --git a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpected.move.pass.cpp b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpected.move.pass.cpp index 2ddcb63c085f0..33a5e7293df21 100644 --- a/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpected.move.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.void/ctor/ctor.unexpected.move.pass.cpp @@ -27,6 +27,7 @@ #include "MoveOnly.h" #include "test_macros.h" +#include "../../types.h" // Test Constraints static_assert(std::is_constructible_v, std::unexpected>); @@ -69,14 +70,13 @@ constexpr bool test() { testInt(); testInt(); testInt(); + testInt>(); testMoveOnly(); return true; } void testException() { #ifndef TEST_HAS_NO_EXCEPTIONS - struct Except {}; - struct Throwing { Throwing(int) { throw Except{}; } }; diff --git a/libcxx/test/std/utilities/expected/expected.void/observers/has_value.pass.cpp b/libcxx/test/std/utilities/expected/expected.void/observers/has_value.pass.cpp index 42a173d60c898..fe92bb401643d 100644 --- a/libcxx/test/std/utilities/expected/expected.void/observers/has_value.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.void/observers/has_value.pass.cpp @@ -16,6 +16,7 @@ #include #include "test_macros.h" +#include "../../types.h" // Test noexcept template @@ -43,6 +44,17 @@ constexpr bool test() { assert(!e.has_value()); } + // See comments of the corresponding test in + // "expected.expected/observers/has_value.pass.cpp". + { + const std::expected> e(std::unexpect); + // clang-cl does not support [[no_unique_address]] yet. +#if !(defined(TEST_COMPILER_CLANG) && defined(_MSC_VER)) + LIBCPP_STATIC_ASSERT(sizeof(TailClobberer<1>) == sizeof(e)); +#endif + assert(!e.has_value()); + } + return true; } diff --git a/libcxx/test/std/utilities/expected/expected.void/swap/free.swap.pass.cpp b/libcxx/test/std/utilities/expected/expected.void/swap/free.swap.pass.cpp index b293fe27c8d80..9587b891b1a4a 100644 --- a/libcxx/test/std/utilities/expected/expected.void/swap/free.swap.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.void/swap/free.swap.pass.cpp @@ -93,7 +93,7 @@ constexpr bool test() { std::expected e1(std::in_place); std::expected e2(std::unexpect, s, 10); - e1.swap(e2); + swap(e1, e2); assert(!e1.has_value()); assert(e1.error().data_ == 10); @@ -109,7 +109,7 @@ constexpr bool test() { std::expected e1(std::unexpect, s, 10); std::expected e2(std::in_place); - e1.swap(e2); + swap(e1, e2); assert(e1.has_value()); assert(!e2.has_value()); @@ -119,6 +119,19 @@ constexpr bool test() { assert(s.dtorCalled); } + // TailClobberer + { + std::expected> x(std::in_place); + std::expected> y(std::unexpect); + + swap(x, y); + + // The next line would fail if adjusting the "has value" flag happened + // _before_ constructing the member object inside the `swap`. + assert(!x.has_value()); + assert(y.has_value()); + } + return true; } @@ -153,6 +166,21 @@ void testException() { assert(!e2Destroyed); } } + + // TailClobberer + { + std::expected> x(std::in_place); + std::expected> y(std::unexpect); + try { + swap(x, y); + assert(false); + } catch (Except) { + // This would fail if `TailClobbererNonTrivialMove<0, false, true>` + // clobbered the flag before throwing the exception. + assert(x.has_value()); + assert(!y.has_value()); + } + } #endif // TEST_HAS_NO_EXCEPTIONS } diff --git a/libcxx/test/std/utilities/expected/expected.void/swap/member.swap.pass.cpp b/libcxx/test/std/utilities/expected/expected.void/swap/member.swap.pass.cpp index 3eed916d431ee..82554dae04355 100644 --- a/libcxx/test/std/utilities/expected/expected.void/swap/member.swap.pass.cpp +++ b/libcxx/test/std/utilities/expected/expected.void/swap/member.swap.pass.cpp @@ -54,7 +54,7 @@ struct MoveMayThrow { }; template -concept MemberSwapNoexcept = +concept MemberSwapNoexcept = // requires(std::expected x, std::expected y) { { x.swap(y) } noexcept; }; @@ -128,6 +128,19 @@ constexpr bool test() { assert(s.dtorCalled); } + // TailClobberer + { + std::expected> x(std::in_place); + std::expected> y(std::unexpect); + + x.swap(y); + + // The next line would fail if adjusting the "has value" flag happened + // _before_ constructing the member object inside the `swap`. + assert(!x.has_value()); + assert(y.has_value()); + } + return true; } @@ -162,6 +175,21 @@ void testException() { assert(!e2Destroyed); } } + + // TailClobberer + { + std::expected> x(std::in_place); + std::expected> y(std::unexpect); + try { + x.swap(y); + assert(false); + } catch (Except) { + // This would fail if `TailClobbererNonTrivialMove<0, false, true>` + // clobbered the flag before throwing the exception. + assert(x.has_value()); + assert(!y.has_value()); + } + } #endif // TEST_HAS_NO_EXCEPTIONS } diff --git a/libcxx/test/std/utilities/expected/types.h b/libcxx/test/std/utilities/expected/types.h index 278ab0f0ec746..ecab3ce454c98 100644 --- a/libcxx/test/std/utilities/expected/types.h +++ b/libcxx/test/std/utilities/expected/types.h @@ -9,7 +9,9 @@ #ifndef TEST_STD_UTILITIES_EXPECTED_TYPES_H #define TEST_STD_UTILITIES_EXPECTED_TYPES_H +#include #include +#include #include "test_macros.h" template @@ -142,4 +144,51 @@ struct ThrowOnMove { #endif // TEST_HAS_NO_EXCEPTIONS +// This type has one byte of tail padding where `std::expected` may put its +// "has value" flag. The constructor will clobber all bytes including the +// tail padding. With this type we can check that `std::expected` handles +// the case where the "has value" flag is an overlapping subobject correctly. +// +// See https://github.com/llvm/llvm-project/issues/68552 for details. +template +struct TailClobberer { + constexpr TailClobberer() noexcept { + if (!std::is_constant_evaluated()) { + std::memset(this, Constant, sizeof(*this)); + } + // Always set `b` itself to `false` so that the comparison works. + b = false; + } + constexpr TailClobberer(const TailClobberer&) : TailClobberer() {} + constexpr TailClobberer(TailClobberer&&) = default; + // Converts from `int`/`std::initializer_list, used in some tests. + constexpr TailClobberer(int) : TailClobberer() {} + constexpr TailClobberer(std::initializer_list) noexcept : TailClobberer() {} + + friend constexpr bool operator==(const TailClobberer&, const TailClobberer&) = default; + + friend constexpr void swap(TailClobberer&, TailClobberer&){}; + +private: + alignas(2) bool b; +}; +static_assert(!std::is_trivially_copy_constructible_v>); +static_assert(std::is_trivially_move_constructible_v>); + +template +struct TailClobbererNonTrivialMove : TailClobberer { + using TailClobberer::TailClobberer; + constexpr TailClobbererNonTrivialMove(TailClobbererNonTrivialMove&&) noexcept(Noexcept) : TailClobberer() { +#ifndef TEST_HAS_NO_EXCEPTIONS + if constexpr (!Noexcept && ThrowOnMove) + throw Except{}; +#endif + } +}; +static_assert(!std::is_trivially_copy_constructible_v>); +static_assert(std::is_move_constructible_v>); +static_assert(!std::is_trivially_move_constructible_v>); +static_assert(std::is_nothrow_move_constructible_v>); +static_assert(!std::is_nothrow_move_constructible_v>); + #endif // TEST_STD_UTILITIES_EXPECTED_TYPES_H