[clang-tools-extra] [libc] [clang] [flang] [llvm] [libcxx] [compiler-rt] [lld] [mlir] [libc++][hardening] Don't trigger redundant checks in the fast mode. (PR #77176)
Konstantin Varlamov via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 20 23:46:44 PST 2024
https://github.com/var-const updated https://github.com/llvm/llvm-project/pull/77176
>From f86839d0bfc8b2070127dc3b2c609c2b3f7239ad Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconsteq at gmail.com>
Date: Fri, 5 Jan 2024 20:08:27 -0800
Subject: [PATCH 1/2] [libc++][hardening] Don't trigger redundant checks in the
fast mode.
Sometimes we essentially check the same condition twice -- for example,
a class might check that an index into its vector member variable is
valid before accessing it, but `vector::operator[]` contains the same
check. These "redundant" checks allow catching errors closer to the
source and providing a better error message, but they also impose
additional overhead. Marking the "early" checks as redundant allows
ignoring them in the fast mode (while still getting a guaranteed trap)
while still getting better error messages in the extensive mode and
above. Introducing a separate wrapper macro allows making the concept of
redundant assertions orthogonal to assertion categories and retaining
the actual category of a check.
This is a follow-up to https://github.com/llvm/llvm-project/pull/75918,
specifically to [this discussion](https://github.com/llvm/llvm-project/pull/75918#discussion_r1434493455).
---
libcxx/include/__config | 16 ++++++++++++++++
.../include/__filesystem/directory_iterator.h | 3 ++-
libcxx/include/__iterator/next.h | 5 +++--
libcxx/include/__iterator/prev.h | 5 +++--
libcxx/include/__mdspan/layout_left.h | 5 +++--
libcxx/include/__mdspan/layout_right.h | 5 +++--
libcxx/include/__mdspan/layout_stride.h | 5 +++--
libcxx/include/__ranges/chunk_by_view.h | 17 +++++++++++------
libcxx/include/__ranges/drop_while_view.h | 9 +++++----
libcxx/include/__ranges/filter_view.h | 5 +++--
libcxx/include/regex | 3 ++-
11 files changed, 54 insertions(+), 24 deletions(-)
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 082c73e672c7492..b20e8abed0939cc 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -290,6 +290,18 @@
// user input.
//
// - `_LIBCPP_ASSERT_UNCATEGORIZED` -- for assertions that haven't been properly classified yet.
+//
+// In addition to these categories, `_LIBCPP_REDUNDANT_ASSERTION` should be used to wrap assertions that duplicate other
+// assertions (for example, a range view might check that its `optional` data member holds a value before dereferencing
+// it, but this is already checked by `optional` itself). Redundant assertions incur an additional performance overhead
+// and don't provide any extra security benefit, but catching an error earlier allows halting the program closer to the
+// root cause and giving the user an error message that contains more context. Due to these tradeoffs, redundant
+// assertions are disabled in the fast mode but are enabled in the extensive mode and above. Thus, going back to the
+// example above, if a view attempts to dereference an empty optional member variable:
+// - in the fast mode, the program will only perform one check and will trap inside the optional (with an error
+// amounting to "Attempting to dereference an empty optional");
+// - in the extensive mode, the program will normally perform two checks (in the non-error case), and if the optional is
+// empty, it will trap inside the view (with an error like "`foo_view` doesn't have a valid predicate").
// clang-format off
# define _LIBCPP_HARDENING_MODE_NONE (1 << 1)
@@ -331,6 +343,7 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSUME(expression)
+# define _LIBCPP_REDUNDANT_ASSERTION(expression) _LIBCPP_ASSUME(expression)
// Extensive hardening mode checks.
@@ -344,6 +357,7 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSERT(expression, message)
+# define _LIBCPP_REDUNDANT_ASSERTION(expression) expression
// Disabled checks.
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression)
@@ -360,6 +374,7 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSERT(expression, message)
+# define _LIBCPP_REDUNDANT_ASSERTION(expression) expression
// Disable all checks if hardening is not enabled.
@@ -374,6 +389,7 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSUME(expression)
+# define _LIBCPP_REDUNDANT_ASSERTION(expression) _LIBCPP_ASSUME(expression)
# endif // _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_FAST
// clang-format on
diff --git a/libcxx/include/__filesystem/directory_iterator.h b/libcxx/include/__filesystem/directory_iterator.h
index 5287a4d8b055fd5..9b55513c87e5087 100644
--- a/libcxx/include/__filesystem/directory_iterator.h
+++ b/libcxx/include/__filesystem/directory_iterator.h
@@ -74,7 +74,8 @@ class directory_iterator {
_LIBCPP_HIDE_FROM_ABI const directory_entry& operator*() const {
// Note: this check duplicates a check in `__dereference()`.
- _LIBCPP_ASSERT_NON_NULL(__imp_, "The end iterator cannot be dereferenced");
+ _LIBCPP_REDUNDANT_ASSERTION( //
+ _LIBCPP_ASSERT_NON_NULL(__imp_, "The end iterator cannot be dereferenced"));
return __dereference();
}
diff --git a/libcxx/include/__iterator/next.h b/libcxx/include/__iterator/next.h
index 21d3688ad9eb600..b08881e1313e59d 100644
--- a/libcxx/include/__iterator/next.h
+++ b/libcxx/include/__iterator/next.h
@@ -29,8 +29,9 @@ inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 _InputIter
next(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n = 1) {
// Calling `advance` with a negative value on a non-bidirectional iterator is a no-op in the current implementation.
// Note that this check duplicates the similar check in `std::advance`.
- _LIBCPP_ASSERT_PEDANTIC(__n >= 0 || __has_bidirectional_iterator_category<_InputIter>::value,
- "Attempt to next(it, n) with negative n on a non-bidirectional iterator");
+ _LIBCPP_REDUNDANT_ASSERTION( //
+ _LIBCPP_ASSERT_PEDANTIC(__n >= 0 || __has_bidirectional_iterator_category<_InputIter>::value,
+ "Attempt to next(it, n) with negative n on a non-bidirectional iterator"));
std::advance(__x, __n);
return __x;
diff --git a/libcxx/include/__iterator/prev.h b/libcxx/include/__iterator/prev.h
index 2f0e6a088edb366..2590ea27d2b9b9b 100644
--- a/libcxx/include/__iterator/prev.h
+++ b/libcxx/include/__iterator/prev.h
@@ -29,8 +29,9 @@ inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 _InputIter
prev(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n = 1) {
// Calling `advance` with a negative value on a non-bidirectional iterator is a no-op in the current implementation.
// Note that this check duplicates the similar check in `std::advance`.
- _LIBCPP_ASSERT_PEDANTIC(__n <= 0 || __has_bidirectional_iterator_category<_InputIter>::value,
- "Attempt to prev(it, n) with a positive n on a non-bidirectional iterator");
+ _LIBCPP_REDUNDANT_ASSERTION( //
+ _LIBCPP_ASSERT_PEDANTIC(__n <= 0 || __has_bidirectional_iterator_category<_InputIter>::value,
+ "Attempt to prev(it, n) with a positive n on a non-bidirectional iterator"));
std::advance(__x, -__n);
return __x;
}
diff --git a/libcxx/include/__mdspan/layout_left.h b/libcxx/include/__mdspan/layout_left.h
index fd644fa0c53226b..74fc4398b742c26 100644
--- a/libcxx/include/__mdspan/layout_left.h
+++ b/libcxx/include/__mdspan/layout_left.h
@@ -152,8 +152,9 @@ class layout_left::mapping {
// 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...),
- "layout_left::mapping: out of bounds indexing");
+ _LIBCPP_REDUNDANT_ASSERTION( //
+ _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idx...),
+ "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;
diff --git a/libcxx/include/__mdspan/layout_right.h b/libcxx/include/__mdspan/layout_right.h
index 8e64d07dd52309c..18b4c4c223cb4aa 100644
--- a/libcxx/include/__mdspan/layout_right.h
+++ b/libcxx/include/__mdspan/layout_right.h
@@ -151,8 +151,9 @@ class layout_right::mapping {
// 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...),
- "layout_right::mapping: out of bounds indexing");
+ _LIBCPP_REDUNDANT_ASSERTION( //
+ _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idx...),
+ "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), ...);
diff --git a/libcxx/include/__mdspan/layout_stride.h b/libcxx/include/__mdspan/layout_stride.h
index 77934bfa11d9de0..5a3ab6aa7953535 100644
--- a/libcxx/include/__mdspan/layout_stride.h
+++ b/libcxx/include/__mdspan/layout_stride.h
@@ -284,8 +284,9 @@ class layout_stride::mapping {
// 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...),
- "layout_stride::mapping: out of bounds indexing");
+ _LIBCPP_REDUNDANT_ASSERTION( //
+ _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idx...),
+ "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)>());
diff --git a/libcxx/include/__ranges/chunk_by_view.h b/libcxx/include/__ranges/chunk_by_view.h
index c5b3240a7d0bed2..262dc4ef1cf90a5 100644
--- a/libcxx/include/__ranges/chunk_by_view.h
+++ b/libcxx/include/__ranges/chunk_by_view.h
@@ -67,8 +67,10 @@ class _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG chunk_by_view
_LIBCPP_HIDE_FROM_ABI constexpr iterator_t<_View> __find_next(iterator_t<_View> __current) {
// Note: this duplicates a check in `optional` but provides a better error message.
- _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
- __pred_.__has_value(), "Trying to call __find_next() on a chunk_by_view that does not have a valid predicate.");
+ _LIBCPP_REDUNDANT_ASSERTION( //
+ _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+ __pred_.__has_value(),
+ "Trying to call __find_next() on a chunk_by_view that does not have a valid predicate."));
auto __reversed_pred = [this]<class _Tp, class _Up>(_Tp&& __x, _Up&& __y) -> bool {
return !std::invoke(*__pred_, std::forward<_Tp>(__x), std::forward<_Up>(__y));
};
@@ -82,8 +84,10 @@ class _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG chunk_by_view
// Attempting to decrement a begin iterator is a no-op (`__find_prev` would return the same argument given to it).
_LIBCPP_ASSERT_PEDANTIC(__current != ranges::begin(__base_), "Trying to call __find_prev() on a begin iterator.");
// Note: this duplicates a check in `optional` but provides a better error message.
- _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
- __pred_.__has_value(), "Trying to call __find_prev() on a chunk_by_view that does not have a valid predicate.");
+ _LIBCPP_REDUNDANT_ASSERTION( //
+ _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+ __pred_.__has_value(),
+ "Trying to call __find_prev() on a chunk_by_view that does not have a valid predicate."));
auto __first = ranges::begin(__base_);
reverse_view __reversed{subrange{__first, __current}};
@@ -113,8 +117,9 @@ class _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG chunk_by_view
_LIBCPP_HIDE_FROM_ABI constexpr __iterator begin() {
// Note: this duplicates a check in `optional` but provides a better error message.
- _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
- __pred_.__has_value(), "Trying to call begin() on a chunk_by_view that does not have a valid predicate.");
+ _LIBCPP_REDUNDANT_ASSERTION( //
+ _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+ __pred_.__has_value(), "Trying to call begin() on a chunk_by_view that does not have a valid predicate."));
auto __first = ranges::begin(__base_);
if (!__cached_begin_.__has_value()) {
diff --git a/libcxx/include/__ranges/drop_while_view.h b/libcxx/include/__ranges/drop_while_view.h
index b367f735c1417ea..9271b834fbfaf0f 100644
--- a/libcxx/include/__ranges/drop_while_view.h
+++ b/libcxx/include/__ranges/drop_while_view.h
@@ -67,10 +67,11 @@ class _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG drop_while_view
_LIBCPP_HIDE_FROM_ABI constexpr auto begin() {
// Note: this duplicates a check in `optional` but provides a better error message.
- _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
- __pred_.__has_value(),
- "drop_while_view needs to have a non-empty predicate before calling begin() -- did a previous "
- "assignment to this drop_while_view fail?");
+ _LIBCPP_REDUNDANT_ASSERTION( //
+ _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+ __pred_.__has_value(),
+ "drop_while_view needs to have a non-empty predicate before calling begin() -- did a previous "
+ "assignment to this drop_while_view fail?"));
if constexpr (_UseCache) {
if (!__cached_begin_.__has_value()) {
__cached_begin_.__emplace(ranges::find_if_not(__base_, std::cref(*__pred_)));
diff --git a/libcxx/include/__ranges/filter_view.h b/libcxx/include/__ranges/filter_view.h
index ecb78eee381008a..0d36757464ccdd3 100644
--- a/libcxx/include/__ranges/filter_view.h
+++ b/libcxx/include/__ranges/filter_view.h
@@ -84,8 +84,9 @@ class _LIBCPP_ABI_2023_OVERLAPPING_SUBOBJECT_FIX_TAG filter_view : public view_i
_LIBCPP_HIDE_FROM_ABI constexpr __iterator begin() {
// Note: this duplicates a check in `optional` but provides a better error message.
- _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
- __pred_.__has_value(), "Trying to call begin() on a filter_view that does not have a valid predicate.");
+ _LIBCPP_REDUNDANT_ASSERTION( //
+ _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+ __pred_.__has_value(), "Trying to call begin() on a filter_view that does not have a valid predicate."));
if constexpr (_UseCache) {
if (!__cached_begin_.__has_value()) {
__cached_begin_.__emplace(ranges::find_if(__base_, std::ref(*__pred_)));
diff --git a/libcxx/include/regex b/libcxx/include/regex
index b575a267583b5fe..51c83c244fd4d2e 100644
--- a/libcxx/include/regex
+++ b/libcxx/include/regex
@@ -4731,7 +4731,8 @@ _OutputIter match_results<_BidirectionalIterator, _Allocator>::format(
const char_type* __fmt_last,
regex_constants::match_flag_type __flags) const {
// Note: this duplicates a check in `vector::operator[]` but provides a better error message.
- _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(ready(), "match_results::format() called when not ready");
+ _LIBCPP_REDUNDANT_ASSERTION(
+ _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(ready(), "match_results::format() called when not ready"));
if (__flags & regex_constants::format_sed) {
for (; __fmt_first != __fmt_last; ++__fmt_first) {
if (*__fmt_first == '&')
>From dcc510c51dd493bfb04299f99e45d1c56c67f657 Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconsteq at gmail.com>
Date: Fri, 19 Jan 2024 13:51:13 -0800
Subject: [PATCH 2/2] Feedback
---
libcxx/include/__config | 9 +++++---
libcxx/include/__mdspan/layout_left.h | 2 +-
libcxx/include/__mdspan/layout_right.h | 2 +-
libcxx/include/__mdspan/layout_stride.h | 2 +-
.../assertions/single_expression.pass.cpp | 22 +++++++++++++------
5 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/libcxx/include/__config b/libcxx/include/__config
index b20e8abed0939cc..75a130e31c9e1e5 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -297,7 +297,8 @@
// and don't provide any extra security benefit, but catching an error earlier allows halting the program closer to the
// root cause and giving the user an error message that contains more context. Due to these tradeoffs, redundant
// assertions are disabled in the fast mode but are enabled in the extensive mode and above. Thus, going back to the
-// example above, if a view attempts to dereference an empty optional member variable:
+// example above, the view should wrap its check for the empty optional member variable in
+// `_LIBCPP_REDUNDANT_ASSERTION`. Then:
// - in the fast mode, the program will only perform one check and will trap inside the optional (with an error
// amounting to "Attempting to dereference an empty optional");
// - in the extensive mode, the program will normally perform two checks (in the non-error case), and if the optional is
@@ -343,7 +344,9 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSUME(expression)
-# define _LIBCPP_REDUNDANT_ASSERTION(expression) _LIBCPP_ASSUME(expression)
+// TODO(hardening): if `_LIBCPP_ASSUME` becomes functional again (it's currently a no-op), this would essentially
+// discard the assumption.
+# define _LIBCPP_REDUNDANT_ASSERTION(expression) ((void)0)
// Extensive hardening mode checks.
@@ -389,7 +392,7 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSUME(expression)
-# define _LIBCPP_REDUNDANT_ASSERTION(expression) _LIBCPP_ASSUME(expression)
+# define _LIBCPP_REDUNDANT_ASSERTION(expression) ((void)0)
# endif // _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_FAST
// clang-format on
diff --git a/libcxx/include/__mdspan/layout_left.h b/libcxx/include/__mdspan/layout_left.h
index 74fc4398b742c26..0935e7e8f5688d0 100644
--- a/libcxx/include/__mdspan/layout_left.h
+++ b/libcxx/include/__mdspan/layout_left.h
@@ -151,7 +151,7 @@ class layout_left::mapping {
// 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
+ // However, mdspan does check this on its own, so we avoid double checking in hardened mode
_LIBCPP_REDUNDANT_ASSERTION( //
_LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idx...),
"layout_left::mapping: out of bounds indexing"));
diff --git a/libcxx/include/__mdspan/layout_right.h b/libcxx/include/__mdspan/layout_right.h
index 18b4c4c223cb4aa..ac2e38b33a09cf6 100644
--- a/libcxx/include/__mdspan/layout_right.h
+++ b/libcxx/include/__mdspan/layout_right.h
@@ -150,7 +150,7 @@ class layout_right::mapping {
// 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
+ // However, mdspan does check this on its own, so we avoid double checking in hardened mode
_LIBCPP_REDUNDANT_ASSERTION( //
_LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idx...),
"layout_right::mapping: out of bounds indexing"));
diff --git a/libcxx/include/__mdspan/layout_stride.h b/libcxx/include/__mdspan/layout_stride.h
index 5a3ab6aa7953535..04e7c2a9b0490b2 100644
--- a/libcxx/include/__mdspan/layout_stride.h
+++ b/libcxx/include/__mdspan/layout_stride.h
@@ -283,7 +283,7 @@ class layout_stride::mapping {
// 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
+ // However, mdspan does check this on its own, so we avoid double checking in hardened mode
_LIBCPP_REDUNDANT_ASSERTION( //
_LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idx...),
"layout_stride::mapping: out of bounds indexing"));
diff --git a/libcxx/test/libcxx/assertions/single_expression.pass.cpp b/libcxx/test/libcxx/assertions/single_expression.pass.cpp
index 13253e4cb6ef5af..15b77f24944cd94 100644
--- a/libcxx/test/libcxx/assertions/single_expression.pass.cpp
+++ b/libcxx/test/libcxx/assertions/single_expression.pass.cpp
@@ -6,27 +6,35 @@
//
//===----------------------------------------------------------------------===//
-// Make sure that `_LIBCPP_ASSERT` and `_LIBCPP_ASSUME` are each a single expression.
-// This is useful so we can use them in places that require an expression, such as
-// in a constructor initializer list.
+// Make sure that `_LIBCPP_ASSERT` and `_LIBCPP_ASSUME` are each a single expression, and that it still holds when an
+// assertion is wrapped in the `_LIBCPP_REDUNDANT_ASSERTION` macro. This is useful so we can use them in places that
+// require an expression, such as in a constructor initializer list.
#include <__assert>
#include <cassert>
-void f() {
+void test_assert() {
int i = (_LIBCPP_ASSERT(true, "message"), 3);
assert(i == 3);
return _LIBCPP_ASSERT(true, "message");
}
-void g() {
+void test_assume() {
int i = (_LIBCPP_ASSUME(true), 3);
assert(i == 3);
return _LIBCPP_ASSUME(true);
}
+void test_redundant() {
+ int i = (_LIBCPP_REDUNDANT_ASSERTION(_LIBCPP_ASSERT(true, "message")), 3);
+ assert(i == 3);
+ return _LIBCPP_REDUNDANT_ASSERTION(_LIBCPP_ASSERT(true, "message"));
+}
+
int main(int, char**) {
- f();
- g();
+ test_assert();
+ test_assume();
+ test_redundant();
+
return 0;
}
More information about the cfe-commits
mailing list