Re: [PATCH] PR libstdc++/67843 set shared_ptr lock policy at build-time

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Nov 28, 2018 at 3:09 PM Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On 28/11/18 14:46 +0100, Richard Biener wrote:
> >On Wed, Nov 28, 2018 at 1:30 PM Jonathan Wakely <jwakely@redhat.com> wrote:
> >>
> >> On 28/11/18 11:11 +0000, Jonathan Wakely wrote:
> >> >On 28/11/18 11:46 +0100, Richard Biener wrote:
> >> >>On Wed, Nov 28, 2018 at 12:26 AM Jonathan Wakely <jwakely@redhat.com> wrote:
> >> >>>
> >> >>>This resolves a longstanding issue where the lock policy for shared_ptr
> >> >>>reference counting depends on compilation options when the header is
> >> >>>included, so that different -march options can cause ABI changes. For
> >> >>>example, objects compiled with -march=armv7 will use atomics to
> >> >>>synchronize reference counts, and objects compiled with -march=armv5t
> >> >>>will use a mutex. That means the shared_ptr control block will have a
> >> >>>different layout in different objects, causing ODR violations and
> >> >>>undefined behaviour. This was the root cause of PR libstdc++/42734 as
> >> >>>well as PR libstdc++/67843.
> >> >>>
> >> >>>The solution is to decide on the lock policy at build time, when
> >> >>>libstdc++ is configured. The configure script checks for the
> >> >>>availability of the necessary atomic built-ins for the target and fixes
> >> >>>that choice permanently. Different -march flags used to compile user
> >> >>>code will not cause changes to the lock policy. This results in an ABI
> >> >>>change for certain compilations, but only where there was already an ABI
> >> >>>incompatibility between the libstdc++.so library and objects built with
> >> >>>an incompatible -march option. In general, this means a more stable ABI
> >> >>>that isn't silently altered when -march flags make addition atomic ops
> >> >>>available.
> >> >>>
> >> >>>To force a target to use "atomic" or "mutex" the new configure option
> >> >>>--with-libstdcxx-lock-policy can be used.
> >> >>>
> >> >>>In order to turn ODR violations into linker errors, the uses of
> >> >>>shared_ptr in filesystem directory iterators have been replaced
> >> >>>with __shared_ptr, and explicit instantiations are declared. This
> >> >>>ensures that object files using those types cannot link to libstdc++
> >> >>>libs unless they use the same lock policy.
> >> >>
> >> >>Would it be possible to have both in libstdc++ and with differnet mangling of
> >> >>different kind ensure compatibility between different user CUs?  Or is
> >> >>that too awkward for the user to get right?
> >> >
> >> >It would mean duplicating a lot more code, which is already duplicated
> >> >once for the std::string ABI, so we'd have four permuations!
> >> >
> >> >It still wouldn't ensure compatibility between different user CUs,
> >> >only between any user CU and any build of libstdc++. Different user
> >> >CUs would still disagree on the ABI of the types, and so couldn't pass
> >> >them between CUs. I see no advantage to supporting that for the
> >> >std::filesystem library (unlike std::string and std::iostream, which
> >> >are probably used in the majority of CUs).
> >> >
> >> >I do not want to get to the point where every type in libstdc++ exists
> >> >multiple times and you select some combination via command-line flags.
> >> >It's already becoming unmanageable with multiple std::string and long
> >> >double ABIs.
> >>
> >> Also, in practice, I don't see a need. The common cases where this bug
> >> arises are limited to 32-bit ARM, and for arm-linux the kernel helpers
> >> mean that you can always use "atomic". For bare metal ARM toolchains
> >> you probably don't want to be mixing CUs built against different
> >> versions of libstdc++ anyway. You have your toolchain for the board,
> >> and you use that. If it is configured to use "atomic", then that's
> >> what you get. If it's configured to use "mutex", then that's what you
> >> get. You probably don't want to use a toolchain configured for a
> >> different board, but you could use --with-libstdcxx-lock-policy to
> >> ensure a consistent policy across those toolchains if needed.
> >>
> >> It's also possible for problems to arise on i386. If you build GCC for
> >> i386 then it will choose "mutex" and be incompatible with a libstdc++
> >> built for i486 or later. In that case, you could use the option
> >> --with-libstdcxx-lock-policy=atomic to force the use of "atomic" (and
> >> then you'd need to link to libatomic to provide the ops, as there are
> >> no kernel helpers for i386 in libgcc).
> >
> >I think the default should be part of the platform ABI somehow,
> >like the i386 incompatibility should better not arise.
>
> What do you suggest to ensure that?
>
> Just imake i386 default to the "atomic" lock policy, and require anybody
> silly enough to configure for i386 to either link to libatomic, or
> explicitly use --with-libstdcxx-lock-policy=mutex if they really want
> to be incompatible with default i486 builds?

Something like that, yes.

> >I suppose
> >libstdc++ configury doesn't read in gcc/config.gcc but does it
> >have sth similar where it's easy to see defaults for plaforms?
> >There's configure.host but I'm not sure this is related at all.
>
> No, I don't think we do. When there are target-specific things
> hardcoded it's spread out around acinclude.m4 and configure.ac
>
>