[libcxx-commits] [libcxx] [libc++] Fix value category issues in `mdspan::operator[]` bounds checking (PR #192269)
via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Apr 15 07:39:30 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: eiytoq (eiytoq)
<details>
<summary>Changes</summary>
Reproducer: https://godbolt.org/z/3bejsdYjq
---
Full diff: https://github.com/llvm/llvm-project/pull/192269.diff
6 Files Affected:
- (modified) libcxx/include/__mdspan/extents.h (+6-18)
- (modified) libcxx/include/__mdspan/layout_left.h (+4-4)
- (modified) libcxx/include/__mdspan/layout_right.h (+5-3)
- (modified) libcxx/include/__mdspan/layout_stride.h (+6-4)
- (modified) libcxx/include/__mdspan/mdspan.h (+9-5)
- (modified) libcxx/test/std/containers/views/mdspan/mdspan/index_operator.pass.cpp (+9)
``````````diff
diff --git a/libcxx/include/__mdspan/extents.h b/libcxx/include/__mdspan/extents.h
index d16bbd2af44f1..115a84bb9519b 100644
--- a/libcxx/include/__mdspan/extents.h
+++ b/libcxx/include/__mdspan/extents.h
@@ -493,7 +493,6 @@ inline constexpr bool __is_extents_v = __is_extents<_Tp>::value;
// the respective extents.
template <integral _IndexType, class _From>
- requires(integral<_From>)
_LIBCPP_HIDE_FROM_ABI constexpr bool __is_index_in_extent(_IndexType __extent, _From __value) {
if constexpr (is_signed_v<_From>) {
if (__value < 0)
@@ -503,26 +502,15 @@ _LIBCPP_HIDE_FROM_ABI constexpr bool __is_index_in_extent(_IndexType __extent, _
return static_cast<_Tp>(__value) < static_cast<_Tp>(__extent);
}
-template <integral _IndexType, class _From>
- requires(!integral<_From>)
-_LIBCPP_HIDE_FROM_ABI constexpr bool __is_index_in_extent(_IndexType __extent, _From __value) {
- if constexpr (is_signed_v<_IndexType>) {
- if (static_cast<_IndexType>(__value) < 0)
- return false;
- }
- return static_cast<_IndexType>(__value) < __extent;
-}
-
-template <size_t... _Idxs, class _Extents, class... _From>
+template <size_t... _Idxs, class _Extents, class _From>
_LIBCPP_HIDE_FROM_ABI constexpr bool
-__is_multidimensional_index_in_impl(index_sequence<_Idxs...>, const _Extents& __ext, _From... __values) {
- return (__mdspan_detail::__is_index_in_extent(__ext.extent(_Idxs), __values) && ...);
+__is_multidimensional_index_in_impl(index_sequence<_Idxs...>, const _Extents& __ext, const _From& __values) {
+ return (__mdspan_detail::__is_index_in_extent(__ext.extent(_Idxs), __values[_Idxs]) && ...);
}
-template <class _Extents, class... _From>
-_LIBCPP_HIDE_FROM_ABI constexpr bool __is_multidimensional_index_in(const _Extents& __ext, _From... __values) {
- return __mdspan_detail::__is_multidimensional_index_in_impl(
- make_index_sequence<_Extents::rank()>(), __ext, __values...);
+template <class _Extents, class _From>
+_LIBCPP_HIDE_FROM_ABI constexpr bool __is_multidimensional_index_in(const _Extents& __ext, const _From& __values) {
+ return __mdspan_detail::__is_multidimensional_index_in_impl(make_index_sequence<_Extents::rank()>(), __ext, __values);
}
} // namespace __mdspan_detail
diff --git a/libcxx/include/__mdspan/layout_left.h b/libcxx/include/__mdspan/layout_left.h
index fb1b59555b254..0a27559180e15 100644
--- a/libcxx/include/__mdspan/layout_left.h
+++ b/libcxx/include/__mdspan/layout_left.h
@@ -146,17 +146,17 @@ class layout_left::mapping {
template <class... _Indices>
requires((sizeof...(_Indices) == extents_type::rank()) && (is_convertible_v<_Indices, index_type> && ...) &&
(is_nothrow_constructible_v<index_type, _Indices> && ...))
- _LIBCPP_HIDE_FROM_ABI constexpr index_type operator()(_Indices... __idx) const noexcept {
+ _LIBCPP_HIDE_FROM_ABI constexpr index_type operator()(_Indices... __indices) const noexcept {
+ const array<index_type, extents_type::rank()> __idxs{static_cast<index_type>(__indices)...};
// Mappings are generally meant to be used for accessing allocations and are meant to guarantee to never
// return a value exceeding required_span_size(), which is used to know how large an allocation one needs
// Thus, this is a canonical point in multi-dimensional data structures to make invalid element access checks
// However, mdspan does check this on its own, so for now we avoid double checking in hardened mode
- _LIBCPP_ASSERT_UNCATEGORIZED(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idx...),
+ _LIBCPP_ASSERT_UNCATEGORIZED(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idxs),
"layout_left::mapping: out of bounds indexing");
- array<index_type, extents_type::rank()> __idx_a{static_cast<index_type>(__idx)...};
return [&]<size_t... _Pos>(index_sequence<_Pos...>) {
index_type __res = 0;
- ((__res = __idx_a[extents_type::rank() - 1 - _Pos] + __extents_.extent(extents_type::rank() - 1 - _Pos) * __res),
+ ((__res = __idxs[extents_type::rank() - 1 - _Pos] + __extents_.extent(extents_type::rank() - 1 - _Pos) * __res),
...);
return __res;
}(make_index_sequence<sizeof...(_Indices)>());
diff --git a/libcxx/include/__mdspan/layout_right.h b/libcxx/include/__mdspan/layout_right.h
index 7cb828022ea8e..bec2e3187c1f2 100644
--- a/libcxx/include/__mdspan/layout_right.h
+++ b/libcxx/include/__mdspan/layout_right.h
@@ -146,16 +146,18 @@ class layout_right::mapping {
template <class... _Indices>
requires((sizeof...(_Indices) == extents_type::rank()) && (is_convertible_v<_Indices, index_type> && ...) &&
(is_nothrow_constructible_v<index_type, _Indices> && ...))
- _LIBCPP_HIDE_FROM_ABI constexpr index_type operator()(_Indices... __idx) const noexcept {
+ _LIBCPP_HIDE_FROM_ABI constexpr index_type operator()(_Indices... __indices) const noexcept {
+ const array<index_type, extents_type::rank()> __idxs{static_cast<index_type>(__indices)...};
// Mappings are generally meant to be used for accessing allocations and are meant to guarantee to never
// return a value exceeding required_span_size(), which is used to know how large an allocation one needs
// Thus, this is a canonical point in multi-dimensional data structures to make invalid element access checks
// However, mdspan does check this on its own, so for now we avoid double checking in hardened mode
- _LIBCPP_ASSERT_UNCATEGORIZED(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idx...),
+ _LIBCPP_ASSERT_UNCATEGORIZED(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idxs),
"layout_right::mapping: out of bounds indexing");
+
return [&]<size_t... _Pos>(index_sequence<_Pos...>) {
index_type __res = 0;
- ((__res = static_cast<index_type>(__idx) + __extents_.extent(_Pos) * __res), ...);
+ ((__res = __idxs[_Pos] + __extents_.extent(_Pos) * __res), ...);
return __res;
}(make_index_sequence<sizeof...(_Indices)>());
}
diff --git a/libcxx/include/__mdspan/layout_stride.h b/libcxx/include/__mdspan/layout_stride.h
index 66d5a7f499e4c..383ff048931e6 100644
--- a/libcxx/include/__mdspan/layout_stride.h
+++ b/libcxx/include/__mdspan/layout_stride.h
@@ -283,16 +283,18 @@ class layout_stride::mapping {
template <class... _Indices>
requires((sizeof...(_Indices) == __rank_) && (is_convertible_v<_Indices, index_type> && ...) &&
(is_nothrow_constructible_v<index_type, _Indices> && ...))
- _LIBCPP_HIDE_FROM_ABI constexpr index_type operator()(_Indices... __idx) const noexcept {
+ _LIBCPP_HIDE_FROM_ABI constexpr index_type operator()(_Indices... __indices) const noexcept {
+ const array<index_type, extents_type::rank()> __idxs{static_cast<index_type>(__indices)...};
// Mappings are generally meant to be used for accessing allocations and are meant to guarantee to never
// return a value exceeding required_span_size(), which is used to know how large an allocation one needs
// Thus, this is a canonical point in multi-dimensional data structures to make invalid element access checks
// However, mdspan does check this on its own, so for now we avoid double checking in hardened mode
- _LIBCPP_ASSERT_UNCATEGORIZED(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idx...),
+ _LIBCPP_ASSERT_UNCATEGORIZED(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idxs),
"layout_stride::mapping: out of bounds indexing");
+
return [&]<size_t... _Pos>(index_sequence<_Pos...>) {
- return ((static_cast<index_type>(__idx) * __strides_[_Pos]) + ... + index_type(0));
- }(make_index_sequence<sizeof...(_Indices)>());
+ return ((__idxs[_Pos] * __strides_[_Pos]) + ... + index_type(0));
+ }(make_index_sequence<extents_type::rank()>{});
}
_LIBCPP_HIDE_FROM_ABI static constexpr bool is_always_unique() noexcept { return true; }
diff --git a/libcxx/include/__mdspan/mdspan.h b/libcxx/include/__mdspan/mdspan.h
index 54685b56a76ff..6001b2676ea92 100644
--- a/libcxx/include/__mdspan/mdspan.h
+++ b/libcxx/include/__mdspan/mdspan.h
@@ -191,11 +191,15 @@ class mdspan {
(is_nothrow_constructible_v<index_type, _OtherIndexTypes> && ...) &&
(sizeof...(_OtherIndexTypes) == rank()))
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr reference operator[](_OtherIndexTypes... __indices) const {
- // Note the standard layouts would also check this, but user provided ones may not, so we
- // check the precondition here
- _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__mdspan_detail::__is_multidimensional_index_in(extents(), __indices...),
- "mdspan: operator[] out of bounds access");
- return __acc_.access(__ptr_, __map_(static_cast<index_type>(std::move(__indices))...));
+ const array<index_type, extents_type::rank()> __idxs{static_cast<index_type>(std::move(__indices))...};
+ // Note the standard layouts would also check this, but user provided ones may not, so we check the precondition
+ // here
+ _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+ __mdspan_detail::__is_multidimensional_index_in(extents(), __idxs), "mdspan: operator[] out of bounds access");
+
+ return __acc_.access(__ptr_, [&]<size_t... _Idxs>(index_sequence<_Idxs...>) {
+ return __map_(static_cast<index_type>(__idxs[_Idxs])...);
+ }(make_index_sequence<rank()>()));
}
template <class _OtherIndexType>
diff --git a/libcxx/test/std/containers/views/mdspan/mdspan/index_operator.pass.cpp b/libcxx/test/std/containers/views/mdspan/mdspan/index_operator.pass.cpp
index 9124bd2314806..03f9d8c3ffb85 100644
--- a/libcxx/test/std/containers/views/mdspan/mdspan/index_operator.pass.cpp
+++ b/libcxx/test/std/containers/views/mdspan/mdspan/index_operator.pass.cpp
@@ -231,10 +231,19 @@ constexpr void test_layout_large() {
// mapping requirements only require the index operator to mixed integer types not anything convertible to index_type
constexpr void test_index_cast_happens() {}
+struct RValueInt {
+ constexpr operator int() && noexcept { return 0; }
+};
+
constexpr bool test() {
test_layout<std::layout_left>();
test_layout<std::layout_right>();
test_layout<layout_wrapping_integral<4>>();
+
+ int data[1]{};
+ std::mdspan m(data, std::extents<int, 1>{1});
+ TEST_IGNORE_NODISCARD m[RValueInt{}];
+
return true;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/192269
More information about the libcxx-commits
mailing list