Skip to content

Commit

Permalink
[libc++] Use bounded iterators in std::string_view when the debug mod…
Browse files Browse the repository at this point in the history
…e is enabled

Differential Revision: https://reviews.llvm.org/D142903
  • Loading branch information
ldionne committed Jan 31, 2023
1 parent 742bcbf commit a845b5b
Show file tree
Hide file tree
Showing 13 changed files with 155 additions and 44 deletions.
3 changes: 3 additions & 0 deletions libcxx/docs/ReleaseNotes.rst
Expand Up @@ -41,6 +41,9 @@ Implemented Papers
Improvements and New Features
-----------------------------

- ``std::string_view`` now provides iterators that check for out-of-bounds accesses when the safe
libc++ mode is enabled.

Deprecations and Removals
-------------------------

Expand Down
14 changes: 7 additions & 7 deletions libcxx/include/__chrono/formatter.h
Expand Up @@ -169,7 +169,7 @@ _LIBCPP_HIDE_FROM_ABI void __format_chrono_using_chrono_specs(
if (__year < 1000 || __year > 9999)
__formatter::__format_century(__year, __sstr);
else
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), __s, __it + 1);
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), std::to_address(__s), std::to_address(__it + 1));
} break;

case _CharT('j'):
Expand All @@ -180,7 +180,7 @@ _LIBCPP_HIDE_FROM_ABI void __format_chrono_using_chrono_specs(
// an intemediate step.
__sstr << chrono::duration_cast<chrono::days>(chrono::duration_cast<chrono::seconds>(__value)).count();
else
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), __s, __it + 1);
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), std::to_address(__s), std::to_address(__it + 1));
break;

case _CharT('q'):
Expand Down Expand Up @@ -208,7 +208,7 @@ _LIBCPP_HIDE_FROM_ABI void __format_chrono_using_chrono_specs(

case _CharT('S'):
case _CharT('T'):
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), __s, __it + 1);
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), std::to_address(__s), std::to_address(__it + 1));
if constexpr (__use_fraction<_Tp>())
__formatter::__format_sub_seconds(__value, __sstr);
break;
Expand Down Expand Up @@ -252,7 +252,7 @@ _LIBCPP_HIDE_FROM_ABI void __format_chrono_using_chrono_specs(
if (__year < 1000)
__formatter::__format_year(__year, __sstr);
else
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), __s, __it + 1);
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), std::to_address(__s), std::to_address(__it + 1));
} break;

case _CharT('F'): {
Expand All @@ -261,7 +261,7 @@ _LIBCPP_HIDE_FROM_ABI void __format_chrono_using_chrono_specs(
__formatter::__format_year(__year, __sstr);
__sstr << std::format(_LIBCPP_STATICALLY_WIDEN(_CharT, "-{:02}-{:02}"), __t.tm_mon + 1, __t.tm_mday);
} else
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), __s, __it + 1);
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), std::to_address(__s), std::to_address(__it + 1));
} break;

case _CharT('O'):
Expand All @@ -271,7 +271,7 @@ _LIBCPP_HIDE_FROM_ABI void __format_chrono_using_chrono_specs(
// fractional part should be formatted.
if (*(__it + 1) == 'S') {
++__it;
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), __s, __it + 1);
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), std::to_address(__s), std::to_address(__it + 1));
__formatter::__format_sub_seconds(__value, __sstr);
break;
}
Expand All @@ -281,7 +281,7 @@ _LIBCPP_HIDE_FROM_ABI void __format_chrono_using_chrono_specs(
++__it;
[[fallthrough]];
default:
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), __s, __it + 1);
__facet.put({__sstr}, __sstr, _CharT(' '), std::addressof(__t), std::to_address(__s), std::to_address(__it + 1));
break;
}
} else {
Expand Down
20 changes: 11 additions & 9 deletions libcxx/include/__chrono/parser_std_format_spec.h
Expand Up @@ -137,17 +137,19 @@ _LIBCPP_HIDE_FROM_ABI constexpr void __validate_time_zone(__flags __flags) {

template <class _CharT>
class _LIBCPP_TEMPLATE_VIS __parser_chrono {
using _ConstIterator = typename basic_format_parse_context<_CharT>::const_iterator;

public:
_LIBCPP_HIDE_FROM_ABI constexpr auto
__parse(basic_format_parse_context<_CharT>& __parse_ctx, __fields __fields, __flags __flags)
-> decltype(__parse_ctx.begin()) {
const _CharT* __begin = __parser_.__parse(__parse_ctx, __fields);
const _CharT* __end = __parse_ctx.end();
_ConstIterator __begin = __parser_.__parse(__parse_ctx, __fields);
_ConstIterator __end = __parse_ctx.end();
if (__begin == __end)
return __begin;

const _CharT* __last = __parse_chrono_specs(__begin, __end, __flags);
__chrono_specs_ = basic_string_view<_CharT>{__begin, __last};
_ConstIterator __last = __parse_chrono_specs(__begin, __end, __flags);
__chrono_specs_ = basic_string_view<_CharT>{__begin, __last};

return __last;
}
Expand All @@ -156,8 +158,8 @@ class _LIBCPP_TEMPLATE_VIS __parser_chrono {
basic_string_view<_CharT> __chrono_specs_;

private:
_LIBCPP_HIDE_FROM_ABI constexpr const _CharT*
__parse_chrono_specs(const _CharT* __begin, const _CharT* __end, __flags __flags) {
_LIBCPP_HIDE_FROM_ABI constexpr _ConstIterator
__parse_chrono_specs(_ConstIterator __begin, _ConstIterator __end, __flags __flags) {
_LIBCPP_ASSERT(__begin != __end,
"When called with an empty input the function will cause "
"undefined behavior by evaluating data not in the input");
Expand Down Expand Up @@ -190,7 +192,7 @@ class _LIBCPP_TEMPLATE_VIS __parser_chrono {
/// \pre *__begin == '%'
/// \post __begin points at the end parsed conversion-spec
_LIBCPP_HIDE_FROM_ABI constexpr void
__parse_conversion_spec(const _CharT*& __begin, const _CharT* __end, __flags __flags) {
__parse_conversion_spec(_ConstIterator& __begin, _ConstIterator __end, __flags __flags) {
++__begin;
if (__begin == __end)
std::__throw_format_error("End of input while parsing the modifier chrono conversion-spec");
Expand Down Expand Up @@ -304,7 +306,7 @@ class _LIBCPP_TEMPLATE_VIS __parser_chrono {
/// \pre *__begin == 'E'
/// \post __begin is incremented by one.
_LIBCPP_HIDE_FROM_ABI constexpr void
__parse_modifier_E(const _CharT*& __begin, const _CharT* __end, __flags __flags) {
__parse_modifier_E(_ConstIterator& __begin, _ConstIterator __end, __flags __flags) {
++__begin;
if (__begin == __end)
std::__throw_format_error("End of input while parsing the modifier E");
Expand Down Expand Up @@ -343,7 +345,7 @@ class _LIBCPP_TEMPLATE_VIS __parser_chrono {
/// \pre *__begin == 'O'
/// \post __begin is incremented by one.
_LIBCPP_HIDE_FROM_ABI constexpr void
__parse_modifier_O(const _CharT*& __begin, const _CharT* __end, __flags __flags) {
__parse_modifier_O(_ConstIterator& __begin, _ConstIterator __end, __flags __flags) {
++__begin;
if (__begin == __end)
std::__throw_format_error("End of input while parsing the modifier O");
Expand Down
2 changes: 1 addition & 1 deletion libcxx/include/__format/formatter_output.h
Expand Up @@ -503,7 +503,7 @@ __escape(basic_string<_CharT>& __str, basic_string_view<_CharT> __values, __esca
__unicode::__code_point_view<_CharT> __view{__values.begin(), __values.end()};

while (!__view.__at_end()) {
const _CharT* __first = __view.__position();
auto __first = __view.__position();
typename __unicode::__consume_p2286_result __result = __view.__consume_p2286();
if (__result.__ill_formed_size == 0) {
if (!__formatter::__is_escaped_sequence_written(__str, __result.__value, __mark))
Expand Down
4 changes: 2 additions & 2 deletions libcxx/include/__format/formatter_tuple.h
Expand Up @@ -51,7 +51,7 @@ struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT __formatter_tuple {

template <class _ParseContext>
_LIBCPP_HIDE_FROM_ABI constexpr typename _ParseContext::iterator parse(_ParseContext& __parse_ctx) {
const _CharT* __begin = __parser_.__parse(__parse_ctx, __format_spec::__fields_tuple);
auto __begin = __parser_.__parse(__parse_ctx, __format_spec::__fields_tuple);

// [format.tuple]/7
// ... For each element e in underlying_, if e.set_debug_format()
Expand All @@ -61,7 +61,7 @@ struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT __formatter_tuple {
std::__set_debug_format(std::get<_Index>(__underlying_));
});

const _CharT* __end = __parse_ctx.end();
auto __end = __parse_ctx.end();
if (__begin == __end)
return __begin;

Expand Down
7 changes: 4 additions & 3 deletions libcxx/include/__format/range_formatter.h
Expand Up @@ -55,8 +55,8 @@ struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT range_formatter {

template <class _ParseContext>
_LIBCPP_HIDE_FROM_ABI constexpr typename _ParseContext::iterator parse(_ParseContext& __parse_ctx) {
const _CharT* __begin = __parser_.__parse(__parse_ctx, __format_spec::__fields_range);
const _CharT* __end = __parse_ctx.end();
auto __begin = __parser_.__parse(__parse_ctx, __format_spec::__fields_range);
auto __end = __parse_ctx.end();
if (__begin == __end)
return __begin;

Expand Down Expand Up @@ -211,7 +211,8 @@ struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT range_formatter {
__format_spec::__parser<_CharT> __parser_{.__alignment_ = __format_spec::__alignment::__left};

private:
_LIBCPP_HIDE_FROM_ABI constexpr void __parse_type(const _CharT*& __begin, const _CharT* __end) {
template <contiguous_iterator _Iterator>
_LIBCPP_HIDE_FROM_ABI constexpr void __parse_type(_Iterator& __begin, _Iterator __end) {
switch (*__begin) {
case _CharT('m'):
if constexpr (__fmt_pair_like<_Tp>) {
Expand Down
21 changes: 19 additions & 2 deletions libcxx/include/string_view
Expand Up @@ -208,6 +208,7 @@ namespace std {
#include <__functional/hash.h>
#include <__functional/unary_function.h>
#include <__fwd/string_view.h>
#include <__iterator/bounded_iter.h>
#include <__iterator/concepts.h>
#include <__iterator/readable_traits.h>
#include <__iterator/reverse_iterator.h>
Expand Down Expand Up @@ -265,7 +266,11 @@ public:
using const_pointer = const _CharT*;
using reference = _CharT&;
using const_reference = const _CharT&;
#ifdef _LIBCPP_DEBUG_ITERATOR_BOUNDS_CHECKING
using const_iterator = __bounded_iter<const_pointer>;
#else
using const_iterator = const_pointer; // See [string.view.iterators]
#endif
using iterator = const_iterator;
using const_reverse_iterator = _VSTD::reverse_iterator<const_iterator>;
using reverse_iterator = const_reverse_iterator;
Expand Down Expand Up @@ -343,10 +348,22 @@ public:
const_iterator end() const _NOEXCEPT { return cend(); }

_LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY
const_iterator cbegin() const _NOEXCEPT { return __data_; }
const_iterator cbegin() const _NOEXCEPT {
#ifdef _LIBCPP_DEBUG_ITERATOR_BOUNDS_CHECKING
return std::__make_bounded_iter(data(), data(), data() + size());
#else
return __data_;
#endif
}

_LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY
const_iterator cend() const _NOEXCEPT { return __data_ + __size_; }
const_iterator cend() const _NOEXCEPT {
#ifdef _LIBCPP_DEBUG_ITERATOR_BOUNDS_CHECKING
return std::__make_bounded_iter(data() + size(), data(), data() + size());
#else
return __data_ + __size_;
#endif
}

_LIBCPP_CONSTEXPR_SINCE_CXX17 _LIBCPP_INLINE_VISIBILITY
const_reverse_iterator rbegin() const _NOEXCEPT { return const_reverse_iterator(cend()); }
Expand Down
@@ -0,0 +1,88 @@
//===----------------------------------------------------------------------===//
//
// 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
//
//===----------------------------------------------------------------------===//

// Make sure that std::string_view's iterators check for OOB accesses when the debug mode is enabled.

// REQUIRES: has-unix-headers
// UNSUPPORTED: !libcpp-has-debug-mode

#include <string_view>

#include "check_assertion.h"

int main(int, char**) {
// string_view::iterator
{
std::string_view const str("hello world");
{
auto it = str.end();
TEST_LIBCPP_ASSERT_FAILURE(*it, "__bounded_iter::operator*: Attempt to dereference an out-of-range iterator");
}
{
auto it = str.end();
TEST_LIBCPP_ASSERT_FAILURE(it.operator->(), "__bounded_iter::operator->: Attempt to dereference an out-of-range iterator");
}
{
auto it = str.begin();
TEST_LIBCPP_ASSERT_FAILURE(it[99], "__bounded_iter::operator[]: Attempt to index an iterator out-of-range");
}
}

// string_view::const_iterator
{
std::string_view const str("hello world");
{
auto it = str.cend();
TEST_LIBCPP_ASSERT_FAILURE(*it, "__bounded_iter::operator*: Attempt to dereference an out-of-range iterator");
}
{
auto it = str.cend();
TEST_LIBCPP_ASSERT_FAILURE(it.operator->(), "__bounded_iter::operator->: Attempt to dereference an out-of-range iterator");
}
{
auto it = str.cbegin();
TEST_LIBCPP_ASSERT_FAILURE(it[99], "__bounded_iter::operator[]: Attempt to index an iterator out-of-range");
}
}

// string_view::reverse_iterator
{
std::string_view const str("hello world");
{
auto it = str.rend();
TEST_LIBCPP_ASSERT_FAILURE(*it, "__bounded_iter::operator*: Attempt to dereference an out-of-range iterator");
}
{
auto it = str.rend();
TEST_LIBCPP_ASSERT_FAILURE(it.operator->(), "__bounded_iter::operator->: Attempt to dereference an out-of-range iterator");
}
{
auto it = str.rbegin();
TEST_LIBCPP_ASSERT_FAILURE(it[99], "__bounded_iter::operator*: Attempt to dereference an out-of-range iterator");
}
}

// string_view::const_reverse_iterator
{
std::string_view const str("hello world");
{
auto it = str.crend();
TEST_LIBCPP_ASSERT_FAILURE(*it, "__bounded_iter::operator*: Attempt to dereference an out-of-range iterator");
}
{
auto it = str.crend();
TEST_LIBCPP_ASSERT_FAILURE(it.operator->(), "__bounded_iter::operator->: Attempt to dereference an out-of-range iterator");
}
{
auto it = str.crbegin();
TEST_LIBCPP_ASSERT_FAILURE(it[99], "__bounded_iter::operator*: Attempt to dereference an out-of-range iterator");
}
}

return 0;
}
Expand Up @@ -62,11 +62,11 @@ constexpr void test(const Data& data) {
for (const auto& d : data) {
assert(d.code_points.size() == d.breaks.size());

std::__unicode::__extended_grapheme_cluster_view view{d.input.data(), d.input.data() + d.input.size()};
std::__unicode::__extended_grapheme_cluster_view view{d.input.begin(), d.input.end()};
for (size_t i = 0; i < d.breaks.size(); ++i) {
auto r = view.__consume();
assert(r.__code_point_ == d.code_points[i]);
assert(r.__last_ == d.input.data() + d.breaks[i]);
assert(r.__last_ == d.input.begin() + d.breaks[i]);
}
}
}
Expand Down
Expand Up @@ -25,8 +25,8 @@ struct MoveOnlyView : std::ranges::view_base {
constexpr MoveOnlyView(std::string_view v) : view_(v) {}
constexpr MoveOnlyView(MoveOnlyView&&) = default;
constexpr MoveOnlyView& operator=(MoveOnlyView&&) = default;
constexpr const char* begin() const { return view_.begin(); }
constexpr const char* end() const { return view_.end(); }
constexpr std::string_view::const_iterator begin() const { return view_.begin(); }
constexpr std::string_view::const_iterator end() const { return view_.end(); }
constexpr bool operator==(MoveOnlyView rhs) const { return view_ == rhs.view_; }
};
static_assert( std::ranges::view<MoveOnlyView>);
Expand Down
Expand Up @@ -80,8 +80,8 @@ struct StrView : std::ranges::view_base {
// C++23 and later.
template <std::ranges::range R>
constexpr StrView(R&& r) : buffer_(r.begin(), r.end()) {}
constexpr const char* begin() const { return buffer_.begin(); }
constexpr const char* end() const { return buffer_.end(); }
constexpr std::string_view::const_iterator begin() const { return buffer_.begin(); }
constexpr std::string_view::const_iterator end() const { return buffer_.end(); }
constexpr bool operator==(const StrView& rhs) const { return buffer_ == rhs.buffer_; }
};
static_assert( std::ranges::random_access_range<StrView>);
Expand Down
Expand Up @@ -29,8 +29,8 @@ struct ForwardViewCommonIfConst : std::ranges::view_base {
constexpr ForwardViewCommonIfConst& operator=(const ForwardViewCommonIfConst&) = default;
constexpr forward_iterator<char*> begin() { return forward_iterator<char*>(nullptr); }
constexpr std::default_sentinel_t end() { return std::default_sentinel; }
constexpr forward_iterator<const char*> begin() const { return forward_iterator<const char*>(view_.begin()); }
constexpr forward_iterator<const char*> end() const { return forward_iterator<const char*>(view_.end()); }
constexpr forward_iterator<std::string_view::const_iterator> begin() const { return forward_iterator<std::string_view::const_iterator>(view_.begin()); }
constexpr forward_iterator<std::string_view::const_iterator> end() const { return forward_iterator<std::string_view::const_iterator>(view_.end()); }
};
bool operator==(forward_iterator<char*>, std::default_sentinel_t) { return false; }

Expand All @@ -45,10 +45,10 @@ struct ForwardViewNonCommonRange : std::ranges::view_base {
constexpr ForwardViewNonCommonRange& operator=(const ForwardViewNonCommonRange&) = default;
constexpr forward_iterator<char*> begin() { return forward_iterator<char*>(nullptr); }
constexpr std::default_sentinel_t end() { return std::default_sentinel; }
constexpr forward_iterator<const char*> begin() const { return forward_iterator<const char*>(view_.begin()); }
constexpr forward_iterator<std::string_view::const_iterator> begin() const { return forward_iterator<std::string_view::const_iterator>(view_.begin()); }
constexpr std::default_sentinel_t end() const { return std::default_sentinel; }
};
bool operator==(forward_iterator<const char*>, std::default_sentinel_t) { return false; }
bool operator==(forward_iterator<std::string_view::const_iterator>, std::default_sentinel_t) { return false; }

constexpr bool test() {
// non-const: forward_range<V> && simple_view<V> && simple_view<P> -> outer-iterator<Const = true>
Expand Down

0 comments on commit a845b5b

Please sign in to comment.