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

[Christopher Di Bella via libcxx-commits]

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

>From d642ffee047dc5ee6a1ff1a37ee1d0913b5fd5e4 Mon Sep 17 00:00:00 2001
From: Christopher Di Bella <cjdb at google.com>
Date: Thu, 2 May 2024 23:56:48 +0000
Subject: [PATCH 1/2] [libcxx] adds additional checks to RAI containers'
 `erase`

This ensures that the iterators passed in are in-range for the
container. Thanks to @mclow for identifying this opportunity.
---
 libcxx/include/deque  | 11 +++++++++--
 libcxx/include/string | 11 +++++++++--
 libcxx/include/vector | 11 +++++++++--
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/libcxx/include/deque b/libcxx/include/deque
index 3c33e04e9f05f..2df644cd3d80e 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 4e3dd278c12b0..3f15e0b766edd 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 976bde9b9048c..16cb3bab942dd 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));

>From 4f36eb47800ee0dd7fe96092849c606be8370ed9 Mon Sep 17 00:00:00 2001
From: Christopher Di Bella <cjdb.ns at gmail.com>
Date: Mon, 10 Jun 2024 10:07:55 -0700
Subject: [PATCH 2/2] Update libcxx/include/deque

Co-authored-by: Louis Dionne <ldionne.2 at gmail.com>
---
 libcxx/include/deque | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/include/deque b/libcxx/include/deque
index 2df644cd3d80e..232285c84724b 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -2401,7 +2401,7 @@ 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 <= begin(), "deque::erase(first, last) called with an iterator range starting before 'begin()'");
+    __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(