Skip to content

Commit 2990385

Browse files
committed
[libc++][hardening] Use bounded iterators in std::vector and std::string
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 #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 capacaity. 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 invalided 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 #78805
1 parent 1eb7f05 commit 2990385

17 files changed

+214
-44
lines changed

libcxx/include/__iterator/bounded_iter.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,8 @@ struct __bounded_iter {
225225
private:
226226
template <class>
227227
friend struct pointer_traits;
228+
template <class, class>
229+
friend struct __bounded_iter;
228230
_Iterator __current_; // current iterator
229231
_Iterator __begin_, __end_; // valid range represented as [begin, end]
230232
};

libcxx/include/string

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,7 @@ basic_string<char32_t> operator""s( const char32_t *str, size_t len );
580580
#include <__functional/unary_function.h>
581581
#include <__fwd/string.h>
582582
#include <__ios/fpos.h>
583+
#include <__iterator/bounded_iter.h>
583584
#include <__iterator/distance.h>
584585
#include <__iterator/iterator_traits.h>
585586
#include <__iterator/reverse_iterator.h>
@@ -786,9 +787,16 @@ public:
786787
"[allocator.requirements] states that rebinding an allocator to the same type should result in the "
787788
"original allocator");
788789

789-
// TODO: Implement iterator bounds checking without requiring the global database.
790+
#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS
791+
// The pointer must be passed through __wrap_iter because
792+
// __alloc_traits::pointer may not be detected as a continguous iterator on
793+
// its own.
794+
typedef __bounded_iter<__wrap_iter<pointer>> iterator;
795+
typedef __bounded_iter<__wrap_iter<const_pointer>> const_iterator;
796+
#else
790797
typedef __wrap_iter<pointer> iterator;
791798
typedef __wrap_iter<const_pointer> const_iterator;
799+
#endif
792800
typedef std::reverse_iterator<iterator> reverse_iterator;
793801
typedef std::reverse_iterator<const_iterator> const_reverse_iterator;
794802

@@ -918,11 +926,27 @@ private:
918926
__init_with_sentinel(std::move(__first), std::move(__last));
919927
}
920928

929+
#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS
930+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 iterator __make_iterator(pointer __p) {
931+
return __make_bounded_iter(
932+
__wrap_iter<pointer>(__p),
933+
__wrap_iter<pointer>(__get_pointer()),
934+
__wrap_iter<pointer>(__get_pointer() + size()));
935+
}
936+
937+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 const_iterator __make_const_iterator(const_pointer __p) const {
938+
return __make_bounded_iter(
939+
__wrap_iter<const_pointer>(__p),
940+
__wrap_iter<const_pointer>(__get_pointer()),
941+
__wrap_iter<const_pointer>(__get_pointer() + size()));
942+
}
943+
#else
921944
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 iterator __make_iterator(pointer __p) { return iterator(__p); }
922945

923946
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 const_iterator __make_const_iterator(const_pointer __p) const {
924947
return const_iterator(__p);
925948
}
949+
#endif
926950

927951
public:
928952
_LIBCPP_TEMPLATE_DATA_VIS static const size_type npos = -1;

libcxx/include/vector

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,7 @@ template<class T, class charT> requires is-vector-bool-reference<T> // Since C++
328328
#include <__functional/unary_function.h>
329329
#include <__fwd/vector.h>
330330
#include <__iterator/advance.h>
331+
#include <__iterator/bounded_iter.h>
331332
#include <__iterator/distance.h>
332333
#include <__iterator/iterator_traits.h>
333334
#include <__iterator/reverse_iterator.h>
@@ -401,9 +402,16 @@ public:
401402
typedef typename __alloc_traits::difference_type difference_type;
402403
typedef typename __alloc_traits::pointer pointer;
403404
typedef typename __alloc_traits::const_pointer const_pointer;
404-
// TODO: Implement iterator bounds checking without requiring the global database.
405+
#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS
406+
// The pointer must be passed through __wrap_iter because
407+
// __alloc_traits::pointer may not be detected as a continguous iterator on
408+
// its own.
409+
typedef __bounded_iter<__wrap_iter<pointer>> iterator;
410+
typedef __bounded_iter<__wrap_iter<const_pointer>> const_iterator;
411+
#else
405412
typedef __wrap_iter<pointer> iterator;
406413
typedef __wrap_iter<const_pointer> const_iterator;
414+
#endif
407415
typedef std::reverse_iterator<iterator> reverse_iterator;
408416
typedef std::reverse_iterator<const_iterator> const_reverse_iterator;
409417

@@ -798,10 +806,30 @@ private:
798806
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __append(size_type __n);
799807
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void __append(size_type __n, const_reference __x);
800808
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator __make_iter(pointer __p) _NOEXCEPT {
809+
#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS
810+
// Bound the iterator according to the capacity, rather than the size.
811+
// Resizing a vector up to the capacity will not invalidate iterators, so,
812+
// Without a way to update all live iterators on resize, we must
813+
// conservatively bound the iterator by the capacity rather than the size.
814+
return __make_bounded_iter(
815+
__wrap_iter<pointer>(__p), __wrap_iter<pointer>(this->__begin_), __wrap_iter<pointer>(this->__end_cap()));
816+
#else
801817
return iterator(__p);
818+
#endif
802819
}
803820
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI const_iterator __make_iter(const_pointer __p) const _NOEXCEPT {
821+
#ifdef _LIBCPP_ABI_BOUNDED_ITERATORS
822+
// Bound the iterator according to the capacity, rather than the size.
823+
// Resizing a vector up to the capacity will not invalidate iterators, so,
824+
// Without a way to update all live iterators on resize, we must
825+
// conservatively bound the iterator by the capacity rather than the size.
826+
return __make_bounded_iter(
827+
__wrap_iter<const_pointer>(__p),
828+
__wrap_iter<const_pointer>(this->__begin_),
829+
__wrap_iter<const_pointer>(this->__end_cap()));
830+
#else
804831
return const_iterator(__p);
832+
#endif
805833
}
806834
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
807835
__swap_out_circular_buffer(__split_buffer<value_type, allocator_type&>& __v);

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

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,36 +10,48 @@
1010

1111
// Add to iterator out of bounds.
1212

13-
// REQUIRES: has-unix-headers
14-
// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
13+
// REQUIRES: has-unix-headers, libcpp-has-abi-bounded-iterators
14+
// UNSUPPORTED: libcpp-hardening-mode=none, c++03
1515

1616
#include <vector>
1717
#include <cassert>
1818

1919
#include "check_assertion.h"
2020
#include "min_allocator.h"
2121

22+
template <typename T, typename A>
23+
void fill_to_capacity(std::vector<T, A>& vec) {
24+
// Fill vec up to its capacity. Our bounded iterators currently unable to
25+
// catch accesses between size and capacity due to iterator stability
26+
// guarantees. This function clears those effects.
27+
while (vec.size() < vec.capacity()) {
28+
vec.push_back(T());
29+
}
30+
}
31+
2232
int main(int, char**) {
2333
{
2434
typedef int T;
2535
typedef std::vector<T> C;
2636
C c(1);
37+
fill_to_capacity(c);
2738
C::iterator i = c.begin();
28-
i += 1;
39+
i += c.size();
2940
assert(i == c.end());
3041
i = c.begin();
31-
TEST_LIBCPP_ASSERT_FAILURE(i + 2, "Attempted to add/subtract an iterator outside its valid range");
42+
TEST_LIBCPP_ASSERT_FAILURE(i + 2, "__bounded_iter::operator+=: Attempt to advance an iterator past the end");
3243
}
3344

3445
{
3546
typedef int T;
3647
typedef std::vector<T, min_allocator<T> > C;
3748
C c(1);
49+
fill_to_capacity(c);
3850
C::iterator i = c.begin();
39-
i += 1;
51+
i += c.size();
4052
assert(i == c.end());
4153
i = c.begin();
42-
TEST_LIBCPP_ASSERT_FAILURE(i + 2, "Attempted to add/subtract an iterator outside its valid range");
54+
TEST_LIBCPP_ASSERT_FAILURE(i + 2, "__bounded_iter::operator+=: Attempt to advance an iterator past the end");
4355
}
4456

4557
return 0;

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010

1111
// Decrement iterator prior to begin.
1212

13-
// REQUIRES: has-unix-headers
14-
// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
13+
// REQUIRES: has-unix-headers, libcpp-has-abi-bounded-iterators
14+
// UNSUPPORTED: libcpp-hardening-mode=none, c++03
1515

1616
#include <vector>
1717
#include <cassert>
@@ -27,7 +27,7 @@ int main(int, char**) {
2727
C::iterator i = c.end();
2828
--i;
2929
assert(i == c.begin());
30-
TEST_LIBCPP_ASSERT_FAILURE(--i, "Attempted to decrement a non-decrementable iterator");
30+
TEST_LIBCPP_ASSERT_FAILURE(--i, "__bounded_iter::operator--: Attempt to rewind an iterator past the start");
3131
}
3232

3333
{
@@ -37,7 +37,7 @@ int main(int, char**) {
3737
C::iterator i = c.end();
3838
--i;
3939
assert(i == c.begin());
40-
TEST_LIBCPP_ASSERT_FAILURE(--i, "Attempted to decrement a non-decrementable iterator");
40+
TEST_LIBCPP_ASSERT_FAILURE(--i, "__bounded_iter::operator--: Attempt to rewind an iterator past the start");
4141
}
4242

4343
return 0;

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,29 +10,41 @@
1010

1111
// Dereference non-dereferenceable iterator.
1212

13-
// REQUIRES: has-unix-headers
14-
// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
13+
// REQUIRES: has-unix-headers, libcpp-has-abi-bounded-iterators
14+
// UNSUPPORTED: libcpp-hardening-mode=none, c++03
1515

1616
#include <vector>
1717

1818
#include "check_assertion.h"
1919
#include "min_allocator.h"
2020

21+
template <typename T, typename A>
22+
void fill_to_capacity(std::vector<T, A>& vec) {
23+
// Fill vec up to its capacity. Our bounded iterators currently unable to
24+
// catch accesses between size and capacity due to iterator stability
25+
// guarantees. This function clears those effects.
26+
while (vec.size() < vec.capacity()) {
27+
vec.push_back(T());
28+
}
29+
}
30+
2131
int main(int, char**) {
2232
{
2333
typedef int T;
2434
typedef std::vector<T> C;
2535
C c(1);
36+
fill_to_capacity(c);
2637
C::iterator i = c.end();
27-
TEST_LIBCPP_ASSERT_FAILURE(*i, "Attempted to dereference a non-dereferenceable iterator");
38+
TEST_LIBCPP_ASSERT_FAILURE(*i, "__bounded_iter::operator*: Attempt to dereference an iterator at the end");
2839
}
2940

3041
{
3142
typedef int T;
3243
typedef std::vector<T, min_allocator<T> > C;
3344
C c(1);
45+
fill_to_capacity(c);
3446
C::iterator i = c.end();
35-
TEST_LIBCPP_ASSERT_FAILURE(*i, "Attempted to dereference a non-dereferenceable iterator");
47+
TEST_LIBCPP_ASSERT_FAILURE(*i, "__bounded_iter::operator*: Attempt to dereference an iterator at the end");
3648
}
3749

3850
return 0;

libcxx/test/libcxx/containers/sequences/vector/debug.iterator.increment.pass.cpp

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,34 +10,46 @@
1010

1111
// Increment iterator past end.
1212

13-
// REQUIRES: has-unix-headers
14-
// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
13+
// REQUIRES: has-unix-headers, libcpp-has-abi-bounded-iterators
14+
// UNSUPPORTED: libcpp-hardening-mode=none, c++03
1515

1616
#include <vector>
1717
#include <cassert>
1818

1919
#include "check_assertion.h"
2020
#include "min_allocator.h"
2121

22+
template <typename T, typename A>
23+
void fill_to_capacity(std::vector<T, A>& vec) {
24+
// Fill vec up to its capacity. Our bounded iterators currently unable to
25+
// catch accesses between size and capacity due to iterator stability
26+
// guarantees. This function clears those effects.
27+
while (vec.size() < vec.capacity()) {
28+
vec.push_back(T());
29+
}
30+
}
31+
2232
int main(int, char**) {
2333
{
2434
typedef int T;
2535
typedef std::vector<T> C;
2636
C c(1);
37+
fill_to_capacity(c);
2738
C::iterator i = c.begin();
28-
++i;
39+
i += c.size();
2940
assert(i == c.end());
30-
TEST_LIBCPP_ASSERT_FAILURE(++i, "Attempted to increment a non-incrementable iterator");
41+
TEST_LIBCPP_ASSERT_FAILURE(++i, "__bounded_iter::operator++: Attempt to advance an iterator past the end");
3142
}
3243

3344
{
3445
typedef int T;
3546
typedef std::vector<T, min_allocator<T> > C;
3647
C c(1);
48+
fill_to_capacity(c);
3749
C::iterator i = c.begin();
38-
++i;
50+
i += c.size();
3951
assert(i == c.end());
40-
TEST_LIBCPP_ASSERT_FAILURE(++i, "Attempted to increment a non-incrementable iterator");
52+
TEST_LIBCPP_ASSERT_FAILURE(++i, "__bounded_iter::operator++: Attempt to advance an iterator past the end");
4153
}
4254

4355
return 0;

libcxx/test/libcxx/containers/sequences/vector/debug.iterator.index.pass.cpp

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,32 +10,50 @@
1010

1111
// Index iterator out of bounds.
1212

13-
// REQUIRES: has-unix-headers
14-
// UNSUPPORTED: !libcpp-has-legacy-debug-mode, c++03
13+
// REQUIRES: has-unix-headers, libcpp-has-abi-bounded-iterators
14+
// UNSUPPORTED: libcpp-hardening-mode=none, c++03
1515

1616
#include <vector>
1717
#include <cassert>
1818

1919
#include "check_assertion.h"
2020
#include "min_allocator.h"
2121

22+
template <typename T, typename A>
23+
void fill_to_capacity(std::vector<T, A>& vec) {
24+
// Fill vec up to its capacity. Our bounded iterators currently unable to
25+
// catch accesses between size and capacity due to iterator stability
26+
// guarantees. This function clears those effects.
27+
while (vec.size() < vec.capacity()) {
28+
vec.push_back(T());
29+
}
30+
}
31+
2232
int main(int, char**) {
2333
{
2434
typedef int T;
2535
typedef std::vector<T> C;
2636
C c(1);
37+
fill_to_capacity(c);
2738
C::iterator i = c.begin();
2839
assert(i[0] == 0);
29-
TEST_LIBCPP_ASSERT_FAILURE(i[1], "Attempted to subscript an iterator outside its valid range");
40+
TEST_LIBCPP_ASSERT_FAILURE(
41+
i[c.size()], "__bounded_iter::operator[]: Attempt to index an iterator at or past the end");
42+
TEST_LIBCPP_ASSERT_FAILURE(
43+
i[c.size() + 1], "__bounded_iter::operator[]: Attempt to index an iterator at or past the end");
3044
}
3145

3246
{
3347
typedef int T;
3448
typedef std::vector<T, min_allocator<T> > C;
3549
C c(1);
50+
fill_to_capacity(c);
3651
C::iterator i = c.begin();
3752
assert(i[0] == 0);
38-
TEST_LIBCPP_ASSERT_FAILURE(i[1], "Attempted to subscript an iterator outside its valid range");
53+
TEST_LIBCPP_ASSERT_FAILURE(
54+
i[c.size()], "__bounded_iter::operator[]: Attempt to index an iterator at or past the end");
55+
TEST_LIBCPP_ASSERT_FAILURE(
56+
i[c.size() + 1], "__bounded_iter::operator[]: Attempt to index an iterator at or past the end");
3957
}
4058

4159
return 0;

libcxx/test/libcxx/strings/basic.string/alignof.compile.pass.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
// UNSUPPORTED: c++03
1212

13+
#include <iterator>
1314
#include <string>
1415

1516
#include "test_macros.h"
@@ -18,6 +19,14 @@
1819

1920
template <class T>
2021
class small_pointer {
22+
public:
23+
using value_type = T;
24+
using difference_type = std::int16_t;
25+
using pointer = small_pointer;
26+
using reference = T&;
27+
using iterator_category = std::random_access_iterator_tag;
28+
29+
private:
2130
std::uint16_t offset;
2231
};
2332

0 commit comments

Comments
 (0)