[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