diff options
author | Wladimir J. van der Laan <laanwj@protonmail.com> | 2020-09-15 12:51:09 +0200 |
---|---|---|
committer | Wladimir J. van der Laan <laanwj@protonmail.com> | 2020-09-15 13:03:02 +0200 |
commit | 48a9968e508581e726feefe34e3bc0bf4a0b61ed (patch) | |
tree | 3f8047a505c54d17ac1bbdf53e621c2d2f6351ea /src/txmempool.h | |
parent | a33651866c863328e8aca870465cc1c99a4240d6 (diff) | |
parent | fc9278d162c342bace8a147da6bc4f9941d8d9d7 (diff) |
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 'src/txmempool.h')
0 files changed, 0 insertions, 0 deletions