[libcxx-commits] [libcxx] [libc++][hardening] Check bounds on arithmetic in __bounded_iter (PR #78876)

[David Benjamin via libcxx-commits]

================
@@ -31,13 +31,10 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 // Iterator wrapper that carries the valid range it is allowed to access.
 //
 // This is a simple iterator wrapper for contiguous iterators that points
-// within a [begin, end) range and carries these bounds with it. The iterator
-// ensures that it is pointing within that [begin, end) range when it is
-// dereferenced.
-//
-// Arithmetic operations are allowed and the bounds of the resulting iterator
-// are not checked. Hence, it is possible to create an iterator pointing outside
-// its range, but it is not possible to dereference it.
+// within a [begin, end] range and carries these bounds with it. The iterator
+// ensures that it is pointing within [begin, end) range when it is
+// dereferenced. It also ensures that it is never iterated outside of
+// [begin, end].
----------------
davidben wrote:

> Do we want to expand on this a little here?

Done.

> Do we want to add any discussion on why it's desirable to disallow creating invalid iterators,

Also done.

> Is something like this library-level UB?

That is a good question! I think it is indeed library-level UB, per the table that @var-const pointed at, but I didn't scour the spec carefully.

Alas, this means that bounded iterators on `std::span` and `std::string_view` do indeed introduce crashes that weren't strictly necessary. In fact, @danakj and I just ran into an analogous issue with `std::string_view` iterators in Chromium and have to go back and rewrite it! 😁 However, I cannot think of any reasonable way to bounds-check iterators without paying that one, so I think we'll just have to run with it and hope it's not too bad.

Either way, I think this is unrelated to this PR. The existing bounded iterators design _already_ breaks that code. (Which is how we managed to run into it.) This PR _does_ newly break cases where you compute `sub.begin() + 20` without dereferencing it, but I think almost all code that relies on interchanging subspan or substr iterators is also going to dereference it, so that delta isn't likely to be meaningful.

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