[libc++][hardening] Constrain construction for `__{(static_)bounded,wrap}_iter` #115271

frederick-vs-ja · 2024-11-07T06:40:18Z

This PR restricts construction to cases where reference types of source/destination iterators are (T&, T&) or (T&, const T&) ( where T can be const).

Fixes #50058.

llvmbot · 2024-11-07T06:40:58Z

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

This PR restricts construction to cases where reference types of source/destination iterators are (T&, T&) or (T&, const T&) ( where T can be const).

Fixes #50058. Fixes #115002.

Full diff: https://github.com/llvm/llvm-project/pull/115271.diff

4 Files Affected:

(modified) libcxx/include/__iterator/bounded_iter.h (+16-3)
(modified) libcxx/include/__iterator/wrap_iter.h (+16-3)
(added) libcxx/test/libcxx/iterators/contiguous_iterators.verify.cpp (+25)
(added) libcxx/test/libcxx/iterators/wrap.bounded.iter.conv.compile.pass.cpp (+49)

diff --git a/libcxx/include/__iterator/bounded_iter.h b/libcxx/include/__iterator/bounded_iter.h
index 5a86bd98e71940..e3f5d5838ff854 100644
--- a/libcxx/include/__iterator/bounded_iter.h
+++ b/libcxx/include/__iterator/bounded_iter.h
@@ -16,9 +16,13 @@
 #include <__config>
 #include <__iterator/iterator_traits.h>
 #include <__memory/pointer_traits.h>
+#include <__type_traits/conjunction.h>
+#include <__type_traits/disjunction.h>
 #include <__type_traits/enable_if.h>
 #include <__type_traits/integral_constant.h>
 #include <__type_traits/is_convertible.h>
+#include <__type_traits/is_same.h>
+#include <__type_traits/make_const_lvalue_ref.h>
 #include <__utility/move.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -47,8 +51,11 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 //    pointer, it is undefined at the language level (see [expr.add]). If
 //    bounded iterators exhibited this undefined behavior, we risk compiler
 //    optimizations deleting non-redundant bounds checks.
-template <class _Iterator, class = __enable_if_t< __libcpp_is_contiguous_iterator<_Iterator>::value > >
+template <class _Iterator>
 struct __bounded_iter {
+  static_assert(__libcpp_is_contiguous_iterator<_Iterator>::value,
+                "Only contiguous iterators can be adapted by __bounded_iter.");
+
   using value_type        = typename iterator_traits<_Iterator>::value_type;
   using difference_type   = typename iterator_traits<_Iterator>::difference_type;
   using pointer           = typename iterator_traits<_Iterator>::pointer;
@@ -67,7 +74,13 @@ struct __bounded_iter {
   _LIBCPP_HIDE_FROM_ABI __bounded_iter(__bounded_iter const&) = default;
   _LIBCPP_HIDE_FROM_ABI __bounded_iter(__bounded_iter&&)      = default;
 
-  template <class _OtherIterator, __enable_if_t< is_convertible<_OtherIterator, _Iterator>::value, int> = 0>
+  template < class _OtherIterator,
+             __enable_if_t<
+                 _And<is_constructible<_Iterator, const _OtherIterator&>,
+                      is_convertible<const _OtherIterator&, _Iterator>,
+                      _Or<is_same<reference, __iter_reference<_OtherIterator> >,
+                          is_same<reference, __make_const_lvalue_ref<__iter_reference<_OtherIterator> > > > >::value,
+                 int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __bounded_iter(__bounded_iter<_OtherIterator> const& __other) _NOEXCEPT
       : __current_(__other.__current_),
         __begin_(__other.__begin_),
@@ -247,7 +260,7 @@ struct __bounded_iter {
 private:
   template <class>
   friend struct pointer_traits;
-  template <class, class>
+  template <class>
   friend struct __bounded_iter;
   _Iterator __current_;       // current iterator
   _Iterator __begin_, __end_; // valid range represented as [begin, end]
diff --git a/libcxx/include/__iterator/wrap_iter.h b/libcxx/include/__iterator/wrap_iter.h
index 2856833e600798..2ceea9c77ecbe3 100644
--- a/libcxx/include/__iterator/wrap_iter.h
+++ b/libcxx/include/__iterator/wrap_iter.h
@@ -17,9 +17,13 @@
 #include <__iterator/iterator_traits.h>
 #include <__memory/addressof.h>
 #include <__memory/pointer_traits.h>
+#include <__type_traits/conjunction.h>
+#include <__type_traits/disjunction.h>
 #include <__type_traits/enable_if.h>
 #include <__type_traits/integral_constant.h>
 #include <__type_traits/is_convertible.h>
+#include <__type_traits/is_same.h>
+#include <__type_traits/make_const_lvalue_ref.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -29,6 +33,9 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _Iter>
 class __wrap_iter {
+  static_assert(__libcpp_is_contiguous_iterator<_Iter>::value,
+                "Only contiguous iterators can be adapted by __wrap_iter.");
+
 public:
   typedef _Iter iterator_type;
   typedef typename iterator_traits<iterator_type>::value_type value_type;
@@ -45,9 +52,15 @@ class __wrap_iter {
 
 public:
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __wrap_iter() _NOEXCEPT : __i_() {}
-  template <class _Up, __enable_if_t<is_convertible<_Up, iterator_type>::value, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __wrap_iter(const __wrap_iter<_Up>& __u) _NOEXCEPT
-      : __i_(__u.base()) {}
+  template <
+      class _OtherIter,
+      __enable_if_t< _And<is_constructible<_Iter, const _OtherIter&>,
+                          is_convertible<const _OtherIter&, _Iter>,
+                          _Or<is_same<reference, __iter_reference<_OtherIter> >,
+                              is_same<reference, __make_const_lvalue_ref<__iter_reference<_OtherIter> > > > >::value,
+                     int> = 0>
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __wrap_iter(const __wrap_iter<_OtherIter>& __u) _NOEXCEPT
+      : __i_(__u.__i_) {}
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 reference operator*() const _NOEXCEPT { return *__i_; }
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 pointer operator->() const _NOEXCEPT {
     return std::__to_address(__i_);
diff --git a/libcxx/test/libcxx/iterators/contiguous_iterators.verify.cpp b/libcxx/test/libcxx/iterators/contiguous_iterators.verify.cpp
new file mode 100644
index 00000000000000..d21e5d4211b86f
--- /dev/null
+++ b/libcxx/test/libcxx/iterators/contiguous_iterators.verify.cpp
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+
+// <iterator>
+
+// __bounded_iter<_Iter>
+// __wrap_iter<_Iter>
+
+// Verify that these wrappers do not accept non-contiguous iterators as determined by
+// __libcpp_is_contiguous_iterator.
+// static_assert should be used, see https://github.com/llvm/llvm-project/issues/115002.
+
+#include <deque>
+#include <iterator>
+
+// expected-error-re@*:* {{static assertion failed due to requirement {{.*}}Only contiguous iterators can be adapted by __bounded_iter.}}
+std::__bounded_iter<std::deque<int>::iterator> bit;
+// expected-error-re@*:* {{static assertion failed due to requirement {{.*}}Only contiguous iterators can be adapted by __wrap_iter.}}
+std::__wrap_iter<std::deque<int>::iterator> wit;
diff --git a/libcxx/test/libcxx/iterators/wrap.bounded.iter.conv.compile.pass.cpp b/libcxx/test/libcxx/iterators/wrap.bounded.iter.conv.compile.pass.cpp
new file mode 100644
index 00000000000000..86f33eaef8ad7e
--- /dev/null
+++ b/libcxx/test/libcxx/iterators/wrap.bounded.iter.conv.compile.pass.cpp
@@ -0,0 +1,49 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+
+// <iterator>
+
+// __bounded_iter<_Iter>
+// __wrap_iter<_Iter>
+
+// Verify that libc++-wrapped iterators do not permit slicing conversion or construction.
+
+#include <array>
+#include <vector>
+#include <span>
+#include <type_traits>
+
+#include "test_macros.h"
+
+struct Base {};
+struct Derived : Base {};
+
+static_assert(!std::is_convertible<std::array<Derived, 1>::iterator, std::array<Base, 1>::iterator>::value, "");
+static_assert(!std::is_convertible<std::array<Derived, 1>::iterator, std::array<Base, 1>::const_iterator>::value, "");
+static_assert(!std::is_convertible<std::array<Derived, 1>::const_iterator, std::array<Base, 1>::const_iterator>::value,
+              "");
+static_assert(!std::is_constructible<std::array<Base, 1>::iterator, std::array<Derived, 1>::iterator>::value, "");
+static_assert(!std::is_constructible<std::array<Base, 1>::iterator, std::array<Derived, 1>::const_iterator>::value, "");
+static_assert(!std::is_constructible<std::array<Base, 1>::const_iterator, std::array<Derived, 1>::const_iterator>::value, "");
+
+static_assert(!std::is_convertible<std::vector<Derived>::iterator, std::vector<Base>::iterator>::value, "");
+static_assert(!std::is_convertible<std::vector<Derived>::iterator, std::vector<Base>::const_iterator>::value, "");
+static_assert(!std::is_convertible<std::vector<Derived>::const_iterator, std::vector<Base>::const_iterator>::value, "");
+static_assert(!std::is_constructible<std::vector<Base>::iterator, std::vector<Derived>::iterator>::value, "");
+static_assert(!std::is_constructible<std::vector<Base>::iterator, std::vector<Derived>::const_iterator>::value, "");
+static_assert(!std::is_constructible<std::vector<Base>::const_iterator, std::vector<Derived>::const_iterator>::value, "");
+
+#if TEST_STD_VER >= 20
+static_assert(!std::is_convertible_v<std::span<Derived>::iterator, std::span<Base>::iterator>);
+static_assert(!std::is_convertible_v<std::span<Derived>::iterator, std::span<Base>::const_iterator>);
+static_assert(!std::is_convertible_v<std::span<Derived>::const_iterator, std::span<Base>::const_iterator>);
+static_assert(!std::is_constructible_v<std::span<Base>::iterator, std::vector<Derived>::iterator>);
+static_assert(!std::is_constructible_v<std::span<Base>::iterator, std::vector<Derived>::const_iterator>);
+static_assert(!std::is_constructible_v<std::span<Base>::const_iterator, std::vector<Derived>::const_iterator>);
+#endif

github-actions · 2024-11-07T06:45:07Z

✅ With the latest revision this PR passed the C/C++ code formatter.

... for `__{bounded.wrap}_iter`. This PR restricts construction to cases where reference types of source/destination iterators are (`T&`, `T&`) or (`T&`, `const T&`) ( where T can be const). We can't `static_assert` `__libcpp_is_contiguous_iterator` for `__wrap_iter` currently because `__wrap_iter` is also used for wrapping user-defined fancy pointers.

frederick-vs-ja · 2024-11-07T10:15:10Z

Timed out in std/utilities/charconv/charconv.msvc/test.pass.cpp, which seems unrelated.

https://github.com/llvm/llvm-project/actions/runs/11718999636/job/32643141533

philnik777 · 2024-11-07T11:23:20Z

Please make these separate PRs. The changes are unrelated AFAICT.

frederick-vs-ja · 2024-11-07T11:53:45Z

Please make these separate PRs. The changes are unrelated AFAICT.

Separated to #115304.

libcxx/include/__iterator/bounded_iter.h

libcxx/test/libcxx/iterators/wrap.bounded.iter.conv.compile.pass.cpp

ldionne · 2024-11-07T14:36:23Z

Can you also make this change for __static_bounded_iter which was added just now?

- Drop uses of `is_constructible`. - Generalize to `__static_bounded_iter`. - Improve conditional compilation in test.

frederick-vs-ja · 2024-11-09T07:25:45Z

Can you also make this change for __static_bounded_iter which was added just now?

Changed. Also added a previously missed change to one friend declaration.

…rap}_iter` (llvm#115271) This PR restricts construction to cases where reference types of source/destination iterators are (`T&`, `T&`) or (`T&`, `const T&`) ( where `T` can be const). Fixes llvm#50058.

frederick-vs-ja requested a review from a team as a code owner November 7, 2024 06:40

frederick-vs-ja changed the title ~~[libc++][hardening] Constrain construction and use static_assert~~ [libc++][hardening] Constrain construction and use static_assert for __{bounded.wrap}_iter. Nov 7, 2024

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 7, 2024

frederick-vs-ja changed the title ~~[libc++][hardening] Constrain construction and use static_assert for __{bounded.wrap}_iter.~~ [libc++][hardening] Constrain construction and use static_assert for __{bounded.wrap}_iter Nov 7, 2024

frederick-vs-ja force-pushed the restrict-bounded-wrap-iter branch from 9115be1 to 80ce177 Compare November 7, 2024 06:51

var-const added the hardening Issues related to the hardening effort label Nov 7, 2024

frederick-vs-ja force-pushed the restrict-bounded-wrap-iter branch 2 times, most recently from f8bea98 to f0676e0 Compare November 7, 2024 07:44

frederick-vs-ja changed the title ~~[libc++][hardening] Constrain construction and use static_assert for __{bounded.wrap}_iter~~ [libc++][hardening] Constrain construction and use static_assert for __{bounded,wrap}_iter Nov 7, 2024

frederick-vs-ja force-pushed the restrict-bounded-wrap-iter branch from f0676e0 to 78dee26 Compare November 7, 2024 08:10

frederick-vs-ja added 2 commits November 7, 2024 19:38

Merge branch 'llvm:main' into restrict-bounded-wrap-iter

4177189

Revert static_assert

a1b9e60

frederick-vs-ja changed the title ~~[libc++][hardening] Constrain construction and use static_assert for __{bounded,wrap}_iter~~ [libc++][hardening] Constrain construction for __{bounded,wrap}_iter Nov 7, 2024

ldionne reviewed Nov 7, 2024

View reviewed changes

libcxx/include/__iterator/bounded_iter.h Outdated Show resolved Hide resolved

libcxx/include/__iterator/bounded_iter.h Outdated Show resolved Hide resolved

libcxx/test/libcxx/iterators/wrap.bounded.iter.conv.compile.pass.cpp Outdated Show resolved Hide resolved

frederick-vs-ja added 2 commits November 9, 2024 00:53

Merge branch 'main' into restrict-bounded-wrap-iter

2780880

Address review comments

a40207a

- Drop uses of `is_constructible`. - Generalize to `__static_bounded_iter`. - Improve conditional compilation in test.

frederick-vs-ja changed the title ~~[libc++][hardening] Constrain construction for __{bounded,wrap}_iter~~ [libc++][hardening] Constrain construction for __{(static_)bounded,wrap}_iter Nov 8, 2024

Fix old friend declaration

8f8a4e1

frederick-vs-ja mentioned this pull request Nov 10, 2024

[libcxx][algorithm] Optimize std::stable_sort via radix sort algorithm #104683

Merged

ldionne approved these changes Nov 11, 2024

View reviewed changes

frederick-vs-ja merged commit 9f471fd into llvm:main Nov 11, 2024
66 checks passed

frederick-vs-ja deleted the restrict-bounded-wrap-iter branch November 11, 2024 15:04

frederick-vs-ja mentioned this pull request Apr 11, 2025

[libc++][WIP] ADL-proof std::__wrap_iter #107766

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++][hardening] Constrain construction for `__{(static_)bounded,wrap}_iter` #115271

[libc++][hardening] Constrain construction for `__{(static_)bounded,wrap}_iter` #115271

Uh oh!

frederick-vs-ja commented Nov 7, 2024 •

edited

Loading

Uh oh!

llvmbot commented Nov 7, 2024

Uh oh!

github-actions bot commented Nov 7, 2024 •

edited

Loading

Uh oh!

frederick-vs-ja commented Nov 7, 2024

Uh oh!

philnik777 commented Nov 7, 2024

Uh oh!

frederick-vs-ja commented Nov 7, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldionne commented Nov 7, 2024

Uh oh!

frederick-vs-ja commented Nov 9, 2024

Uh oh!

Uh oh!

Uh oh!

[libc++][hardening] Constrain construction for __{(static_)bounded,wrap}_iter #115271

[libc++][hardening] Constrain construction for __{(static_)bounded,wrap}_iter #115271

Uh oh!

Conversation

frederick-vs-ja commented Nov 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Nov 7, 2024

Uh oh!

github-actions bot commented Nov 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

frederick-vs-ja commented Nov 7, 2024

Uh oh!

philnik777 commented Nov 7, 2024

Uh oh!

frederick-vs-ja commented Nov 7, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldionne commented Nov 7, 2024

Uh oh!

frederick-vs-ja commented Nov 9, 2024

Uh oh!

Uh oh!

Uh oh!

[libc++][hardening] Constrain construction for `__{(static_)bounded,wrap}_iter` #115271

[libc++][hardening] Constrain construction for `__{(static_)bounded,wrap}_iter` #115271

frederick-vs-ja commented Nov 7, 2024 •

edited

Loading

github-actions bot commented Nov 7, 2024 •

edited

Loading