On Wed, 28 Nov 2018 at 00:25, Jonathan Wakely <jwak...@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. > > 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. >
Hi Jonathan, Your patch passed my tests, thanks! directory_iterator.cc tests used to be killed by a timeout until last night where now they do PASS in my configurations involving -march=armv5t. That being said, I'm not sure I fully understand the fix. In my tests involving -march=armv5t, I still configure GCC --with-cpu=cortex-a9, and I pass -march=armv5t as an override with the --target-board runtest option. In these cases libstdc++ configure detects support for "atomic", so I suspect the tests pass only because I use QEMU with --cpu cortex-a9. I think if I used a different QEMU cpu (eg arm926), the tests would try to use atomics, and fail? The reason I'm still using cortex-a9 at QEMU level is that many tests override -mcpu/-march, and I used that as a compromise. However, I do have configurations using --with-cpu=arm10tdmi or --with-cpu=default and QEMU --cpu arm926. In these 2 cases, the tests do PASS, but they used to before your patch. libstdc++ configure does detect "mutex" in these cases. To be hopefully clearer, the subset of relevant configurations I use is as follows: GCC target / with-cpu / RUNTEST target-board / QEMU cpu 1- arm-none-linux-gnueabi[hf] / cortex-a9 / -march=armv5t / cortex-a9 2- arm-none-linux-gnueabihf / arm10tdmi / "" / arm926 3- arm-none-eabi / default / "" / arm926 (1) uses atomics (2) and (3) use mutex All of them now say "PASS", but maybe I should give a try switch to arm926 for QEMU in (1) ? Thanks, Christophe
