[libcxx-commits] [libcxx] [libc++] Ensure that `std::expected` has no tail padding (PR #69673)

[Jan Kokemüller via libcxx-commits]

https://github.com/jiixyj created https://github.com/llvm/llvm-project/pull/69673

Currently `std::expected` can have some padding bytes in its tail due to `[[no_unique_address]]`. Those padding bytes can be used by other objects. For example, in the current implementation:

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

...so the data layout of an `std::expected<std::expected<std::optional<int>, bool>, bool>` can look like this:

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

```

This is problematic because `emplace()`ing the "inner" expected can not only overwrite the "inner" expected "has value" flag (issue <https://github.com/llvm/llvm-project/issues/68552>) but _also_ the tail padding where other objects might live.

@philnik777 proposed to add a "char __padding_[]" array to the end of the `expected` to make sure that the `expected` itself never has any tail padding bytes that might get used by other objects.

This is an ABI breaking change because
`sizeof(std::expected<std::optional<int>, bool>) < sizeof(std::expected<std::expected<std::optional<int>, bool>, bool>);` afterwards. The data layout will change in the following cases where tail padding can be reused by other objects:

```c++
class foo : std::expected<std::optional<int>, bool> {
  bool b;
};
```

or using `[[no_unique_address]]`:

```c++
struct foo {
  [[no_unique_address]] std::expected<std::optional<int>, bool> e;
  bool b;
};
```

>From 74b266db78accc3f23e0cc1d96aa7277eeccdb2f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= <jan.kokemueller at gmail.com>
Date: Sun, 15 Oct 2023 22:09:51 +0200
Subject: [PATCH] ensure that expected has no tail padding

---
 libcxx/include/__expected/expected.h          | 20 +++++++++++++++++++
 .../expected.expected/assign/emplace.pass.cpp | 16 +++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/libcxx/include/__expected/expected.h b/libcxx/include/__expected/expected.h
index 045370a486fae6b..463060702235742 100644
--- a/libcxx/include/__expected/expected.h
+++ b/libcxx/include/__expected/expected.h
@@ -18,6 +18,7 @@
 #include <__memory/addressof.h>
 #include <__memory/construct_at.h>
 #include <__type_traits/conjunction.h>
+#include <__type_traits/datasizeof.h>
 #include <__type_traits/disjunction.h>
 #include <__type_traits/integral_constant.h>
 #include <__type_traits/is_assignable.h>
@@ -955,8 +956,17 @@ class expected {
     _LIBCPP_NO_UNIQUE_ADDRESS _ErrorType __unex_;
   };
 
+  _LIBCPP_HIDE_FROM_ABI static constexpr auto __calculate_padding() {
+    struct __calc_expected {
+      _LIBCPP_NO_UNIQUE_ADDRESS __union_t<_Tp, _Err> __union_;
+      bool __has_val_;
+    };
+    return sizeof(__calc_expected) - __libcpp_datasizeof<__calc_expected>::value;
+  }
+
   _LIBCPP_NO_UNIQUE_ADDRESS __union_t<_Tp, _Err> __union_;
   bool __has_val_;
+  char __padding_[__calculate_padding()]{};
 };
 
 template <class _Tp, class _Err>
@@ -1552,8 +1562,18 @@ class expected<_Tp, _Err> {
     _LIBCPP_NO_UNIQUE_ADDRESS _ErrorType __unex_;
   };
 
+
+  _LIBCPP_HIDE_FROM_ABI static constexpr auto __calculate_padding() {
+    struct __calc_expected {
+      _LIBCPP_NO_UNIQUE_ADDRESS __union_t<_Err> __union_;
+      bool __has_val_;
+    };
+    return sizeof(__calc_expected) - __libcpp_datasizeof<__calc_expected>::value;
+  }
+
   _LIBCPP_NO_UNIQUE_ADDRESS __union_t<_Err> __union_;
   bool __has_val_;
+  char __padding_[__calculate_padding()]{};
 };
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/test/std/utilities/expected/expected.expected/assign/emplace.pass.cpp b/libcxx/test/std/utilities/expected/expected.expected/assign/emplace.pass.cpp
index c62e6289350201a..5be8d2b5fb74e40 100644
--- a/libcxx/test/std/utilities/expected/expected.expected/assign/emplace.pass.cpp
+++ b/libcxx/test/std/utilities/expected/expected.expected/assign/emplace.pass.cpp
@@ -73,6 +73,22 @@ constexpr bool test() {
     assert(e.value() == 10);
   }
 
+  // Test that `expected` has no tail padding.
+  {
+    struct BoolWithPadding {
+      explicit operator bool() { return b; }
+
+    private:
+      alignas(4) bool b = false;
+    };
+
+    static_assert(sizeof(std::expected<BoolWithPadding, bool>) ==
+                  std::__libcpp_datasizeof<std::expected<BoolWithPadding, bool>>::value);
+
+    static_assert(sizeof(std::expected<void, BoolWithPadding>) ==
+                  std::__libcpp_datasizeof<std::expected<void, BoolWithPadding>>::value);
+  }
+
   return true;
 }