[libc++][hardening] Use bounded iterators in std::vector and std::str…

…ing (llvm#78929)

~~NB: This PR depends on llvm#78876. Ignore the first commit when reviewing,
and don't merge it until llvm#78876 is resolved. When/if llvm#78876 lands, I'll
clean this up.~~

This partially restores parity with the old, since removed debug build.
We now can re-enable a bunch of the disabled tests. Some things of note:

- `bounded_iter`'s converting constructor has never worked. It needs a
friend declaration to access the other `bound_iter` instantiation's
private fields.

- The old debug iterators also checked that callers did not try to
compare iterators from different objects. `bounded_iter` does not
currently do this, so I've left those disabled. However, I think we
probably should add those. See
llvm#78771 (comment)

- The `std::vector` iterators are bounded up to capacity, not size. This
makes for a weaker safety check. This is because the STL promises not to
invalidate iterators when appending up to the capacity. Since we cannot
retroactively update all the iterators on `push_back()`, I've instead
sized it to the capacity. This is not as good, but at least will stop
the iterator from going off the end of the buffer.

There was also no test for this, so I've added one in the `std`
directory.

- `std::string` has two ambiguities to deal with. First, I opted not to
size it against the capacity. https://eel.is/c++draft/string.require#4
says iterators are invalidated on an non-const operation. Second,
whether the iterator can reach the NUL terminator. The previous debug
tests and the special-case in https://eel.is/c++draft/string.access#2
suggest no. If either of these causes widespread problems, I figure we
can revisit.

- `resize_and_overwrite.pass.cpp` assumed `std::string`'s iterator
supported `s.begin().base()`, but I see no promise of this in the
standard. GCC also doesn't support this. I fixed the test to use
`std::to_address`.

- `alignof.compile.pass.cpp`'s pointer isn't enough of a real pointer.
(It needs to satisfy `NullablePointer`, `LegacyRandomAccessIterator`,
and `LegacyContiguousIterator`.) `__bounded_iter` seems to instantiate
enough to notice. I've added a few more bits to satisfy it.

Fixes llvm#78805

libcxx/cmake/caches/Generic-hardening-mode-fast-with-abi-breaks.cmake

@@ -1,2 +1,4 @@
 set(LIBCXX_HARDENING_MODE "fast" CACHE STRING "")
 set(LIBCXX_ABI_DEFINES "_LIBCPP_ABI_BOUNDED_ITERATORS" CACHE STRING "")
+set(LIBCXX_ABI_DEFINES "_LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING" CACHE STRING "")
+set(LIBCXX_ABI_DEFINES "_LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR" CACHE STRING "")

libcxx/docs/Hardening.rst

@@ -325,6 +325,22 @@ Vendors can use the following ABI options to enable additional hardening checks:
   - ``span``;
   - ``string_view``.
 
+- ``_LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING`` -- changes the iterator type of
+  ``basic_string`` to a bounded iterator that keeps track of whether it's within
+  the bounds of the original container and asserts it on every dereference and
+  when performing iterator arithmetics.
+
+  ABI impact: changes the iterator type of ``basic_string`` and its
+  specializations, such as ``string`` and ``wstring``.
+
+- ``_LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR`` -- changes the iterator type of
+  ``vector`` to a bounded iterator that keeps track of whether it's within the
+  bounds of the original container and asserts it on every dereference and when
+  performing iterator arithmetics. Note: this doesn't yet affect
+  ``vector<bool>``.
+
+  ABI impact: changes the iterator type of ``vector`` (except ``vector<bool>``).
+
 ABI tags
 --------
 
@@ -367,10 +383,10 @@ Hardened containers status
       - ❌
     * - ``vector``
       - ✅
-      - ❌
+      - ✅ (see note)
     * - ``string``
       - ✅
-      - ❌
+      - ✅ (see note)
     * - ``list``
       - ✅
       - ❌
@@ -429,6 +445,12 @@ Hardened containers status
       - ❌
       - N/A
 
+Note: for ``vector`` and ``string``, the iterator does not check for
+invalidation (accesses made via an invalidated iterator still lead to undefined
+behavior)
+
+Note: ``vector<bool>`` iterator is not currently hardened.
+
 Testing
 =======
 

libcxx/docs/ReleaseNotes/19.rst

@@ -87,6 +87,20 @@ Improvements and New Features
 - ``std::ignore``\s ``const __ignore_t& operator=(_Tp&&) const`` was changed to
   ``const __ignore_type& operator=(const _Tp&) const noexcept`` for all language versions.
 
+- Vendors can now configure the ABI so that ``string`` and ``vector`` will use bounded iterators when hardening is
+  enabled. Note that checks for iterator invalidation are currently not supported -- any accesses made through an
+  invalidated bounded iterator will still result in undefined behavior (bounded iterators follow the normal invalidation
+  rules of the associated container). ``string`` bounded iterators use the logical size of the container (``index
+  < str.size()``) whereas ``vector`` bounded iterators use the "physical" size of the container (``index
+  < vec.capacity()``) which is a less strict check; refer to the implementation for further details.
+
+  Bounded iterators can be enabled via the ``_LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING`` ABI macro for ``string`` and via
+  the ``_LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR`` ABI macro for ``vector``; note that checks will only be performed if
+  the hardening mode is set to ``fast`` or above (i.e., no checking is performed in the unchecked mode, even if bounded
+  iterators are enabled in the ABI configuration).
+
+  Note: bounded iterators currently are not supported for ``vector<bool>``.
+
 Deprecations and Removals
 -------------------------
 

libcxx/include/__configuration/abi.h

@@ -141,6 +141,19 @@
 // - `string_view`.
 // #define _LIBCPP_ABI_BOUNDED_ITERATORS
 
+// Changes the iterator type of `basic_string` to a bounded iterator that keeps track of whether it's within the bounds
+// of the original container and asserts it on every dereference and when performing iterator arithmetics.
+//
+// ABI impact: changes the iterator type of `basic_string` and its specializations, such as `string` and `wstring`.
+// #define _LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING
+
+// Changes the iterator type of `vector` to a bounded iterator that keeps track of whether it's within the bounds of the
+// original container and asserts it on every dereference and when performing iterator arithmetics. Note: this doesn't
+// yet affect `vector<bool>`.
+//
+// ABI impact: changes the iterator type of `vector` (except `vector<bool>`).
+// #define _LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR
+
 #if defined(_LIBCPP_COMPILER_CLANG_BASED)
 #  if defined(__APPLE__)
 #    if defined(__i386__) || defined(__x86_64__)

libcxx/include/__iterator/bounded_iter.h

@@ -225,6 +225,8 @@ struct __bounded_iter {
 private:
   template <class>
   friend struct pointer_traits;
+  template <class, class>
+  friend struct __bounded_iter;
   _Iterator __current_;       // current iterator
   _Iterator __begin_, __end_; // valid range represented as [begin, end]
 };

libcxx/include/string

@@ -598,6 +598,7 @@ basic_string<char32_t> operator""s( const char32_t *str, size_t len );
 #include <__functional/unary_function.h>
 #include <__fwd/string.h>
 #include <__ios/fpos.h>
+#include <__iterator/bounded_iter.h>
 #include <__iterator/distance.h>
 #include <__iterator/iterator_traits.h>
 #include <__iterator/reverse_iterator.h>
@@ -822,9 +823,16 @@ public:
                 "Allocator::value_type must be same type as value_type");
   static_assert(__check_valid_allocator<allocator_type>::value, "");
 
-  // TODO: Implement iterator bounds checking without requiring the global database.
+#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING
+  // Users might provide custom allocators, and prior to C++20 we have no existing way to detect whether the allocator's
+  // pointer type is contiguous (though it has to be by the Standard). Using the wrapper type ensures the iterator is
+  // considered contiguous.
+  typedef __bounded_iter<__wrap_iter<pointer>> iterator;
+  typedef __bounded_iter<__wrap_iter<const_pointer>> const_iterator;
+#else
   typedef __wrap_iter<pointer> iterator;
   typedef __wrap_iter<const_pointer> const_iterator;
+#endif
   typedef std::reverse_iterator<iterator> reverse_iterator;
   typedef std::reverse_iterator<const_iterator> const_reverse_iterator;
 
@@ -940,10 +948,33 @@ private:
     __init_with_sentinel(std::move(__first), std::move(__last));
   }
 
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 iterator __make_iterator(pointer __p) { return iterator(__p); }
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 iterator __make_iterator(pointer __p) {
+#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING
+    // Bound the iterator according to the size (and not the capacity, unlike vector).
+    //
+    // By the Standard, string iterators are generally not guaranteed to stay valid when the container is modified,
+    // regardless of whether reallocation occurs. This allows us to check for out-of-bounds accesses using logical size,
+    // a stricter check, since correct code can never rely on being able to access newly-added elements via an existing
+    // iterator.
+    return std::__make_bounded_iter(
+        std::__wrap_iter<pointer>(__p),
+        std::__wrap_iter<pointer>(__get_pointer()),
+        std::__wrap_iter<pointer>(__get_pointer() + size()));
+#else
+    return iterator(__p);
+#endif // _LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING
+  }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 const_iterator __make_const_iterator(const_pointer __p) const {
+#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING
+    // Bound the iterator according to the size (and not the capacity, unlike vector).
+    return std::__make_bounded_iter(
+        std::__wrap_iter<const_pointer>(__p),
+        std::__wrap_iter<const_pointer>(__get_pointer()),
+        std::__wrap_iter<const_pointer>(__get_pointer() + size()));
+#else
     return const_iterator(__p);
+#endif // _LIBCPP_ABI_BOUNDED_ITERATORS_IN_STRING
   }
 
 public:

libcxx/include/vector

@@ -327,6 +327,7 @@ template<class T, class charT> requires is-vector-bool-reference<T> // Since C++
 #include <__functional/unary_function.h>
 #include <__fwd/vector.h>
 #include <__iterator/advance.h>
+#include <__iterator/bounded_iter.h>
 #include <__iterator/distance.h>
 #include <__iterator/iterator_traits.h>
 #include <__iterator/reverse_iterator.h>
@@ -400,9 +401,16 @@ public:
   typedef typename __alloc_traits::difference_type difference_type;
   typedef typename __alloc_traits::pointer pointer;
   typedef typename __alloc_traits::const_pointer const_pointer;
-  // TODO: Implement iterator bounds checking without requiring the global database.
+#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR
+  // Users might provide custom allocators, and prior to C++20 we have no existing way to detect whether the allocator's
+  // pointer type is contiguous (though it has to be by the Standard). Using the wrapper type ensures the iterator is
+  // considered contiguous.
+  typedef __bounded_iter<__wrap_iter<pointer>> iterator;
+  typedef __bounded_iter<__wrap_iter<const_pointer>> const_iterator;
+#else
   typedef __wrap_iter<pointer> iterator;
   typedef __wrap_iter<const_pointer> const_iterator;
+#endif
   typedef std::reverse_iterator<iterator> reverse_iterator;
   typedef std::reverse_iterator<const_iterator> const_reverse_iterator;
 
@@ -827,12 +835,39 @@ private:
 
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __append(size_type __n);
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __append(size_type __n, const_reference __x);
+
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator __make_iter(pointer __p) _NOEXCEPT {
+#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR
+    // Bound the iterator according to the capacity, rather than the size.
+    //
+    // Vector guarantees that iterators stay valid as long as no reallocation occurs even if new elements are inserted
+    // into the container; for these cases, we need to make sure that the newly-inserted elements can be accessed
+    // through the bounded iterator without failing checks. The downside is that the bounded iterator won't catch
+    // access that is logically out-of-bounds, i.e., goes beyond the size, but is still within the capacity. With the
+    // current implementation, there is no connection between a bounded iterator and its associated container, so we
+    // don't have a way to update existing valid iterators when the container is resized and thus have to go with
+    // a laxer approach.
+    return std::__make_bounded_iter(
+        std::__wrap_iter<pointer>(__p),
+        std::__wrap_iter<pointer>(this->__begin_),
+        std::__wrap_iter<pointer>(this->__end_cap()));
+#else
     return iterator(__p);
+#endif // _LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR
   }
+
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI const_iterator __make_iter(const_pointer __p) const _NOEXCEPT {
+#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR
+    // Bound the iterator according to the capacity, rather than the size.
+    return std::__make_bounded_iter(
+        std::__wrap_iter<const_pointer>(__p),
+        std::__wrap_iter<const_pointer>(this->__begin_),
+        std::__wrap_iter<const_pointer>(this->__end_cap()));
+#else
     return const_iterator(__p);
+#endif // _LIBCPP_ABI_BOUNDED_ITERATORS_IN_VECTOR
   }
+
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
   __swap_out_circular_buffer(__split_buffer<value_type, allocator_type&>& __v);
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI pointer

libcxx/test/libcxx/containers/sequences/vector/abi.compile.pass.cpp

@@ -14,6 +14,14 @@
 
 template <class T>
 class small_pointer {
+public:
+  using value_type        = T;
+  using difference_type   = std::int16_t;
+  using pointer           = small_pointer;
+  using reference         = T&;
+  using iterator_category = std::random_access_iterator_tag;
+
+private:
   std::uint16_t offset;
 };
 

libcxx/test/libcxx/containers/sequences/vector/debug.iterator.add.pass.cpp → libcxx/test/libcxx/containers/sequences/vector/assert.iterator.add.pass.cpp

@@ -10,36 +10,39 @@
 
 // Add to iterator out of bounds.
 
-// REQUIRES: has-unix-headers
-// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
+// REQUIRES: has-unix-headers, libcpp-has-abi-bounded-iterators-in-vector
+// UNSUPPORTED: libcpp-hardening-mode=none, c++03
 
 #include <vector>
 #include <cassert>
 
 #include "check_assertion.h"
+#include "fill_to_capacity.h"
 #include "min_allocator.h"
 
 int main(int, char**) {
   {
     typedef int T;
     typedef std::vector<T> C;
     C c(1);
+    fill_to_capacity(c);
     C::iterator i = c.begin();
-    i += 1;
+    i += c.size();
     assert(i == c.end());
     i = c.begin();
-    TEST_LIBCPP_ASSERT_FAILURE(i + 2, "Attempted to add/subtract an iterator outside its valid range");
+    TEST_LIBCPP_ASSERT_FAILURE(i + 2, "__bounded_iter::operator+=: Attempt to advance an iterator past the end");
   }
 
   {
     typedef int T;
     typedef std::vector<T, min_allocator<T> > C;
     C c(1);
+    fill_to_capacity(c);
     C::iterator i = c.begin();
-    i += 1;
+    i += c.size();
     assert(i == c.end());
     i = c.begin();
-    TEST_LIBCPP_ASSERT_FAILURE(i + 2, "Attempted to add/subtract an iterator outside its valid range");
+    TEST_LIBCPP_ASSERT_FAILURE(i + 2, "__bounded_iter::operator+=: Attempt to advance an iterator past the end");
   }
 
   return 0;

libcxx/test/libcxx/containers/sequences/vector/debug.iterator.decrement.pass.cpp → libcxx/test/libcxx/containers/sequences/vector/assert.iterator.decrement.pass.cpp

@@ -10,8 +10,8 @@
 
 // Decrement iterator prior to begin.
 
-// REQUIRES: has-unix-headers
-// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
+// REQUIRES: has-unix-headers, libcpp-has-abi-bounded-iterators-in-vector
+// UNSUPPORTED: libcpp-hardening-mode=none, c++03
 
 #include <vector>
 #include <cassert>
@@ -27,7 +27,7 @@ int main(int, char**) {
     C::iterator i = c.end();
     --i;
     assert(i == c.begin());
-    TEST_LIBCPP_ASSERT_FAILURE(--i, "Attempted to decrement a non-decrementable iterator");
+    TEST_LIBCPP_ASSERT_FAILURE(--i, "__bounded_iter::operator--: Attempt to rewind an iterator past the start");
   }
 
   {
@@ -37,7 +37,7 @@ int main(int, char**) {
     C::iterator i = c.end();
     --i;
     assert(i == c.begin());
-    TEST_LIBCPP_ASSERT_FAILURE(--i, "Attempted to decrement a non-decrementable iterator");
+    TEST_LIBCPP_ASSERT_FAILURE(--i, "__bounded_iter::operator--: Attempt to rewind an iterator past the start");
   }
 
   return 0;

libcxx/test/libcxx/containers/sequences/vector/debug.iterator.dereference.pass.cpp → libcxx/test/libcxx/containers/sequences/vector/assert.iterator.dereference.pass.cpp

@@ -10,29 +10,32 @@
 
 // Dereference non-dereferenceable iterator.
 
-// REQUIRES: has-unix-headers
-// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
+// REQUIRES: has-unix-headers, libcpp-has-abi-bounded-iterators-in-vector
+// UNSUPPORTED: libcpp-hardening-mode=none, c++03
 
 #include <vector>
 
 #include "check_assertion.h"
+#include "fill_to_capacity.h"
 #include "min_allocator.h"
 
 int main(int, char**) {
   {
     typedef int T;
     typedef std::vector<T> C;
     C c(1);
+    fill_to_capacity(c);
     C::iterator i = c.end();
-    TEST_LIBCPP_ASSERT_FAILURE(*i, "Attempted to dereference a non-dereferenceable iterator");
+    TEST_LIBCPP_ASSERT_FAILURE(*i, "__bounded_iter::operator*: Attempt to dereference an iterator at the end");
   }
 
   {
     typedef int T;
     typedef std::vector<T, min_allocator<T> > C;
     C c(1);
+    fill_to_capacity(c);
     C::iterator i = c.end();
-    TEST_LIBCPP_ASSERT_FAILURE(*i, "Attempted to dereference a non-dereferenceable iterator");
+    TEST_LIBCPP_ASSERT_FAILURE(*i, "__bounded_iter::operator*: Attempt to dereference an iterator at the end");
   }
 
   return 0;