[libcxx-commits] [libcxx] [libc++] Fix UB in <expected> related to "has value" flag (#68552) (PR #68733)

[Louis Dionne via libcxx-commits]

Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>
Message-ID:
In-Reply-To: <llvm/llvm-project/pull/68733/libcxx at github.com>


================
@@ -43,6 +45,36 @@ constexpr bool test() {
     assert(!e.has_value());
   }
 
+  // The following tests check that the "has_value" flag is not overwritten
+  // by the constructor of the value. This could happen because the flag is
+  // stored in the tail padding of the value.
+  //
+  // The first test is a simplified version of the real code where this was
+  // first observed.
+  //
+  // The other tests use a synthetic struct that clobbers its tail padding
+  // on construction, making the issue easier to reproduce.
+  //
+  // See https://github.com/llvm/llvm-project/issues/68552 and the linked PR.
+  {
+    auto f1 = [] -> std::expected<std::optional<int>, long> { return 0; };
+
+    auto f2 = [&f1] -> std::expected<std::optional<int>, int> {
+      return f1().transform_error([](auto) { return 0; });
+    };
+
+    auto e = f2();
+    assert(e.has_value());
+  }
+  {
+    const std::expected<TailClobberer<0>, bool> e = {};
+    // clang-cl does not support [[no_unique_address]] yet.
+#if !(defined(TEST_COMPILER_CLANG) && defined(_MSC_VER))
+    static_assert(sizeof(TailClobberer<0>) == sizeof(e));
+#endif
----------------
ldionne wrote:

Since this is just a single `static_assert` and it is strongly related to `TailClobberer` (which is defined in this directory), I would be okay with just using `LIBCPP_STATIC_ASSERT(sizeof(TailClobberer<0>) == sizeof(e))` here instead. I think this keeps the test more readable.

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