[libc++][hardening] Categorize more 'valid-element-access' checks. #71620

var-const · 2023-11-08T01:57:58Z

No description provided.

llvmbot · 2023-11-08T01:58:31Z

@llvm/pr-subscribers-libcxx

Author: Konstantin Varlamov (var-const)

Changes

Patch is 27.30 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71620.diff

14 Files Affected:

(modified) libcxx/include/__algorithm/ranges_max.h (+3-2)
(modified) libcxx/include/__algorithm/ranges_min.h (+3-2)
(modified) libcxx/include/__algorithm/ranges_minmax.h (+3-2)
(modified) libcxx/include/__algorithm/sample.h (+1-1)
(modified) libcxx/include/__format/formatter_output.h (+3-3)
(modified) libcxx/include/__format/parser_std_format_spec.h (+9-9)
(modified) libcxx/include/__iterator/common_iterator.h (+32-32)
(modified) libcxx/include/__iterator/counted_iterator.h (+6-6)
(modified) libcxx/include/__ranges/subrange.h (+1-1)
(modified) libcxx/include/__ranges/view_interface.h (+8-8)
(modified) libcxx/include/__utility/is_pointer_in_range.h (+1-1)
(modified) libcxx/include/experimental/__simd/vec_ext.h (+2-2)
(modified) libcxx/src/support/ibm/xlocale_zos.cpp (+1-1)
(modified) libcxx/test/libcxx/algorithms/alg.sorting/assert.min.max.pass.cpp (+1-1)

diff --git a/libcxx/include/__algorithm/ranges_max.h b/libcxx/include/__algorithm/ranges_max.h
index 5cc418d3393cef6..d51c5a933b2ab23 100644
--- a/libcxx/include/__algorithm/ranges_max.h
+++ b/libcxx/include/__algorithm/ranges_max.h
@@ -54,7 +54,8 @@ struct __fn {
             indirect_strict_weak_order<projected<const _Tp*, _Proj>> _Comp = ranges::less>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr _Tp
   operator()(initializer_list<_Tp> __il, _Comp __comp = {}, _Proj __proj = {}) const {
-    _LIBCPP_ASSERT_UNCATEGORIZED(__il.begin() != __il.end(), "initializer_list must contain at least one element");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+        __il.begin() != __il.end(), "initializer_list must contain at least one element");
 
     auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) { return std::invoke(__comp, __rhs, __lhs); };
     return *ranges::__min_element_impl(__il.begin(), __il.end(), __comp_lhs_rhs_swapped, __proj);
@@ -69,7 +70,7 @@ struct __fn {
     auto __first = ranges::begin(__r);
     auto __last  = ranges::end(__r);
 
-    _LIBCPP_ASSERT_UNCATEGORIZED(__first != __last, "range must contain at least one element");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__first != __last, "range must contain at least one element");
 
     if constexpr (forward_range<_Rp> && !__is_cheap_to_copy<range_value_t<_Rp>>) {
       auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) { return std::invoke(__comp, __rhs, __lhs); };
diff --git a/libcxx/include/__algorithm/ranges_min.h b/libcxx/include/__algorithm/ranges_min.h
index be15b4536734df2..8757358cdf37d95 100644
--- a/libcxx/include/__algorithm/ranges_min.h
+++ b/libcxx/include/__algorithm/ranges_min.h
@@ -53,7 +53,8 @@ struct __fn {
             indirect_strict_weak_order<projected<const _Tp*, _Proj>> _Comp = ranges::less>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr _Tp
   operator()(initializer_list<_Tp> __il, _Comp __comp = {}, _Proj __proj = {}) const {
-    _LIBCPP_ASSERT_UNCATEGORIZED(__il.begin() != __il.end(), "initializer_list must contain at least one element");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+        __il.begin() != __il.end(), "initializer_list must contain at least one element");
     return *ranges::__min_element_impl(__il.begin(), __il.end(), __comp, __proj);
   }
 
@@ -65,7 +66,7 @@ struct __fn {
   operator()(_Rp&& __r, _Comp __comp = {}, _Proj __proj = {}) const {
     auto __first = ranges::begin(__r);
     auto __last  = ranges::end(__r);
-    _LIBCPP_ASSERT_UNCATEGORIZED(__first != __last, "range must contain at least one element");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__first != __last, "range must contain at least one element");
     if constexpr (forward_range<_Rp> && !__is_cheap_to_copy<range_value_t<_Rp>>) {
       return *ranges::__min_element_impl(__first, __last, __comp, __proj);
     } else {
diff --git a/libcxx/include/__algorithm/ranges_minmax.h b/libcxx/include/__algorithm/ranges_minmax.h
index a5b5cf9bd0ab98f..22a62b620c936f4 100644
--- a/libcxx/include/__algorithm/ranges_minmax.h
+++ b/libcxx/include/__algorithm/ranges_minmax.h
@@ -65,7 +65,8 @@ struct __fn {
             indirect_strict_weak_order<projected<const _Type*, _Proj>> _Comp = ranges::less>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr ranges::minmax_result<_Type>
   operator()(initializer_list<_Type> __il, _Comp __comp = {}, _Proj __proj = {}) const {
-    _LIBCPP_ASSERT_UNCATEGORIZED(__il.begin() != __il.end(), "initializer_list has to contain at least one element");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+        __il.begin() != __il.end(), "initializer_list has to contain at least one element");
     auto __iters = std::__minmax_element_impl(__il.begin(), __il.end(), __comp, __proj);
     return ranges::minmax_result<_Type>{*__iters.first, *__iters.second};
   }
@@ -80,7 +81,7 @@ struct __fn {
     auto __last   = ranges::end(__r);
     using _ValueT = range_value_t<_Range>;
 
-    _LIBCPP_ASSERT_UNCATEGORIZED(__first != __last, "range has to contain at least one element");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__first != __last, "range has to contain at least one element");
 
     if constexpr (forward_range<_Range>) {
       // Special-case the one element case. Avoid repeatedly initializing objects from the result of an iterator
diff --git a/libcxx/include/__algorithm/sample.h b/libcxx/include/__algorithm/sample.h
index c7a1898e5308b11..ab49c7643de7f3c 100644
--- a/libcxx/include/__algorithm/sample.h
+++ b/libcxx/include/__algorithm/sample.h
@@ -77,7 +77,7 @@ _LIBCPP_INLINE_VISIBILITY
 _SampleIterator __sample(_PopulationIterator __first,
                          _PopulationSentinel __last, _SampleIterator __output_iter,
                          _Distance __n, _UniformRandomNumberGenerator& __g) {
-  _LIBCPP_ASSERT_UNCATEGORIZED(__n >= 0, "N must be a positive number.");
+  _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__n >= 0, "N must be a positive number.");
 
   using _PopIterCategory = typename _IterOps<_AlgPolicy>::template __iterator_category<_PopulationIterator>;
   using _Difference = typename _IterOps<_AlgPolicy>::template __difference_type<_PopulationIterator>;
diff --git a/libcxx/include/__format/formatter_output.h b/libcxx/include/__format/formatter_output.h
index 072305b6dbca10c..6fa0c2f5ba95898 100644
--- a/libcxx/include/__format/formatter_output.h
+++ b/libcxx/include/__format/formatter_output.h
@@ -243,7 +243,7 @@ __write(_Iterator __first,
         output_iterator<const iter_value_t<_Iterator>&> auto __out_it,
         __format_spec::__parsed_specifications<_ParserCharT> __specs,
         ptrdiff_t __size) -> decltype(__out_it) {
-  _LIBCPP_ASSERT_UNCATEGORIZED(__first <= __last, "Not a valid range");
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(__first <= __last, "Not a valid range");
   return __formatter::__write(basic_string_view{__first, __last}, _VSTD::move(__out_it), __specs, __size);
 }
 
@@ -256,7 +256,7 @@ __write(_Iterator __first,
         _Iterator __last,
         output_iterator<const iter_value_t<_Iterator>&> auto __out_it,
         __format_spec::__parsed_specifications<_ParserCharT> __specs) -> decltype(__out_it) {
-  _LIBCPP_ASSERT_UNCATEGORIZED(__first <= __last, "Not a valid range");
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(__first <= __last, "Not a valid range");
   return __formatter::__write(__first, __last, _VSTD::move(__out_it), __specs, __last - __first);
 }
 
@@ -265,7 +265,7 @@ _LIBCPP_HIDE_FROM_ABI auto __write_transformed(const _CharT* __first, const _Cha
                                                output_iterator<const _CharT&> auto __out_it,
                                                __format_spec::__parsed_specifications<_ParserCharT> __specs,
                                                _UnaryOperation __op) -> decltype(__out_it) {
-  _LIBCPP_ASSERT_UNCATEGORIZED(__first <= __last, "Not a valid range");
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(__first <= __last, "Not a valid range");
 
   ptrdiff_t __size = __last - __first;
   if (__size >= __specs.__width_)
diff --git a/libcxx/include/__format/parser_std_format_spec.h b/libcxx/include/__format/parser_std_format_spec.h
index e79fc8fc481bcfc..c0aeddc2830bd65 100644
--- a/libcxx/include/__format/parser_std_format_spec.h
+++ b/libcxx/include/__format/parser_std_format_spec.h
@@ -593,9 +593,9 @@ class _LIBCPP_TEMPLATE_VIS __parser {
           || (same_as<_CharT, wchar_t> && sizeof(wchar_t) == 2)
 #    endif
   _LIBCPP_HIDE_FROM_ABI constexpr bool __parse_fill_align(_Iterator& __begin, _Iterator __end, bool __use_range_fill) {
-    _LIBCPP_ASSERT_UNCATEGORIZED(__begin != __end,
-                                 "when called with an empty input the function will cause "
-                                 "undefined behavior by evaluating data not in the input");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__begin != __end,
+                                        "when called with an empty input the function will cause "
+                                        "undefined behavior by evaluating data not in the input");
     __unicode::__code_point_view<_CharT> __view{__begin, __end};
     __unicode::__consume_result __consumed = __view.__consume();
     if (__consumed.__status != __unicode::__consume_result::__ok)
@@ -625,9 +625,9 @@ class _LIBCPP_TEMPLATE_VIS __parser {
   template <contiguous_iterator _Iterator>
     requires(same_as<_CharT, wchar_t> && sizeof(wchar_t) == 4)
   _LIBCPP_HIDE_FROM_ABI constexpr bool __parse_fill_align(_Iterator& __begin, _Iterator __end, bool __use_range_fill) {
-    _LIBCPP_ASSERT_UNCATEGORIZED(__begin != __end,
-                                 "when called with an empty input the function will cause "
-                                 "undefined behavior by evaluating data not in the input");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__begin != __end,
+                                        "when called with an empty input the function will cause "
+                                        "undefined behavior by evaluating data not in the input");
     if (__begin + 1 != __end && __parse_alignment(*(__begin + 1))) {
       if (!__unicode::__is_scalar_value(*__begin))
         std::__throw_format_error("The fill option contains an invalid value");
@@ -652,9 +652,9 @@ class _LIBCPP_TEMPLATE_VIS __parser {
   // range-fill and tuple-fill are identical
   template <contiguous_iterator _Iterator>
   _LIBCPP_HIDE_FROM_ABI constexpr bool __parse_fill_align(_Iterator& __begin, _Iterator __end, bool __use_range_fill) {
-    _LIBCPP_ASSERT_UNCATEGORIZED(__begin != __end,
-                                 "when called with an empty input the function will cause "
-                                 "undefined behavior by evaluating data not in the input");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__begin != __end,
+                                        "when called with an empty input the function will cause "
+                                        "undefined behavior by evaluating data not in the input");
     if (__begin + 1 != __end) {
       if (__parse_alignment(*(__begin + 1))) {
         __validate_fill_character(*__begin, __use_range_fill);
diff --git a/libcxx/include/__iterator/common_iterator.h b/libcxx/include/__iterator/common_iterator.h
index 95e248d83f4b460..3c344f4be18ad8d 100644
--- a/libcxx/include/__iterator/common_iterator.h
+++ b/libcxx/include/__iterator/common_iterator.h
@@ -75,8 +75,8 @@ class common_iterator {
     requires convertible_to<const _I2&, _Iter> && convertible_to<const _S2&, _Sent>
   _LIBCPP_HIDE_FROM_ABI constexpr common_iterator(const common_iterator<_I2, _S2>& __other)
     : __hold_([&]() -> variant<_Iter, _Sent> {
-      _LIBCPP_ASSERT_UNCATEGORIZED(!__other.__hold_.valueless_by_exception(),
-                                   "Attempted to construct from a valueless common_iterator");
+      _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!__other.__hold_.valueless_by_exception(),
+                                          "Attempted to construct from a valueless common_iterator");
       if (__other.__hold_.index() == 0)
         return variant<_Iter, _Sent>{in_place_index<0>, _VSTD::__unchecked_get<0>(__other.__hold_)};
       return variant<_Iter, _Sent>{in_place_index<1>, _VSTD::__unchecked_get<1>(__other.__hold_)};
@@ -86,8 +86,8 @@ class common_iterator {
     requires convertible_to<const _I2&, _Iter> && convertible_to<const _S2&, _Sent> &&
              assignable_from<_Iter&, const _I2&> && assignable_from<_Sent&, const _S2&>
   _LIBCPP_HIDE_FROM_ABI common_iterator& operator=(const common_iterator<_I2, _S2>& __other) {
-    _LIBCPP_ASSERT_UNCATEGORIZED(!__other.__hold_.valueless_by_exception(),
-                                 "Attempted to assign from a valueless common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!__other.__hold_.valueless_by_exception(),
+                                        "Attempted to assign from a valueless common_iterator");
 
     auto __idx = __hold_.index();
     auto __other_idx = __other.__hold_.index();
@@ -109,16 +109,16 @@ class common_iterator {
 
   _LIBCPP_HIDE_FROM_ABI constexpr decltype(auto) operator*()
   {
-    _LIBCPP_ASSERT_UNCATEGORIZED(std::holds_alternative<_Iter>(__hold_),
-                                 "Attempted to dereference a non-dereferenceable common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(std::holds_alternative<_Iter>(__hold_),
+                                        "Attempted to dereference a non-dereferenceable common_iterator");
     return *_VSTD::__unchecked_get<_Iter>(__hold_);
   }
 
   _LIBCPP_HIDE_FROM_ABI constexpr decltype(auto) operator*() const
     requires __dereferenceable<const _Iter>
   {
-    _LIBCPP_ASSERT_UNCATEGORIZED(std::holds_alternative<_Iter>(__hold_),
-                                 "Attempted to dereference a non-dereferenceable common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(std::holds_alternative<_Iter>(__hold_),
+                                        "Attempted to dereference a non-dereferenceable common_iterator");
     return *_VSTD::__unchecked_get<_Iter>(__hold_);
   }
 
@@ -129,8 +129,8 @@ class common_iterator {
      is_reference_v<iter_reference_t<_I2>> ||
      constructible_from<iter_value_t<_I2>, iter_reference_t<_I2>>)
   {
-    _LIBCPP_ASSERT_UNCATEGORIZED(std::holds_alternative<_Iter>(__hold_),
-                                 "Attempted to dereference a non-dereferenceable common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(std::holds_alternative<_Iter>(__hold_),
+                                        "Attempted to dereference a non-dereferenceable common_iterator");
     if constexpr (is_pointer_v<_Iter> || requires(const _Iter& __i) { __i.operator->(); })    {
       return _VSTD::__unchecked_get<_Iter>(__hold_);
     } else if constexpr (is_reference_v<iter_reference_t<_Iter>>) {
@@ -142,14 +142,14 @@ class common_iterator {
   }
 
   _LIBCPP_HIDE_FROM_ABI common_iterator& operator++() {
-    _LIBCPP_ASSERT_UNCATEGORIZED(std::holds_alternative<_Iter>(__hold_),
-                                 "Attempted to increment a non-dereferenceable common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(std::holds_alternative<_Iter>(__hold_),
+                                        "Attempted to increment a non-dereferenceable common_iterator");
     ++_VSTD::__unchecked_get<_Iter>(__hold_); return *this;
   }
 
   _LIBCPP_HIDE_FROM_ABI decltype(auto) operator++(int) {
-    _LIBCPP_ASSERT_UNCATEGORIZED(std::holds_alternative<_Iter>(__hold_),
-                                 "Attempted to increment a non-dereferenceable common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(std::holds_alternative<_Iter>(__hold_),
+                                        "Attempted to increment a non-dereferenceable common_iterator");
     if constexpr (forward_iterator<_Iter>) {
       auto __tmp = *this;
       ++*this;
@@ -168,10 +168,10 @@ class common_iterator {
     requires sentinel_for<_Sent, _I2>
   _LIBCPP_HIDE_FROM_ABI
   friend constexpr bool operator==(const common_iterator& __x, const common_iterator<_I2, _S2>& __y) {
-    _LIBCPP_ASSERT_UNCATEGORIZED(!__x.__hold_.valueless_by_exception(),
-                                 "Attempted to compare a valueless common_iterator");
-    _LIBCPP_ASSERT_UNCATEGORIZED(!__y.__hold_.valueless_by_exception(),
-                                 "Attempted to compare a valueless common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!__x.__hold_.valueless_by_exception(),
+                                        "Attempted to compare a valueless common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!__y.__hold_.valueless_by_exception(),
+                                        "Attempted to compare a valueless common_iterator");
 
     auto __x_index = __x.__hold_.index();
     auto __y_index = __y.__hold_.index();
@@ -189,10 +189,10 @@ class common_iterator {
     requires sentinel_for<_Sent, _I2> && equality_comparable_with<_Iter, _I2>
   _LIBCPP_HIDE_FROM_ABI
   friend constexpr bool operator==(const common_iterator& __x, const common_iterator<_I2, _S2>& __y) {
-    _LIBCPP_ASSERT_UNCATEGORIZED(!__x.__hold_.valueless_by_exception(),
-                                 "Attempted to compare a valueless common_iterator");
-    _LIBCPP_ASSERT_UNCATEGORIZED(!__y.__hold_.valueless_by_exception(),
-                                 "Attempted to compare a valueless common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!__x.__hold_.valueless_by_exception(),
+                                        "Attempted to compare a valueless common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!__y.__hold_.valueless_by_exception(),
+                                        "Attempted to compare a valueless common_iterator");
 
     auto __x_index = __x.__hold_.index();
     auto __y_index = __y.__hold_.index();
@@ -213,10 +213,10 @@ class common_iterator {
     requires sized_sentinel_for<_Sent, _I2>
   _LIBCPP_HIDE_FROM_ABI
   friend constexpr iter_difference_t<_I2> operator-(const common_iterator& __x, const common_iterator<_I2, _S2>& __y) {
-    _LIBCPP_ASSERT_UNCATEGORIZED(!__x.__hold_.valueless_by_exception(),
-                                 "Attempted to subtract from a valueless common_iterator");
-    _LIBCPP_ASSERT_UNCATEGORIZED(!__y.__hold_.valueless_by_exception(),
-                                 "Attempted to subtract a valueless common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!__x.__hold_.valueless_by_exception(),
+                                        "Attempted to subtract from a valueless common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(!__y.__hold_.valueless_by_exception(),
+                                        "Attempted to subtract a valueless common_iterator");
 
     auto __x_index = __x.__hold_.index();
     auto __y_index = __y.__hold_.index();
@@ -237,8 +237,8 @@ class common_iterator {
     noexcept(noexcept(ranges::iter_move(std::declval<const _Iter&>())))
       requires input_iterator<_Iter>
   {
-    _LIBCPP_ASSERT_UNCATEGORIZED(std::holds_alternative<_Iter>(__i.__hold_),
-                                 "Attempted to iter_move a non-dereferenceable common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(std::holds_alternative<_Iter>(__i.__hold_),
+                                        "Attempted to iter_move a non-dereferenceable common_iterator");
     return ranges::iter_move( _VSTD::__unchecked_get<_Iter>(__i.__hold_));
   }
 
@@ -246,10 +246,10 @@ class common_iterator {
   _LIBCPP_HIDE_FROM_ABI friend constexpr void iter_swap(const common_iterator& __x, const common_iterator<_I2, _S2>& __y)
       noexcept(noexcept(ranges::iter_swap(std::declval<const _Iter&>(), std::declval<const _I2&>())))
   {
-    _LIBCPP_ASSERT_UNCATEGORIZED(std::holds_alternative<_Iter>(__x.__hold_),
-                                 "Attempted to iter_swap a non-dereferenceable common_iterator");
-    _LIBCPP_ASSERT_UNCATEGORIZED(std::holds_alternative<_I2>(__y.__hold_),
-                                 "Attempted to iter_swap a non-dereferenceable common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(std::holds_alternative<_Iter>(__x.__hold_),
+                                        "Attempted to iter_swap a non-dereferenceable common_iterator");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(std::holds_alternative<_I2>(__y.__hold_),
+                                        "Attempted to iter_swap a non-dereferenceable common_iterator");
     return ranges::iter_swap(_VSTD::__unchecked_get<_Iter>(__x.__hold_), _VSTD::__unchecked_get<_I2>(__y.__hold_));
   }
 };
diff --git a/libcxx/include/__iterator/counted_iterator.h b/libcxx/include/__iterator/counted_iterator.h
index 41b7e57d28c1451..7e210b653fb01ba 100644
--- a/libcxx/include/__iterator/counted_iterator.h
+++ b/libcxx/include/__iterator/counted_iterator.h
@@ -115,7 +115,7 @@ class counted_iterator
 
   _LIBCPP_HIDE_FROM_ABI
   constexpr decltype(auto) operator*() {
-    _LIBCPP_ASSERT_UNCATEGORIZED(__count_ > 0, "Iterator is equal to or past end.");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__count_ > 0, "Iterator is equal to or past end.");
     return *__current_;
   }
 
@@ -123,7 +123,7 @@ class counted_iterator
   constexpr decltype(auto) operator*() const
     requires __dereferenceable<const _Iter>
   {
-    _LIBCPP_ASSERT_UNCATEGORIZED(__count_ > 0, "Iterator is equal to or past end.");
+    _LIBCPP...
[truncated]

ldionne

This mostly LGTM, but I'd like to add tests for the iterator operations at least. It's reasonable for some of the other utilities not to add a bunch of tests, but for e.g. counted_iterator I think it's sufficiently high-value that we should be adding assertion tests.

libcxx/include/__iterator/common_iterator.h

libcxx/include/__iterator/counted_iterator.h

mordante

Thanks for working on this. No additional comments for me.

github-actions · 2023-12-16T00:21:15Z

✅ With the latest revision this PR passed the C/C++ code formatter.

hawkinsw

Adding a note about capitalization. I hope that it's helpful.

hawkinsw · 2023-12-16T02:23:40Z

libcxx/include/__format/parser_std_format_spec.h

-                                        "undefined behavior by evaluating data not in the input");
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+        __begin != __end,
+        "when called with an empty input the function will cause "


Nit: (hopefully helpful) All other assertion messages begin with capital letters.

Suggested change

"when called with an empty input the function will cause "

"When called with an empty input, the function will cause "

If you think that this suggestion is helpful, I can put them on the others to make it easier for you.

Thanks! It's a good suggestion, but for this PR specifically, I'd prefer to keep the delta as small as possible, even if it means leaving some less-than-perfect error messages in place. It would be good to do a follow-up to improve some messages, though.

ldionne

LGTM, thanks!

libcxx/test/libcxx/iterators/predef.iterators/iterators.common/assert.pass.cpp

var-const requested a review from a team as a code owner November 8, 2023 01:57

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 8, 2023

var-const added the hardening Issues related to the hardening effort label Nov 8, 2023

ldionne requested changes Nov 8, 2023

View reviewed changes

libcxx/include/__iterator/common_iterator.h Outdated Show resolved Hide resolved

libcxx/include/__iterator/common_iterator.h Outdated Show resolved Hide resolved

libcxx/include/__iterator/counted_iterator.h Show resolved Hide resolved

mordante reviewed Nov 25, 2023

View reviewed changes

var-const force-pushed the varconst/hardening-categorization-2 branch from 6331da5 to 6460a34 Compare December 16, 2023 00:18

hawkinsw reviewed Dec 16, 2023

View reviewed changes

var-const added 3 commits December 19, 2023 01:38

[libc++][hardening] Categorize more 'valid-element-access' checks.

4b7d2e8

Address feedback (test iterator operations)

6e80158

Quick fix to new tests

aca4fd3

var-const force-pushed the varconst/hardening-categorization-2 branch from 93005e2 to aca4fd3 Compare December 19, 2023 09:40

var-const added 2 commits December 19, 2023 01:48

Fix bad merge

1864bb1

Fix the name of the mode in tests

38f0ba9

var-const mentioned this pull request Dec 19, 2023

[libc++][hardening] Categorize more assertions. #75918

Merged

ldionne approved these changes Dec 20, 2023

View reviewed changes

libcxx/test/libcxx/iterators/predef.iterators/iterators.common/assert.pass.cpp Outdated Show resolved Hide resolved

Remove a TODO

4946f40

var-const merged commit 1638657 into llvm:main Dec 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++][hardening] Categorize more 'valid-element-access' checks. #71620

[libc++][hardening] Categorize more 'valid-element-access' checks. #71620

Uh oh!

var-const commented Nov 8, 2023

Uh oh!

llvmbot commented Nov 8, 2023

Uh oh!

ldionne left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mordante left a comment

Uh oh!

github-actions bot commented Dec 16, 2023 •

edited

Loading

Uh oh!

hawkinsw left a comment

Uh oh!

hawkinsw Dec 16, 2023

Uh oh!

hawkinsw Dec 16, 2023

Uh oh!

var-const Dec 19, 2023

Uh oh!

ldionne left a comment

Uh oh!

Uh oh!

Uh oh!

	"when called with an empty input the function will cause "
	"When called with an empty input, the function will cause "

[libc++][hardening] Categorize more 'valid-element-access' checks. #71620

[libc++][hardening] Categorize more 'valid-element-access' checks. #71620

Uh oh!

Conversation

var-const commented Nov 8, 2023

Uh oh!

llvmbot commented Nov 8, 2023

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mordante left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hawkinsw left a comment

Choose a reason for hiding this comment

Uh oh!

hawkinsw Dec 16, 2023

Choose a reason for hiding this comment

Uh oh!

hawkinsw Dec 16, 2023

Choose a reason for hiding this comment

Uh oh!

var-const Dec 19, 2023

Choose a reason for hiding this comment

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 16, 2023 •

edited

Loading