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

Richard.

>         PR libstdc++/67843
>         * acinclude.m4 (GLIBCXX_ENABLE_LOCK_POLICY): Add new macro
>         that defines _GLIBCXX_HAVE_ATOMIC_LOCK_POLICY.
>         * config.h.in: Regenerate.
>         * configure: Regenerate.
>         * configure.ac: Use GLIBCXX_ENABLE_LOCK_POLICY.
>         * doc/xml/manual/configure.xml: Document new configure option.
>         * include/bits/fs_dir.h (directory_iterator): Use __shared_ptr
>         instead of shared_ptr.
>         (recursive_directory_iterator): Likewise.
>         (__shared_ptr<_Dir>): Add explicit instantiation declaration.
>         (__shared_ptr<recursive_directory_iterator::_Dir_stack>): Likewise.
>         * include/bits/shared_ptr_base.h (__allocate_shared, __make_shared):
>         Add default template argument for _Lock_policy template parameter.
>         * include/ext/concurrence.h (__default_lock_policy): Check macro
>         _GLIBCXX_HAVE_ATOMIC_LOCK_POLICY instead of checking if the current
>         target supports the builtins for compare-and-swap.
>         * src/filesystem/std-dir.cc (__shared_ptr<_Dir>): Add explicit
>         instantiation definition.
>         (__shared_ptr<recursive_directory_iterator::_Dir_stack>): Likewise.
>         (directory_iterator, recursive_directory_iterator): Use __make_shared
>         instead of make_shared.
>
> Tested x86_64-linux and aarch64-linux, committed to trunk.
>
> I would appreciate testing on ARM, especially Christophe's
> -march=armv5t set up.
>