Add configure check for -latomic

[ajtowns]

Add a configure check to use libatomic for std::atomic operations if needed.

Per https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_concurrency.html "Some uses of std::atomic also require linking to libatomic." This has been observed to be necessary when building with gcc on Debian's mips and mipsel architectures.

[ajtowns]

Build logs that failed (without the patch):

https://buildd.debian.org/status/fetch.php?pkg=bitcoin&arch=mipsel&ver=0.13.0~rc3-0.1&stamp=1471929506
https://buildd.debian.org/status/fetch.php?pkg=bitcoin&arch=mips&ver=0.13.0~rc3-0.1&stamp=1471898920

Successful versions:

https://buildd.debian.org/status/fetch.php?pkg=bitcoin&arch=mipsel&ver=0.13.0-0.1~exp1&stamp=1471946370
https://buildd.debian.org/status/fetch.php?pkg=bitcoin&arch=mips&ver=0.13.0-0.1~exp1&stamp=1471942990

[sipa]

Concept ACK

[luke-jr]

Concept ACK, but I wonder if we should try to link with -latomic first, in case linking without it merely fails at runtime (or worse, works without providing the atomic guarantees).

[laanwj]

Hm, you have a point @luke-jr but I also like the approach of trying -latomic only when necessary. This will make sure that no spurious library is linked that happens to be called 'atomic' on completely unrelated platforms/compilers, and that the configure log clearly states why a certain library is linked.

without it merely fails at runtime (or worse, works without providing the atomic guarantees).

That would be really messed-up. Linking a library or not should not affect program behavior in itself (that's determined during compile time), and missing symbols give an error at link time.
(Maybe something could be concocted with ELF weak symbols that gives this kind of behavior, even making it dependent on the link order, but bleh)

[laanwj]

utACK 878faac

[luke-jr]

I agree, it would indeed be messed-up, but wasn't that almost exactly what happened in CVE-2012-1910?

[theuni]

Assuming we're only doing link tests, is an AC_SEARCH_LIBS not enough here?

[laanwj]

I agree, it would indeed be messed-up, but wasn't that almost exactly what happened in CVE-2012-1910?

IIRC there the problem was linking the wrong library, not forgetting to link a library.

[laanwj]

Assuming we're only doing link tests, is an AC_SEARCH_LIBS not enough here?

I think the problem is that it is unspecified which symbols are exported. The requirement is that the atomic library is linked when c++11 atomics are used. How these relate to ELF-level symbols is probably an implementation detail that may change over time.

[luke-jr]

main is always exported ;)

[laanwj]

I mean these symbols exported by atomic library:

libbitcoin_server.a(libbitcoin_server_a-main.o): In function `std::__atomic_base<long long>::store(long long, std::memory_order)':
/usr/include/c++/6/bits/atomic_base.h:374: undefined reference to `__atomic_store_8'
libbitcoin_server.a(libbitcoin_server_a-main.o): In function `std::__atomic_base<long long>::load(std::memory_order) const':
/usr/include/c++/6/bits/atomic_base.h:396: undefined reference to `__atomic_load_8'
/usr/include/c++/6/bits/atomic_base.h:396: undefined reference to `__atomic_load_8'
libbitcoin_server.a(libbitcoin_server_a-main.o): In function `std::__atomic_base<long long>::store(long
...

Would be easy enough to check for one of them using AC_SEARCH_LIBS, but I think the test in this pull is fine and less dependent on implementation details.

[laanwj]

ACK 878faac, reproduced with a mipsel-unknown-linux-gnu toolchain (gcc 5.3.0) built with crosstool-ng.
Without this patch it fails, with this patch it proceeds. Haven't tested the executable, but it's a clear step forward at least.

[luke-jr]

Ugh, this m4 code is GPL licensed at best... checking with upstream if we can get MIT license on it.

[ajtowns]

Looks like @luke-jr was successful; https://svn.filezilla-project.org/filezilla/FileZilla3/trunk/m4/check_atomic.m4?r1=7216&r2=7788

[laanwj]

This was backported in #8772, removing tag