[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:16 PST 2023


================
@@ -134,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>);
----------------
ldionne wrote:

Also not attached to this line: We need to introduce an ABI tag for everything that is impacted by this change. For now, I think this should be:

```
take_while_view
filter_view
single_view
drop_while_view
repeat_view
transform_view
chunk_by_view
__movable_box
```

I suggest doing something along those lines:

```
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 7f66042f9025..1a06070ce3bf 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -193,6 +193,9 @@
 #    endif
 #  endif
 
+// TODO: Comprehensive explanation of what this is.
+#  define _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG __attribute__((__abi_tag__("whatever")))
+
 #  if defined(_LIBCPP_BUILDING_LIBRARY) || _LIBCPP_ABI_VERSION >= 2
 // Define a key function for `bad_function_call` in the library, to centralize
 // its vtable and typeinfo to libc++ rather than having all other libraries
diff --git a/libcxx/include/__ranges/filter_view.h b/libcxx/include/__ranges/filter_view.h
index 1cef94ca6744..7647790aeed2 100644
--- a/libcxx/include/__ranges/filter_view.h
+++ b/libcxx/include/__ranges/filter_view.h
@@ -51,6 +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>
+  _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG
   class 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 6615533d3743..eddc1fb27739 100644
--- a/libcxx/include/__ranges/movable_box.h
+++ b/libcxx/include/__ranges/movable_box.h
@@ -55,6 +55,7 @@ concept __movable_box_object =
 namespace ranges {
 // Primary template - uses std::optional and introduces an empty state in case assignment fails.
 template <__movable_box_object _Tp>
+_LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG
 class __movable_box {
   _LIBCPP_NO_UNIQUE_ADDRESS optional<_Tp> __val_;
 
diff --git a/libcxx/include/__ranges/transform_view.h b/libcxx/include/__ranges/transform_view.h
index 3678f9d64f7b..3c49ac09fc94 100644
--- a/libcxx/include/__ranges/transform_view.h
+++ b/libcxx/include/__ranges/transform_view.h
@@ -69,6 +69,7 @@ template <input_range _View, move_constructible _Fn>
 template <input_range _View, copy_constructible _Fn>
 #  endif
   requires __transform_view_constraints<_View, _Fn>
+_LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG
 class transform_view : public view_interface<transform_view<_View, _Fn>> {
   template<bool> class __iterator;
   template<bool> class __sentinel;

```

So basically we add an ABI tag in `__config`, we document it there, and then we apply that ABI tag to `__movable_box` itself but also all the public types that use it, which fortunately doesn't seem to be too many types.

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


More information about the libcxx-commits mailing list