[libcxx-commits] [libcxx] [libc++][ranges][abi-break] Fix `movable_box` overwriting memory of data that lives in the tail padding (PR #71314)
via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Dec 22 13:10:26 PST 2023
https://github.com/huixie90 updated https://github.com/llvm/llvm-project/pull/71314
>From 6e155fab9d483fac83ea63dd18e4ad07ff37b9cd Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Sun, 5 Nov 2023 10:48:04 +0000
Subject: [PATCH 1/5] [libc++][ranges][abi-break] Fix `movable_box` overwriting
memory of data that lives in the tail padding
---
libcxx/include/__ranges/movable_box.h | 45 ++++++++++++++-----
.../no_unique_address.pass.cpp | 14 ++++++
.../range.transform/general.pass.cpp | 17 +++++++
3 files changed, 64 insertions(+), 12 deletions(-)
diff --git a/libcxx/include/__ranges/movable_box.h b/libcxx/include/__ranges/movable_box.h
index 6615533d374349..13e28954fd862f 100644
--- a/libcxx/include/__ranges/movable_box.h
+++ b/libcxx/include/__ranges/movable_box.h
@@ -16,6 +16,7 @@
#include <__config>
#include <__memory/addressof.h>
#include <__memory/construct_at.h>
+#include <__type_traits/is_empty.h>
#include <__type_traits/is_nothrow_constructible.h>
#include <__type_traits/is_nothrow_copy_constructible.h>
#include <__type_traits/is_nothrow_default_constructible.h>
@@ -146,21 +147,41 @@ template <class _Tp>
concept __doesnt_need_empty_state = __doesnt_need_empty_state_for_copy<_Tp> && __doesnt_need_empty_state_for_move<_Tp>;
# endif
+template <class _Tp, bool _NoUniqueAddress>
+struct __no_unique_address_if {
+ _Tp __val_;
+
+ template <class... _Args>
+ requires is_constructible_v<_Tp, _Args&&...>
+ _LIBCPP_HIDE_FROM_ABI constexpr explicit __no_unique_address_if(_Args&&... __args)
+ : __val_(std::forward<_Args>(__args)...) {}
+};
+
+template <class _Tp>
+struct __no_unique_address_if<_Tp, true> {
+ _LIBCPP_NO_UNIQUE_ADDRESS _Tp __val_;
+
+ template <class... _Args>
+ requires is_constructible_v<_Tp, _Args&&...>
+ _LIBCPP_HIDE_FROM_ABI constexpr explicit __no_unique_address_if(_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_;
+class __movable_box<_Tp> : private __no_unique_address_if<_Tp, is_empty_v<_Tp>> {
+ using __base = __no_unique_address_if<_Tp, is_empty_v<_Tp>>;
public:
template <class... _Args>
requires is_constructible_v<_Tp, _Args...>
_LIBCPP_HIDE_FROM_ABI constexpr explicit __movable_box(in_place_t, _Args&&... __args) noexcept(
is_nothrow_constructible_v<_Tp, _Args...>)
- : __val_(std::forward<_Args>(__args)...) {}
+ : __base(std::forward<_Args>(__args)...) {}
_LIBCPP_HIDE_FROM_ABI constexpr __movable_box() noexcept(is_nothrow_default_constructible_v<_Tp>)
requires default_initializable<_Tp>
- : __val_() {}
+ : __base() {}
_LIBCPP_HIDE_FROM_ABI __movable_box(__movable_box const&) = default;
_LIBCPP_HIDE_FROM_ABI __movable_box(__movable_box&&) = default;
@@ -177,8 +198,8 @@ class __movable_box<_Tp> {
_LIBCPP_HIDE_FROM_ABI constexpr __movable_box& operator=(__movable_box const& __other) noexcept {
static_assert(is_nothrow_copy_constructible_v<_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;
}
@@ -186,17 +207,17 @@ class __movable_box<_Tp> {
_LIBCPP_HIDE_FROM_ABI constexpr __movable_box& operator=(__movable_box&& __other) noexcept {
static_assert(is_nothrow_move_constructible_v<_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; }
};
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..6a33dab96280af 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
@@ -15,6 +15,20 @@
#include <cassert>
#include <utility>
+// movable_box should not use no_unique_address internally if T is not empty
+struct WithPadding {
+ WithPadding() {}
+ int i;
+ bool b;
+};
+
+struct Test {
+ [[no_unique_address]] std::ranges::__movable_box<WithPadding> box_;
+ bool b2;
+};
+
+static_assert(sizeof(Test) > sizeof(std::ranges::__movable_box<WithPadding>));
+
bool copied = false;
bool moved = false;
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;
}
>From 867bd7c3277661baf198a6a57236ffca6622039b Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Mon, 6 Nov 2023 13:25:38 +0000
Subject: [PATCH 2/5] only apply no_unique_address when we don't need to supply
assignment operator
---
libcxx/include/__ranges/movable_box.h | 28 ++++--
.../no_unique_address.pass.cpp | 89 ++++++++++---------
2 files changed, 68 insertions(+), 49 deletions(-)
diff --git a/libcxx/include/__ranges/movable_box.h b/libcxx/include/__ranges/movable_box.h
index 13e28954fd862f..a03cd70fa09ed9 100644
--- a/libcxx/include/__ranges/movable_box.h
+++ b/libcxx/include/__ranges/movable_box.h
@@ -16,7 +16,6 @@
#include <__config>
#include <__memory/addressof.h>
#include <__memory/construct_at.h>
-#include <__type_traits/is_empty.h>
#include <__type_traits/is_nothrow_constructible.h>
#include <__type_traits/is_nothrow_copy_constructible.h>
#include <__type_traits/is_nothrow_default_constructible.h>
@@ -135,6 +134,13 @@ 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>);
+
+// we can only use no_unique_address if _Tp has assignment operators,
+// so that we don't need to add our own assignment operator, which
+// contains problematic construct_at
+template <class _Tp>
+concept __can_use_no_unique_address = (copy_constructible<_Tp> ? copyable<_Tp> : movable<_Tp>);
+
# else
template <class _Tp>
@@ -145,32 +151,36 @@ 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, bool _NoUniqueAddress>
-struct __no_unique_address_if {
+template <class _Tp>
+struct __movable_box_base {
_Tp __val_;
template <class... _Args>
requires is_constructible_v<_Tp, _Args&&...>
- _LIBCPP_HIDE_FROM_ABI constexpr explicit __no_unique_address_if(_Args&&... __args)
+ _LIBCPP_HIDE_FROM_ABI constexpr explicit __movable_box_base(_Args&&... __args)
: __val_(std::forward<_Args>(__args)...) {}
};
template <class _Tp>
-struct __no_unique_address_if<_Tp, true> {
+ requires __can_use_no_unique_address<_Tp>
+struct __movable_box_base<_Tp> {
_LIBCPP_NO_UNIQUE_ADDRESS _Tp __val_;
template <class... _Args>
requires is_constructible_v<_Tp, _Args&&...>
- _LIBCPP_HIDE_FROM_ABI constexpr explicit __no_unique_address_if(_Args&&... __args)
+ _LIBCPP_HIDE_FROM_ABI constexpr explicit __movable_box_base(_Args&&... __args)
: __val_(std::forward<_Args>(__args)...) {}
};
template <__movable_box_object _Tp>
requires __doesnt_need_empty_state<_Tp>
-class __movable_box<_Tp> : private __no_unique_address_if<_Tp, is_empty_v<_Tp>> {
- using __base = __no_unique_address_if<_Tp, is_empty_v<_Tp>>;
+class __movable_box<_Tp> : private __movable_box_base<_Tp> {
+ using __base = __movable_box_base<_Tp>;
public:
template <class... _Args>
@@ -197,6 +207,7 @@ class __movable_box<_Tp> : private __no_unique_address_if<_Tp, is_empty_v<_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(this->__val_));
std::construct_at(std::addressof(this->__val_), __other.__val_);
@@ -206,6 +217,7 @@ class __movable_box<_Tp> : private __no_unique_address_if<_Tp, is_empty_v<_Tp>>
_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(this->__val_));
std::construct_at(std::addressof(this->__val_), std::move(__other.__val_));
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 6a33dab96280af..c425a67351bfb6 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
@@ -15,56 +15,63 @@
#include <cassert>
#include <utility>
-// movable_box should not use no_unique_address internally if T is not empty
-struct WithPadding {
- WithPadding() {}
- int i;
- bool b;
-};
-
-struct Test {
- [[no_unique_address]] std::ranges::__movable_box<WithPadding> box_;
- bool b2;
-};
-
-static_assert(sizeof(Test) > sizeof(std::ranges::__movable_box<WithPadding>));
+#include "test_macros.h"
+
+template <class T, bool ExpectNoUniqueAddress>
+void test_no_unique_address() {
+ struct Test {
+ [[no_unique_address]] std::ranges::__movable_box<T> box_;
+ bool b2;
+ };
+
+ if constexpr (ExpectNoUniqueAddress) {
+ static_assert(sizeof(Test) == sizeof(bool));
+ } else {
+ static_assert(sizeof(Test) > sizeof(bool));
+ }
+}
-bool copied = false;
-bool moved = false;
+struct Copyable {};
-struct Empty {
- Empty() noexcept {}
- Empty(Empty const&) noexcept { copied = true; }
- Empty(Empty&&) noexcept { moved = true; }
- Empty& operator=(Empty const&) = delete;
- Empty& operator=(Empty&&) = delete;
+struct NotCopyAssignable {
+ constexpr NotCopyAssignable() = default;
+ constexpr NotCopyAssignable(const NotCopyAssignable&) = default;
+ NotCopyAssignable& operator=(const NotCopyAssignable&) = delete;
};
-using Box = std::ranges::__movable_box<Empty>;
+struct NotMoveAssignable {
+ constexpr NotMoveAssignable() = default;
+ constexpr NotMoveAssignable(const NotMoveAssignable&) = default;
+ NotMoveAssignable& operator=(const NotMoveAssignable&) = default;
+ constexpr NotMoveAssignable(NotMoveAssignable&&) = default;
+ NotMoveAssignable& operator=(NotMoveAssignable&&) = delete;
+};
-struct Inherit : Box {};
+struct MoveOnly {
+ constexpr MoveOnly() = default;
+ constexpr MoveOnly(const MoveOnly&) = delete;
+ MoveOnly& operator=(const MoveOnly&) = delete;
+ constexpr MoveOnly(MoveOnly&&) = default;
+ MoveOnly& operator=(MoveOnly&&) = default;
+};
-struct Hold : Box {
- [[no_unique_address]] Inherit member;
+struct MoveOnlyNotAssignable {
+ constexpr MoveOnlyNotAssignable() = default;
+ constexpr MoveOnlyNotAssignable(const MoveOnlyNotAssignable&) = delete;
+ MoveOnlyNotAssignable& operator=(const MoveOnlyNotAssignable&) = delete;
+ constexpr MoveOnlyNotAssignable(MoveOnlyNotAssignable&&) = default;
+ MoveOnlyNotAssignable& operator=(MoveOnlyNotAssignable&&) = delete;
};
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 <copyable-box>s had the same address.
- base = member;
- assert(copied);
-
- base = std::move(member);
- assert(moved);
+ test_no_unique_address<Copyable, true>();
+ test_no_unique_address<NotCopyAssignable, false>();
+ test_no_unique_address<NotMoveAssignable, false>();
+
+#if TEST_STD_VER >= 23
+ test_no_unique_address<MoveOnly, true>();
+ test_no_unique_address<MoveOnlyNotAssignable, false>();
+#endif
return 0;
}
>From a3df3446636c3df3e2f5208e43f089ec5189f6ae Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Mon, 6 Nov 2023 15:47:39 +0000
Subject: [PATCH 3/5] fix no_unique_address ignored in the tests on win
---
.../range.adaptors/range.move.wrap/no_unique_address.pass.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 c425a67351bfb6..b7dee217e4945e 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
@@ -20,7 +20,7 @@
template <class T, bool ExpectNoUniqueAddress>
void test_no_unique_address() {
struct Test {
- [[no_unique_address]] std::ranges::__movable_box<T> box_;
+ _LIBCPP_NO_UNIQUE_ADDRESS std::ranges::__movable_box<T> box_;
bool b2;
};
>From c7327cb37fd593145b0bbeb3bc9b0773e54988f5 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Wed, 20 Dec 2023 15:10:01 +0000
Subject: [PATCH 4/5] address feedback and rebase
---
libcxx/docs/ReleaseNotes/18.rst | 4 ++
libcxx/include/__config | 12 ++++
libcxx/include/__ranges/chunk_by_view.h | 3 +-
libcxx/include/__ranges/drop_while_view.h | 3 +-
libcxx/include/__ranges/filter_view.h | 2 +-
libcxx/include/__ranges/movable_box.h | 49 +++++++-------
libcxx/include/__ranges/repeat_view.h | 4 +-
libcxx/include/__ranges/single_view.h | 4 +-
libcxx/include/__ranges/take_while_view.h | 3 +-
libcxx/include/__ranges/transform_view.h | 3 +-
.../no_unique_address.pass.cpp | 5 +-
.../no_unique_address.compile.pass.cpp | 21 ++++++
.../no_unique_address.compile.pass.cpp | 21 ++++++
..._robust_against_no_unique_address.pass.cpp | 64 +++++++++++++++++++
14 files changed, 166 insertions(+), 32 deletions(-)
create mode 100644 libcxx/test/libcxx/ranges/range.factories/range.repeat.view/no_unique_address.compile.pass.cpp
create mode 100644 libcxx/test/libcxx/ranges/range.factories/range.single.view/no_unique_address.compile.pass.cpp
create mode 100644 libcxx/test/std/ranges/range_robust_against_no_unique_address.pass.cpp
diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 79608c631f1e62..2a8f2a0fd07260 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -166,6 +166,10 @@ ABI Affecting Changes
to throw a different exception when attempting allocations that are too large
(``std::bad_alloc`` vs ``std::length_error``).
+- The ABI of some classes inside `std::ranges` that use `movable-box` have changed in order to fix a bug which could
+ result in overwriting user data followed by the `movable-box`. The affected views are `take_while_view`, `filter_view`,
+ `single_view`, `drop_while_view`, `repeat_view`, `transform_view`, `chunk_by_view`
+
Build System Changes
--------------------
diff --git a/libcxx/include/__config b/libcxx/include/__config
index adff13e714cb64..04abcb9cda8dda 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -200,6 +200,18 @@
# define _LIBCPP_ABI_BAD_FUNCTION_CALL_KEY_FUNCTION
# endif
+# ifndef _LIBCPP_NO_ABI_TAG
+// 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 that have ABI changed as part of the bug fix
+# define _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG __attribute__((__abi_tag__("2023_subobj_fix")))
+# endif
+
// 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 a03cd70fa09ed9..80773927bfd488 100644
--- a/libcxx/include/__ranges/movable_box.h
+++ b/libcxx/include/__ranges/movable_box.h
@@ -135,9 +135,16 @@ concept __doesnt_need_empty_state =
// is_nothrow_move_constructible_v<T> is true.
: movable<_Tp> || is_nothrow_move_constructible_v<_Tp>);
-// we can only use no_unique_address if _Tp has assignment operators,
-// so that we don't need to add our own assignment operator, which
-// contains problematic construct_at
+// 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>);
@@ -157,41 +164,39 @@ concept __can_use_no_unique_address = copyable<_Tp>;
# endif
template <class _Tp>
-struct __movable_box_base {
+struct __movable_box_holder {
_Tp __val_;
template <class... _Args>
- requires is_constructible_v<_Tp, _Args&&...>
- _LIBCPP_HIDE_FROM_ABI constexpr explicit __movable_box_base(_Args&&... __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_base<_Tp> {
+struct __movable_box_holder<_Tp> {
_LIBCPP_NO_UNIQUE_ADDRESS _Tp __val_;
template <class... _Args>
- requires is_constructible_v<_Tp, _Args&&...>
- _LIBCPP_HIDE_FROM_ABI constexpr explicit __movable_box_base(_Args&&... __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> : private __movable_box_base<_Tp> {
- using __base = __movable_box_base<_Tp>;
+class __movable_box<_Tp> {
+ _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...>)
- : __base(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>
- : __base() {}
+ : __holder_(in_place_t{}) {}
_LIBCPP_HIDE_FROM_ABI __movable_box(__movable_box const&) = default;
_LIBCPP_HIDE_FROM_ABI __movable_box(__movable_box&&) = default;
@@ -209,8 +214,8 @@ class __movable_box<_Tp> : private __movable_box_base<_Tp> {
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(this->__val_));
- std::construct_at(std::addressof(this->__val_), __other.__val_);
+ std::destroy_at(std::addressof(__holder_.__val_));
+ std::construct_at(std::addressof(__holder_.__val_), __other.__holder_.__val_);
}
return *this;
}
@@ -219,17 +224,17 @@ class __movable_box<_Tp> : private __movable_box_base<_Tp> {
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(this->__val_));
- std::construct_at(std::addressof(this->__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 this->__val_; }
- _LIBCPP_HIDE_FROM_ABI constexpr _Tp& operator*() noexcept { return this->__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(this->__val_); }
- _LIBCPP_HIDE_FROM_ABI constexpr _Tp* operator->() noexcept { return std::addressof(this->__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_difference_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.move.wrap/no_unique_address.pass.cpp b/libcxx/test/libcxx/ranges/range.adaptors/range.move.wrap/no_unique_address.pass.cpp
index b7dee217e4945e..e8916059ac2456 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,6 +8,9 @@
// UNSUPPORTED: c++03, c++11, c++14, c++17
+// clang-cl and cl currently don't support [[no_unique_address]]
+// XFAIL: msvc
+
// This test ensures that <copyable-box> behaves correctly when it holds an empty type.
#include <ranges>
@@ -20,7 +23,7 @@
template <class T, bool ExpectNoUniqueAddress>
void test_no_unique_address() {
struct Test {
- _LIBCPP_NO_UNIQUE_ADDRESS std::ranges::__movable_box<T> box_;
+ [[no_unique_address]] std::ranges::__movable_box<T> box_;
bool b2;
};
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..f6e0e25494daca
--- /dev/null
+++ b/libcxx/test/libcxx/ranges/range.factories/range.repeat.view/no_unique_address.compile.pass.cpp
@@ -0,0 +1,21 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+#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..8106131d3c54b2
--- /dev/null
+++ b/libcxx/test/libcxx/ranges/range.factories/range.single.view/no_unique_address.compile.pass.cpp
@@ -0,0 +1,21 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+#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_robust_against_no_unique_address.pass.cpp b/libcxx/test/std/ranges/range_robust_against_no_unique_address.pass.cpp
new file mode 100644
index 00000000000000..a35ecf1ed72e59
--- /dev/null
+++ b/libcxx/test/std/ranges/range_robust_against_no_unique_address.pass.cpp
@@ -0,0 +1,64 @@
+//===----------------------------------------------------------------------===//
+//
+// 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.
+
+#include <cassert>
+#include <ranges>
+
+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 T>
+struct S {
+ [[no_unique_address]] T t{};
+ char c = 42;
+};
+
+template <class T>
+constexpr void testOne() {
+ S<T> s1;
+ assert(s1.c == 42);
+ s1.t = T{};
+ assert(s1.c == 42);
+}
+
+constexpr bool test() {
+ testOne<std::ranges::transform_view<View, Pred>>();
+ testOne<std::ranges::filter_view<View, Pred>>();
+ testOne<std::ranges::chunk_by_view<View, Pred>>();
+ testOne<std::ranges::drop_while_view<View, Pred>>();
+ testOne<std::ranges::take_while_view<View, Pred>>();
+ testOne<std::ranges::repeat_view<Pred>>();
+ testOne<std::ranges::single_view<Pred>>();
+ return true;
+}
+
+int main(int, char**) {
+ static_assert(test());
+ test();
+
+ return 0;
+}
>From 7033d0499c251b8da418f163a2431437e6082000 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Fri, 22 Dec 2023 21:10:09 +0000
Subject: [PATCH 5/5] address feedback
---
libcxx/docs/ReleaseNotes/18.rst | 14 ++++-
libcxx/include/__config | 6 +-
libcxx/include/__ranges/movable_box.h | 2 +-
.../no_unique_address.compile.pass.cpp | 46 +++++++++++++++
.../range.move.wrap/empty_object.pass.cpp | 56 +++++++++++++++++++
.../no_unique_address.pass.cpp | 2 -
.../no_unique_address.compile.pass.cpp | 2 +
.../no_unique_address.compile.pass.cpp | 2 +
...robust_against_no_unique_address.pass.cpp} | 11 ++--
9 files changed, 126 insertions(+), 15 deletions(-)
create mode 100644 libcxx/test/libcxx/ranges/range.adaptors/range.chunk.by/no_unique_address.compile.pass.cpp
create mode 100644 libcxx/test/libcxx/ranges/range.adaptors/range.move.wrap/empty_object.pass.cpp
rename libcxx/test/std/ranges/{range_robust_against_no_unique_address.pass.cpp => ranges_robust_against_no_unique_address.pass.cpp} (90%)
diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 2a8f2a0fd07260..cf6c5171288545 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -166,9 +166,17 @@ ABI Affecting Changes
to throw a different exception when attempting allocations that are too large
(``std::bad_alloc`` vs ``std::length_error``).
-- The ABI of some classes inside `std::ranges` that use `movable-box` have changed in order to fix a bug which could
- result in overwriting user data followed by the `movable-box`. The affected views are `take_while_view`, `filter_view`,
- `single_view`, `drop_while_view`, `repeat_view`, `transform_view`, `chunk_by_view`
+- 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 different 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 04abcb9cda8dda..40e6da8bc03afa 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -200,7 +200,6 @@
# define _LIBCPP_ABI_BAD_FUNCTION_CALL_KEY_FUNCTION
# endif
-# ifndef _LIBCPP_NO_ABI_TAG
// 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
//
@@ -208,9 +207,8 @@
// 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 that have ABI changed as part of the bug fix
-# define _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG __attribute__((__abi_tag__("2023_subobj_fix")))
-# endif
+// 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/movable_box.h b/libcxx/include/__ranges/movable_box.h
index 80773927bfd488..9b38877494eaa4 100644
--- a/libcxx/include/__ranges/movable_box.h
+++ b/libcxx/include/__ranges/movable_box.h
@@ -137,7 +137,7 @@ concept __doesnt_need_empty_state =
// 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
+// _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.
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..b17ebb4daa3adb
--- /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
+// 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 e8916059ac2456..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
@@ -11,8 +11,6 @@
// clang-cl and cl currently don't support [[no_unique_address]]
// XFAIL: msvc
-// This test ensures that <copyable-box> behaves correctly when it holds an empty type.
-
#include <ranges>
#include <cassert>
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
index f6e0e25494daca..2b153ea80fc509 100644
--- 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
@@ -9,6 +9,8 @@
// UNSUPPORTED: c++03, c++11, c++14, c++17
// XFAIL: msvc
+// This test ensures that we use `[[no_unique_address]]` in `repeat_view`.
+
#include <ranges>
struct Empty {};
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
index 8106131d3c54b2..10883b8385780e 100644
--- 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
@@ -9,6 +9,8 @@
// 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 {};
diff --git a/libcxx/test/std/ranges/range_robust_against_no_unique_address.pass.cpp b/libcxx/test/std/ranges/ranges_robust_against_no_unique_address.pass.cpp
similarity index 90%
rename from libcxx/test/std/ranges/range_robust_against_no_unique_address.pass.cpp
rename to libcxx/test/std/ranges/ranges_robust_against_no_unique_address.pass.cpp
index a35ecf1ed72e59..5f82a10efadeb0 100644
--- a/libcxx/test/std/ranges/range_robust_against_no_unique_address.pass.cpp
+++ b/libcxx/test/std/ranges/ranges_robust_against_no_unique_address.pass.cpp
@@ -9,6 +9,7 @@
// 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>
@@ -31,17 +32,17 @@ struct View : std::ranges::view_base {
constexpr int* end() const { return nullptr; }
};
-template <class T>
+template <class View>
struct S {
- [[no_unique_address]] T t{};
+ [[no_unique_address]] View view{};
char c = 42;
};
-template <class T>
+template <class View>
constexpr void testOne() {
- S<T> s1;
+ S<View> s1;
assert(s1.c == 42);
- s1.t = T{};
+ s1.view = View{};
assert(s1.c == 42);
}
More information about the libcxx-commits
mailing list