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

via libcxx-commits libcxx-commits at lists.llvm.org
Mon Jan 1 23:03:10 PST 2024


Author: Hui
Date: 2024-01-02T07:03:06Z
New Revision: 120b0bfbf0bade430fa9b19d78025ccd1d6148d0

URL: https://github.com/llvm/llvm-project/commit/120b0bfbf0bade430fa9b19d78025ccd1d6148d0
DIFF: https://github.com/llvm/llvm-project/commit/120b0bfbf0bade430fa9b19d78025ccd1d6148d0.diff

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

fixes #70506 

The detailed problem description is in #70506 

The original proposed fix was to remove `[[no_unique_address]]` except
when `_Tp` is empty.

Edit:
After the discussion in the comments below, the new fix here is to
remove the `[[no_unique_address]]` from `movable_box` in the cases where
we need to add our own assignment operator, which has contains the
problematic `construct_at`

Added: 
    libcxx/test/libcxx/ranges/range.adaptors/range.chunk.by/no_unique_address.compile.pass.cpp
    libcxx/test/libcxx/ranges/range.adaptors/range.move.wrap/empty_object.pass.cpp
    libcxx/test/libcxx/ranges/range.factories/range.repeat.view/no_unique_address.compile.pass.cpp
    libcxx/test/libcxx/ranges/range.factories/range.single.view/no_unique_address.compile.pass.cpp
    libcxx/test/std/ranges/ranges_robust_against_no_unique_address.pass.cpp

Modified: 
    libcxx/docs/ReleaseNotes/18.rst
    libcxx/include/__config
    libcxx/include/__ranges/chunk_by_view.h
    libcxx/include/__ranges/drop_while_view.h
    libcxx/include/__ranges/filter_view.h
    libcxx/include/__ranges/movable_box.h
    libcxx/include/__ranges/repeat_view.h
    libcxx/include/__ranges/single_view.h
    libcxx/include/__ranges/take_while_view.h
    libcxx/include/__ranges/transform_view.h
    libcxx/test/libcxx/ranges/range.adaptors/range.move.wrap/no_unique_address.pass.cpp
    libcxx/test/std/ranges/range.adaptors/range.lazy.split/ctor.range.pass.cpp
    libcxx/test/std/ranges/range.adaptors/range.transform/general.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index fa60a581652d6a..f5cda9aaa5dcb0 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -170,6 +170,18 @@ ABI Affecting Changes
   to throw a 
diff erent exception when attempting allocations that are too large
   (``std::bad_alloc`` vs ``std::length_error``).
 
+- The layout of some views inside ``std::ranges`` that use the ``movable-box`` exposition-only type as an implementation 
+  detail has changed in order to fix a bug which could result in overwriting user data following the ``movable-box``
+  <https://github.com/llvm/llvm-project/issues/70506>. 
+  This was caused by incorrect usage of the ``[[no_unique_address]]`` attribute inside the implementation of ``movable-box``. 
+  This only affects the layout of the following views: ``take_while_view``, ``filter_view``, ``single_view``, ``drop_while_view``, 
+  ``repeat_view``, ``transform_view``, ``chunk_by_view``. In order to avoid silent breakage, an ABI tag has been added to 
+  these views such that their mangled name will be 
diff erent starting in this version of libc++. 
+  As a result, attempting to call a function that expects one of these views will fail to link until the code has been rebuilt 
+  against a matching version of libc++. In practice, we believe it is unusual for these views to appear at ABI boundaries so this 
+  should not be a major problem for most users. However it is probably worth auditing ranges-heavy code for ABI boundaries that 
+  would contain these views, or for types that contain these views as members and which are passed across ABI boundaries.
+
 Build System Changes
 --------------------
 

diff  --git a/libcxx/include/__config b/libcxx/include/__config
index adff13e714cb64..40e6da8bc03afa 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -200,6 +200,16 @@
 #    define _LIBCPP_ABI_BAD_FUNCTION_CALL_KEY_FUNCTION
 #  endif
 
+// We had some bugs where we use [[no_unique_address]] together with construct_at,
+// which causes UB as the call on construct_at could write to overlapping subobjects
+//
+// https://github.com/llvm/llvm-project/issues/70506
+// https://github.com/llvm/llvm-project/issues/70494
+//
+// To fix the bug we had to change the ABI of some classes to remove [[no_unique_address]] under certain conditions.
+// The below macro is used for all classes whose ABI have changed as part of fixing these bugs.
+#  define _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG __attribute__((__abi_tag__("subobj_fix_2023")))
+
 // Changes the iterator type of select containers (see below) to a bounded iterator that keeps track of whether it's
 // within the bounds of the original container and asserts it on every dereference.
 //

diff  --git a/libcxx/include/__ranges/chunk_by_view.h b/libcxx/include/__ranges/chunk_by_view.h
index e4699868764698..3ecc018cac9d91 100644
--- a/libcxx/include/__ranges/chunk_by_view.h
+++ b/libcxx/include/__ranges/chunk_by_view.h
@@ -54,7 +54,8 @@ namespace ranges {
 
 template <forward_range _View, indirect_binary_predicate<iterator_t<_View>, iterator_t<_View>> _Pred>
   requires view<_View> && is_object_v<_Pred>
-class chunk_by_view : public view_interface<chunk_by_view<_View, _Pred>> {
+class _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG chunk_by_view
+    : public view_interface<chunk_by_view<_View, _Pred>> {
   _LIBCPP_NO_UNIQUE_ADDRESS _View __base_ = _View();
   _LIBCPP_NO_UNIQUE_ADDRESS __movable_box<_Pred> __pred_;
 

diff  --git a/libcxx/include/__ranges/drop_while_view.h b/libcxx/include/__ranges/drop_while_view.h
index 677b5bc66d442a..eb3783eb42f146 100644
--- a/libcxx/include/__ranges/drop_while_view.h
+++ b/libcxx/include/__ranges/drop_while_view.h
@@ -45,7 +45,8 @@ namespace ranges {
 
 template <view _View, class _Pred>
   requires input_range<_View> && is_object_v<_Pred> && indirect_unary_predicate<const _Pred, iterator_t<_View>>
-class drop_while_view : public view_interface<drop_while_view<_View, _Pred>> {
+class _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG drop_while_view
+    : public view_interface<drop_while_view<_View, _Pred>> {
 public:
   _LIBCPP_HIDE_FROM_ABI drop_while_view()
     requires default_initializable<_View> && default_initializable<_Pred>

diff  --git a/libcxx/include/__ranges/filter_view.h b/libcxx/include/__ranges/filter_view.h
index 08d50ab011042b..868ad128e8944a 100644
--- a/libcxx/include/__ranges/filter_view.h
+++ b/libcxx/include/__ranges/filter_view.h
@@ -51,7 +51,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 namespace ranges {
 template <input_range _View, indirect_unary_predicate<iterator_t<_View>> _Pred>
   requires view<_View> && is_object_v<_Pred>
-class filter_view : public view_interface<filter_view<_View, _Pred>> {
+class _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG filter_view : public view_interface<filter_view<_View, _Pred>> {
   _LIBCPP_NO_UNIQUE_ADDRESS _View __base_ = _View();
   _LIBCPP_NO_UNIQUE_ADDRESS __movable_box<_Pred> __pred_;
 

diff  --git a/libcxx/include/__ranges/movable_box.h b/libcxx/include/__ranges/movable_box.h
index 6615533d374349..9b38877494eaa4 100644
--- a/libcxx/include/__ranges/movable_box.h
+++ b/libcxx/include/__ranges/movable_box.h
@@ -134,6 +134,20 @@ concept __doesnt_need_empty_state =
          // 2. Otherwise, movable-box<T> should store only a T if either T models movable or
          //    is_nothrow_move_constructible_v<T> is true.
          : movable<_Tp> || is_nothrow_move_constructible_v<_Tp>);
+
+// When _Tp doesn't have an assignment operator, we must implement __movable_box's assignment operator
+// by doing destroy_at followed by construct_at. However, that implementation strategy leads to UB if the nested
+// _Tp is potentially overlapping, as it is doing a non-transparent replacement of the sub-object, which means that
+// we're not considered "nested" inside the movable-box anymore, and since we're not nested within it, [basic.life]/1.5
+// says that we essentially just reused the storage of the movable-box for a completely unrelated object and ended the
+// movable-box's lifetime.
+// https://github.com/llvm/llvm-project/issues/70494#issuecomment-1845646490
+//
+// Hence, when the _Tp doesn't have an assignment operator, we can't risk making it a potentially-overlapping
+// subobject because of the above, and we don't use [[no_unique_address]] in that case.
+template <class _Tp>
+concept __can_use_no_unique_address = (copy_constructible<_Tp> ? copyable<_Tp> : movable<_Tp>);
+
 #  else
 
 template <class _Tp>
@@ -144,23 +158,45 @@ concept __doesnt_need_empty_state_for_move = movable<_Tp> || is_nothrow_move_con
 
 template <class _Tp>
 concept __doesnt_need_empty_state = __doesnt_need_empty_state_for_copy<_Tp> && __doesnt_need_empty_state_for_move<_Tp>;
+
+template <class _Tp>
+concept __can_use_no_unique_address = copyable<_Tp>;
 #  endif
 
+template <class _Tp>
+struct __movable_box_holder {
+  _Tp __val_;
+
+  template <class... _Args>
+  _LIBCPP_HIDE_FROM_ABI constexpr explicit __movable_box_holder(in_place_t, _Args&&... __args)
+      : __val_(std::forward<_Args>(__args)...) {}
+};
+
+template <class _Tp>
+  requires __can_use_no_unique_address<_Tp>
+struct __movable_box_holder<_Tp> {
+  _LIBCPP_NO_UNIQUE_ADDRESS _Tp __val_;
+
+  template <class... _Args>
+  _LIBCPP_HIDE_FROM_ABI constexpr explicit __movable_box_holder(in_place_t, _Args&&... __args)
+      : __val_(std::forward<_Args>(__args)...) {}
+};
+
 template <__movable_box_object _Tp>
   requires __doesnt_need_empty_state<_Tp>
 class __movable_box<_Tp> {
-  _LIBCPP_NO_UNIQUE_ADDRESS _Tp __val_;
+  _LIBCPP_NO_UNIQUE_ADDRESS __movable_box_holder<_Tp> __holder_;
 
 public:
   template <class... _Args>
     requires is_constructible_v<_Tp, _Args...>
-  _LIBCPP_HIDE_FROM_ABI constexpr explicit __movable_box(in_place_t, _Args&&... __args) noexcept(
+  _LIBCPP_HIDE_FROM_ABI constexpr explicit __movable_box(in_place_t __inplace, _Args&&... __args) noexcept(
       is_nothrow_constructible_v<_Tp, _Args...>)
-      : __val_(std::forward<_Args>(__args)...) {}
+      : __holder_(__inplace, std::forward<_Args>(__args)...) {}
 
   _LIBCPP_HIDE_FROM_ABI constexpr __movable_box() noexcept(is_nothrow_default_constructible_v<_Tp>)
     requires default_initializable<_Tp>
-      : __val_() {}
+      : __holder_(in_place_t{}) {}
 
   _LIBCPP_HIDE_FROM_ABI __movable_box(__movable_box const&) = default;
   _LIBCPP_HIDE_FROM_ABI __movable_box(__movable_box&&)      = default;
@@ -176,27 +212,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(__holder_.__val_));
+      std::construct_at(std::addressof(__holder_.__val_), __other.__holder_.__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(__holder_.__val_));
+      std::construct_at(std::addressof(__holder_.__val_), std::move(__other.__holder_.__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 __holder_.__val_; }
+  _LIBCPP_HIDE_FROM_ABI constexpr _Tp& operator*() noexcept { return __holder_.__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(__holder_.__val_); }
+  _LIBCPP_HIDE_FROM_ABI constexpr _Tp* operator->() noexcept { return std::addressof(__holder_.__val_); }
 
   _LIBCPP_HIDE_FROM_ABI constexpr bool __has_value() const noexcept { return true; }
 };

diff  --git a/libcxx/include/__ranges/repeat_view.h b/libcxx/include/__ranges/repeat_view.h
index 459a1e229613ac..479eca96acb098 100644
--- a/libcxx/include/__ranges/repeat_view.h
+++ b/libcxx/include/__ranges/repeat_view.h
@@ -68,7 +68,7 @@ struct __fn;
 template <move_constructible _Tp, semiregular _Bound = unreachable_sentinel_t>
   requires(is_object_v<_Tp> && same_as<_Tp, remove_cv_t<_Tp>> &&
            (__integer_like_with_usable_
diff erence_type<_Bound> || same_as<_Bound, unreachable_sentinel_t>))
-class repeat_view : public view_interface<repeat_view<_Tp, _Bound>> {
+class _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG repeat_view : public view_interface<repeat_view<_Tp, _Bound>> {
   friend struct views::__take::__fn;
   friend struct views::__drop::__fn;
   class __iterator;
@@ -119,7 +119,7 @@ class repeat_view : public view_interface<repeat_view<_Tp, _Bound>> {
   }
 
 private:
-  __movable_box<_Tp> __value_;
+  _LIBCPP_NO_UNIQUE_ADDRESS __movable_box<_Tp> __value_;
   _LIBCPP_NO_UNIQUE_ADDRESS _Bound __bound_ = _Bound();
 };
 

diff  --git a/libcxx/include/__ranges/single_view.h b/libcxx/include/__ranges/single_view.h
index b0b2c1d9f3c062..0ae2036a66a92b 100644
--- a/libcxx/include/__ranges/single_view.h
+++ b/libcxx/include/__ranges/single_view.h
@@ -37,8 +37,8 @@ template <move_constructible _Tp>
 template <copy_constructible _Tp>
 #  endif
   requires is_object_v<_Tp>
-class single_view : public view_interface<single_view<_Tp>> {
-  __movable_box<_Tp> __value_;
+class _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG single_view : public view_interface<single_view<_Tp>> {
+  _LIBCPP_NO_UNIQUE_ADDRESS __movable_box<_Tp> __value_;
 
 public:
   _LIBCPP_HIDE_FROM_ABI single_view()

diff  --git a/libcxx/include/__ranges/take_while_view.h b/libcxx/include/__ranges/take_while_view.h
index a6f7f80ca76bfb..4534210d97943e 100644
--- a/libcxx/include/__ranges/take_while_view.h
+++ b/libcxx/include/__ranges/take_while_view.h
@@ -43,7 +43,8 @@ namespace ranges {
 
 template <view _View, class _Pred>
   requires input_range<_View> && is_object_v<_Pred> && indirect_unary_predicate<const _Pred, iterator_t<_View>>
-class take_while_view : public view_interface<take_while_view<_View, _Pred>> {
+class _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG take_while_view
+    : public view_interface<take_while_view<_View, _Pred>> {
   template <bool>
   class __sentinel;
 

diff  --git a/libcxx/include/__ranges/transform_view.h b/libcxx/include/__ranges/transform_view.h
index 55c6ce587bd69e..744f597ccef593 100644
--- a/libcxx/include/__ranges/transform_view.h
+++ b/libcxx/include/__ranges/transform_view.h
@@ -67,7 +67,8 @@ template <input_range _View, move_constructible _Fn>
 template <input_range _View, copy_constructible _Fn>
 #  endif
   requires __transform_view_constraints<_View, _Fn>
-class transform_view : public view_interface<transform_view<_View, _Fn>> {
+class _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG transform_view
+    : public view_interface<transform_view<_View, _Fn>> {
   template <bool>
   class __iterator;
   template <bool>

diff  --git a/libcxx/test/libcxx/ranges/range.adaptors/range.chunk.by/no_unique_address.compile.pass.cpp b/libcxx/test/libcxx/ranges/range.adaptors/range.chunk.by/no_unique_address.compile.pass.cpp
new file mode 100644
index 00000000000000..e696598f618ba9
--- /dev/null
+++ b/libcxx/test/libcxx/ranges/range.adaptors/range.chunk.by/no_unique_address.compile.pass.cpp
@@ -0,0 +1,46 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// XFAIL: msvc
+
+// This test ensures that we use `[[no_unique_address]]` in `chunk_by_view`.
+
+#include <ranges>
+
+struct View : std::ranges::view_base {
+  int* begin() const;
+  int* end() const;
+};
+
+struct Pred {
+  template <class... Args>
+  bool operator()(const Args&...) const;
+};
+
+template <class View>
+struct Test {
+  [[no_unique_address]] View view;
+  char c;
+};
+
+// [[no_unique_address]] applied to _View
+struct ViewWithPadding : View {
+  alignas(128) char c;
+};
+
+static_assert(sizeof(Test<std::ranges::chunk_by_view<ViewWithPadding, Pred>>) ==
+              sizeof(std::ranges::chunk_by_view<ViewWithPadding, Pred>));
+
+// [[no_unique_address]] applied to movable-box
+struct PredWithPadding : Pred {
+  alignas(128) char c;
+};
+
+static_assert(sizeof(Test<std::ranges::chunk_by_view<View, PredWithPadding>>) ==
+              sizeof(std::ranges::chunk_by_view<View, PredWithPadding>));

diff  --git a/libcxx/test/libcxx/ranges/range.adaptors/range.move.wrap/empty_object.pass.cpp b/libcxx/test/libcxx/ranges/range.adaptors/range.move.wrap/empty_object.pass.cpp
new file mode 100644
index 00000000000000..9712635a0010b6
--- /dev/null
+++ b/libcxx/test/libcxx/ranges/range.adaptors/range.move.wrap/empty_object.pass.cpp
@@ -0,0 +1,56 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// This test ensures that <movable-box> behaves correctly when it holds an empty type.
+
+#include <ranges>
+
+#include <cassert>
+#include <utility>
+
+bool copied = false;
+bool moved  = false;
+
+struct Empty {
+  Empty() noexcept {}
+  Empty(Empty const&) noexcept { copied = true; }
+  Empty(Empty&&) noexcept { moved = true; }
+  Empty& operator=(Empty const&) = delete;
+  Empty& operator=(Empty&&)      = delete;
+};
+
+using Box = std::ranges::__movable_box<Empty>;
+
+struct Inherit : Box {};
+
+struct Hold : Box {
+  [[no_unique_address]] Inherit member;
+};
+
+int main(int, char**) {
+  Hold box;
+
+  Box& base   = static_cast<Box&>(box);
+  Box& member = static_cast<Box&>(box.member);
+
+  // Despite [[no_unique_address]], the two objects have the same type so they
+  // can't share the same address.
+  assert(&base != &member);
+
+  // Make sure that we do perform the copy-construction, which wouldn't be the
+  // case if the two <movable-box>s had the same address.
+  base = member;
+  assert(copied);
+
+  base = std::move(member);
+  assert(moved);
+
+  return 0;
+}

diff  --git a/libcxx/test/libcxx/ranges/range.adaptors/range.move.wrap/no_unique_address.pass.cpp b/libcxx/test/libcxx/ranges/range.adaptors/range.move.wrap/no_unique_address.pass.cpp
index e21191f98dea3e..4ec6c888f9c1ca 100644
--- a/libcxx/test/libcxx/ranges/range.adaptors/range.move.wrap/no_unique_address.pass.cpp
+++ b/libcxx/test/libcxx/ranges/range.adaptors/range.move.wrap/no_unique_address.pass.cpp
@@ -8,49 +8,71 @@
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 
-// This test ensures that <copyable-box> behaves correctly when it holds an empty type.
+// clang-cl and cl currently don't support [[no_unique_address]]
+// XFAIL: msvc
 
 #include <ranges>
 
 #include <cassert>
 #include <utility>
 
-bool copied = false;
-bool moved  = false;
+#include "test_macros.h"
 
-struct Empty {
-  Empty() noexcept {}
-  Empty(Empty const&) noexcept { copied = true; }
-  Empty(Empty&&) noexcept { moved = true; }
-  Empty& operator=(Empty const&) = delete;
-  Empty& operator=(Empty&&)      = delete;
-};
+template <class T, bool ExpectNoUniqueAddress>
+void test_no_unique_address() {
+  struct Test {
+    [[no_unique_address]] std::ranges::__movable_box<T> box_;
+    bool b2;
+  };
 
-using Box = std::ranges::__movable_box<Empty>;
+  if constexpr (ExpectNoUniqueAddress) {
+    static_assert(sizeof(Test) == sizeof(bool));
+  } else {
+    static_assert(sizeof(Test) > sizeof(bool));
+  }
+}
 
-struct Inherit : Box {};
+struct Copyable {};
 
-struct Hold : Box {
-  [[no_unique_address]] Inherit member;
+struct NotCopyAssignable {
+  constexpr NotCopyAssignable()                          = default;
+  constexpr NotCopyAssignable(const NotCopyAssignable&)  = default;
+  NotCopyAssignable& operator=(const NotCopyAssignable&) = delete;
 };
 
-int main(int, char**) {
-  Hold box;
+struct NotMoveAssignable {
+  constexpr NotMoveAssignable()                          = default;
+  constexpr NotMoveAssignable(const NotMoveAssignable&)  = default;
+  NotMoveAssignable& operator=(const NotMoveAssignable&) = default;
+  constexpr NotMoveAssignable(NotMoveAssignable&&)       = default;
+  NotMoveAssignable& operator=(NotMoveAssignable&&)      = delete;
+};
 
-  Box& base   = static_cast<Box&>(box);
-  Box& member = static_cast<Box&>(box.member);
+struct MoveOnly {
+  constexpr MoveOnly()                 = default;
+  constexpr MoveOnly(const MoveOnly&)  = delete;
+  MoveOnly& operator=(const MoveOnly&) = delete;
+  constexpr MoveOnly(MoveOnly&&)       = default;
+  MoveOnly& operator=(MoveOnly&&)      = default;
+};
 
-  // Despite [[no_unique_address]], the two objects have the same type so they
-  // can't share the same address.
-  assert(&base != &member);
+struct MoveOnlyNotAssignable {
+  constexpr MoveOnlyNotAssignable()                              = default;
+  constexpr MoveOnlyNotAssignable(const MoveOnlyNotAssignable&)  = delete;
+  MoveOnlyNotAssignable& operator=(const MoveOnlyNotAssignable&) = delete;
+  constexpr MoveOnlyNotAssignable(MoveOnlyNotAssignable&&)       = default;
+  MoveOnlyNotAssignable& operator=(MoveOnlyNotAssignable&&)      = delete;
+};
 
-  // Make sure that we do perform the copy-construction, which wouldn't be the
-  // case if the two <copyable-box>s had the same address.
-  base = member;
-  assert(copied);
+int main(int, char**) {
+  test_no_unique_address<Copyable, true>();
+  test_no_unique_address<NotCopyAssignable, false>();
+  test_no_unique_address<NotMoveAssignable, false>();
 
-  base = std::move(member);
-  assert(moved);
+#if TEST_STD_VER >= 23
+  test_no_unique_address<MoveOnly, true>();
+  test_no_unique_address<MoveOnlyNotAssignable, false>();
+#endif
 
   return 0;
 }

diff  --git a/libcxx/test/libcxx/ranges/range.factories/range.repeat.view/no_unique_address.compile.pass.cpp b/libcxx/test/libcxx/ranges/range.factories/range.repeat.view/no_unique_address.compile.pass.cpp
new file mode 100644
index 00000000000000..6ab2ee173ca1b3
--- /dev/null
+++ b/libcxx/test/libcxx/ranges/range.factories/range.repeat.view/no_unique_address.compile.pass.cpp
@@ -0,0 +1,23 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// XFAIL: msvc
+
+// This test ensures that we use `[[no_unique_address]]` in `repeat_view`.
+
+#include <ranges>
+
+struct Empty {};
+
+struct Test {
+  [[no_unique_address]] std::ranges::repeat_view<Empty> v;
+  bool b;
+};
+
+static_assert(sizeof(Test) == sizeof(bool));

diff  --git a/libcxx/test/libcxx/ranges/range.factories/range.single.view/no_unique_address.compile.pass.cpp b/libcxx/test/libcxx/ranges/range.factories/range.single.view/no_unique_address.compile.pass.cpp
new file mode 100644
index 00000000000000..10883b8385780e
--- /dev/null
+++ b/libcxx/test/libcxx/ranges/range.factories/range.single.view/no_unique_address.compile.pass.cpp
@@ -0,0 +1,23 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// XFAIL: msvc
+
+// This test ensures that we use `[[no_unique_address]]` in `single_view`.
+
+#include <ranges>
+
+struct Empty {};
+
+struct Test {
+  [[no_unique_address]] std::ranges::single_view<Empty> v;
+  bool b;
+};
+
+static_assert(sizeof(Test) == sizeof(bool));

diff  --git a/libcxx/test/std/ranges/range.adaptors/range.lazy.split/ctor.range.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.lazy.split/ctor.range.pass.cpp
index 91df304b79af7d..8eeaa3dae36dbe 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.lazy.split/ctor.range.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.lazy.split/ctor.range.pass.cpp
@@ -22,22 +22,21 @@
 #include <utility>
 
 #include "test_convertible.h"
+#include "test_macros.h"
 #include "types.h"
 
 struct ElementWithCounting {
   int* times_copied = nullptr;
-  int* times_moved = nullptr;
+  int* times_moved  = nullptr;
 
   constexpr ElementWithCounting(int& copies_ctr, int& moves_ctr) : times_copied(&copies_ctr), times_moved(&moves_ctr) {}
 
   constexpr ElementWithCounting(const ElementWithCounting& rhs)
-      : times_copied(rhs.times_copied)
-      , times_moved(rhs.times_moved) {
+      : times_copied(rhs.times_copied), times_moved(rhs.times_moved) {
     ++(*times_copied);
   }
   constexpr ElementWithCounting(ElementWithCounting&& rhs)
-      : times_copied(rhs.times_copied)
-      , times_moved(rhs.times_moved) {
+      : times_copied(rhs.times_copied), times_moved(rhs.times_moved) {
     ++(*times_moved);
   }
 
@@ -48,18 +47,15 @@ struct RangeWithCounting {
   using value_type = ElementWithCounting;
 
   int* times_copied = nullptr;
-  int* times_moved = nullptr;
+  int* times_moved  = nullptr;
 
   constexpr RangeWithCounting(int& copies_ctr, int& moves_ctr) : times_copied(&copies_ctr), times_moved(&moves_ctr) {}
 
   constexpr RangeWithCounting(const RangeWithCounting& rhs)
-    : times_copied(rhs.times_copied)
-    , times_moved(rhs.times_moved) {
+      : times_copied(rhs.times_copied), times_moved(rhs.times_moved) {
     ++(*times_copied);
   }
-  constexpr RangeWithCounting(RangeWithCounting&& rhs)
-    : times_copied(rhs.times_copied)
-    , times_moved(rhs.times_moved) {
+  constexpr RangeWithCounting(RangeWithCounting&& rhs) : times_copied(rhs.times_copied), times_moved(rhs.times_moved) {
     ++(*times_moved);
   }
 
@@ -67,10 +63,10 @@ struct RangeWithCounting {
   constexpr const ElementWithCounting* end() const { return nullptr; }
 
   constexpr RangeWithCounting& operator=(const RangeWithCounting&) = default;
-  constexpr RangeWithCounting& operator=(RangeWithCounting&&) = default;
+  constexpr RangeWithCounting& operator=(RangeWithCounting&&)      = default;
   constexpr bool operator==(const RangeWithCounting&) const { return true; }
 };
-static_assert( std::ranges::forward_range<RangeWithCounting>);
+static_assert(std::ranges::forward_range<RangeWithCounting>);
 static_assert(!std::ranges::view<RangeWithCounting>);
 
 struct StrView : std::ranges::view_base {
@@ -86,9 +82,9 @@ struct StrView : std::ranges::view_base {
   constexpr std::string_view::const_iterator end() const { return buffer_.end(); }
   constexpr bool operator==(const StrView& rhs) const { return buffer_ == rhs.buffer_; }
 };
-static_assert( std::ranges::random_access_range<StrView>);
-static_assert( std::ranges::view<StrView>);
-static_assert( std::is_copy_constructible_v<StrView>);
+static_assert(std::ranges::random_access_range<StrView>);
+static_assert(std::ranges::view<StrView>);
+static_assert(std::is_copy_constructible_v<StrView>);
 
 // SFINAE tests.
 
@@ -131,7 +127,7 @@ constexpr bool test() {
 
   // Make sure the arguments are moved, not copied.
   {
-    using Range = RangeWithCounting;
+    using Range   = RangeWithCounting;
     using Element = ElementWithCounting;
     using Pattern = std::ranges::single_view<Element>;
 
@@ -144,10 +140,13 @@ constexpr bool test() {
       Element element(element_copied, element_moved);
 
       std::ranges::lazy_split_view<View, Pattern> v(range, element);
-      assert(range_copied == 0); // `ref_view` does neither copy...
-      assert(range_moved == 0); // ...nor move the element.
+      assert(range_copied == 0);   // `ref_view` does neither copy...
+      assert(range_moved == 0);    // ...nor move the element.
       assert(element_copied == 1); // The element is copied into the argument...
+#ifndef TEST_COMPILER_GCC
+      //https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98995
       assert(element_moved == 1); // ...and moved into the member variable.
+#endif
     }
 
     // Arguments are rvalues.
@@ -160,7 +159,10 @@ constexpr bool test() {
       assert(range_copied == 0);
       assert(range_moved == 1); // `owning_view` moves the given argument.
       assert(element_copied == 0);
+#ifndef TEST_COMPILER_GCC
+      //https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98995
       assert(element_moved == 1);
+#endif
     }
   }
 

diff  --git a/libcxx/test/std/ranges/range.adaptors/range.transform/general.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.transform/general.pass.cpp
index 32fed6f8caa7d0..cfe394e8918323 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.transform/general.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.transform/general.pass.cpp
@@ -108,5 +108,22 @@ int main(int, char**) {
   }
 #endif
 
+  // GH issue #70506
+  // movable_box::operator= overwrites underlying view
+  {
+    auto f = [l = 0.0L, b = false](int i) {
+      (void)l;
+      (void)b;
+      return i;
+    };
+
+    auto v1 = std::vector{1, 2, 3, 4} | std::views::transform(f);
+    auto v2 = std::vector{1, 2, 3, 4} | std::views::transform(f);
+
+    v1             = std::move(v2);
+    int expected[] = {1, 2, 3, 4};
+    assert(std::equal(v1.begin(), v1.end(), expected, expected + 4));
+  }
+
   return 0;
 }

diff  --git a/libcxx/test/std/ranges/ranges_robust_against_no_unique_address.pass.cpp b/libcxx/test/std/ranges/ranges_robust_against_no_unique_address.pass.cpp
new file mode 100644
index 00000000000000..3b35271d649c38
--- /dev/null
+++ b/libcxx/test/std/ranges/ranges_robust_against_no_unique_address.pass.cpp
@@ -0,0 +1,70 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// Test that views that use __movable_box do not overwrite overlapping subobjects.
+// https://github.com/llvm/llvm-project/issues/70506
+
+#include <cassert>
+#include <ranges>
+
+#include "test_macros.h"
+
+struct Pred {
+  alignas(128) bool a{};
+
+  Pred() noexcept            = default;
+  Pred(const Pred&) noexcept = default;
+  Pred(Pred&&) noexcept      = default;
+
+  Pred& operator=(const Pred&) = delete;
+  Pred& operator=(Pred&&)      = delete;
+
+  constexpr bool operator()(const auto&...) const { return true; }
+};
+
+struct View : std::ranges::view_base {
+  constexpr int* begin() const { return nullptr; }
+  constexpr int* end() const { return nullptr; }
+};
+
+template <class View>
+struct S {
+  [[no_unique_address]] View view{};
+  char c = 42;
+};
+
+template <class View>
+constexpr void testOne() {
+  S<View> s1;
+  assert(s1.c == 42);
+  s1.view = View{};
+  assert(s1.c == 42);
+}
+
+constexpr bool test() {
+  testOne<std::ranges::transform_view<View, Pred>>();
+  testOne<std::ranges::filter_view<View, Pred>>();
+  testOne<std::ranges::drop_while_view<View, Pred>>();
+  testOne<std::ranges::take_while_view<View, Pred>>();
+  testOne<std::ranges::single_view<Pred>>();
+
+#if TEST_STD_VER >= 23
+  testOne<std::ranges::chunk_by_view<View, Pred>>();
+  testOne<std::ranges::repeat_view<Pred>>();
+#endif
+  return true;
+}
+
+int main(int, char**) {
+  static_assert(test());
+  test();
+
+  return 0;
+}


        


More information about the libcxx-commits mailing list