[PATCH] libstdc++: Make debug iterator pointer sequence const [PR116369]
François Dumont
frs.dumont@gmail.com
Mon Jun 23 17:18:51 GMT 2025
Hi
Even if you have no time to review this for now could you only answer
the question below that is to say:
Should the current _GLIBCXX_INLINE_VERSION abi be preserved ?
Thanks
On 16/06/2025 19:36, François Dumont wrote:
>
> I eventually wonder if it is such a big deal to add the new symbols
> for _GLIBCXX_DEBUG mode.
>
> Here is the patch doing this. It avoids to add many const_cast which
> is what we are trying to achieve here.
>
> I've updated the PR keeping 2 commits so that if this last step is not
> good I can just drop it.
>
> I even updated the versioned namespace mode breaking this mode abi, I
> think it's fine, no ?
>
> François
>
>
> On 08/06/2025 22:00, François Dumont wrote:
>>
>> Here is a new attempt preserving symbols.
>>
>> libstdc++: Make debug iterator pointer sequence const [PR116369]
>>
>> In revision a35dd276cbf6236e08bcf6e56e62c2be41cf6e3c the debug
>> sequence
>> have been made mutable to allow attach iterators to const containers.
>> This change completes this fix by also declaring debug unordered
>> container
>> members mutable.
>>
>> Additionally the debug iterator sequence is now a
>> pointer-to-const and so
>> _Safe_sequence_base _M_attach and all other methods are const
>> qualified.
>> Not-const methods exported are preserved for abi backward
>> compatibility. The
>> new const methods are calling the latter thanks to a safe use of
>> const_cast.
>>
>> libstdc++-v3/ChangeLog:
>>
>> PR c++/116369
>> * include/debug/safe_base.h
>> (_Safe_iterator_base::_M_sequence): Declare as
>> pointer-to-const.
>> (_Safe_iterator_base::_M_attach, _M_attach_single): New,
>> take pointer-to-const
>> _Safe_sequence_base.
>> (_Safe_sequence_base::_M_detach_all, _M_detach_singular,
>> _M_revalidate_singular)
>> (_M_swap, _M_get_mutex): New, const qualified.
>> (_Safe_sequence_base::_M_attach, _M_attach_single,
>> _M_detach, _M_detach_single):
>> const qualify.
>> * include/debug/safe_container.h
>> (_Safe_container<>::_M_cont): Add const qualifier.
>> (_Safe_container<>::_M_swap_base): New.
>> (_Safe_container(_Safe_container&&, const _Alloc&,
>> std::false_type)):
>> Adapt to use latter.
>> (_Safe_container<>::operator=(_Safe_container&&)): Likewise.
>> (_Safe_container<>::_M_swap): Likewise and take parameter
>> as const reference.
>> * include/debug/safe_unordered_base.h
>> (_Safe_local_iterator_base::_M_safe_container): New.
>> (_Safe_local_iterator_base::_Safe_local_iterator_base): Take
>> _Safe_unordered_container_base as pointer-to-const.
>> (_Safe_unordered_container_base::_M_attach,
>> _M_attach_single): New, take
>> container as _Safe_unordered_container_base pointer-to-const.
>> (_Safe_unordered_container_base::_M_local_iterators,
>> _M_const_local_iterators):
>> Add mutable.
>> (_Safe_unordered_container_base::_M_detach_all, _M_swap):
>> New, const qualify.
>> (_Safe_unordered_container_base::_M_attach_local,
>> _M_attach_local_single)
>> (_M_detach_local, _M_detach_local_single): Add const
>> qualifier.
>> * include/debug/safe_iterator.h
>> (_Safe_iterator<>::_M_attach, _M_attach_single):
>> Take _Safe_sequence_base as pointer-to-const.
>> (_Safe_iterator<>::_M_get_sequence): Add const_cast and
>> comment about it.
>> * include/debug/safe_local_iterator.h
>> (_Safe_local_iterator<>): Replace usages
>> of _M_sequence member by _M_safe_container().
>> (_Safe_local_iterator<>::_M_attach, _M_attach_single): Take
>> _Safe_unordered_container_base as pointer-to-const.
>> (_Safe_local_iterator<>::_M_get_sequence): Rename into...
>> (_Safe_local_iterator<>::_M_get_ucontainer): ...this. Add
>> necessary const_cast and
>> comment to explain it.
>> (_Safe_local_iterator<>::_M_is_begin, _M_is_end): Adapt.
>> * include/debug/safe_local_iterator.tcc: Adapt.
>> * include/debug/safe_sequence.h
>> (_Safe_sequence<>::_M_invalidate_if,
>> _M_transfer_from_if): Add const qualifier.
>> * include/debug/safe_sequence.tcc: Adapt.
>> * include/debug/deque (std::__debug::deque::erase): Adapt
>> to use new const
>> qualified methods.
>> * include/debug/formatter.h: Adapt.
>> * include/debug/forward_list
>> (_Safe_forward_list::_M_this): Add const
>> qualification.
>> (_Safe_forward_list::_M_swap_aux): Rename into...
>> (_Safe_forward_list::_S_swap_aux): ...this and take
>> sequence as const reference.
>> (forward_list<>::resize): Adapt to use const methods.
>> * include/debug/list (list<>::resize): Likewise.
>> * src/c++11/debug.cc: Adapt to const qualification.
>> * testsuite/util/testsuite_containers.h
>> (forward_members_unordered::forward_members_unordered):
>> Add check on local_iterator
>> conversion to const_local_iterator.
>> (forward_members::forward_members): Add check on iterator
>> conversion to
>> const_iterator.
>> *
>> testsuite/23_containers/unordered_map/const_container.cc: New test case.
>> *
>> testsuite/23_containers/unordered_multimap/const_container.cc: New
>> test case.
>> *
>> testsuite/23_containers/unordered_multiset/const_container.cc: New
>> test case.
>> *
>> testsuite/23_containers/unordered_set/const_container.cc: New test case.
>> *
>> testsuite/23_containers/vector/debug/mutex_association.cc: Adapt.
>>
>> Tested under Linux x86_64.
>>
>> Ok to commit ?
>>
>> François
>>
>>
>> On 26/05/2025 19:07, François Dumont wrote:
>>>
>>> Ok, I'll give it another try.
>>>
>>> Trying to use the same approach for targets using gnu.ver and others
>>> thought, seems more reasonable to me.
>>>
>>> François
>>>
>>>
>>> On 22/05/2025 09:28, Jonathan Wakely wrote:
>>>>
>>>>
>>>> On Thu, 22 May 2025, 08:26 Jonathan Wakely, <jwakely.gcc@gmail.com>
>>>> wrote:
>>>>
>>>>
>>>>
>>>> On Thu, 15 May 2025, 06:26 François Dumont,
>>>> <frs.dumont@gmail.com> wrote:
>>>>
>>>> Got
>>>>
>>>> On 14/05/2025 18:46, Jonathan Wakely wrote:
>>>> > On Wed, 14 May 2025 at 17:31, François Dumont
>>>> <frs.dumont@gmail.com> wrote:
>>>> >> On 12/05/2025 23:03, Jonathan Wakely wrote:
>>>> >>> On 31/03/25 22:20 +0200, François Dumont wrote:
>>>> >>>> Hi
>>>> >>>>
>>>> >>>> Following this previous patch
>>>> >>>>
>>>> https://gcc.gnu.org/pipermail/libstdc++/2024-August/059418.html
>>>> I've
>>>> >>>> completed it for the _Safe_unordered_container_base
>>>> type and
>>>> >>>> implemented the rest of the change to store the safe
>>>> iterator
>>>> >>>> sequence as a pointer-to-const.
>>>> >>>>
>>>> >>>> libstdc++: Make debug iterator pointer sequence
>>>> const [PR116369]
>>>> >>>>
>>>> >>>> In revision
>>>> a35dd276cbf6236e08bcf6e56e62c2be41cf6e3c the debug
>>>> >>>> sequence
>>>> >>>> have been made mutable to allow attach iterators
>>>> to const
>>>> >>>> containers.
>>>> >>>> This change completes this fix by also declaring
>>>> debug unordered
>>>> >>>> container
>>>> >>>> members mutable.
>>>> >>>>
>>>> >>>> Additionally the debug iterator sequence is now a
>>>> >>>> pointer-to-const and so
>>>> >>>> _Safe_sequence_base _M_attach and all other
>>>> methods are const
>>>> >>>> qualified.
>>>> >>>> Symbols export are maintained thanks to __asm
>>>> directives.
>>>> >>>>
>>>> >>> I can't compile this, it seems to be missing changes to
>>>> >>> safe_local_iterator.tcc:
>>>> >>>
>>>> >>> In file included from
>>>> >>>
>>>> /home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_local_iterator.h:444,
>>>> >>> from
>>>> >>>
>>>> /home/jwakely/src/gcc/gcc/libstdc++-v3/src/c++11/debug.cc:33:
>>>> >>>
>>>> /home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_local_iterator.tcc:
>>>> >>> In member function ‘typename
>>>> >>> __gnu_debug::_Distance_traits<_Iterator>::__type
>>>> >>> __gnu_debug::_Safe_local_iterator<_Iterator,
>>>> >>> _Sequence>::_M_get_distance_to(const
>>>> >>> __gnu_debug::_Safe_local_iterator<_Iterator,
>>>> _Sequence>&) const’:
>>>> >>>
>>>> /home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_local_iterator.tcc:47:17:
>>>> >>> error: there are no arguments to ‘_M_get_sequence’ that
>>>> depend on a
>>>> >>> template parameter, so a declaration of
>>>> ‘_M_get_sequence’ must be
>>>> >>> available [-Wtemplate-body]
>>>> >>> 47 | _M_get_sequence()->bucket_size(bucket()),
>>>> >>> | ^~~~~~~~~~~~~~~
>>>> >>>
>>>> /home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_local_iterator.tcc:47:17:
>>>> >>> note: (if you use ‘-fpermissive’, G++ will accept your
>>>> code, but
>>>> >>> allowing the use of an undeclared name is deprecated)
>>>> >>>
>>>> /home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_local_iterator.tcc:59:18:
>>>> >>> error: there are no arguments to ‘_M_get_sequence’ that
>>>> depend on a
>>>> >>> template parameter, so a declaration of
>>>> ‘_M_get_sequence’ must be
>>>> >>> available [-Wtemplate-body]
>>>> >>> 59 | -_M_get_sequence()->bucket_size(bucket()),
>>>> >>> | ^~~~~~~~~~~~~~~
>>>> >>>
>>>> >> Yes, sorry, I had already spotted this problem, but only
>>>> updated the PR
>>>> >> and not re-sending patch here.
>>>> >>
>>>> >>
>>>> >>>> Also available as a PR
>>>> >>>>
>>>> >>>> https://forge.sourceware.org/gcc/gcc-TEST/pulls/47
>>>> >>>>
>>>> >>>> /** Detach all singular iterators.
>>>> >>>> * @post for all iterators i attached to this
>>>> sequence,
>>>> >>>> * i->_M_version == _M_version.
>>>> >>>> */
>>>> >>>> void
>>>> >>>> - _M_detach_singular();
>>>> >>>> + _M_detach_singular() const
>>>> >>>> +
>>>> __asm("_ZN11__gnu_debug19_Safe_sequence_base18_M_detach_singularEv");
>>>> >>> Does this work on all targets?
>>>> >> No idea ! I thought the symbol name used here just had
>>>> to match the
>>>> >> entries in config/abi/pre/gnu.ver.
>>>> > That linker script is not used for all targets.
>>>>
>>>> Ok, got it, I only need to use this when symbol versioning
>>>> is activated.
>>>>
>>>>
>>>> I don't think that's right. For targets that don't use gnu.ver
>>>> we still want to preserve the same symbols. They just aren't
>>>> versioned on those targets.
>>>> And e.g. Solaris uses versioning, but a different format, not
>>>> gnu.ver, and I don't remember it the same macro is defined.
>>>>
>>>> Isn't it possible to do this without asm somehow? At least as a
>>>> fallback for targets that don't use gnu.ver
>>>>
>>>>
>>>> Basically this needs more research, and then testing on other targets.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> I think this new patch should do it if so.
>>>>
>>>> François
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20250623/d934542e/attachment-0001.htm>
More information about the Libstdc++
mailing list