[libcxx-commits] [libcxx] [libcxx] adds additional checks to RAI containers' `erase` (PR #90919)

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Christopher Di Bella (cjdb)

<details>
<summary>Changes</summary>

This ensures that the iterators passed in are in-range for the container. Thanks to @<!-- -->mclow for identifying this opportunity.

---
Full diff: https://github.com/llvm/llvm-project/pull/90919.diff


3 Files Affected:

- (modified) libcxx/include/deque (+9-2) 
- (modified) libcxx/include/string (+9-2) 
- (modified) libcxx/include/vector (+9-2) 


``````````diff
diff --git a/libcxx/include/deque b/libcxx/include/deque
index 3c33e04e9f05f8..2df644cd3d80ea 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -2372,7 +2372,9 @@ void deque<_Tp, _Allocator>::__move_construct_backward_and_check(
 template <class _Tp, class _Allocator>
 typename deque<_Tp, _Allocator>::iterator deque<_Tp, _Allocator>::erase(const_iterator __f) {
   _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
-      __f != end(), "deque::erase(iterator) called with a non-dereferenceable iterator");
+      __f >= begin(), "deque::erase(iterator) called with an iterator outside of the container's control");
+  _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+      __f < end(), "deque::erase(iterator) called with an iterator outside of the container's control");
   size_type __old_sz    = size();
   size_type __old_start = __start_;
   iterator __b          = begin();
@@ -2398,7 +2400,12 @@ typename deque<_Tp, _Allocator>::iterator deque<_Tp, _Allocator>::erase(const_it
 
 template <class _Tp, class _Allocator>
 typename deque<_Tp, _Allocator>::iterator deque<_Tp, _Allocator>::erase(const_iterator __f, const_iterator __l) {
-  _LIBCPP_ASSERT_VALID_INPUT_RANGE(__f <= __l, "deque::erase(first, last) called with an invalid range");
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(
+    __f <= begin(), "deque::erase(first, last) called with an iterator range starting before 'begin()'");
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(
+    __l <= end(), "deque::erase(first, last) called with an iterator range finishing after 'end()'");
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(
+    __f <= __l, "deque::erase(first, last) called with an invalid range");
   size_type __old_sz    = size();
   size_type __old_start = __start_;
   difference_type __n   = __l - __f;
diff --git a/libcxx/include/string b/libcxx/include/string
index 4e3dd278c12b0c..3f15e0b766edd8 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -3155,7 +3155,9 @@ template <class _CharT, class _Traits, class _Allocator>
 inline _LIBCPP_CONSTEXPR_SINCE_CXX20 typename basic_string<_CharT, _Traits, _Allocator>::iterator
 basic_string<_CharT, _Traits, _Allocator>::erase(const_iterator __pos) {
   _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
-      __pos != end(), "string::erase(iterator) called with a non-dereferenceable iterator");
+      __pos >= begin(), "string::erase(iterator) called with an iterator outside of the container's control");
+  _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+      __pos < end(), "string::erase(iterator) called with an iterator outside of the container's control");
   iterator __b  = begin();
   size_type __r = static_cast<size_type>(__pos - __b);
   erase(__r, 1);
@@ -3165,7 +3167,12 @@ basic_string<_CharT, _Traits, _Allocator>::erase(const_iterator __pos) {
 template <class _CharT, class _Traits, class _Allocator>
 inline _LIBCPP_CONSTEXPR_SINCE_CXX20 typename basic_string<_CharT, _Traits, _Allocator>::iterator
 basic_string<_CharT, _Traits, _Allocator>::erase(const_iterator __first, const_iterator __last) {
-  _LIBCPP_ASSERT_VALID_INPUT_RANGE(__first <= __last, "string::erase(first, last) called with invalid range");
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(
+    __first >= begin(), "string::erase(first, last) called with an iterator range starting before 'begin()'");
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(
+    __last <= end(), "string::erase(first, last) called with an iterator range finishing after 'end()'");
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(
+    __first <= __last, "string::erase(first, last) called with invalid range");
   iterator __b  = begin();
   size_type __r = static_cast<size_type>(__first - __b);
   erase(__r, static_cast<size_type>(__last - __first));
diff --git a/libcxx/include/vector b/libcxx/include/vector
index 976bde9b9048c8..16cb3bab942dd6 100644
--- a/libcxx/include/vector
+++ b/libcxx/include/vector
@@ -1537,7 +1537,9 @@ template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 inline _LIBCPP_HIDE_FROM_ABI typename vector<_Tp, _Allocator>::iterator
 vector<_Tp, _Allocator>::erase(const_iterator __position) {
   _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
-      __position != end(), "vector::erase(iterator) called with a non-dereferenceable iterator");
+      __position >= begin(), "vector::erase(iterator) called with an iterator outside of the container's control");
+  _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+      __position < end(), "vector::erase(iterator) called with an iterator outside of the container's control");
   difference_type __ps = __position - cbegin();
   pointer __p          = this->__begin_ + __ps;
   this->__destruct_at_end(std::move(__p + 1, this->__end_, __p));
@@ -1547,7 +1549,12 @@ vector<_Tp, _Allocator>::erase(const_iterator __position) {
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 typename vector<_Tp, _Allocator>::iterator
 vector<_Tp, _Allocator>::erase(const_iterator __first, const_iterator __last) {
-  _LIBCPP_ASSERT_VALID_INPUT_RANGE(__first <= __last, "vector::erase(first, last) called with invalid range");
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(
+    __first >= begin(), "vector::erase(first, last) called with an iterator range starting before 'begin()'");
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(
+    __last <= end(), "vector::erase(first, last) called with an iterator range finishing after 'end()'");
+  _LIBCPP_ASSERT_VALID_INPUT_RANGE(
+    __first <= __last, "vector::erase(first, last) called with invalid range");
   pointer __p = this->__begin_ + (__first - begin());
   if (__first != __last) {
     this->__destruct_at_end(std::move(__p + (__last - __first), this->__end_, __p));

``````````

</details>


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