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

Konstantin Varlamov via libcxx-commits libcxx-commits at lists.llvm.org
Wed Feb 14 00:06:37 PST 2024


================
@@ -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].
----------------
var-const wrote:

Do we want to expand on this a little here? 
1. Do we want to mention that iterating outside these bounds is potentially undefined behavior? For pointers, it's stated quite explicitly in [`[expr.add#4]`](http://eel.is/c++draft/expr.add#4):
> Otherwise, if `P` points to an array element `i` of an array object `x` with `n` elements [...], the expressions `P + J` and `J + P` [...] point to the (possibly-hypothetical) array element `i+j` of `x` **if `0 ≤ i+j ≤n`** [...] Otherwise, the behavior is undefined.

For iterators, it's a little bit less explicit, but I think the best reference is the [table of `Cpp17InputIterator` requirements](http://eel.is/c++draft/iterators#tab:inputiterator) which states that the result of incrementing an iterator must be either dereferenceable or point to the past-the-end element (then addition is defined in terms of repeated increment). I can't immediately find the equivalent requirement on `input_iterator`, but I presume it must be somewhere (or if it's not, I'd expect it to be an omission).

(Note: I say "potentially" because if we have `int a[32]; __bounded_iter i(a, a + 5);`, IIUC calling `i + 20` is not UB since it's still within the bounds of the actual underlying array, but the bounded iterator has no way of knowing that so it would still assert)

2. Do we want to add any discussion on why it's desirable to disallow creating invalid iterators, even if they are never dereferenced? I don't feel very strongly because `git blame` can always bring a curious reader to this patch with all the discussion and links, but perhaps a brief rationale would still be helpful.

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


More information about the libcxx-commits mailing list