[libcxx-commits] [libcxx] [libc++] Fix UB in <expected> related to "has value" flag (#68552) (PR #68733)
Jan Kokemüller via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Oct 10 11:33:31 PDT 2023
https://github.com/jiixyj created https://github.com/llvm/llvm-project/pull/68733
The calls to `std::construct_at` might overwrite the previously set `__has_value_` flag, so reverse the order everywhere.
>From 779aedda85828357f58b3eaf38fa1350ffa2d16f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= <jan.kokemueller at gmail.com>
Date: Tue, 10 Oct 2023 20:24:06 +0200
Subject: [PATCH] [libc++] Fix UB in <expected> related to "has value" flag
(#68552)
The calls to `std::construct_at` might overwrite the previously set
`__has_value_` flag, so reverse the order everywhere.
---
libcxx/include/__expected/expected.h | 98 ++++++++++---------
.../observers/has_value_clobber.pass.cpp | 36 +++++++
2 files changed, 86 insertions(+), 48 deletions(-)
create mode 100644 libcxx/test/std/utilities/expected/expected.expected/observers/has_value_clobber.pass.cpp
diff --git a/libcxx/include/__expected/expected.h b/libcxx/include/__expected/expected.h
index 045370a486fae6b..cd31694d8b53c41 100644
--- a/libcxx/include/__expected/expected.h
+++ b/libcxx/include/__expected/expected.h
@@ -118,9 +118,9 @@ class expected {
// [expected.object.ctor], constructors
_LIBCPP_HIDE_FROM_ABI constexpr expected()
noexcept(is_nothrow_default_constructible_v<_Tp>) // strengthened
- requires is_default_constructible_v<_Tp>
- : __has_val_(true) {
+ requires is_default_constructible_v<_Tp> {
std::construct_at(std::addressof(__union_.__val_));
+ __has_val_ = true;
}
_LIBCPP_HIDE_FROM_ABI constexpr expected(const expected&) = delete;
@@ -135,13 +135,13 @@ class expected {
_LIBCPP_HIDE_FROM_ABI constexpr expected(const expected& __other)
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_) {
+ !(is_trivially_copy_constructible_v<_Tp> && is_trivially_copy_constructible_v<_Err>)) {
+ if (__other.__has_val_) {
std::construct_at(std::addressof(__union_.__val_), __other.__union_.__val_);
} else {
std::construct_at(std::addressof(__union_.__unex_), __other.__union_.__unex_);
}
+ __has_val_ = __other.__has_val_;
}
@@ -153,13 +153,13 @@ class expected {
_LIBCPP_HIDE_FROM_ABI constexpr expected(expected&& __other)
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_) {
+ !(is_trivially_move_constructible_v<_Tp> && is_trivially_move_constructible_v<_Err>)) {
+ if (__other.__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_));
}
+ __has_val_ = __other.__has_val_;
}
private:
@@ -199,26 +199,26 @@ class expected {
!is_convertible_v<const _OtherErr&, _Err>)
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_) {
+ is_nothrow_constructible_v<_Err, const _OtherErr&>) { // strengthened
+ if (__other.__has_val_) {
std::construct_at(std::addressof(__union_.__val_), __other.__union_.__val_);
} else {
std::construct_at(std::addressof(__union_.__unex_), __other.__union_.__unex_);
}
+ __has_val_ = __other.__has_val_;
}
template <class _Up, class _OtherErr>
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_) {
+ noexcept(is_nothrow_constructible_v<_Tp, _Up> && is_nothrow_constructible_v<_Err, _OtherErr>) { // strengthened
+ if (__other.__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_));
}
+ __has_val_ = __other.__has_val_;
}
template <class _Up = _Tp>
@@ -226,61 +226,61 @@ class expected {
is_constructible_v<_Tp, _Up> && !__is_std_unexpected<remove_cvref_t<_Up>>::value &&
(!is_same_v<remove_cv_t<_Tp>, bool> || !__is_std_expected<remove_cvref_t<_Up>>::value))
_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) {
+ expected(_Up&& __u) noexcept(is_nothrow_constructible_v<_Tp, _Up>) { // strengthened
std::construct_at(std::addressof(__union_.__val_), std::forward<_Up>(__u));
+ __has_val_ = true;
}
template <class _OtherErr>
requires is_constructible_v<_Err, const _OtherErr&>
_LIBCPP_HIDE_FROM_ABI constexpr explicit(!is_convertible_v<const _OtherErr&, _Err>)
expected(const unexpected<_OtherErr>& __unex)
- noexcept(is_nothrow_constructible_v<_Err, const _OtherErr&>) // strengthened
- : __has_val_(false) {
+ noexcept(is_nothrow_constructible_v<_Err, const _OtherErr&>) { // strengthened
std::construct_at(std::addressof(__union_.__unex_), __unex.error());
+ __has_val_ = false;
}
template <class _OtherErr>
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) {
+ noexcept(is_nothrow_constructible_v<_Err, _OtherErr>) { // strengthened
std::construct_at(std::addressof(__union_.__unex_), std::move(__unex.error()));
+ __has_val_ = false;
}
template <class... _Args>
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) {
+ noexcept(is_nothrow_constructible_v<_Tp, _Args...>) { // strengthened
std::construct_at(std::addressof(__union_.__val_), std::forward<_Args>(__args)...);
+ __has_val_ = true;
}
template <class _Up, class... _Args>
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) {
+ noexcept(is_nothrow_constructible_v<_Tp, initializer_list<_Up>&, _Args...>) { // strengthened
std::construct_at(std::addressof(__union_.__val_), __il, std::forward<_Args>(__args)...);
+ __has_val_ = true;
}
template <class... _Args>
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) {
+ noexcept(is_nothrow_constructible_v<_Err, _Args...>) { // strengthened
std::construct_at(std::addressof(__union_.__unex_), std::forward<_Args>(__args)...);
+ __has_val_ = false;
}
template <class _Up, class... _Args>
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) {
+ noexcept(is_nothrow_constructible_v<_Err, initializer_list<_Up>&, _Args...>) { // strengthened
std::construct_at(std::addressof(__union_.__unex_), __il, std::forward<_Args>(__args)...);
+ __has_val_ = false;
}
// [expected.object.dtor], destructor
@@ -440,9 +440,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 *std::addressof(__union_.__val_);
}
template <class _Up, class... _Args>
@@ -452,9 +453,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 *std::addressof(__union_.__val_);
}
@@ -997,11 +999,11 @@ 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_) {
+ requires(is_copy_constructible_v<_Err> && !is_trivially_copy_constructible_v<_Err>) {
if (!__rhs.__has_val_) {
std::construct_at(std::addressof(__union_.__unex_), __rhs.__union_.__unex_);
}
+ __has_val_ = __rhs.__has_val_;
}
_LIBCPP_HIDE_FROM_ABI constexpr expected(expected&&)
@@ -1010,51 +1012,51 @@ 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_) {
+ requires(is_move_constructible_v<_Err> && !is_trivially_move_constructible_v<_Err>) {
if (!__rhs.__has_val_) {
std::construct_at(std::addressof(__union_.__unex_), std::move(__rhs.__union_.__unex_));
}
+ __has_val_ = __rhs.__has_val_;
}
template <class _Up, class _OtherErr>
requires __can_convert<_Up, _OtherErr, const _OtherErr&>::value
_LIBCPP_HIDE_FROM_ABI constexpr explicit(!is_convertible_v<const _OtherErr&, _Err>)
expected(const expected<_Up, _OtherErr>& __rhs)
- noexcept(is_nothrow_constructible_v<_Err, const _OtherErr&>) // strengthened
- : __has_val_(__rhs.__has_val_) {
+ noexcept(is_nothrow_constructible_v<_Err, const _OtherErr&>) { // strengthened
if (!__rhs.__has_val_) {
std::construct_at(std::addressof(__union_.__unex_), __rhs.__union_.__unex_);
}
+ __has_val_ = __rhs.__has_val_;
}
template <class _Up, class _OtherErr>
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_) {
+ noexcept(is_nothrow_constructible_v<_Err, _OtherErr>) { // strengthened
if (!__rhs.__has_val_) {
std::construct_at(std::addressof(__union_.__unex_), std::move(__rhs.__union_.__unex_));
}
+ __has_val_ = __rhs.__has_val_;
}
template <class _OtherErr>
requires is_constructible_v<_Err, const _OtherErr&>
_LIBCPP_HIDE_FROM_ABI constexpr explicit(!is_convertible_v<const _OtherErr&, _Err>)
expected(const unexpected<_OtherErr>& __unex)
- noexcept(is_nothrow_constructible_v<_Err, const _OtherErr&>) // strengthened
- : __has_val_(false) {
+ noexcept(is_nothrow_constructible_v<_Err, const _OtherErr&>) { // strengthened
std::construct_at(std::addressof(__union_.__unex_), __unex.error());
+ __has_val_ = false;
}
template <class _OtherErr>
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) {
+ noexcept(is_nothrow_constructible_v<_Err, _OtherErr>) { // strengthened
std::construct_at(std::addressof(__union_.__unex_), std::move(__unex.error()));
+ __has_val_ = false;
}
_LIBCPP_HIDE_FROM_ABI constexpr explicit expected(in_place_t) noexcept : __has_val_(true) {}
@@ -1062,17 +1064,17 @@ class expected<_Tp, _Err> {
template <class... _Args>
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) {
+ noexcept(is_nothrow_constructible_v<_Err, _Args...>) { // strengthened
std::construct_at(std::addressof(__union_.__unex_), std::forward<_Args>(__args)...);
+ __has_val_ = false;
}
template <class _Up, class... _Args>
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) {
+ noexcept(is_nothrow_constructible_v<_Err, initializer_list<_Up>&, _Args...>) { // strengthened
std::construct_at(std::addressof(__union_.__unex_), __il, std::forward<_Args>(__args)...);
+ __has_val_ = false;
}
private:
diff --git a/libcxx/test/std/utilities/expected/expected.expected/observers/has_value_clobber.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/observers/has_value_clobber.pass.cpp
new file mode 100644
index 000000000000000..ef5499258b7f0ac
--- /dev/null
+++ b/libcxx/test/std/utilities/expected/expected.expected/observers/has_value_clobber.pass.cpp
@@ -0,0 +1,36 @@
+//===----------------------------------------------------------------------===//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// ADDITIONAL_COMPILE_FLAGS: -O2
+
+// constexpr bool has_value() const noexcept;
+
+#include <cassert>
+#include <expected>
+#include <optional>
+
+#include "test_macros.h"
+
+// See https://github.com/llvm/llvm-project/issues/68552
+void testClobberHasValue() {
+ static constexpr auto f1 = [] -> std::expected<std::optional<int>, long> {
+ return 0;
+ };
+
+ static constexpr auto f2 = [] -> std::expected<std::optional<int>, int> {
+ return f1().transform_error([](auto) { return 0; });
+ };
+
+ auto e = f2();
+ assert(e.has_value());
+}
+
+int main(int, char**) {
+ testClobberHasValue();
+ return 0;
+}
More information about the libcxx-commits
mailing list