From 78dee26eb5a3bc074ea0269e2e33f1c12667df94 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Thu, 7 Nov 2024 16:09:59 +0800 Subject: [PATCH 1/4] [libc++][hardening] Constrain construction and use `static_assert` ... 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. --- libcxx/include/__iterator/bounded_iter.h | 20 +++++-- libcxx/include/__iterator/wrap_iter.h | 17 ++++-- .../iterators/contiguous_iterators.verify.cpp | 22 ++++++++ .../wrap.bounded.iter.conv.compile.pass.cpp | 53 +++++++++++++++++++ 4 files changed, 106 insertions(+), 6 deletions(-) create mode 100644 libcxx/test/libcxx/iterators/contiguous_iterators.verify.cpp create mode 100644 libcxx/test/libcxx/iterators/wrap.bounded.iter.conv.compile.pass.cpp diff --git a/libcxx/include/__iterator/bounded_iter.h b/libcxx/include/__iterator/bounded_iter.h index 5a86bd98e7194..7b14d765ce826 100644 --- a/libcxx/include/__iterator/bounded_iter.h +++ b/libcxx/include/__iterator/bounded_iter.h @@ -16,9 +16,14 @@ #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_constructible.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 +52,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 ::value > > +template 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 +75,13 @@ struct __bounded_iter { _LIBCPP_HIDE_FROM_ABI __bounded_iter(__bounded_iter const&) = default; _LIBCPP_HIDE_FROM_ABI __bounded_iter(__bounded_iter&&) = default; - template ::value, int> = 0> + template < class _OtherIterator, + __enable_if_t< + _And, + is_convertible, + _Or >, + is_same > > > >::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 +261,7 @@ struct __bounded_iter { private: template friend struct pointer_traits; - template + template 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 2856833e60079..352e94630d855 100644 --- a/libcxx/include/__iterator/wrap_iter.h +++ b/libcxx/include/__iterator/wrap_iter.h @@ -17,9 +17,14 @@ #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_constructible.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 @@ -45,9 +50,15 @@ class __wrap_iter { public: _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __wrap_iter() _NOEXCEPT : __i_() {} - template ::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_convertible, + _Or >, + is_same > > > >::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 0000000000000..c211104bef727 --- /dev/null +++ b/libcxx/test/libcxx/iterators/contiguous_iterators.verify.cpp @@ -0,0 +1,22 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// +// + +// + +// __bounded_iter<_Iter> + +// Verify that __bounded_iter does 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. +// __wrap_iter cannot be so handled because it may directly wrap user-defined fancy pointers in libc++'s vector. + +#include +#include + +// expected-error-re@*:* {{static assertion failed due to requirement {{.*}}Only contiguous iterators can be adapted by __bounded_iter.}} +std::__bounded_iter::iterator> bit; 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 0000000000000..4fd08f0356204 --- /dev/null +++ b/libcxx/test/libcxx/iterators/wrap.bounded.iter.conv.compile.pass.cpp @@ -0,0 +1,53 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// +// + +// + +// __bounded_iter<_Iter> +// __wrap_iter<_Iter> + +// Verify that libc++-wrapped iterators do not permit slicing conversion or construction. + +#include +#include +#include +#include + +#include "test_macros.h" + +struct Base {}; +struct Derived : Base {}; + +#ifdef _LIBCPP_ABI_USE_WRAP_ITER_IN_STD_ARRAY +static_assert(!std::is_convertible::iterator, std::array::iterator>::value, ""); +static_assert(!std::is_convertible::iterator, std::array::const_iterator>::value, ""); +static_assert(!std::is_convertible::const_iterator, std::array::const_iterator>::value, + ""); +static_assert(!std::is_constructible::iterator, std::array::iterator>::value, ""); +static_assert(!std::is_constructible::const_iterator, std::array::iterator>::value, ""); +static_assert( + !std::is_constructible::const_iterator, std::array::const_iterator>::value, ""); +#endif + +static_assert(!std::is_convertible::iterator, std::vector::iterator>::value, ""); +static_assert(!std::is_convertible::iterator, std::vector::const_iterator>::value, ""); +static_assert(!std::is_convertible::const_iterator, std::vector::const_iterator>::value, ""); +static_assert(!std::is_constructible::iterator, std::vector::iterator>::value, ""); +static_assert(!std::is_constructible::const_iterator, std::vector::iterator>::value, ""); +static_assert(!std::is_constructible::const_iterator, std::vector::const_iterator>::value, + ""); + +#if TEST_STD_VER >= 20 +static_assert(!std::is_convertible_v::iterator, std::span::iterator>); +static_assert(!std::is_convertible_v::iterator, std::span::iterator>); +static_assert(!std::is_convertible_v::iterator, std::span::iterator>); +static_assert(!std::is_constructible_v::iterator, std::span::iterator>); +static_assert(!std::is_constructible_v::iterator, std::span::iterator>); +static_assert(!std::is_constructible_v::iterator, std::span::iterator>); +#endif From a1b9e604b98ed99fe3a0c5f75bde20ca3494c74b Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Thu, 7 Nov 2024 19:44:22 +0800 Subject: [PATCH 2/4] Revert `static_assert` --- libcxx/include/__iterator/bounded_iter.h | 7 ++---- .../iterators/contiguous_iterators.verify.cpp | 22 ------------------- 2 files changed, 2 insertions(+), 27 deletions(-) delete mode 100644 libcxx/test/libcxx/iterators/contiguous_iterators.verify.cpp diff --git a/libcxx/include/__iterator/bounded_iter.h b/libcxx/include/__iterator/bounded_iter.h index 7b14d765ce826..788a1cd0a85e2 100644 --- a/libcxx/include/__iterator/bounded_iter.h +++ b/libcxx/include/__iterator/bounded_iter.h @@ -52,11 +52,8 @@ _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 +template ::value > > 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; @@ -261,7 +258,7 @@ struct __bounded_iter { private: template friend struct pointer_traits; - template + template friend struct __bounded_iter; _Iterator __current_; // current iterator _Iterator __begin_, __end_; // valid range represented as [begin, end] diff --git a/libcxx/test/libcxx/iterators/contiguous_iterators.verify.cpp b/libcxx/test/libcxx/iterators/contiguous_iterators.verify.cpp deleted file mode 100644 index c211104bef727..0000000000000 --- a/libcxx/test/libcxx/iterators/contiguous_iterators.verify.cpp +++ /dev/null @@ -1,22 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// 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 -// -//===----------------------------------------------------------------------===// -// - -// - -// __bounded_iter<_Iter> - -// Verify that __bounded_iter does 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. -// __wrap_iter cannot be so handled because it may directly wrap user-defined fancy pointers in libc++'s vector. - -#include -#include - -// expected-error-re@*:* {{static assertion failed due to requirement {{.*}}Only contiguous iterators can be adapted by __bounded_iter.}} -std::__bounded_iter::iterator> bit; From a40207a497edcd8b91d28a5f6aa432e2c9afef81 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Sat, 9 Nov 2024 01:35:43 +0800 Subject: [PATCH 3/4] Address review comments - Drop uses of `is_constructible`. - Generalize to `__static_bounded_iter`. - Improve conditional compilation in test. --- libcxx/include/__iterator/bounded_iter.h | 8 ++--- .../include/__iterator/static_bounded_iter.h | 11 ++++++- libcxx/include/__iterator/wrap_iter.h | 8 ++--- ...ontiguous_iterators.conv.compile.pass.cpp} | 32 ++++++++++++------- 4 files changed, 37 insertions(+), 22 deletions(-) rename libcxx/test/libcxx/iterators/{wrap.bounded.iter.conv.compile.pass.cpp => contiguous_iterators.conv.compile.pass.cpp} (65%) diff --git a/libcxx/include/__iterator/bounded_iter.h b/libcxx/include/__iterator/bounded_iter.h index 44b330ec84b3a..d12750d1f81ac 100644 --- a/libcxx/include/__iterator/bounded_iter.h +++ b/libcxx/include/__iterator/bounded_iter.h @@ -20,7 +20,6 @@ #include <__type_traits/disjunction.h> #include <__type_traits/enable_if.h> #include <__type_traits/integral_constant.h> -#include <__type_traits/is_constructible.h> #include <__type_traits/is_convertible.h> #include <__type_traits/is_same.h> #include <__type_traits/make_const_lvalue_ref.h> @@ -77,10 +76,9 @@ struct __bounded_iter { template < class _OtherIterator, __enable_if_t< - _And, - is_convertible, - _Or >, - is_same > > > >::value, + _And< is_convertible, + _Or >, + is_same > > > >::value, int> = 0> _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __bounded_iter(__bounded_iter<_OtherIterator> const& __other) _NOEXCEPT : __current_(__other.__current_), diff --git a/libcxx/include/__iterator/static_bounded_iter.h b/libcxx/include/__iterator/static_bounded_iter.h index 9794c220384f5..a570c12c9e089 100644 --- a/libcxx/include/__iterator/static_bounded_iter.h +++ b/libcxx/include/__iterator/static_bounded_iter.h @@ -17,9 +17,13 @@ #include <__cstddef/size_t.h> #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) @@ -93,7 +97,12 @@ struct __static_bounded_iter { _LIBCPP_HIDE_FROM_ABI __static_bounded_iter(__static_bounded_iter const&) = default; _LIBCPP_HIDE_FROM_ABI __static_bounded_iter(__static_bounded_iter&&) = default; - template ::value, int> = 0> + template , + _Or >, + is_same > > > >::value, + int> = 0> _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __static_bounded_iter(__static_bounded_iter<_OtherIterator, _Size> const& __other) _NOEXCEPT : __storage_(__other.__storage_.__current(), __other.__storage_.__begin()) {} diff --git a/libcxx/include/__iterator/wrap_iter.h b/libcxx/include/__iterator/wrap_iter.h index 352e94630d855..966c4675b7049 100644 --- a/libcxx/include/__iterator/wrap_iter.h +++ b/libcxx/include/__iterator/wrap_iter.h @@ -21,7 +21,6 @@ #include <__type_traits/disjunction.h> #include <__type_traits/enable_if.h> #include <__type_traits/integral_constant.h> -#include <__type_traits/is_constructible.h> #include <__type_traits/is_convertible.h> #include <__type_traits/is_same.h> #include <__type_traits/make_const_lvalue_ref.h> @@ -52,10 +51,9 @@ class __wrap_iter { _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __wrap_iter() _NOEXCEPT : __i_() {} template < class _OtherIter, - __enable_if_t< _And, - is_convertible, - _Or >, - is_same > > > >::value, + __enable_if_t< _And< is_convertible, + _Or >, + is_same > > > >::value, int> = 0> _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 __wrap_iter(const __wrap_iter<_OtherIter>& __u) _NOEXCEPT : __i_(__u.__i_) {} diff --git a/libcxx/test/libcxx/iterators/wrap.bounded.iter.conv.compile.pass.cpp b/libcxx/test/libcxx/iterators/contiguous_iterators.conv.compile.pass.cpp similarity index 65% rename from libcxx/test/libcxx/iterators/wrap.bounded.iter.conv.compile.pass.cpp rename to libcxx/test/libcxx/iterators/contiguous_iterators.conv.compile.pass.cpp index 4fd08f0356204..372559594143e 100644 --- a/libcxx/test/libcxx/iterators/wrap.bounded.iter.conv.compile.pass.cpp +++ b/libcxx/test/libcxx/iterators/contiguous_iterators.conv.compile.pass.cpp @@ -10,30 +10,40 @@ // // __bounded_iter<_Iter> +// __static_bounded_iter<_Iter> // __wrap_iter<_Iter> // Verify that libc++-wrapped iterators do not permit slicing conversion or construction. #include -#include #include #include +#include #include "test_macros.h" struct Base {}; struct Derived : Base {}; -#ifdef _LIBCPP_ABI_USE_WRAP_ITER_IN_STD_ARRAY -static_assert(!std::is_convertible::iterator, std::array::iterator>::value, ""); -static_assert(!std::is_convertible::iterator, std::array::const_iterator>::value, ""); -static_assert(!std::is_convertible::const_iterator, std::array::const_iterator>::value, - ""); -static_assert(!std::is_constructible::iterator, std::array::iterator>::value, ""); -static_assert(!std::is_constructible::const_iterator, std::array::iterator>::value, ""); -static_assert( - !std::is_constructible::const_iterator, std::array::const_iterator>::value, ""); -#endif +template ::iterator>::value> +struct test_array_helper : std::true_type { + typedef typename std::array::iterator BaseIter; + typedef typename std::array::iterator DerivedIter; + typedef typename std::array::const_iterator BaseConstIter; + typedef typename std::array::const_iterator DerivedConstIter; + + static_assert(!std::is_convertible::value, ""); + static_assert(!std::is_convertible::value, ""); + static_assert(!std::is_convertible::value, ""); + static_assert(!std::is_constructible::value, ""); + static_assert(!std::is_constructible::value, ""); + static_assert(!std::is_constructible::value, ""); +}; + +template +struct test_array_helper : std::true_type {}; + +static_assert(test_array_helper::value, ""); static_assert(!std::is_convertible::iterator, std::vector::iterator>::value, ""); static_assert(!std::is_convertible::iterator, std::vector::const_iterator>::value, ""); From 8f8a4e19b5a9e919f812c726401ee5b38d313436 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Sat, 9 Nov 2024 11:42:43 +0800 Subject: [PATCH 4/4] Fix old friend declaration --- libcxx/include/__iterator/static_bounded_iter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcxx/include/__iterator/static_bounded_iter.h b/libcxx/include/__iterator/static_bounded_iter.h index a570c12c9e089..8f4fbdf6dff96 100644 --- a/libcxx/include/__iterator/static_bounded_iter.h +++ b/libcxx/include/__iterator/static_bounded_iter.h @@ -273,7 +273,7 @@ struct __static_bounded_iter { private: template friend struct pointer_traits; - template + template friend struct __static_bounded_iter; __static_bounded_iter_storage<_Iterator, _Size> __storage_;