[libc++][hardening] Don't trigger redundant checks in the fast mode.

libcxx/include/__config

@@ -308,6 +308,19 @@
 //   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, 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
+//   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)
@@ -350,6 +363,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)
+// 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.
 
@@ -363,6 +379,7 @@ _LIBCPP_HARDENING_MODE_DEBUG
 #    define _LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR(expression, message)     _LIBCPP_ASSERT(expression, message)
 #    define _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(expression, message)   _LIBCPP_ASSERT(expression, message)
 #    define _LIBCPP_ASSERT_PEDANTIC(expression, message)                 _LIBCPP_ASSERT(expression, message)
+#    define _LIBCPP_REDUNDANT_ASSERTION(expression)                      expression
 #    define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message)            _LIBCPP_ASSERT(expression, message)
 // Disabled checks.
 #    define _LIBCPP_ASSERT_INTERNAL(expression, message)                 _LIBCPP_ASSUME(expression)
@@ -381,6 +398,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.
 
@@ -396,6 +414,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)                       ((void)0)
 
 #  endif // _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_FAST
 // clang-format on

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();
   }
 

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;

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;
 }

libcxx/include/__mdspan/layout_left.h

@@ -151,9 +151,10 @@ 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
-    _LIBCPP_ASSERT_UNCATEGORIZED(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idx...),
-                                 "layout_left::mapping: out of bounds indexing");
+    // 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"));
     array<index_type, extents_type::rank()> __idx_a{static_cast<index_type>(__idx)...};
     return [&]<size_t... _Pos>(index_sequence<_Pos...>) {
       index_type __res = 0;

libcxx/include/__mdspan/layout_right.h

@@ -150,9 +150,10 @@ 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
-    _LIBCPP_ASSERT_UNCATEGORIZED(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idx...),
-                                 "layout_right::mapping: out of bounds indexing");
+    // 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"));
     return [&]<size_t... _Pos>(index_sequence<_Pos...>) {
       index_type __res = 0;
       ((__res = static_cast<index_type>(__idx) + __extents_.extent(_Pos) * __res), ...);

libcxx/include/__mdspan/layout_stride.h

@@ -283,9 +283,10 @@ 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
-    _LIBCPP_ASSERT_UNCATEGORIZED(__mdspan_detail::__is_multidimensional_index_in(__extents_, __idx...),
-                                 "layout_stride::mapping: out of bounds indexing");
+    // 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"));
     return [&]<size_t... _Pos>(index_sequence<_Pos...>) {
       return ((static_cast<index_type>(__idx) * __strides_[_Pos]) + ... + index_type(0));
     }(make_index_sequence<sizeof...(_Indices)>());

libcxx/include/__ranges/chunk_by_view.h

@@ -66,8 +66,10 @@ class _LIBCPP_ABI_LLVM18_NO_UNIQUE_ADDRESS chunk_by_view : public view_interface
 
   _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));
     };
@@ -81,8 +83,10 @@ class _LIBCPP_ABI_LLVM18_NO_UNIQUE_ADDRESS chunk_by_view : public view_interface
     // 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}};
@@ -112,8 +116,9 @@ class _LIBCPP_ABI_LLVM18_NO_UNIQUE_ADDRESS chunk_by_view : public view_interface
 
   _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()) {

libcxx/include/__ranges/drop_while_view.h

@@ -66,10 +66,11 @@ class _LIBCPP_ABI_LLVM18_NO_UNIQUE_ADDRESS drop_while_view : public view_interfa
 
   _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_)));

libcxx/include/__ranges/filter_view.h

@@ -84,8 +84,9 @@ class _LIBCPP_ABI_LLVM18_NO_UNIQUE_ADDRESS filter_view : public view_interface<f
 
   _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_)));

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 == '&')

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;
 }