Skip to content

Commit

Permalink
[libc++] Fix UB in <expected> related to "has value" flag (llvm#68552) (
Browse files Browse the repository at this point in the history
llvm#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 llvm#68552
  • Loading branch information
jiixyj committed Oct 30, 2023
1 parent 849f963 commit 134c915
Show file tree
Hide file tree
Showing 30 changed files with 534 additions and 162 deletions.
177 changes: 82 additions & 95 deletions libcxx/include/__expected/expected.h

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ constexpr bool test() {
assert(e.value().i == 10);
}

// TailClobberer
{
std::expected<TailClobberer<0>, bool> e(std::unexpect);
auto list = {4, 5, 6};
e.emplace(list);
assert(e.has_value());
}

return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ constexpr bool test() {
assert(e.value() == 10);
}

// TailClobberer
{
std::expected<TailClobberer<0>, bool> e(std::unexpect);
e.emplace();
assert(e.has_value());
}

return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include <utility>

#include "test_macros.h"
#include "../../types.h"

// Test Constraints:
template <class T1, class Err1, class T2, class Err2>
Expand Down Expand Up @@ -161,13 +162,19 @@ constexpr bool test() {
assert(e1.error() == 5);
}

// convert TailClobberer
{
const std::expected<TailClobbererNonTrivialMove<0>, char> e1;
std::expected<TailClobberer<0>, 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{}; }
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@

#include "MoveOnly.h"
#include "test_macros.h"
#include "../../types.h"

// Test Constraints:
template <class T1, class Err1, class T2, class Err2>
Expand Down Expand Up @@ -160,13 +161,19 @@ constexpr bool test() {
assert(e1.error().get() == 0);
}

// convert TailClobberer
{
std::expected<TailClobbererNonTrivialMove<0>, char> e1;
std::expected<TailClobberer<0>, 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{}; }
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <utility>

#include "test_macros.h"
#include "../../types.h"

struct NonCopyable {
NonCopyable(const NonCopyable&) = delete;
Expand Down Expand Up @@ -93,13 +94,26 @@ constexpr bool test() {
assert(!e2.has_value());
assert(e2.error() == 5);
}

// copy TailClobberer as value
{
const std::expected<TailClobberer<0>, bool> e1;
auto e2 = e1;
assert(e2.has_value());
}

// copy TailClobberer as error
{
const std::expected<bool, TailClobberer<1>> 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{}; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <type_traits>

#include "test_macros.h"
#include "../../types.h"

struct NoDedefaultCtor {
NoDedefaultCtor() = delete;
Expand All @@ -45,20 +46,20 @@ constexpr void testDefaultCtor() {

template <class T>
constexpr void testTypes() {
testDefaultCtor<T, bool>();
testDefaultCtor<T, int>();
testDefaultCtor<T, NoDedefaultCtor>();
}

constexpr bool test() {
testTypes<int>();
testTypes<MyInt>();
testTypes<TailClobberer<0>>();
return true;
}

void testException() {
#ifndef TEST_HAS_NO_EXCEPTIONS
struct Except {};

struct Throwing {
Throwing() { throw Except{}; };
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include "MoveOnly.h"
#include "test_macros.h"
#include "../../types.h"

// Test Constraints:
static_assert(std::is_constructible_v<std::expected<int, int>, std::in_place_t>);
Expand Down Expand Up @@ -54,24 +55,24 @@ struct CopyOnly {
friend constexpr bool operator==(const CopyOnly& mi, int ii) { return mi.i == ii; }
};

template <class T>
template <class T, class E = int>
constexpr void testInt() {
std::expected<T, int> e(std::in_place, 5);
std::expected<T, E> e(std::in_place, 5);
assert(e.has_value());
assert(e.value() == 5);
}

template <class T>
template <class T, class E = int>
constexpr void testLValue() {
T t(5);
std::expected<T, int> e(std::in_place, t);
std::expected<T, E> e(std::in_place, t);
assert(e.has_value());
assert(e.value() == 5);
}

template <class T>
template <class T, class E = int>
constexpr void testRValue() {
std::expected<T, int> e(std::in_place, T(5));
std::expected<T, E> e(std::in_place, T(5));
assert(e.has_value());
assert(e.value() == 5);
}
Expand All @@ -80,10 +81,13 @@ constexpr bool test() {
testInt<int>();
testInt<CopyOnly>();
testInt<MoveOnly>();
testInt<TailClobberer<0>, bool>();
testLValue<int>();
testLValue<CopyOnly>();
testLValue<TailClobberer<0>, bool>();
testRValue<int>();
testRValue<MoveOnly>();
testRValue<TailClobberer<0>, bool>();

// no arg
{
Expand Down Expand Up @@ -111,8 +115,6 @@ constexpr bool test() {

void testException() {
#ifndef TEST_HAS_NO_EXCEPTIONS
struct Except {};

struct Throwing {
Throwing(int) { throw Except{}; };
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

#include "MoveOnly.h"
#include "test_macros.h"
#include "../../types.h"

// Test Constraints:
static_assert(
Expand Down Expand Up @@ -90,13 +91,17 @@ constexpr bool test() {
assert(m.get() == 0);
}

// TailClobberer
{
std::expected<TailClobberer<0>, 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>, int) { throw Except{}; };
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include <utility>

#include "test_macros.h"
#include "../../types.h"

struct NonMovable {
NonMovable(NonMovable&&) = delete;
Expand Down Expand Up @@ -112,13 +113,28 @@ constexpr bool test() {
assert(e2.error() == 5);
assert(!e1.has_value());
}

// move TailClobbererNonTrivialMove as value
{
std::expected<TailClobbererNonTrivialMove<0>, bool> e1;
auto e2 = std::move(e1);
assert(e2.has_value());
assert(e1.has_value());
}

// move TailClobbererNonTrivialMove as error
{
std::expected<bool, TailClobbererNonTrivialMove<1>> 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{}; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

#include "MoveOnly.h"
#include "test_macros.h"
#include "../../types.h"

// Test Constraints:
static_assert(std::is_constructible_v<std::expected<int, int>, int>);
Expand Down Expand Up @@ -70,24 +71,24 @@ struct CopyOnly {
struct BaseError {};
struct DerivedError : BaseError {};

template <class T>
template <class T, class E = int>
constexpr void testInt() {
std::expected<T, int> e(5);
std::expected<T, E> e(5);
assert(e.has_value());
assert(e.value() == 5);
}

template <class T>
template <class T, class E = int>
constexpr void testLValue() {
T t(5);
std::expected<T, int> e(t);
std::expected<T, E> e(t);
assert(e.has_value());
assert(e.value() == 5);
}

template <class T>
template <class T, class E = int>
constexpr void testRValue() {
std::expected<T, int> e(T(5));
std::expected<T, E> e(T(5));
assert(e.has_value());
assert(e.value() == 5);
}
Expand All @@ -96,10 +97,13 @@ constexpr bool test() {
testInt<int>();
testInt<CopyOnly>();
testInt<MoveOnly>();
testInt<TailClobberer<0>, bool>();
testLValue<int>();
testLValue<CopyOnly>();
testLValue<TailClobberer<0>, bool>();
testRValue<int>();
testRValue<MoveOnly>();
testRValue<TailClobberer<0>, bool>();

// Test default template argument.
// Without it, the template parameter cannot be deduced from an initializer list
Expand Down Expand Up @@ -153,8 +157,6 @@ constexpr bool test() {

void testException() {
#ifndef TEST_HAS_NO_EXCEPTIONS
struct Except {};

struct Throwing {
Throwing(int) { throw Except{}; };
};
Expand Down

0 comments on commit 134c915

Please sign in to comment.