| Message ID | 20190118212843.GA7905@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR libstdc++/88782 avoid ODR problems in std::make_shared | expand |
On Fri, Jan 18, 2019 at 10:29 PM Jonathan Wakely <jwakely@redhat.com> wrote: > > The old version of _Sp_counted_ptr_inplace::_M_get_deleter (up to GCC > 8.2.0) expects to be passed a real std::typeinfo object, so mixing that > with the new definition of the __shared_ptr constructor (which always > passes the fake tag) leads to accessing the fake object as a real > std::typeinfo. Instead of trying to make it safe to mix the old and new > definitions, just stop using that function. By passing a reference to > __shared_ptr::_M_ptr to the __shared_count constructor it can be set > directly, without needing to obtain the pointer via the _M_get_deleter > back-channel. This avoids a virtual dispatch (which fixes PR 87514). > > This means that code built against new libstdc++ headers doesn't use > _M_get_deleter at all, and so make_shared works the same whether RTTI is > enabled or not. > > Also change _M_get_deleter so that it checks for a real type_info object > even when RTTI is disabled, by calling a library function. Unless > libstdc++ itself is built without RTTI that library function will be > able to test if it's the right type_info. This means the new definition > of _M_get_deleter can handle both the fake type_info tag and a real > type_info object, even if built without RTTI. > > If linking to objects built against older versions of libstdc++ then if > all objects use -frtti or all use -fno-rtti, then the caller of > _M_get_deleter and the definition of _M_get_deleter will be consistent > and it will work. If mixing -frtti with -fno-rtti it can still fail if > the linker picks an old definition of _M_get_deleter and an old > __shared_ptr constructor that are incompatible. In that some or all > objects might need to be recompiled. Can you try summarizing whatever caveats result from this in the 8.3/9 changes.html parts? I have a hard time extracting that myself from the above ;) (small example what kind of simplest code might run into this helps) Richard. > PR libstdc++/87514 > PR libstdc++/87520 > PR libstdc++/88782 > * config/abi/pre/gnu.ver (GLIBCXX_3.4.26): Export new symbol. > * include/bits/shared_ptr.h > (shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)) > (allocate_shared): Change to use new tag type. > * include/bits/shared_ptr_base.h (_Sp_make_shared_tag::_S_eq): > Declare new member function. > (_Sp_alloc_shared_tag): Define new type. > (_Sp_counted_ptr_inplace): Declare __shared_count<_Lp> as a friend. > (_Sp_counted_ptr_inplace::_M_get_deleter) [!__cpp_rtti]: Use > _Sp_make_shared_tag::_S_eq to check type_info. > (__shared_count(Ptr, Deleter),__shared_count(Ptr, Deleter, Alloc)): > Constrain to prevent being called with _Sp_alloc_shared_tag. > (__shared_count(_Sp_make_shared_tag, const _Alloc&, Args&&...)): > Replace constructor with ... > (__shared_count(Tp*&, _Sp_alloc_shared_tag<_Alloc>, Args&&...)): Use > reference parameter so address of the new object can be returned to > the caller. Obtain the allocator from the tag type. > (__shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)): Replace > constructor with ... > (__shared_ptr(_Sp_alloc_shared_tag<Alloc>, Args&&...)): Pass _M_ptr > to the __shared_count constructor. > (__allocate_shared): Change to use new tag type. > * src/c++11/shared_ptr.cc (_Sp_make_shared_tag::_S_eq): Define. > > Tested powerpc64le-linux, committed to trunk. > > I'll backport this to gcc-8-branch without the new symbol in the library. > >
On 21/01/19 09:13 +0100, Richard Biener wrote: >On Fri, Jan 18, 2019 at 10:29 PM Jonathan Wakely <jwakely@redhat.com> wrote: >> >> The old version of _Sp_counted_ptr_inplace::_M_get_deleter (up to GCC >> 8.2.0) expects to be passed a real std::typeinfo object, so mixing that >> with the new definition of the __shared_ptr constructor (which always >> passes the fake tag) leads to accessing the fake object as a real >> std::typeinfo. Instead of trying to make it safe to mix the old and new >> definitions, just stop using that function. By passing a reference to >> __shared_ptr::_M_ptr to the __shared_count constructor it can be set >> directly, without needing to obtain the pointer via the _M_get_deleter >> back-channel. This avoids a virtual dispatch (which fixes PR 87514). >> >> This means that code built against new libstdc++ headers doesn't use >> _M_get_deleter at all, and so make_shared works the same whether RTTI is >> enabled or not. >> >> Also change _M_get_deleter so that it checks for a real type_info object >> even when RTTI is disabled, by calling a library function. Unless >> libstdc++ itself is built without RTTI that library function will be >> able to test if it's the right type_info. This means the new definition >> of _M_get_deleter can handle both the fake type_info tag and a real >> type_info object, even if built without RTTI. >> >> If linking to objects built against older versions of libstdc++ then if >> all objects use -frtti or all use -fno-rtti, then the caller of >> _M_get_deleter and the definition of _M_get_deleter will be consistent >> and it will work. If mixing -frtti with -fno-rtti it can still fail if >> the linker picks an old definition of _M_get_deleter and an old >> __shared_ptr constructor that are incompatible. In that some or all >> objects might need to be recompiled. > >Can you try summarizing whatever caveats result from this in >the 8.3/9 changes.html parts? I have a hard time extracting that >myself from the above ;) (small example what kind of simplest code might >run into this helps) There are *fewer* caveats compared to previous releases. There's an example in PR 87520 showing problems mixing -frtti and -fno-rtti code, although it's a contrived example that uses explicit instantiation of shared_ptr internals to demonstrate the problem. The real issue reported privately to me is harder to reproduce, and will be even harder with this fix committed. I'll try to document this potential problem mixing -frtti and -fno-rtti (there are still others, e.g. PR 43105 lists one). The problems described in PR 88782 only happen if you use 8.2.1 snapshots post-20181122 and mix code compiled by that snapshot with code compiled by earlier releases. If you only use 8.3.0 (or 9.1.0) and no snapshots post-20181122 then, the problem can't happen. I don't think we need to document transient issues that only existed for two months with snapshots. >Richard. > >> PR libstdc++/87514 >> PR libstdc++/87520 >> PR libstdc++/88782 >> * config/abi/pre/gnu.ver (GLIBCXX_3.4.26): Export new symbol. >> * include/bits/shared_ptr.h >> (shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)) >> (allocate_shared): Change to use new tag type. >> * include/bits/shared_ptr_base.h (_Sp_make_shared_tag::_S_eq): >> Declare new member function. >> (_Sp_alloc_shared_tag): Define new type. >> (_Sp_counted_ptr_inplace): Declare __shared_count<_Lp> as a friend. >> (_Sp_counted_ptr_inplace::_M_get_deleter) [!__cpp_rtti]: Use >> _Sp_make_shared_tag::_S_eq to check type_info. >> (__shared_count(Ptr, Deleter),__shared_count(Ptr, Deleter, Alloc)): >> Constrain to prevent being called with _Sp_alloc_shared_tag. >> (__shared_count(_Sp_make_shared_tag, const _Alloc&, Args&&...)): >> Replace constructor with ... >> (__shared_count(Tp*&, _Sp_alloc_shared_tag<_Alloc>, Args&&...)): Use >> reference parameter so address of the new object can be returned to >> the caller. Obtain the allocator from the tag type. >> (__shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)): Replace >> constructor with ... >> (__shared_ptr(_Sp_alloc_shared_tag<Alloc>, Args&&...)): Pass _M_ptr >> to the __shared_count constructor. >> (__allocate_shared): Change to use new tag type. >> * src/c++11/shared_ptr.cc (_Sp_make_shared_tag::_S_eq): Define. >> >> Tested powerpc64le-linux, committed to trunk. >> >> I'll backport this to gcc-8-branch without the new symbol in the library. >> >>
On Mon, Jan 21, 2019 at 12:08 PM Jonathan Wakely <jwakely@redhat.com> wrote: > > On 21/01/19 09:13 +0100, Richard Biener wrote: > >On Fri, Jan 18, 2019 at 10:29 PM Jonathan Wakely <jwakely@redhat.com> wrote: > >> > >> The old version of _Sp_counted_ptr_inplace::_M_get_deleter (up to GCC > >> 8.2.0) expects to be passed a real std::typeinfo object, so mixing that > >> with the new definition of the __shared_ptr constructor (which always > >> passes the fake tag) leads to accessing the fake object as a real > >> std::typeinfo. Instead of trying to make it safe to mix the old and new > >> definitions, just stop using that function. By passing a reference to > >> __shared_ptr::_M_ptr to the __shared_count constructor it can be set > >> directly, without needing to obtain the pointer via the _M_get_deleter > >> back-channel. This avoids a virtual dispatch (which fixes PR 87514). > >> > >> This means that code built against new libstdc++ headers doesn't use > >> _M_get_deleter at all, and so make_shared works the same whether RTTI is > >> enabled or not. > >> > >> Also change _M_get_deleter so that it checks for a real type_info object > >> even when RTTI is disabled, by calling a library function. Unless > >> libstdc++ itself is built without RTTI that library function will be > >> able to test if it's the right type_info. This means the new definition > >> of _M_get_deleter can handle both the fake type_info tag and a real > >> type_info object, even if built without RTTI. > >> > >> If linking to objects built against older versions of libstdc++ then if > >> all objects use -frtti or all use -fno-rtti, then the caller of > >> _M_get_deleter and the definition of _M_get_deleter will be consistent > >> and it will work. If mixing -frtti with -fno-rtti it can still fail if > >> the linker picks an old definition of _M_get_deleter and an old > >> __shared_ptr constructor that are incompatible. In that some or all > >> objects might need to be recompiled. > > > >Can you try summarizing whatever caveats result from this in > >the 8.3/9 changes.html parts? I have a hard time extracting that > >myself from the above ;) (small example what kind of simplest code might > >run into this helps) > > There are *fewer* caveats compared to previous releases. > > There's an example in PR 87520 showing problems mixing -frtti and > -fno-rtti code, although it's a contrived example that uses explicit > instantiation of shared_ptr internals to demonstrate the problem. The > real issue reported privately to me is harder to reproduce, and will > be even harder with this fix committed. I'll try to document this > potential problem mixing -frtti and -fno-rtti (there are still others, > e.g. PR 43105 lists one). > > The problems described in PR 88782 only happen if you use 8.2.1 > snapshots post-20181122 and mix code compiled by that snapshot with > code compiled by earlier releases. If you only use 8.3.0 (or 9.1.0) > and no snapshots post-20181122 then, the problem can't happen. I don't > think we need to document transient issues that only existed for two > months with snapshots. OK, I had the impression that with the fix in there's still more cases that have issues than before whatever rev. triggered the original issue on the GCC 8 branch. If that's not so then I'm fine. I wasn't aware of any -f[no-]rtti mixing issues at all so documenting those might still be nice. Richard. > >Richard. > > > >> PR libstdc++/87514 > >> PR libstdc++/87520 > >> PR libstdc++/88782 > >> * config/abi/pre/gnu.ver (GLIBCXX_3.4.26): Export new symbol. > >> * include/bits/shared_ptr.h > >> (shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)) > >> (allocate_shared): Change to use new tag type. > >> * include/bits/shared_ptr_base.h (_Sp_make_shared_tag::_S_eq): > >> Declare new member function. > >> (_Sp_alloc_shared_tag): Define new type. > >> (_Sp_counted_ptr_inplace): Declare __shared_count<_Lp> as a friend. > >> (_Sp_counted_ptr_inplace::_M_get_deleter) [!__cpp_rtti]: Use > >> _Sp_make_shared_tag::_S_eq to check type_info. > >> (__shared_count(Ptr, Deleter),__shared_count(Ptr, Deleter, Alloc)): > >> Constrain to prevent being called with _Sp_alloc_shared_tag. > >> (__shared_count(_Sp_make_shared_tag, const _Alloc&, Args&&...)): > >> Replace constructor with ... > >> (__shared_count(Tp*&, _Sp_alloc_shared_tag<_Alloc>, Args&&...)): Use > >> reference parameter so address of the new object can be returned to > >> the caller. Obtain the allocator from the tag type. > >> (__shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)): Replace > >> constructor with ... > >> (__shared_ptr(_Sp_alloc_shared_tag<Alloc>, Args&&...)): Pass _M_ptr > >> to the __shared_count constructor. > >> (__allocate_shared): Change to use new tag type. > >> * src/c++11/shared_ptr.cc (_Sp_make_shared_tag::_S_eq): Define. > >> > >> Tested powerpc64le-linux, committed to trunk. > >> > >> I'll backport this to gcc-8-branch without the new symbol in the library. > >> > >>
On 21/01/19 12:16 +0100, Richard Biener wrote: >On Mon, Jan 21, 2019 at 12:08 PM Jonathan Wakely <jwakely@redhat.com> wrote: >> >> On 21/01/19 09:13 +0100, Richard Biener wrote: >> >On Fri, Jan 18, 2019 at 10:29 PM Jonathan Wakely <jwakely@redhat.com> wrote: >> >> >> >> The old version of _Sp_counted_ptr_inplace::_M_get_deleter (up to GCC >> >> 8.2.0) expects to be passed a real std::typeinfo object, so mixing that >> >> with the new definition of the __shared_ptr constructor (which always >> >> passes the fake tag) leads to accessing the fake object as a real >> >> std::typeinfo. Instead of trying to make it safe to mix the old and new >> >> definitions, just stop using that function. By passing a reference to >> >> __shared_ptr::_M_ptr to the __shared_count constructor it can be set >> >> directly, without needing to obtain the pointer via the _M_get_deleter >> >> back-channel. This avoids a virtual dispatch (which fixes PR 87514). >> >> >> >> This means that code built against new libstdc++ headers doesn't use >> >> _M_get_deleter at all, and so make_shared works the same whether RTTI is >> >> enabled or not. >> >> >> >> Also change _M_get_deleter so that it checks for a real type_info object >> >> even when RTTI is disabled, by calling a library function. Unless >> >> libstdc++ itself is built without RTTI that library function will be >> >> able to test if it's the right type_info. This means the new definition >> >> of _M_get_deleter can handle both the fake type_info tag and a real >> >> type_info object, even if built without RTTI. >> >> >> >> If linking to objects built against older versions of libstdc++ then if >> >> all objects use -frtti or all use -fno-rtti, then the caller of >> >> _M_get_deleter and the definition of _M_get_deleter will be consistent >> >> and it will work. If mixing -frtti with -fno-rtti it can still fail if >> >> the linker picks an old definition of _M_get_deleter and an old >> >> __shared_ptr constructor that are incompatible. In that some or all >> >> objects might need to be recompiled. >> > >> >Can you try summarizing whatever caveats result from this in >> >the 8.3/9 changes.html parts? I have a hard time extracting that >> >myself from the above ;) (small example what kind of simplest code might >> >run into this helps) >> >> There are *fewer* caveats compared to previous releases. >> >> There's an example in PR 87520 showing problems mixing -frtti and >> -fno-rtti code, although it's a contrived example that uses explicit >> instantiation of shared_ptr internals to demonstrate the problem. The >> real issue reported privately to me is harder to reproduce, and will >> be even harder with this fix committed. I'll try to document this >> potential problem mixing -frtti and -fno-rtti (there are still others, >> e.g. PR 43105 lists one). >> >> The problems described in PR 88782 only happen if you use 8.2.1 >> snapshots post-20181122 and mix code compiled by that snapshot with >> code compiled by earlier releases. If you only use 8.3.0 (or 9.1.0) >> and no snapshots post-20181122 then, the problem can't happen. I don't >> think we need to document transient issues that only existed for two >> months with snapshots. > >OK, I had the impression that with the fix in there's still more cases >that have issues than before whatever rev. triggered the original issue >on the GCC 8 branch. If that's not so then I'm fine. Nope, this makes some problem cases work, and introduces no new problem cases compared to 8.2.0. That's the plan, anyway. I've been wrong before :-( >I wasn't aware of any -f[no-]rtti mixing issues at all so documenting >those might still be nice. Agreed, I'll see what I can do about that ...
On 21/01/19 11:32 +0000, Jonathan Wakely wrote: >On 21/01/19 12:16 +0100, Richard Biener wrote: >>On Mon, Jan 21, 2019 at 12:08 PM Jonathan Wakely <jwakely@redhat.com> wrote: >>> >>>On 21/01/19 09:13 +0100, Richard Biener wrote: >>>>On Fri, Jan 18, 2019 at 10:29 PM Jonathan Wakely <jwakely@redhat.com> wrote: >>>>> >>>>> The old version of _Sp_counted_ptr_inplace::_M_get_deleter (up to GCC >>>>> 8.2.0) expects to be passed a real std::typeinfo object, so mixing that >>>>> with the new definition of the __shared_ptr constructor (which always >>>>> passes the fake tag) leads to accessing the fake object as a real >>>>> std::typeinfo. Instead of trying to make it safe to mix the old and new >>>>> definitions, just stop using that function. By passing a reference to >>>>> __shared_ptr::_M_ptr to the __shared_count constructor it can be set >>>>> directly, without needing to obtain the pointer via the _M_get_deleter >>>>> back-channel. This avoids a virtual dispatch (which fixes PR 87514). >>>>> >>>>> This means that code built against new libstdc++ headers doesn't use >>>>> _M_get_deleter at all, and so make_shared works the same whether RTTI is >>>>> enabled or not. >>>>> >>>>> Also change _M_get_deleter so that it checks for a real type_info object >>>>> even when RTTI is disabled, by calling a library function. Unless >>>>> libstdc++ itself is built without RTTI that library function will be >>>>> able to test if it's the right type_info. This means the new definition >>>>> of _M_get_deleter can handle both the fake type_info tag and a real >>>>> type_info object, even if built without RTTI. >>>>> >>>>> If linking to objects built against older versions of libstdc++ then if >>>>> all objects use -frtti or all use -fno-rtti, then the caller of >>>>> _M_get_deleter and the definition of _M_get_deleter will be consistent >>>>> and it will work. If mixing -frtti with -fno-rtti it can still fail if >>>>> the linker picks an old definition of _M_get_deleter and an old >>>>> __shared_ptr constructor that are incompatible. In that some or all >>>>> objects might need to be recompiled. >>>> >>>>Can you try summarizing whatever caveats result from this in >>>>the 8.3/9 changes.html parts? I have a hard time extracting that >>>>myself from the above ;) (small example what kind of simplest code might >>>>run into this helps) >>> >>>There are *fewer* caveats compared to previous releases. >>> >>>There's an example in PR 87520 showing problems mixing -frtti and >>>-fno-rtti code, although it's a contrived example that uses explicit >>>instantiation of shared_ptr internals to demonstrate the problem. The >>>real issue reported privately to me is harder to reproduce, and will >>>be even harder with this fix committed. I'll try to document this >>>potential problem mixing -frtti and -fno-rtti (there are still others, >>>e.g. PR 43105 lists one). >>> >>>The problems described in PR 88782 only happen if you use 8.2.1 >>>snapshots post-20181122 and mix code compiled by that snapshot with >>>code compiled by earlier releases. If you only use 8.3.0 (or 9.1.0) >>>and no snapshots post-20181122 then, the problem can't happen. I don't >>>think we need to document transient issues that only existed for two >>>months with snapshots. >> >>OK, I had the impression that with the fix in there's still more cases >>that have issues than before whatever rev. triggered the original issue >>on the GCC 8 branch. If that's not so then I'm fine. > >Nope, this makes some problem cases work, and introduces no new >problem cases compared to 8.2.0. Oh, I guess I should mention that mixing std::make_shared compiled with 8.2.0 and compiled with later versions will produce a bit of code bloat compared to using the same version for all objects. That's because they instantiate different constructors now, so the linker will keep both definitions, instead of discarding one. (That's how the bug is fixed, by ensuring the new code generates different symbols which aren't affected by the behaviour of the old symbols). But that happens with a lot of libstdc++ fixes, and we don't usually document it.
On 18/01/19 21:28 +0000, Jonathan Wakely wrote: >The old version of _Sp_counted_ptr_inplace::_M_get_deleter (up to GCC >8.2.0) expects to be passed a real std::typeinfo object, so mixing that >with the new definition of the __shared_ptr constructor (which always >passes the fake tag) leads to accessing the fake object as a real >std::typeinfo. Instead of trying to make it safe to mix the old and new >definitions, just stop using that function. By passing a reference to >__shared_ptr::_M_ptr to the __shared_count constructor it can be set >directly, without needing to obtain the pointer via the _M_get_deleter >back-channel. This avoids a virtual dispatch (which fixes PR 87514). > >This means that code built against new libstdc++ headers doesn't use >_M_get_deleter at all, and so make_shared works the same whether RTTI is >enabled or not. > >Also change _M_get_deleter so that it checks for a real type_info object >even when RTTI is disabled, by calling a library function. Unless >libstdc++ itself is built without RTTI that library function will be >able to test if it's the right type_info. This means the new definition >of _M_get_deleter can handle both the fake type_info tag and a real >type_info object, even if built without RTTI. > >If linking to objects built against older versions of libstdc++ then if >all objects use -frtti or all use -fno-rtti, then the caller of >_M_get_deleter and the definition of _M_get_deleter will be consistent >and it will work. If mixing -frtti with -fno-rtti it can still fail if >the linker picks an old definition of _M_get_deleter and an old >__shared_ptr constructor that are incompatible. In that some or all >objects might need to be recompiled. > > PR libstdc++/87514 > PR libstdc++/87520 > PR libstdc++/88782 > * config/abi/pre/gnu.ver (GLIBCXX_3.4.26): Export new symbol. > * include/bits/shared_ptr.h > (shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)) > (allocate_shared): Change to use new tag type. > * include/bits/shared_ptr_base.h (_Sp_make_shared_tag::_S_eq): > Declare new member function. > (_Sp_alloc_shared_tag): Define new type. > (_Sp_counted_ptr_inplace): Declare __shared_count<_Lp> as a friend. > (_Sp_counted_ptr_inplace::_M_get_deleter) [!__cpp_rtti]: Use > _Sp_make_shared_tag::_S_eq to check type_info. > (__shared_count(Ptr, Deleter),__shared_count(Ptr, Deleter, Alloc)): > Constrain to prevent being called with _Sp_alloc_shared_tag. > (__shared_count(_Sp_make_shared_tag, const _Alloc&, Args&&...)): > Replace constructor with ... > (__shared_count(Tp*&, _Sp_alloc_shared_tag<_Alloc>, Args&&...)): Use > reference parameter so address of the new object can be returned to > the caller. Obtain the allocator from the tag type. > (__shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)): Replace > constructor with ... > (__shared_ptr(_Sp_alloc_shared_tag<Alloc>, Args&&...)): Pass _M_ptr > to the __shared_count constructor. > (__allocate_shared): Change to use new tag type. > * src/c++11/shared_ptr.cc (_Sp_make_shared_tag::_S_eq): Define. > >Tested powerpc64le-linux, committed to trunk. > >I'll backport this to gcc-8-branch without the new symbol in the library. Here's the backport. It's the same, except that it doesn't add the new _Sp_make_shared_tag::_S_eq function, and so there are no changes to _Sp_counted_ptr_inplace::_M_get_deleter. This means that -fno-rtti instantiations of _M_get_deleter will still return null to -frtti callers. That can be fixed by recompiling the callers with GCC 8.3 or later (so they don't use _M_get_deleter at all), or just by relinking so that a -frtti instantiation of _M_get_deleter is kept by the linker. I'll document that as requested by Richi. Tested x86_64-linux, committed to gcc-8-branch as r268114. I've also tested the reproducers from 87520 and 88782 with every combination of GCC 8.2.0, 8.2.1 (after r266380 but before r268114), 8.2.1 (r268114) and 9.0.0 (r268086) and only the expected cases fail (i.e. when both objects use 8.2.0, because obviously the fix isn't present, or when _M_get_deleter is compiled by 8.x with -fno-rtti and a caller is compiled by 8.2.0 with -fno-rtti, as described above).
diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index 3b254b2289f..34c70b6cb8f 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -2235,6 +2235,9 @@ GLIBCXX_3.4.26 { _ZNSolsEDn; _ZNSt13basic_ostreamIwSt11char_traitsIwEElsEDn; + # _Sp_make_shared_tag::_S_eq + _ZNSt19_Sp_make_shared_tag5_S_eqERKSt9type_info; + } GLIBCXX_3.4.25; # Symbols in the support library (libsupc++) have their own tag. diff --git a/libstdc++-v3/include/bits/shared_ptr.h b/libstdc++-v3/include/bits/shared_ptr.h index d504627d1a0..ee815f0d0a1 100644 --- a/libstdc++-v3/include/bits/shared_ptr.h +++ b/libstdc++-v3/include/bits/shared_ptr.h @@ -355,9 +355,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION private: // This constructor is non-standard, it is used by allocate_shared. template<typename _Alloc, typename... _Args> - shared_ptr(_Sp_make_shared_tag __tag, const _Alloc& __a, - _Args&&... __args) - : __shared_ptr<_Tp>(__tag, __a, std::forward<_Args>(__args)...) + shared_ptr(_Sp_alloc_shared_tag<_Alloc> __tag, _Args&&... __args) + : __shared_ptr<_Tp>(__tag, std::forward<_Args>(__args)...) { } template<typename _Yp, typename _Alloc, typename... _Args> @@ -699,7 +698,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION inline shared_ptr<_Tp> allocate_shared(const _Alloc& __a, _Args&&... __args) { - return shared_ptr<_Tp>(_Sp_make_shared_tag(), __a, + return shared_ptr<_Tp>(_Sp_alloc_shared_tag<_Alloc>{__a}, std::forward<_Args>(__args)...); } diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h b/libstdc++-v3/include/bits/shared_ptr_base.h index b45cbf73667..0367c2d51a5 100644 --- a/libstdc++-v3/include/bits/shared_ptr_base.h +++ b/libstdc++-v3/include/bits/shared_ptr_base.h @@ -501,8 +501,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION struct _Sp_make_shared_tag { private: - template<typename _Tp, _Lock_policy _Lp> - friend class __shared_ptr; template<typename _Tp, typename _Alloc, _Lock_policy _Lp> friend class _Sp_counted_ptr_inplace; @@ -512,8 +510,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION alignas(type_info) static constexpr char __tag[sizeof(type_info)] = { }; return reinterpret_cast<const type_info&>(__tag); } + + static bool _S_eq(const type_info&) noexcept; }; + template<typename _Alloc> + struct _Sp_alloc_shared_tag + { + const _Alloc& _M_a; + }; + template<typename _Tp, typename _Alloc, _Lock_policy _Lp> class _Sp_counted_ptr_inplace final : public _Sp_counted_base<_Lp> { @@ -560,24 +566,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION this->~_Sp_counted_ptr_inplace(); } - // Sneaky trick so __shared_ptr can get the managed pointer. + private: + friend class __shared_count<_Lp>; // To be able to call _M_ptr(). + + // No longer used, but code compiled against old libstdc++ headers + // might still call it from __shared_ptr ctor to get the pointer out. virtual void* _M_get_deleter(const std::type_info& __ti) noexcept override { + auto __ptr = const_cast<typename remove_cv<_Tp>::type*>(_M_ptr()); // Check for the fake type_info first, so we don't try to access it - // as a real type_info object. - if (&__ti == &_Sp_make_shared_tag::_S_ti()) - return const_cast<typename remove_cv<_Tp>::type*>(_M_ptr()); + // as a real type_info object. Otherwise, check if it's the real + // type_info for this class. With RTTI enabled we can check directly, + // or call a library function to do it. + if (&__ti == &_Sp_make_shared_tag::_S_ti() + || #if __cpp_rtti - // Callers compiled with old libstdc++ headers and RTTI enabled - // might pass this instead: - else if (__ti == typeid(_Sp_make_shared_tag)) - return const_cast<typename remove_cv<_Tp>::type*>(_M_ptr()); + __ti == typeid(_Sp_make_shared_tag) +#else + _Sp_make_shared_tag::_S_eq(__ti) #endif + ) + return __ptr; return nullptr; } - private: _Tp* _M_ptr() noexcept { return _M_impl._M_storage._M_ptr(); } _Impl _M_impl; @@ -593,6 +606,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<_Lock_policy _Lp> class __shared_count { + template<typename _Tp> + struct __not_alloc_shared_tag { using type = void; }; + + template<typename _Tp> + struct __not_alloc_shared_tag<_Sp_alloc_shared_tag<_Tp>> { }; + public: constexpr __shared_count() noexcept : _M_pi(0) { } @@ -622,12 +641,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : __shared_count(__p, __sp_array_delete{}, allocator<void>()) { } - template<typename _Ptr, typename _Deleter> + template<typename _Ptr, typename _Deleter, + typename = typename __not_alloc_shared_tag<_Deleter>::type> __shared_count(_Ptr __p, _Deleter __d) : __shared_count(__p, std::move(__d), allocator<void>()) { } - template<typename _Ptr, typename _Deleter, typename _Alloc> + template<typename _Ptr, typename _Deleter, typename _Alloc, + typename = typename __not_alloc_shared_tag<_Deleter>::type> __shared_count(_Ptr __p, _Deleter __d, _Alloc __a) : _M_pi(0) { typedef _Sp_counted_deleter<_Ptr, _Deleter, _Alloc, _Lp> _Sp_cd_type; @@ -648,17 +669,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } template<typename _Tp, typename _Alloc, typename... _Args> - __shared_count(_Sp_make_shared_tag, _Tp*, const _Alloc& __a, + __shared_count(_Tp*& __p, _Sp_alloc_shared_tag<_Alloc> __a, _Args&&... __args) - : _M_pi(0) { typedef _Sp_counted_ptr_inplace<_Tp, _Alloc, _Lp> _Sp_cp_type; - typename _Sp_cp_type::__allocator_type __a2(__a); + typename _Sp_cp_type::__allocator_type __a2(__a._M_a); auto __guard = std::__allocate_guarded(__a2); _Sp_cp_type* __mem = __guard.get(); - ::new (__mem) _Sp_cp_type(__a, std::forward<_Args>(__args)...); - _M_pi = __mem; + auto __pi = ::new (__mem) + _Sp_cp_type(__a._M_a, std::forward<_Args>(__args)...); __guard = nullptr; + _M_pi = __pi; + __p = __pi->_M_ptr(); } #if _GLIBCXX_USE_DEPRECATED @@ -1318,17 +1340,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION protected: // This constructor is non-standard, it is used by allocate_shared. template<typename _Alloc, typename... _Args> - __shared_ptr(_Sp_make_shared_tag __tag, const _Alloc& __a, - _Args&&... __args) - : _M_ptr(), _M_refcount(__tag, (_Tp*)0, __a, - std::forward<_Args>(__args)...) - { - // _M_ptr needs to point to the newly constructed object. - // This relies on _Sp_counted_ptr_inplace::_M_get_deleter. - void* __p = _M_refcount._M_get_deleter(_Sp_make_shared_tag::_S_ti()); - _M_ptr = static_cast<_Tp*>(__p); - _M_enable_shared_from_this_with(_M_ptr); - } + __shared_ptr(_Sp_alloc_shared_tag<_Alloc> __tag, _Args&&... __args) + : _M_ptr(), _M_refcount(_M_ptr, __tag, std::forward<_Args>(__args)...) + { _M_enable_shared_from_this_with(_M_ptr); } template<typename _Tp1, _Lock_policy _Lp1, typename _Alloc, typename... _Args> @@ -1808,7 +1822,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION inline __shared_ptr<_Tp, _Lp> __allocate_shared(const _Alloc& __a, _Args&&... __args) { - return __shared_ptr<_Tp, _Lp>(_Sp_make_shared_tag(), __a, + return __shared_ptr<_Tp, _Lp>(_Sp_alloc_shared_tag<_Alloc>{__a}, std::forward<_Args>(__args)...); } diff --git a/libstdc++-v3/src/c++11/shared_ptr.cc b/libstdc++-v3/src/c++11/shared_ptr.cc index 6e838de3c03..1f1323ef89f 100644 --- a/libstdc++-v3/src/c++11/shared_ptr.cc +++ b/libstdc++-v3/src/c++11/shared_ptr.cc @@ -94,5 +94,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } #endif + bool + _Sp_make_shared_tag::_S_eq(const type_info& ti) noexcept + { +#if __cpp_rtti + return ti == typeid(_Sp_make_shared_tag); +#else + // If libstdc++ itself is built with -fno-rtti then just assume that + // make_shared and allocate_shared will never be used with -frtti. + return false; +#endif + } + _GLIBCXX_END_NAMESPACE_VERSION } // namespace
The old version of _Sp_counted_ptr_inplace::_M_get_deleter (up to GCC 8.2.0) expects to be passed a real std::typeinfo object, so mixing that with the new definition of the __shared_ptr constructor (which always passes the fake tag) leads to accessing the fake object as a real std::typeinfo. Instead of trying to make it safe to mix the old and new definitions, just stop using that function. By passing a reference to __shared_ptr::_M_ptr to the __shared_count constructor it can be set directly, without needing to obtain the pointer via the _M_get_deleter back-channel. This avoids a virtual dispatch (which fixes PR 87514). This means that code built against new libstdc++ headers doesn't use _M_get_deleter at all, and so make_shared works the same whether RTTI is enabled or not. Also change _M_get_deleter so that it checks for a real type_info object even when RTTI is disabled, by calling a library function. Unless libstdc++ itself is built without RTTI that library function will be able to test if it's the right type_info. This means the new definition of _M_get_deleter can handle both the fake type_info tag and a real type_info object, even if built without RTTI. If linking to objects built against older versions of libstdc++ then if all objects use -frtti or all use -fno-rtti, then the caller of _M_get_deleter and the definition of _M_get_deleter will be consistent and it will work. If mixing -frtti with -fno-rtti it can still fail if the linker picks an old definition of _M_get_deleter and an old __shared_ptr constructor that are incompatible. In that some or all objects might need to be recompiled. PR libstdc++/87514 PR libstdc++/87520 PR libstdc++/88782 * config/abi/pre/gnu.ver (GLIBCXX_3.4.26): Export new symbol. * include/bits/shared_ptr.h (shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)) (allocate_shared): Change to use new tag type. * include/bits/shared_ptr_base.h (_Sp_make_shared_tag::_S_eq): Declare new member function. (_Sp_alloc_shared_tag): Define new type. (_Sp_counted_ptr_inplace): Declare __shared_count<_Lp> as a friend. (_Sp_counted_ptr_inplace::_M_get_deleter) [!__cpp_rtti]: Use _Sp_make_shared_tag::_S_eq to check type_info. (__shared_count(Ptr, Deleter),__shared_count(Ptr, Deleter, Alloc)): Constrain to prevent being called with _Sp_alloc_shared_tag. (__shared_count(_Sp_make_shared_tag, const _Alloc&, Args&&...)): Replace constructor with ... (__shared_count(Tp*&, _Sp_alloc_shared_tag<_Alloc>, Args&&...)): Use reference parameter so address of the new object can be returned to the caller. Obtain the allocator from the tag type. (__shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)): Replace constructor with ... (__shared_ptr(_Sp_alloc_shared_tag<Alloc>, Args&&...)): Pass _M_ptr to the __shared_count constructor. (__allocate_shared): Change to use new tag type. * src/c++11/shared_ptr.cc (_Sp_make_shared_tag::_S_eq): Define. Tested powerpc64le-linux, committed to trunk. I'll backport this to gcc-8-branch without the new symbol in the library. commit f4334034ec92d0a6cdf6dc3244a25108f32fc89a Author: Jonathan Wakely <jwakely@redhat.com> Date: Thu Jan 17 20:46:09 2019 +0000 PR libstdc++/88782 avoid ODR problems in std::make_shared The old version of _Sp_counted_ptr_inplace::_M_get_deleter (up to GCC 8.2.0) expects to be passed a real std::typeinfo object, so mixing that with the new definition of the __shared_ptr constructor (which always passes the fake tag) leads to accessing the fake object as a real std::typeinfo. Instead of trying to make it safe to mix the old and new definitions, just stop using that function. By passing a reference to __shared_ptr::_M_ptr to the __shared_count constructor it can be set directly, without needing to obtain the pointer via the _M_get_deleter back-channel. This avoids a virtual dispatch (which fixes PR 87514). This means that code built against new libstdc++ headers doesn't use _M_get_deleter at all, and so make_shared works the same whether RTTI is enabled or not. Also change _M_get_deleter so that it checks for a real type_info object even when RTTI is disabled, by calling a library function. Unless libstdc++ itself is built without RTTI that library function will be able to test if it's the right type_info. This means the new definition of _M_get_deleter can handle both the fake type_info tag and a real type_info object, even if built without RTTI. If linking to objects built against older versions of libstdc++ then if all objects use -frtti or all use -fno-rtti, then the caller of _M_get_deleter and the definition of _M_get_deleter will be consistent and it will work. If mixing -frtti with -fno-rtti it can still fail if the linker picks an old definition of _M_get_deleter and an old __shared_ptr constructor that are incompatible. In that some or all objects might need to be recompiled. PR libstdc++/87514 PR libstdc++/87520 PR libstdc++/88782 * config/abi/pre/gnu.ver (GLIBCXX_3.4.26): Export new symbol. * include/bits/shared_ptr.h (shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)) (allocate_shared): Change to use new tag type. * include/bits/shared_ptr_base.h (_Sp_make_shared_tag::_S_eq): Declare new member function. (_Sp_alloc_shared_tag): Define new type. (_Sp_counted_ptr_inplace): Declare __shared_count<_Lp> as a friend. (_Sp_counted_ptr_inplace::_M_get_deleter) [!__cpp_rtti]: Use _Sp_make_shared_tag::_S_eq to check type_info. (__shared_count(Ptr, Deleter),__shared_count(Ptr, Deleter, Alloc)): Constrain to prevent being called with _Sp_alloc_shared_tag. (__shared_count(_Sp_make_shared_tag, const _Alloc&, Args&&...)): Replace constructor with ... (__shared_count(Tp*&, _Sp_alloc_shared_tag<_Alloc>, Args&&...)): Use reference parameter so address of the new object can be returned to the caller. Obtain the allocator from the tag type. (__shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)): Replace constructor with ... (__shared_ptr(_Sp_alloc_shared_tag<Alloc>, Args&&...)): Pass _M_ptr to the __shared_count constructor. (__allocate_shared): Change to use new tag type. * src/c++11/shared_ptr.cc (_Sp_make_shared_tag::_S_eq): Define.