On Mon, 30 Mar 2026 at 21:34, François Dumont <[email protected]> wrote:
>
> In this case your approach is indeed the simplest fix to do.
>
>      libstdc++: [_GLIBCXX_DEBUG] Fix COW string valid range check
>
>      In revision 698a6af5dcbae5d935bcda8a461dea8458c658dc the
> _GLIBCXX_DEBUG code
>      for the Library Defect 438 has been removed for C++11 and after.
>
>      But the COW basic_string implementation used when
> _GLIBCXX_USE_CXX11_ABI=0 is
>      missing the _RequireInputIter constraint on a number method
> resulting in test
>      failures.
>
>      For the moment move the culprit __glibcxx_requires_valid_range call
> in a method
>      where the iterator type has already been checked.

OK for trunk, thanks.


>
>      libstdc++-v3/ChangeLog:
>
>              * include/bits/cow_string.h
>              (basic_string::replace(iterator, iterator, _InputIte,
> _InputIte)): Move
>              __glibcxx_requires_valid_range to...
>              (basic_string::_M_replace_dispatch(iterator, iterator,
> _InputIte,
>              _InputIte, __fase_type)): ...here.
>              * testsuite/21_strings/basic_string/debug/append_neg.cc:
> New test case.
>              * testsuite/21_strings/basic_string/debug/assign_neg.cc:
> New test case.
>              * testsuite/21_strings/basic_string/debug/construct_neg.cc:
> New test case.
>              * testsuite/21_strings/basic_string/debug/insert_neg.cc:
> New test case.
>              * testsuite/21_strings/basic_string/debug/replace_neg.cc:
> New test case.
>
> Tested under Linux x86_64 _GLIBCXX_DEBUG mode C++98 and C++11.
>
> Ok to commit ?
>
> François
>
>
> On 3/30/26 19:13, Jonathan Wakely wrote:
> > On Thu, 26 Mar 2026 at 21:46, François Dumont <[email protected]> wrote:
> >> Hi
> >>
> >> Here is the fix to the current COW basic_string issue now that I've
> >> removed the DR438 _GLIBCXX_DEBUG code.
> >>
> >> I've also revisited the location of the debug checks so that for example
> >> on the append_neg.cc test case we have this assertion message with the
> >> cxx11 string:
> >>
> >> In function:
> >>       constexpr std::cxx11::basic_string<_CharT, _Traits, _Alloc>& std::
> >>       cxx11::basic_string<_CharT, _Traits, _Alloc>::replace(const_iterator,
> >>       const_iterator, _InputIterator, _InputIterator) [with _InputIterator 
> >> =
> >>
> >> so refering to the replace method. Whereas on the cow string:
> >>
> >> In function:
> >>       std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT,
> >>       _Traits, _Alloc>::append(_InputIterator, _InputIterator) [with
> >>       _InputIterator = gnu_debug::_Safe_iterator<gnu_cxx::
> >>
> >> I plan to do something similar on cxx11 string. Let me know if useless.
> >>
> >>       libstdc++: [_GLIBCXX_DEBUG] Fix COW basic_string checks
> >>
> >>       In revision 698a6af5dcbae5d935bcda8a461dea8458c658dc the
> >> _GLIBCXX_DEBUG code
> >>       for the Library Defect 438 has been removed for C++11 and after.
> >> But the COW
> >>       basic_string implementation used when _GLIBCXX_USE_CXX11_ABI=0 was
> >> missing
> >>       the _RequireInputIter constraint on a number method resulting in
> >> test failures.
> >>
> >>       _RequireInputIter is now added where necessary. And _GLIBCXX_DEBUG
> >> checks have
> >>       been added at the right place to benefit from more accurate
> >> assertion messages.
> >>
> >>       libstdc++-v3/ChangeLog:
> >>
> >>               * include/bits/cow_string.h [__cplusplus >= 201103L]
> >>               (basic_string(_InputIte, _InputIte, const _Alloc&)): Add
> >> _RequireInputIter
> >>               template parameter. Add
> >> __glibcxx_requires_valid_constructor_range call.
> >>               (_M_replace_dispatch(_InputIte, _InputIte): New.
> >>               (_M_replace_dispatch(iterator, iterator, _InputIte,
> >> _InputIte): New.
> >>               [__cplusplus >= 201103L]
> >>               (append(_InputIte, _InputIte)): Add _RequireInputIter
> >> template parameter.
> >>               Add __glibcxx_requires_valid_constructor_range call. Use
> >> latter.
> >>               (insert(iterator, _InputIte, _InputIte)): Likewise.
> >>               (replace(iterator, iterator, _InputIte, _InputIte)): 
> >> Likewise.
> >>               (_S_construct(_InputIte, _InputIte, const _Alloc&)):  Add
> >> _RequireInputIter
> >>               template parameter.
> >>               * include/debug/debug.h
> >> (__glibcxx_requires_valid_construtor_range): New.
> >>               * testsuite/21_strings/basic_string/debug/append_neg.cc:
> >> New test case.
> >>               * testsuite/21_strings/basic_string/debug/assign_neg.cc:
> >> New test case.
> >>               * testsuite/21_strings/basic_string/debug/construct_neg.cc:
> >> New test case.
> >>               * testsuite/21_strings/basic_string/debug/insert_neg.cc:
> >> New test case.
> >>               * testsuite/21_strings/basic_string/debug/replace_neg.cc:
> >> New test case.
> >>
> >> Tested under Linux x64_86.
> >>
> >> Ok to commit ?
> >
> > This seems like a large change to stable std::string code, which is
> > inappropriate when we're just a few weeks from releasing GCC 16.
> >
> > I would prefer to just move the assertion macro to
> > _M_replace_dispatch, or only check the assertions for !_Integral, as
> > in the patches I showed.
> >
> > For GCC 17 we can revisit this and apply your fixes.

Reply via email to