[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
Sat Oct 14 11:37:18 PDT 2023


jiixyj wrote:

> Thanks for spotting the issue and send the PR. I am still a bit nervous about object overlapping. Since `expected` can expose its data through several members like `operator*`, `operator->`, `value()`, `error()`, etc.... Could users trigger this UB by other means that we can not stop from. like
> 
> ```
> std::expected<T, U> e = ...;
> e->someMemberThatCouldWriteToObjectThatChangesHasValue()
> ```

I guess `someMemberThatCouldWriteToObjectThatChangesHasValue()` itself is UB already, because every type must assume that there is something important stored in its tail padding.

> 
> or even,
> 
> ```
> std::expected<T, U> e = ...;
> std::construct_at( std::address_of(*e), args...);
> ```
> 
> What do you think?

I now wonder if `std::construct_at` / placement new / the constructor is allowed to overwrite the tail padding. I initially assumed it was, but I just realized the following:

```c++
    static_assert(sizeof(std::expected<std::optional<int>, int>) == sizeof(std::optional<int>));
    static_assert(sizeof(std::expected<std::expected<std::optional<int>, int>, int>) == sizeof(std::optional<int>));
    static_assert(sizeof(std::expected<std::expected<std::expected<std::optional<int>, int>, int>, int>) == sizeof(std::optional<int>));
```

...i.e. that `std::expected<std::optional<int>, int>` still has 2 bytes of tail padding left that can themselves be used by two more "layers" of `std::expected`. So does the `std::destruct_at`/`std::construct_at` magic from the "innermost" `std::expected` need to ensure that _all_ tail padding bytes are left alone? How would that look like? Or can the logic just assume that destructors/constructors won't touch the tail padding bytes?

Maybe the initial UB wasn't from the order of `std::construct_at` and the "has value" flag, but because of missing `std::launder` calls or something that allowed the optimizer to be a bit too aggressive?

https://github.com/llvm/llvm-project/pull/68733


More information about the libcxx-commits mailing list