[PATCH] PR libstdc++/88782 avoid ODR problems in std::make_shared

[Jonathan Wakely]

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 ...