Skip to content

[libc++] std::expected "has value" flag getting clobbered unexpectedly #68552

@jiixyj

Description

@jiixyj

In some situations it seems that the "has value" flag of a std::expected gets overwritten when the value is constructed. Example:

#undef NDEBUG
#include <cassert>

#include <expected>
#include <optional>

std::expected<std::optional<int>, long> f1()
{
    return 0;
}

std::expected<std::optional<int>, int> f2()
{
    return f1().transform_error([](auto) { return 0; });
}

int main()
{
    auto r = f2();
    assert(r.has_value());

Godbolt: https://godbolt.org/z/nbcPr9e5f

I would expect that the assert is not triggered, but it is. I'm testing with commit d408770.

I strongly suspect code like this, i.e. where the call to std::construct_at happens after setting the "has value" flag:

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) {
std::construct_at(std::addressof(__union_.__val_), std::forward<_Args>(__args)...);
}

In the above example, __has_val_ (a bool) and __union_.__val_ (the std::optional<int>) overlap. I think std::construct_at is allowed to overwrite __has_val_ in this case by the language rules as the std::construct_at has no idea that the __has_val_ is there. It just assumes it has full 8 bytes to work with to construct the std::optional<int>.

The data layout of the problematic std::expected<std::optional<int>, int> looks like this:

            +-- optional "has value" flag
            |     +--+--padding
/---int---\ |     |  |
00 00 00 00 01 00 00 00
               |
               |
               +- expected "has value" flag

The expected's "has value" flag reuses one byte of the padding. It should be "1" in this case but is "0".

Suggested fix: move all assignments to the "has value" flag after the std::construct_at calls like this:

  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
  {
    std::construct_at(std::addressof(__union_.__val_), std::forward<_Args>(__args)...);
    __has_val = true;
  }

I wonder if this analysis is correct or if I'm missing some subtleties with std::construct_at.

Metadata

Metadata

Assignees

Labels

ABIApplication Binary Interfacelibc++libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions