[libcxx-commits] [libcxx] [libc++][ranges][abi-break] Fix `movable_box` overwriting memory of data that lives in the tail padding (PR #71314)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Fri Dec 15 09:54:15 PST 2023


================
@@ -176,27 +207,29 @@ class __movable_box<_Tp> {
   // Implementation of assignment operators in case we perform optimization (2)
   _LIBCPP_HIDE_FROM_ABI constexpr __movable_box& operator=(__movable_box const& __other) noexcept {
     static_assert(is_nothrow_copy_constructible_v<_Tp>);
+    static_assert(!__can_use_no_unique_address<_Tp>);
     if (this != std::addressof(__other)) {
-      std::destroy_at(std::addressof(__val_));
-      std::construct_at(std::addressof(__val_), __other.__val_);
+      std::destroy_at(std::addressof(this->__val_));
+      std::construct_at(std::addressof(this->__val_), __other.__val_);
     }
     return *this;
   }
 
   _LIBCPP_HIDE_FROM_ABI constexpr __movable_box& operator=(__movable_box&& __other) noexcept {
     static_assert(is_nothrow_move_constructible_v<_Tp>);
+    static_assert(!__can_use_no_unique_address<_Tp>);
     if (this != std::addressof(__other)) {
-      std::destroy_at(std::addressof(__val_));
-      std::construct_at(std::addressof(__val_), std::move(__other.__val_));
+      std::destroy_at(std::addressof(this->__val_));
+      std::construct_at(std::addressof(this->__val_), std::move(__other.__val_));
     }
     return *this;
   }
 
-  _LIBCPP_HIDE_FROM_ABI constexpr _Tp const& operator*() const noexcept { return __val_; }
-  _LIBCPP_HIDE_FROM_ABI constexpr _Tp& operator*() noexcept { return __val_; }
+  _LIBCPP_HIDE_FROM_ABI constexpr _Tp const& operator*() const noexcept { return this->__val_; }
+  _LIBCPP_HIDE_FROM_ABI constexpr _Tp& operator*() noexcept { return this->__val_; }
 
-  _LIBCPP_HIDE_FROM_ABI constexpr const _Tp* operator->() const noexcept { return std::addressof(__val_); }
-  _LIBCPP_HIDE_FROM_ABI constexpr _Tp* operator->() noexcept { return std::addressof(__val_); }
+  _LIBCPP_HIDE_FROM_ABI constexpr const _Tp* operator->() const noexcept { return std::addressof(this->__val_); }
+  _LIBCPP_HIDE_FROM_ABI constexpr _Tp* operator->() noexcept { return std::addressof(this->__val_); }
 
   _LIBCPP_HIDE_FROM_ABI constexpr bool __has_value() const noexcept { return true; }
 };
----------------
ldionne wrote:

Not attached to this line

The following types use `__movable_box`, but they do not actually apply `[[no_unique_address]]` to it (or they have a following member that is not `[[no_unique_address]]`), which means that they don't technically have a bug right now:

```
single_view
repeat_view
chunk_by_view
```

We should take the opportunity of this ABI break to make them take advantage of the optimization, with a test.

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


More information about the libcxx-commits mailing list