aboutsummaryrefslogtreecommitdiff
path: root/depends
diff options
context:
space:
mode:
authorWladimir J. van der Laan <laanwj@protonmail.com>2020-09-15 12:51:09 +0200
committerWladimir J. van der Laan <laanwj@protonmail.com>2020-09-15 13:03:02 +0200
commit48a9968e508581e726feefe34e3bc0bf4a0b61ed (patch)
tree3f8047a505c54d17ac1bbdf53e621c2d2f6351ea /depends
parenta33651866c863328e8aca870465cc1c99a4240d6 (diff)
parentfc9278d162c342bace8a147da6bc4f9941d8d9d7 (diff)
downloadbitcoin-48a9968e508581e726feefe34e3bc0bf4a0b61ed.tar.xz
Merge #19558: build: split pthread flags out of ldflags and dont use when building libconsensus
fc9278d162c342bace8a147da6bc4f9941d8d9d7 build: AX_PTHREAD serial 27 (fanquake) 15c27c44417ab77a660b53b8574f7eb5261b19f8 build: split PTHREAD_* flags out of AM_LDFLAGS (fanquake) 68e3e2294483cfee6bba8b5481eaee293e981ae8 scripted-diff: add FUZZ_SUITE_LDFLAGS_COMMON (fanquake) afecde8046b5f13253f1a7d687b4a90841b5766c build: add PTHREAD_LIBS to LDFLAGS configure output (fanquake) Pull request description: TLDR: Split pthread flags out of ldflags, and stop using them when building libconsensus. Building libconsensus on Linux using Clang currently warns. i.e: ```bash ./autogen.sh ./configure --disable-tests --disable-bench --with-utils=no --with-daemon=no --with-gui=no --disable-wallet --with-libs=yes CC=clang CXX=clang++ make V=1 -j6 ... -Wl,-z -Wl,relro -Wl,-z -Wl,now -pthread -Wl,-soname -Wl,libbitcoinconsensus.so.0 -o .libs/libbitcoinconsensus.so.0.0.0 clang: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument] clang: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument] ``` Besides wanting to quiet the warnings, after digging into this it seemed we could clean up how we are passing around the pthread flags. I also learnt a bit more about how libtools builds shared libraries, and that passing `-pthread` on the link line wouldn't be enough to link against pthreads anyways, due to libtools usage of -nostdlib (see [related discussion where we build DLLs](https://github.com/bitcoin/bitcoin/blob/476436b2dec254bb988f8c7a6cbec1d7bb7cecfd/configure.ac#L603)). This can be demonstrated with a patch to libconsensus: ```patch diff --git a/src/script/bitcoinconsensus.cpp b/src/script/bitcoinconsensus.cpp index 15e204062..10bf3582f 100644 --- a/src/script/bitcoinconsensus.cpp +++ b/src/script/bitcoinconsensus.cpp @@ -10,6 +10,8 @@ #include <script/interpreter.h> #include <version.h> +#include <pthread.h> + namespace { /** A class that deserializes a single CTransaction one time. */ @@ -127,3 +129,10 @@ unsigned int bitcoinconsensus_version() // Just use the API version for now return BITCOINCONSENSUS_API_VER; } + +void *func_pthread(void *x) { return x; } + +void f() { + pthread_t t; + pthread_create(&t,0,func_pthread,0); +} ``` After building, you'll find you have a `libbitcoinconsensus.so` using pthread symbols, but which isn't linked against libpthread: ```bash ldd -r src/.libs/libbitcoinconsensus.so linux-vdso.so.1 (0x00007ffe49378000) libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f553cee7000) libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f553cda2000) libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f553cd88000) libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f553cbc5000) /lib64/ld-linux-x86-64.so.2 (0x00007f553d15d000) undefined symbol: pthread_create (src/.libs/libbitcoinconsensus.so) ``` This libtool behaviour has been known about for some time, i.e this [thread from 2005](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25460), describes the same issue. The suggestion from libtool maintainers at the time is to add `-lpthread` to LDFLAGS. Also worth noting is that some of the users in those threads were also using the `AX_PTHREADS` macro, same as us, to determine how to compile with/link against pthreads. This macro has [recently been updated](https://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=commitdiff;h=2fb904589643eb6ca6122f834891b58d1d51b347), with reference to this issue. You can compare the output from the version we currently use, to the new version: ```bash # our ax_pthread macro: PTHREAD_CFLAGS = -pthread PTHREAD_LIBS = PTHREAD_CC = gcc / clang # the new ax_pthread macro PTHREAD_CFLAGS = -pthread PTHREAD_LIBS = -lpthread PTHREAD_CC = gcc / clang ``` Note that as part of this PR I've also added `PTHREAD_LIBS` to the split out flags. Although we weren't using it anywhere previously (and wouldn't have seemed to matter for the most part, given it was likely empty for most builders), the macro assumes it's use. i.e: > NOTE: You are assumed to not only compile your program with these flags, > but also to link with them as well. For example, you might link with > $PTHREAD_CC $CFLAGS $PTHREAD_CFLAGS $LDFLAGS ... $PTHREAD_LIBS $LIBS ACKs for top commit: laanwj: Code review ACK fc9278d162c342bace8a147da6bc4f9941d8d9d7 hebasto: re-ACK fc9278d162c342bace8a147da6bc4f9941d8d9d7, only rebased and renamed s/`AM_PTHREAD_FLAGS`/`PTHREAD_FLAGS`/ since my [previous](https://github.com/bitcoin/bitcoin/pull/19558#pullrequestreview-473487730) review.. kallewoof: ACK fc9278d162c342bace8a147da6bc4f9941d8d9d7 Tree-SHA512: 7c0a5b0f0de4f54b1d7dce0e69020b341c37a383bb7c715867cc96c648774a557b1ddb42eb1b676f7bb2b822b69795bec14dc6272362d80662a21f10cb80331c
Diffstat (limited to 'depends')
0 files changed, 0 insertions, 0 deletions