On Mon, 16 Jun 2025 at 18:36, François Dumont <frs.dum...@gmail.com> wrote: > > I eventually wonder if it is such a big deal to add the new symbols for > _GLIBCXX_DEBUG mode.
I like this version much more than the one trying to duplicate symbols with asm. > Here is the patch doing this. It avoids to add many const_cast which is what > we are trying to achieve here. I'm still not really sure if this is worth it though - is it fixing a bug or a correctness problem? (using const_cast is safe if the objects aren't actually const) All the new tests already pass, even without this patch. Are these just tests for const member functions that we aren't currently testing at all? > 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 ? Yes that's fine. > > 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....@gmail.com> wrote: >> >> >> >> On Thu, 15 May 2025, 06:26 François Dumont, <frs.dum...@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.dum...@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 >>>
