[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
Sun Oct 15 00:10:15 PDT 2023


jiixyj wrote:

> I think we can still put at least `__has_val_` into the padding bits, since we control that. It should be good enough to add a `char __padding_[sizeof(expected) - __libcpp_datasizeof<expected>::value];` to avoid accidental overwrites of user data.

Ah, so then `__has_val_` would always be the last byte of the `expected`, so `expected` _itself_ would never have any tail padding? I think this is a great idea!

As an alternative, I wonder if the current ABI is salvageable by saving/restoring tail padding in all "dangerous" functions (i.e. swap, emplace, assign, etc.) with something like:

```c++
  struct __tail_padding_saver {
    static constexpr auto __padding_offset = __libcpp_datasizeof<expected>::value;
    static constexpr auto __padding_size = sizeof(expected) - __padding_offset;

    expected *__e;
    unsigned char __pad[__padding_size];

    explicit __tail_padding_saver(expected *__e) : __e(__e) {
      __builtin_memcpy(__pad, reinterpret_cast<unsigned char *>(__e) + __padding_offset, __padding_size);
    }

    ~__tail_padding_saver() {
      __builtin_memcpy(reinterpret_cast<unsigned char *>(__e) + __padding_offset, __pad, __padding_size);
    }
  };
```

Or even have some helper functions

```c++
construct_at_until(std::addressof(__union_.__val_), &__has_val_, std::forward<_Args>(__args)...);
destroy_at_until(std::addressof(__union_.__val_), &__has_val_, std::forward<_Args>(__args)...);
```

...that save/restore all needed bytes "under the hood"?

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


More information about the libcxx-commits mailing list