From f71a3d95657673fa0353b955dc20dca000c16ea7 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 10 May 2024 16:31:11 -0400 Subject: [PATCH 1/4] Suppress a redundant hardening check in basic_string_view::substr Fixes #91634. This could alternatively be done with an _LIBCPP_ASSUME, after https://github.com/llvm/llvm-project/pull/91801 lands, but would also require https://github.com/llvm/llvm-project/issues/91619 be fixed first. Given the dependencies, it seemed simplest to just make a private ctor. --- libcxx/include/string_view | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/libcxx/include/string_view b/libcxx/include/string_view index 5c4bec742bafa..2c9703cedfd2a 100644 --- a/libcxx/include/string_view +++ b/libcxx/include/string_view @@ -449,8 +449,11 @@ public: } _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI basic_string_view substr(size_type __pos = 0, size_type __n = npos) const { + // Use the `__assume_valid` form of the constructor to avoid an unnecessary check. Any substring of a view is a + // valid view. In particular, `size()` is known to be smaller than `numeric_limits::max()`, so the + // new size is also smaller. See also https://github.com/llvm/llvm-project/issues/91634. return __pos > size() ? (__throw_out_of_range("string_view::substr"), basic_string_view()) - : basic_string_view(data() + __pos, std::min(__n, size() - __pos)); + : basic_string_view(data() + __pos, std::min(__n, size() - __pos), __assume_valid{}); } _LIBCPP_CONSTEXPR_SINCE_CXX14 int compare(basic_string_view __sv) const _NOEXCEPT { @@ -675,6 +678,16 @@ public: #endif private: + struct __assume_valid {}; + + // This is the same as the pointer and length constructor, but it does not include additionally hardening checks. It + // is intended for use within the class, when the class invariants already imply the resulting object is valid. The + // compiler usually cannot apply this optimization itself, because it does not know class invariants. + _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI + basic_string_view(const _CharT* __s, size_type __len, __assume_valid) _NOEXCEPT + : __data_(__s), + __size_(__len) {} + const value_type* __data_; size_type __size_; }; From 350b01e242e7540c7b170920a43294ea51e24fb9 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 22 Jul 2024 17:43:13 -0700 Subject: [PATCH 2/4] Slightly tweak a comment --- libcxx/include/string_view | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libcxx/include/string_view b/libcxx/include/string_view index 2c9703cedfd2a..590271a002bf5 100644 --- a/libcxx/include/string_view +++ b/libcxx/include/string_view @@ -680,9 +680,9 @@ public: private: struct __assume_valid {}; - // This is the same as the pointer and length constructor, but it does not include additionally hardening checks. It - // is intended for use within the class, when the class invariants already imply the resulting object is valid. The - // compiler usually cannot apply this optimization itself, because it does not know class invariants. + // This is the same as the pointer and length constructor, but without the additional hardening checks. It is intended + // for use within the class, when the class invariants already guarantee the resulting object is valid. The compiler + // usually cannot eliminate the redundant checks because it does not know class invariants. _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI basic_string_view(const _CharT* __s, size_type __len, __assume_valid) _NOEXCEPT : __data_(__s), From aafa186173db7e82e2c370ad321cfc7bca0f5db1 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 22 Jul 2024 17:44:55 -0700 Subject: [PATCH 3/4] Don't use list initialization to hopefully fix the C++03 mode --- libcxx/include/string_view | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcxx/include/string_view b/libcxx/include/string_view index 590271a002bf5..2c2898c170986 100644 --- a/libcxx/include/string_view +++ b/libcxx/include/string_view @@ -453,7 +453,7 @@ public: // valid view. In particular, `size()` is known to be smaller than `numeric_limits::max()`, so the // new size is also smaller. See also https://github.com/llvm/llvm-project/issues/91634. return __pos > size() ? (__throw_out_of_range("string_view::substr"), basic_string_view()) - : basic_string_view(data() + __pos, std::min(__n, size() - __pos), __assume_valid{}); + : basic_string_view(data() + __pos, std::min(__n, size() - __pos), __assume_valid()); } _LIBCPP_CONSTEXPR_SINCE_CXX14 int compare(basic_string_view __sv) const _NOEXCEPT { From 15e7af78c4b347bc7043975b686c90dceb03ae9b Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Tue, 23 Jul 2024 12:25:50 -0700 Subject: [PATCH 4/4] Feedback --- libcxx/include/string_view | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libcxx/include/string_view b/libcxx/include/string_view index a748bf39150d2..2a03ee99e9ab5 100644 --- a/libcxx/include/string_view +++ b/libcxx/include/string_view @@ -452,7 +452,7 @@ public: // valid view. In particular, `size()` is known to be smaller than `numeric_limits::max()`, so the // new size is also smaller. See also https://github.com/llvm/llvm-project/issues/91634. return __pos > size() ? (__throw_out_of_range("string_view::substr"), basic_string_view()) - : basic_string_view(data() + __pos, std::min(__n, size() - __pos), __assume_valid()); + : basic_string_view(__assume_valid(), data() + __pos, std::min(__n, size() - __pos)); } _LIBCPP_CONSTEXPR_SINCE_CXX14 int compare(basic_string_view __sv) const _NOEXCEPT { @@ -683,7 +683,7 @@ private: // for use within the class, when the class invariants already guarantee the resulting object is valid. The compiler // usually cannot eliminate the redundant checks because it does not know class invariants. _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI - basic_string_view(const _CharT* __s, size_type __len, __assume_valid) _NOEXCEPT + basic_string_view(__assume_valid, const _CharT* __s, size_type __len) _NOEXCEPT : __data_(__s), __size_(__len) {}