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

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jul 9 12:22:12 PDT 2024


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

>From e3ad6fcbc8c13511c455da2dcad4c716875c365b 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/3] [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 |  8 +++++++-
 libcxx/include/vector | 11 ++++++++---
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/libcxx/include/deque b/libcxx/include/deque
index 4fc994a6e229b..48e732208263b 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -2380,7 +2380,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();
@@ -2406,6 +2408,10 @@ 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()'");
+  _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_;
@@ -2530,8 +2536,7 @@ inline _LIBCPP_HIDE_FROM_ABI bool operator<=(const deque<_Tp, _Allocator>& __x,
 template <class _Tp, class _Allocator>
 _LIBCPP_HIDE_FROM_ABI __synth_three_way_result<_Tp>
 operator<=>(const deque<_Tp, _Allocator>& __x, const deque<_Tp, _Allocator>& __y) {
-  return std::lexicographical_compare_three_way(
-      __x.begin(), __x.end(), __y.begin(), __y.end(), std::__synth_three_way);
+  return std::lexicographical_compare_three_way(__x.begin(), __x.end(), __y.begin(), __y.end(), std::__synth_three_way);
 }
 
 #endif // _LIBCPP_STD_VER <= 17
diff --git a/libcxx/include/string b/libcxx/include/string
index 9a52ab6aef41e..306d3c8227fd9 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -3166,7 +3166,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);
@@ -3176,6 +3178,10 @@ 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 >= 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);
diff --git a/libcxx/include/vector b/libcxx/include/vector
index aaf51d18fe30f..de1e202fa57d7 100644
--- a/libcxx/include/vector
+++ b/libcxx/include/vector
@@ -1534,7 +1534,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));
@@ -1544,6 +1546,10 @@ 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 >= 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) {
@@ -2902,8 +2908,7 @@ inline _LIBCPP_HIDE_FROM_ABI bool operator<=(const vector<_Tp, _Allocator>& __x,
 template <class _Tp, class _Allocator>
 _LIBCPP_HIDE_FROM_ABI constexpr __synth_three_way_result<_Tp>
 operator<=>(const vector<_Tp, _Allocator>& __x, const vector<_Tp, _Allocator>& __y) {
-  return std::lexicographical_compare_three_way(
-      __x.begin(), __x.end(), __y.begin(), __y.end(), std::__synth_three_way);
+  return std::lexicographical_compare_three_way(__x.begin(), __x.end(), __y.begin(), __y.end(), std::__synth_three_way);
 }
 
 #endif // _LIBCPP_STD_VER <= 17

>From 555564113686fb168e3459b1686540876d90e95c 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/3] 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 48e732208263b..28e2ca3b64f2a 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -2409,7 +2409,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(__f <= __l, "deque::erase(first, last) called with an invalid range");

>From 22f1a9820f533b44357866001a660fa3bd2dc409 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Tue, 9 Jul 2024 15:19:33 -0400
Subject: [PATCH 3/3] - Use __is_pointer_in_range for a non-UB comparison - Add
 tests - Downgrade checks we do in deque cause we can't detect iterator
 ownership for non-contiguous containers

---
 libcxx/include/deque                          | 11 +---
 libcxx/include/string                         | 12 ++--
 libcxx/include/vector                         | 15 +++--
 .../deque/assert.erase.iter.pass.cpp}         | 24 ++++----
 .../deque/assert.erase.iter_iter.pass.cpp     | 33 ++++++++++
 .../sequences/deque/assert.pass.cpp           |  4 --
 .../vector/assert.erase.iter.pass.cpp         | 43 +++++++++++++
 .../vector/assert.erase.iter_iter.pass.cpp    | 60 +++++++++++++++++++
 .../assert.erase.iter.pass.cpp                | 47 +++++++++++++++
 ...ss.cpp => assert.erase.iter_iter.pass.cpp} | 36 ++++++-----
 .../debug.erase.iter.pass.cpp                 | 35 -----------
 11 files changed, 233 insertions(+), 87 deletions(-)
 rename libcxx/test/libcxx/{strings/basic.string/string.modifiers/assert.erase_iter.null.pass.cpp => containers/sequences/deque/assert.erase.iter.pass.cpp} (59%)
 create mode 100644 libcxx/test/libcxx/containers/sequences/deque/assert.erase.iter_iter.pass.cpp
 create mode 100644 libcxx/test/libcxx/containers/sequences/vector/assert.erase.iter.pass.cpp
 create mode 100644 libcxx/test/libcxx/containers/sequences/vector/assert.erase.iter_iter.pass.cpp
 create mode 100644 libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.erase.iter.pass.cpp
 rename libcxx/test/libcxx/strings/basic.string/string.modifiers/{debug.erase.iter_iter.pass.cpp => assert.erase.iter_iter.pass.cpp} (50%)
 delete mode 100644 libcxx/test/libcxx/strings/basic.string/string.modifiers/debug.erase.iter.pass.cpp

diff --git a/libcxx/include/deque b/libcxx/include/deque
index 28e2ca3b64f2a..4fc994a6e229b 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -2380,9 +2380,7 @@ 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 >= 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");
+      __f != end(), "deque::erase(iterator) called with a non-dereferenceable iterator");
   size_type __old_sz    = size();
   size_type __old_start = __start_;
   iterator __b          = begin();
@@ -2408,10 +2406,6 @@ 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()'");
-  _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_;
@@ -2536,7 +2530,8 @@ inline _LIBCPP_HIDE_FROM_ABI bool operator<=(const deque<_Tp, _Allocator>& __x,
 template <class _Tp, class _Allocator>
 _LIBCPP_HIDE_FROM_ABI __synth_three_way_result<_Tp>
 operator<=>(const deque<_Tp, _Allocator>& __x, const deque<_Tp, _Allocator>& __y) {
-  return std::lexicographical_compare_three_way(__x.begin(), __x.end(), __y.begin(), __y.end(), std::__synth_three_way);
+  return std::lexicographical_compare_three_way(
+      __x.begin(), __x.end(), __y.begin(), __y.end(), std::__synth_three_way);
 }
 
 #endif // _LIBCPP_STD_VER <= 17
diff --git a/libcxx/include/string b/libcxx/include/string
index 306d3c8227fd9..060d8733243ab 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -3166,9 +3166,8 @@ 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 >= 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");
+      std::__is_pointer_in_range(std::__to_address(begin()), std::__to_address(end()), std::__to_address(__pos)),
+      "string::erase(iterator) called with an iterator that isn't a valid iterator into this string");
   iterator __b  = begin();
   size_type __r = static_cast<size_type>(__pos - __b);
   erase(__r, 1);
@@ -3179,9 +3178,12 @@ 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 >= begin(), "string::erase(first, last) called with an iterator range starting before 'begin()'");
+      std::__is_pointer_in_range(std::__to_address(begin()), std::__to_address(end()), std::__to_address(__first)),
+      "string::erase(first, last) called with an iterator range that doesn't belong to this string");
   _LIBCPP_ASSERT_VALID_INPUT_RANGE(
-      __last <= end(), "string::erase(first, last) called with an iterator range finishing after 'end()'");
+      std::__is_pointer_in_range(std::__to_address(begin()), std::__to_address(end()), std::__to_address(__last)) ||
+          __last == end(),
+      "string::erase(first, last) called with a last iterator that doesn't fall within the string");
   _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);
diff --git a/libcxx/include/vector b/libcxx/include/vector
index de1e202fa57d7..d5a24e6f59b95 100644
--- a/libcxx/include/vector
+++ b/libcxx/include/vector
@@ -1534,9 +1534,8 @@ 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 >= 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");
+      std::__is_pointer_in_range(std::__to_address(begin()), std::__to_address(end()), std::__to_address(__position)),
+      "vector::erase(iterator) called with an iterator that isn't a valid iterator into this vector");
   difference_type __ps = __position - cbegin();
   pointer __p          = this->__begin_ + __ps;
   this->__destruct_at_end(std::move(__p + 1, this->__end_, __p));
@@ -1547,9 +1546,12 @@ 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 >= begin(), "vector::erase(first, last) called with an iterator range starting before 'begin()'");
+      std::__is_pointer_in_range(std::__to_address(begin()), std::__to_address(end()), std::__to_address(__first)),
+      "vector::erase(first, last) called with an iterator range that doesn't belong to this vector");
   _LIBCPP_ASSERT_VALID_INPUT_RANGE(
-      __last <= end(), "vector::erase(first, last) called with an iterator range finishing after 'end()'");
+      std::__is_pointer_in_range(std::__to_address(begin()), std::__to_address(end()), std::__to_address(__last)) ||
+          __last == end(),
+      "vector::erase(first, last) called with a last iterator that doesn't fall within the vector");
   _LIBCPP_ASSERT_VALID_INPUT_RANGE(__first <= __last, "vector::erase(first, last) called with invalid range");
   pointer __p = this->__begin_ + (__first - begin());
   if (__first != __last) {
@@ -2908,7 +2910,8 @@ inline _LIBCPP_HIDE_FROM_ABI bool operator<=(const vector<_Tp, _Allocator>& __x,
 template <class _Tp, class _Allocator>
 _LIBCPP_HIDE_FROM_ABI constexpr __synth_three_way_result<_Tp>
 operator<=>(const vector<_Tp, _Allocator>& __x, const vector<_Tp, _Allocator>& __y) {
-  return std::lexicographical_compare_three_way(__x.begin(), __x.end(), __y.begin(), __y.end(), std::__synth_three_way);
+  return std::lexicographical_compare_three_way(
+      __x.begin(), __x.end(), __y.begin(), __y.end(), std::__synth_three_way);
 }
 
 #endif // _LIBCPP_STD_VER <= 17
diff --git a/libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.erase_iter.null.pass.cpp b/libcxx/test/libcxx/containers/sequences/deque/assert.erase.iter.pass.cpp
similarity index 59%
rename from libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.erase_iter.null.pass.cpp
rename to libcxx/test/libcxx/containers/sequences/deque/assert.erase.iter.pass.cpp
index 036e75965c488..5190bee4998b7 100644
--- a/libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.erase_iter.null.pass.cpp
+++ b/libcxx/test/libcxx/containers/sequences/deque/assert.erase.iter.pass.cpp
@@ -6,30 +6,28 @@
 //
 //===----------------------------------------------------------------------===//
 
-// <string>
+// <deque>
 
-// Call erase(const_iterator position) with end()
+// Make sure we catch invalid uses of std::deque::erase(iterator).
 
 // REQUIRES: has-unix-headers
 // UNSUPPORTED: c++03
 // UNSUPPORTED: libcpp-hardening-mode=none
 // XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing
 
-#include <string>
+#include <deque>
 
 #include "check_assertion.h"
-#include "min_allocator.h"
-
-template <class S>
-void test() {
-  S l1("123");
-  typename S::const_iterator i = l1.end();
-  TEST_LIBCPP_ASSERT_FAILURE(l1.erase(i), "string::erase(iterator) called with a non-dereferenceable iterator");
-}
 
 int main(int, char**) {
-  test<std::string>();
-  test<std::basic_string<char, std::char_traits<char>, min_allocator<char> > >();
+  // With an invalid iterator
+  {
+    std::deque<int> v = {1, 2, 3, 4, 5};
+    TEST_LIBCPP_ASSERT_FAILURE(
+        v.erase(v.end()), "deque::erase(iterator) called with a non-dereferenceable iterator");
+  }
+
+  // Note that we currently can't catch misuse by erasing with an iterator from another container.
 
   return 0;
 }
diff --git a/libcxx/test/libcxx/containers/sequences/deque/assert.erase.iter_iter.pass.cpp b/libcxx/test/libcxx/containers/sequences/deque/assert.erase.iter_iter.pass.cpp
new file mode 100644
index 0000000000000..e430f0ad0f962
--- /dev/null
+++ b/libcxx/test/libcxx/containers/sequences/deque/assert.erase.iter_iter.pass.cpp
@@ -0,0 +1,33 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <deque>
+
+// Make sure we catch invalid uses of std::deque::erase(first, last).
+
+// REQUIRES: has-unix-headers
+// UNSUPPORTED: c++03
+// UNSUPPORTED: libcpp-hardening-mode=none
+// XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing
+
+#include <deque>
+
+#include "check_assertion.h"
+
+int main(int, char**) {
+  // Note that we currently can't catch misuse with using a range that belongs to another container.
+
+  // With an invalid range
+  {
+    std::deque<int> v = {1, 2, 3, 4, 5};
+    TEST_LIBCPP_ASSERT_FAILURE(
+        v.erase(v.begin() + 2, v.begin()), "deque::erase(first, last) called with an invalid range");
+  }
+
+  return 0;
+}
diff --git a/libcxx/test/libcxx/containers/sequences/deque/assert.pass.cpp b/libcxx/test/libcxx/containers/sequences/deque/assert.pass.cpp
index 375a4cdcd58fe..9f332afe0e5df 100644
--- a/libcxx/test/libcxx/containers/sequences/deque/assert.pass.cpp
+++ b/libcxx/test/libcxx/containers/sequences/deque/assert.pass.cpp
@@ -48,9 +48,5 @@ int main(int, char**) {
     TEST_LIBCPP_ASSERT_FAILURE(cc[100], "deque::operator[] index out of bounds");
   }
 
-  TEST_LIBCPP_ASSERT_FAILURE(c.erase(c.end()), "deque::erase(iterator) called with a non-dereferenceable iterator");
-  TEST_LIBCPP_ASSERT_FAILURE(
-      c.erase(c.begin() + 1, c.begin()), "deque::erase(first, last) called with an invalid range");
-
   return 0;
 }
diff --git a/libcxx/test/libcxx/containers/sequences/vector/assert.erase.iter.pass.cpp b/libcxx/test/libcxx/containers/sequences/vector/assert.erase.iter.pass.cpp
new file mode 100644
index 0000000000000..240715df50b24
--- /dev/null
+++ b/libcxx/test/libcxx/containers/sequences/vector/assert.erase.iter.pass.cpp
@@ -0,0 +1,43 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+//
+// iterator erase(const_iterator position);
+
+// Make sure we check that the iterator is within the container.
+
+// REQUIRES: has-unix-headers
+// UNSUPPORTED: c++03
+// UNSUPPORTED: libcpp-hardening-mode=none
+// XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing
+
+#include <vector>
+
+#include "check_assertion.h"
+
+int main(int, char**) {
+  // With an invalid iterator
+  {
+    std::vector<int> v = {1, 2, 3, 4, 5};
+    TEST_LIBCPP_ASSERT_FAILURE(
+        v.erase(v.end()),
+        "vector::erase(iterator) called with an iterator that isn't a valid iterator into this vector");
+  }
+
+  // With an iterator from another container
+  {
+    std::vector<int> v1 = {1, 2, 3, 4, 5};
+    std::vector<int> v2 = {6, 7, 8, 9, 10};
+    TEST_LIBCPP_ASSERT_FAILURE(
+        v1.erase(v2.begin()),
+        "vector::erase(iterator) called with an iterator that isn't a valid iterator into this vector");
+  }
+
+  return 0;
+}
diff --git a/libcxx/test/libcxx/containers/sequences/vector/assert.erase.iter_iter.pass.cpp b/libcxx/test/libcxx/containers/sequences/vector/assert.erase.iter_iter.pass.cpp
new file mode 100644
index 0000000000000..a35db2588d692
--- /dev/null
+++ b/libcxx/test/libcxx/containers/sequences/vector/assert.erase.iter_iter.pass.cpp
@@ -0,0 +1,60 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+//
+// iterator erase(const_iterator first, const_iterator last);
+
+// Make sure we check that the iterator range is valid and within the container.
+
+// REQUIRES: has-unix-headers
+// UNSUPPORTED: c++03
+// UNSUPPORTED: libcpp-hardening-mode=none
+// XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing
+
+#include <vector>
+
+#include "check_assertion.h"
+
+int main(int, char**) {
+  // With first iterator from another container
+  {
+    std::vector<int> v1 = {1, 2, 3, 4, 5};
+    std::vector<int> v2 = {6, 7, 8, 9, 10};
+    TEST_LIBCPP_ASSERT_FAILURE(
+        v1.erase(v2.begin(), v1.end()),
+        "vector::erase(first, last) called with an iterator range that doesn't belong to this vector");
+  }
+
+  // With last iterator from another container
+  {
+    std::vector<int> v1 = {1, 2, 3, 4, 5};
+    std::vector<int> v2 = {6, 7, 8, 9, 10};
+    TEST_LIBCPP_ASSERT_FAILURE(
+        v1.erase(v1.begin(), v2.end()),
+        "vector::erase(first, last) called with a last iterator that doesn't fall within the vector");
+  }
+
+  // With both iterators from another container
+  {
+    std::vector<int> v1 = {1, 2, 3, 4, 5};
+    std::vector<int> v2 = {6, 7, 8, 9, 10};
+    TEST_LIBCPP_ASSERT_FAILURE(
+        v1.erase(v2.begin(), v2.end()),
+        "vector::erase(first, last) called with an iterator range that doesn't belong to this vector");
+  }
+
+  // With an invalid range
+  {
+    std::vector<int> v = {1, 2, 3, 4, 5};
+    TEST_LIBCPP_ASSERT_FAILURE(
+        v.erase(v.begin() + 2, v.begin()), "vector::erase(first, last) called with invalid range");
+  }
+
+  return 0;
+}
diff --git a/libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.erase.iter.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.erase.iter.pass.cpp
new file mode 100644
index 0000000000000..03746eab35137
--- /dev/null
+++ b/libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.erase.iter.pass.cpp
@@ -0,0 +1,47 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <string>
+
+// Make sure we assert when erase(position) is called with an iterator that isn't a
+// valid iterator into the current string.
+
+// REQUIRES: has-unix-headers
+// UNSUPPORTED: c++03
+// UNSUPPORTED: libcpp-hardening-mode=none
+// XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing
+
+#include <string>
+
+#include "check_assertion.h"
+#include "min_allocator.h"
+
+template <class String>
+void test() {
+  {
+    String s("123");
+    TEST_LIBCPP_ASSERT_FAILURE(
+        s.erase(s.end()),
+        "string::erase(iterator) called with an iterator that isn't a valid iterator into this string");
+  }
+
+  {
+    String s1("123");
+    String s2("456");
+    TEST_LIBCPP_ASSERT_FAILURE(
+        s1.erase(s2.begin()),
+        "string::erase(iterator) called with an iterator that isn't a valid iterator into this string");
+  }
+}
+
+int main(int, char**) {
+  test<std::string>();
+  test<std::basic_string<char, std::char_traits<char>, min_allocator<char> > >();
+
+  return 0;
+}
diff --git a/libcxx/test/libcxx/strings/basic.string/string.modifiers/debug.erase.iter_iter.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.erase.iter_iter.pass.cpp
similarity index 50%
rename from libcxx/test/libcxx/strings/basic.string/string.modifiers/debug.erase.iter_iter.pass.cpp
rename to libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.erase.iter_iter.pass.cpp
index ed97e21d9411e..4dc73dd33b849 100644
--- a/libcxx/test/libcxx/strings/basic.string/string.modifiers/debug.erase.iter_iter.pass.cpp
+++ b/libcxx/test/libcxx/strings/basic.string/string.modifiers/assert.erase.iter_iter.pass.cpp
@@ -8,48 +8,52 @@
 
 // <string>
 
-// Call erase(const_iterator first, const_iterator last); with invalid iterators
+// Make sure we assert when erase(first, last) is called with iterators that aren't valid.
 
 // REQUIRES: has-unix-headers
-// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
+// UNSUPPORTED: c++03
+// UNSUPPORTED: libcpp-hardening-mode=none
+// XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing
 
 #include <string>
 
 #include "check_assertion.h"
 #include "min_allocator.h"
 
-template <class S>
+template <class String>
 void test() {
   // With first iterator from another container
   {
-    S l1("123");
-    S l2("123");
+    String s1("123");
+    String s2("123");
     TEST_LIBCPP_ASSERT_FAILURE(
-        l1.erase(l2.cbegin(), l1.cbegin() + 1),
-        "string::erase(iterator,  iterator) called with an iterator not referring to this string");
+        s1.erase(s2.cbegin(), s1.cbegin() + 1),
+        "string::erase(first, last) called with an iterator range that doesn't belong to this string");
   }
 
   // With second iterator from another container
   {
-    S l1("123");
-    S l2("123");
-    TEST_LIBCPP_ASSERT_FAILURE(l1.erase(l1.cbegin(), l2.cbegin() + 1), "Attempted to compare incomparable iterators");
+    String s1("123");
+    String s2("123");
+    TEST_LIBCPP_ASSERT_FAILURE(
+        s1.erase(s1.cbegin(), s2.cbegin() + 1),
+        "string::erase(first, last) called with a last iterator that doesn't fall within the string");
   }
 
   // With both iterators from another container
   {
-    S l1("123");
-    S l2("123");
+    String s1("123");
+    String s2("123");
     TEST_LIBCPP_ASSERT_FAILURE(
-        l1.erase(l2.cbegin(), l2.cbegin() + 1),
-        "string::erase(iterator,  iterator) called with an iterator not referring to this string");
+        s1.erase(s2.cbegin(), s2.cbegin() + 1),
+        "string::erase(first, last) called with an iterator range that doesn't belong to this string");
   }
 
   // With an invalid range
   {
-    S l1("123");
+    String s1("123");
     TEST_LIBCPP_ASSERT_FAILURE(
-        l1.erase(l1.cbegin() + 1, l1.cbegin()), "string::erase(first, last) called with invalid range");
+        s1.erase(s1.cbegin() + 1, s1.cbegin()), "string::erase(first, last) called with invalid range");
   }
 }
 
diff --git a/libcxx/test/libcxx/strings/basic.string/string.modifiers/debug.erase.iter.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.modifiers/debug.erase.iter.pass.cpp
deleted file mode 100644
index 93fd6caf0ba23..0000000000000
--- a/libcxx/test/libcxx/strings/basic.string/string.modifiers/debug.erase.iter.pass.cpp
+++ /dev/null
@@ -1,35 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// <string>
-
-// Call erase(const_iterator position) with an iterator from another container
-
-// REQUIRES: has-unix-headers
-// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
-
-#include <string>
-
-#include "check_assertion.h"
-#include "min_allocator.h"
-
-template <class S>
-void test() {
-  S l1("123");
-  S l2("123");
-  typename S::const_iterator i = l2.begin();
-  TEST_LIBCPP_ASSERT_FAILURE(
-      l1.erase(i), "string::erase(iterator) called with an iterator not referring to this string");
-}
-
-int main(int, char**) {
-  test<std::string>();
-  test<std::basic_string<char, std::char_traits<char>, min_allocator<char> > >();
-
-  return 0;
-}



More information about the libcxx-commits mailing list