Commit graph

468 commits

Author SHA1 Message Date
Pieter Wuille ca8bc42330 Drop --disable-jni from libsecp256k1 configure options 2020-06-10 18:15:38 -07:00
MarcoFalke fa16e7816b
build: Add -Wshadow-field 2020-06-06 08:12:37 -04:00
Vasil Dimov 0012471391
build: turn on --enable-c++17 by --enable-fuzz
Fuzzing code uses C++17 specific code (e.g. std::optional), so it is not
possible to compile with --enable-fuzz and without --enable-c++17.

Thus, turn on --enable-c++17 whenever --enable-fuzz is used.
2020-06-05 11:50:34 +02:00
Wladimir J. van der Laan b46fb5cb10
Merge #19131: refactor: Fix unreachable code in init arg checks
eea8114657 build: Enable unreachable-code-loop-increment (Jonathan Schoeller)
d15db4b1fc refactor: Fix unreachable code in init arg checks (Jonathan Schoeller)

Pull request description:

  Closes: #19017

  In #19015 it's been suggested that we add some new compiler warnings to our build. Some of these, such as `-Wunreachable-code-loop-increment`, generate warnings. We'll likely want to fix these up if we're going to turn these warnings on.

  ```shell
  init.cpp:969:5: warning: loop will run at most once (loop increment never executed) [-Wunreachable-code-loop-increment]
       for (const auto& arg : gArgs.GetUnsuitableSectionOnlyArgs()) {
       ^~~
   1 warning generated.
   ```
   aa8d76806c/src/init.cpp (L968-L972)

  To fix this, collect all errors, and output them in a single error message after the loop completes. This resolves the unreachable code warning, and avoids popup hell that could result from outputting a seperate message for each error or warning one by one.

ACKs for top commit:
  laanwj:
    Code review ACK eea8114657
  hebasto:
    re-ACK eea8114657, only suggested changes applied since the [previous](https://github.com/bitcoin/bitcoin/pull/19131#pullrequestreview-421772387) review.

Tree-SHA512: 2aa3ceb7fab581b6ba2580900668388d8eba1c3001c8ff9c11c1f4a9a10fbc37f30e590249862676858446e3f4950140a252953ba1643ba3bfd772f8eae20583
2020-06-04 16:27:53 +02:00
sachinkm77 0fef60c63d build: improved output of configure for build OS 2020-06-03 04:06:36 -04:00
Jonathan Schoeller eea8114657 build: Enable unreachable-code-loop-increment
Enable unreachable-code-loop-increment and treat as error.
refs: #19015
2020-06-02 06:24:10 +10:00
fanquake a8327fd71f
Merge #19072: doc: Expand section on Getting Started
facef3d413 doc: Explain that anyone can work on good first issues, move text to CONTRIBUTING.md (MarcoFalke)
fae2fb2a19 doc: Expand section on Getting Started (MarcoFalke)
100000d1b2 doc: Add headings to CONTRIBUTING.md (MarcoFalke)
fab893e0ca doc: Fix unrelated typos reported by codespell (MarcoFalke)

Pull request description:

  Some random doc changes:

  * Add sections to docs, so that they can be linked to
  * Explain that anyone (even maintainers) are allowed to work on good first issues
  * Expand section on Getting Started slightly

ACKs for top commit:
  hebasto:
    ACK facef3d413
  fanquake:
    ACK facef3d413

Tree-SHA512: 8998e273a76dbf4ca77e79374c14efe4dfcc5c6df6b7d801e1e1e436711dbe6f76b436f9cbc6cacb45a56827babdd6396f3bd376a9426ee7be3bb9b8a3b8e383
2020-06-01 15:38:57 +08:00
Wladimir J. van der Laan 399d84da37 build: Only allow ASCII identifiers
While emoji and other symbols in C++ identifers (as accepted by newer
compilers) are fun, they might create confusion during code review, for
example because some symbols look very similar. Forbid such extended
identifiers for now.

This is done by providing `-fno-extended-identifiers`. Thanks to sipa
for suggesting this compiler flag.
2020-05-28 19:35:42 +02:00
Hennadii Stepanov 87766b355c
build: Replace -Wthread-safety-analysis with broader -Wthread-safety 2020-05-28 09:56:44 +03:00
MarcoFalke fab893e0ca
doc: Fix unrelated typos reported by codespell 2020-05-27 12:37:08 -04:00
fanquake 97b21b302a
Merge #18677: Multiprocess build support
e2bab2aa16 multiprocess: add multiprocess travis configuration (Russell Yanofsky)
603fd6a2e7 depends: add MULTIPROCESS depends option (Russell Yanofsky)
5d1377b52b build: multiprocess autotools changes (Russell Yanofsky)

Pull request description:

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).

  ---

  This PR consists of build changes only. It adds an `--enable-multiprocess` autoconf option (off by default and marked experimental), that builds new `bitcoin-node` and `bitcoin-gui` binaries. These currently function the same as existing `bitcoind` and `bitcoin-qt` binaries, but are extended in #10102 with IPC features to execute node, wallet, and gui functions in separate processes.

  In addition to adding the `--enable-multiprocess` config flag, it also adds a depends package and autoconf rules to build with the [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) library, and it adds new travis configuration to exercise the build code and run functional tests with the new binaries.

  The changes in this PR were originally part of #10102 but were moved into #16367 to be able to develop and review the multiprocess build changes independently of the code changes. #16367 was briefly merged and then reverted in #18588. Only change since #16367 has been dropping the `native_boost.mk` depends package which was pointed out to be no longer necessary in https://github.com/bitcoin/bitcoin/pull/16367#issuecomment-596484337 and https://github.com/bitcoin/bitcoin/pull/18588#pullrequestreview-391765649

ACKs for top commit:
  practicalswift:
    ACK e2bab2aa16
  Sjors:
    tACK e2bab2aa16 on macOS 10.15.4
  hebasto:
    ACK e2bab2aa16, tested on Linux Mint 19.3 (x86_64):

Tree-SHA512: b5a76eab5abf63d9d8b6d628cbdff4cc1888eef15cafa0a5d56369e2f9d02595fed623f4b74b2cf2830c42c05a774f0943e700f9c768a82d9d348cad199e135c
2020-05-21 15:34:25 +08:00
fanquake e8a8cff07c
build: enforce minimum required Windows version (7)
Instruct the linker to set the major & minor subsystem versions in the PE
header to 6 & 1 (NT 6.1 which corresponds to Windows 7). Similar to
macOS, the binary will now refuse to run on unsupported versions of
Windows.
2020-05-14 09:46:18 +08:00
Wladimir J. van der Laan 04c09553d8
Merge #18887: build: enable -Werror=gnu
a30b0a24e9 build: enable -Werror=gnu (Vasil Dimov)

Pull request description:

  Stop the build if a warning is emitted due to `-Wgnu` and
  `--enable-werror` has been used. As usual - this would help notice such
  a warning that is about to be introduced in new code.

  This is a followup to
  https://github.com/bitcoin/bitcoin/pull/18088 build: ensure we aren't using GNU extensions

ACKs for top commit:
  practicalswift:
    ACK a30b0a24e9
  Empact:
    ACK a30b0a24e9

Tree-SHA512: f81b71cf3ee4db88b6f664c571075e0d30800a604f067f44273f256695a1dea533779db2ac859dd0a4cd8b66289c3e45f4aff1cfadfa160a1c354237167b05e2
2020-05-13 22:20:13 +02:00
Wladimir J. van der Laan 5d18c0ae18
Merge #18862: Remove fdelt_chk back-compat code and sanity check
df6bde031b test: remove glibc fdelt sanity check (fanquake)
8bf1540cc2 build: remove fdelt_chk backwards compatibility code (fanquake)

Pull request description:

  ae30d40e50
  The return type of [`fdelt_chk`](https://sourceware.org/git/?p=glibc.git;a=blob;f=debug/fdelt_chk.c;h=f62ce7349707cb68f55831c1c591fd7387a90258;hb=HEAD) changed from `unsigned  long int` to `long int` in glibc 2.16. See [this commit](https://sourceware.org/git/?p=glibc.git;a=commit;h=ceb9e56b3d1f8c1922e0526c2e841373843460e2). Now that we require [glibc >=2.17](https://github.com/bitcoin/bitcoin/pull/17538) we can remove our back-compat code.

  ab7bce584a
  While looking at the above changes, I noticed that our glibc fdelt sanity check doesn't seem to be checking anything. `fdelt_warn()` also isn't something we'd want to actually "trigger" at runtime, as doing so would cause `bitcoind` to abort.

  The comments:
  > // trigger: Call FD_SET to trigger __fdelt_chk. FORTIFY_SOURCE must be defined
  > //   as >0 and optimizations must be set to at least -O2.

  suggest calling FD_SET to check the invocation of `fdelt_chk` (this is [aliased with fdelt_warn in glibc](https://sourceware.org/git/?p=glibc.git;a=blob;f=debug/fdelt_chk.c;h=f62ce7349707cb68f55831c1c591fd7387a90258;hb=HEAD)). However just calling `FD_SET()` will not necessarily cause the compiler to insert a call to `fd_warn()`.

  Whether or not GCC (recent Clang should work, but may use different heuristics) inserts a call to `fdelt_warn()` depends on if the compiler can determine if the value passed in is a compile time constant (using [`__builtin_constant_p`](https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html)) and whether the value is < 0 or >= `FD_SETSIZE`. The glibc implementation is [here](https://sourceware.org/git/?p=glibc.git;a=blob;f=misc/bits/select2.h;h=7e17430ed94dd1679af10afa3d74795f9c97c0e8;hb=HEAD). This means our check should never cause a call to be inserted.

  Compiling master without `--glibc-back-compat` (if you do pass `--glibc-back-compat` the outcome is still the same; however the abort will only happen with >=`FD_SETSIZE` as that is what our [fdelt_warn()](https://github.com/bitcoin/bitcoin/blob/master/src/compat/glibc_compat.cpp#L24) checks for), there are no calls to `fdelt_warn()` inserted by the compiler:
  ```bash
  objdump -dC bitcoind | grep sanity_fdelt
  ...
  0000000000399d20 <sanity_test_fdelt()>:
    399d20:       48 81 ec 98 00 00 00    sub    $0x98,%rsp
    399d27:       b9 10 00 00 00          mov    $0x10,%ecx
    399d2c:       64 48 8b 04 25 28 00    mov    %fs:0x28,%rax
    399d33:       00 00
    399d35:       48 89 84 24 88 00 00    mov    %rax,0x88(%rsp)
    399d3c:       00
    399d3d:       31 c0                   xor    %eax,%eax
    399d3f:       48 89 e7                mov    %rsp,%rdi
    399d42:       fc                      cld
    399d43:       f3 48 ab                rep stos %rax,%es:(%rdi)
    399d46:       48 8b 84 24 88 00 00    mov    0x88(%rsp),%rax
    399d4d:       00
    399d4e:       64 48 33 04 25 28 00    xor    %fs:0x28,%rax
    399d55:       00 00
    399d57:       75 0d                   jne    399d66 <sanity_test_fdelt()+0x46>
    399d59:       b8 01 00 00 00          mov    $0x1,%eax
    399d5e:       48 81 c4 98 00 00 00    add    $0x98,%rsp
    399d65:       c3                      retq
    399d66:       e8 85 df c8 ff          callq  27cf0 <__stack_chk_fail@plt>
    399d6b:       0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)

  ```

  If you modify the sanity test to pass `-1` or `FD_SETSIZE` to `FD_SET`, you'll see calls to `fdelt_warn` inserted, and the runtime behaviour is an abort as expected.

  ```diff
  diff --git a/src/compat/glibc_sanity_fdelt.cpp b/src/compat/glibc_sanity_fdelt.cpp
  index 87140d0c7..16974bfa0 100644
  --- a/src/compat/glibc_sanity_fdelt.cpp
  +++ b/src/compat/glibc_sanity_fdelt.cpp
  @@ -20,7 +20,7 @@ bool sanity_test_fdelt()
   {
       fd_set fds;
       FD_ZERO(&fds);
  -    FD_SET(0, &fds);
  +    FD_SET(FD_SETSIZE, &fds);
       return FD_ISSET(0, &fds);
   }
   #endif
  ```

  ```bash
  0000000000399d20 <sanity_test_fdelt()>:
    399d20:	48 81 ec 98 00 00 00 	sub    $0x98,%rsp
    399d27:	b9 10 00 00 00       	mov    $0x10,%ecx
    399d2c:	64 48 8b 04 25 28 00 	mov    %fs:0x28,%rax
    399d33:	00 00
    399d35:	48 89 84 24 88 00 00 	mov    %rax,0x88(%rsp)
    399d3c:	00
    399d3d:	31 c0                	xor    %eax,%eax
    399d3f:	48 89 e7             	mov    %rsp,%rdi
    399d42:	fc                   	cld
    399d43:	f3 48 ab             	rep stos %rax,%es:(%rdi)
    399d46:	48 c7 c7 ff ff ff ff 	mov    $0xffffffffffffffff,%rdi
    399d4d:	e8 3e ff ff ff       	callq  399c90 <__fdelt_warn>
    399d52:	0f b6 04 24          	movzbl (%rsp),%eax
    399d56:	83 e0 01             	and    $0x1,%eax
    399d59:	48 8b 94 24 88 00 00 	mov    0x88(%rsp),%rdx
    399d60:	00
    399d61:	64 48 33 14 25 28 00 	xor    %fs:0x28,%rdx
    399d68:	00 00
    399d6a:	75 08                	jne    399d74 <sanity_test_fdelt()+0x54>
    399d6c:	48 81 c4 98 00 00 00 	add    $0x98,%rsp
    399d73:	c3                   	retq
    399d74:	e8 77 df c8 ff       	callq  27cf0 <__stack_chk_fail@plt>
    399d79:	0f 1f 80 00 00 00 00 	nopl   0x0(%rax)
   ```

   ```bash
   src/bitcoind
  *** buffer overflow detected ***: src/bitcoind terminated
  Aborted
   ```

  I think the test should should be removed and replaced (if possible) with additional checks in security-check.py. I was thinking about adding a version of [this script](https://github.com/fanquake/core-review/blob/master/fortify.py) as part of the output, but that needs more thought. I'll address this in a follow up.

ACKs for top commit:
  laanwj:
    ACK  df6bde031b

Tree-SHA512: d8b3af4f4eb2d6c767ca6e72ece51d0ab9042e1bbdfcbbdb7ad713414df21489ba3217662b531b8bfdac0265d2ce5431abfae6e861b6187d182ff26c6e59b32d
2020-05-13 19:35:25 +02:00
fanquake 6c647c89db
Merge #18738: build: Suppress -Wdeprecated-copy warnings
0c63f80854 build: Suppress -Wdeprecated-copy warnings (Hennadii Stepanov)

Pull request description:

  Tomorrow, on Apr 23 the Ubuntu 20.04 release is expected. It packaged with Qt 5.12 LTS that has a nasty peculiarity to cause modern compilers, including Clang 10.0 and GCC 9.3, to emit spammy `-Wdeprecated-copy` warnings (#15822, #18419).

  This PR suppress such warnings _temporarily_, until the [upstream is fixed](https://codereview.qt-project.org/c/qt/qtbase/+/272258).

  Here are some affected systems (with system packages):
  - Ubuntu 20.04 LTS + Qt 5.12.8 LTS + { Clang 10.0 | GCC 9.3 }
  - Fedora 32 + Qt 5.13.2 + Clang 10.0

  Reference: [QTBUG-75210](https://bugreports.qt.io/browse/QTBUG-75210)

  Also see **fanquake**'s [comment](https://github.com/bitcoin/bitcoin/pull/18738#issuecomment-622956100).

ACKs for top commit:
  MarcoFalke:
    ACK 0c63f80854 seems fine to disable this warning for the 0.21.0 release temporarily and then enable it for 0.22.0, when boost is removed.
  fanquake:
    ACK 0c63f80854 - I think it's ok to suppress these for now, given that `-Wdeprecated-copy` is enabled (via `-Wextra`) in GCC 9 and Clang 10. The Qt output is pretty noisy, and there's a few warnings from Boost as well.

Tree-SHA512: 7064a3272bc9eae00b73a16c421ac58be148f374cbef87320e8f092f52761f6e98166eff60346b70867f8a69a9698a79455dc16b42d92f8fbe7c56519571ac08
2020-05-13 21:17:07 +08:00
fanquake 219c55da75
Merge #16710: build: Enable -Wsuggest-override if available
839add193b build: Enable -Wsuggest-override (Hennadii Stepanov)
de5e91c303 refactor: Add BerkeleyDatabaseVersion() function (Hennadii Stepanov)

Pull request description:

  From GCC [docs](https://gcc.gnu.org/onlinedocs/gcc-9.2.0/gcc/Warning-Options.html):
  > `-Wsuggest-override`
  > Warn about overriding virtual functions that are not marked with the override keyword.

  ~This PR is based on #16722 (the first commit).~ See: https://github.com/bitcoin/bitcoin/pull/16722#issuecomment-584111086

ACKs for top commit:
  fanquake:
    ACK 839add193b
  vasild:
    ACK 839add193
  practicalswift:
    ACK 839add193b assuming Travis is happy: patch looks correct

Tree-SHA512: 1e8cc085da30d41536deff9b181962c1882314ab252c2ad958294087ae1e5a0dfa4886bdbe36f21cf6ae71df776a8420f349f007d4b5b49fd79ba98ce308965a
2020-05-13 15:19:05 +08:00
Hennadii Stepanov 839add193b
build: Enable -Wsuggest-override 2020-05-12 18:03:39 +03:00
Russell Yanofsky 5d1377b52b build: multiprocess autotools changes
autoconf and automake changes to support multiprocess gui/node/wallet execution.

This adds a new --enable-multiprocess flag, and build configuration code to
detect libraries needed for multiprocess support. The --enable-multiprocess
flag builds new bitcoin-node and bitcoin-gui executables, which are updated in
https://github.com/bitcoin/bitcoin/pull/10102 to communicate across processes.
But for now they are functionally equivalent to existing bitcoind and
bitcoin-qt executables.
2020-05-12 09:47:06 -04:00
fanquake 49d237ce32
Merge #18928: build: don't pass -w when building for Windows
89fea68ffd build: don't pass -w when building for Windows (fanquake)

Pull request description:

  This has been around since the introduction of autotools. However at
  this point I'm not sure we'd ever want to suppress all warnings when
  performing a build, and given that CXX FLAGS will have been overriden
  when cross-compiling for Windows (using depends), this would rarely,
  if-ever be used anyways.

  From https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html:
  > -w
  >
  >     Inhibit all warning messages.

ACKs for top commit:
  hebasto:
    ACK 89fea68ffd

Tree-SHA512: 2b5bdef7fff5c87b28199f5822cab3cdf600c90c01a40db5cd85053eef5dcb5816e2e97ff61a30ff94b4f0c6cb7be22beaef34d82235bdf05ff9da865d40b381
2020-05-12 16:17:15 +08:00
fanquake 89fea68ffd
build: don't pass -w when building for Windows
This has been around since the introduction of autotools. However at
this point I'm not sure we'd every want to suppress all warnings when
performing a build, and given that CXX FLAGS will have been overriden
when cross-compiling for Windows (using depends), this would rarely,
if-ever be used anyways.

From https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html:
-w

    Inhibit all warning messages.
2020-05-10 19:27:15 +08:00
Ben Woosley 68537275bd
build: Enable -Werror=sign-compare
Explicitly add -Wsign-compare as well - not required for all compilers, as GCC activates it
under -Wall, but may impact clang, etc.
2020-05-09 00:20:09 -07:00
fanquake df6bde031b
test: remove glibc fdelt sanity check
As is, this sanity check doesn't seem to be testing fdelt_chk, because
passing a value of "0" to FD_SET wont cause the compiler to insert any
calls to fdelt_chk().

The documentation is a little misleading. If we actually triggered fdelt_chk
at runtime, bitcoind would abort. I think this check would be better replaced
(if possible) by additional checks in security-check.py.

The compiler may insert a call to fdelt_warn() (aliased with fdelt_chk
in glibc) at compile time if it can determine that an invalid value is
being passed to FD_SET.

These checks are essentially; value < 0 or value >= FD_SETSIZE along
with a check for wether the value is a compile time constant.

If the compiler can determine an invalid value is being passed, a call
to fdelt_warn will be inserted. Passing 0 should never cause a call to
be inserted.

You can check this after compiling:
```bash
objdump -dC bitcoind | grep sanity_fdelt
...
0000000000399d20 <sanity_test_fdelt()>:
  399d20:	48 81 ec 98 00 00 00 	sub    $0x98,%rsp
  399d27:	b9 10 00 00 00       	mov    $0x10,%ecx
  399d2c:	64 48 8b 04 25 28 00 	mov    %fs:0x28,%rax
  399d33:	00 00
  399d35:	48 89 84 24 88 00 00 	mov    %rax,0x88(%rsp)
  399d3c:	00
  399d3d:	31 c0                	xor    %eax,%eax
  399d3f:	48 89 e7             	mov    %rsp,%rdi
  399d42:	fc                   	cld
  399d43:	f3 48 ab             	rep stos %rax,%es:(%rdi)
  399d46:	48 8b 84 24 88 00 00 	mov    0x88(%rsp),%rax
  399d4d:	00
  399d4e:	64 48 33 04 25 28 00 	xor    %fs:0x28,%rax
  399d55:	00 00
  399d57:	75 0d                	jne    399d66 <sanity_test_fdelt()+0x46>
  399d59:	b8 01 00 00 00       	mov    $0x1,%eax
  399d5e:	48 81 c4 98 00 00 00 	add    $0x98,%rsp
  399d65:	c3                   	retq
  399d66:	e8 85 df c8 ff       	callq  27cf0 <__stack_chk_fail@plt>
  399d6b:	0f 1f 44 00 00       	nopl   0x0(%rax,%rax,1)

```

To test, you could modify this test to pass -1 to FD_SET, and check
that a call to fdelt_warn() is inserted, and that running bitcoind
fails. i.e:

```bash
0000000000399d20 <sanity_test_fdelt()>:
  399d20:	48 81 ec 98 00 00 00 	sub    $0x98,%rsp
  399d27:	b9 10 00 00 00       	mov    $0x10,%ecx
  399d2c:	64 48 8b 04 25 28 00 	mov    %fs:0x28,%rax
  399d33:	00 00
  399d35:	48 89 84 24 88 00 00 	mov    %rax,0x88(%rsp)
  399d3c:	00
  399d3d:	31 c0                	xor    %eax,%eax
  399d3f:	48 89 e7             	mov    %rsp,%rdi
  399d42:	fc                   	cld
  399d43:	f3 48 ab             	rep stos %rax,%es:(%rdi)
  399d46:	48 c7 c7 ff ff ff ff 	mov    $0xffffffffffffffff,%rdi
  399d4d:	e8 3e ff ff ff       	callq  399c90 <__fdelt_warn>
  399d52:	0f b6 04 24          	movzbl (%rsp),%eax
  399d56:	83 e0 01             	and    $0x1,%eax
  399d59:	48 8b 94 24 88 00 00 	mov    0x88(%rsp),%rdx
  399d60:	00
  399d61:	64 48 33 14 25 28 00 	xor    %fs:0x28,%rdx
  399d68:	00 00
  399d6a:	75 08                	jne    399d74 <sanity_test_fdelt()+0x54>
  399d6c:	48 81 c4 98 00 00 00 	add    $0x98,%rsp
  399d73:	c3                   	retq
  399d74:	e8 77 df c8 ff       	callq  27cf0 <__stack_chk_fail@plt>
  399d79:	0f 1f 80 00 00 00 00 	nopl   0x0(%rax)

```

```bash
./src/bitcoind
*** buffer overflow detected ***: src/bitcoind terminated
Aborted
```
2020-05-07 15:45:09 +08:00
fanquake 8bf1540cc2
build: remove fdelt_chk backwards compatibility code
Now that we require glibc 2.17 or later, we no longer need to check for
different return types in fdelt_chk. It was changed from unsigned long
int to long int in glibc 2.16 . See this commit:
https://sourceware.org/git/?p=glibc.git;a=commit;h=ceb9e56b3d1f8c1922e0526c2e841373843460e2
and related issue:
https://sourceware.org/bugzilla/show_bug.cgi?id=14210.
2020-05-07 15:44:56 +08:00
fanquake 3b1e289248
Merge #18535: build: remove -Qunused-arguments workaround for clang + ccache
a029805f57 build: remove -Qunused-arguments workaround for clang + ccache (fanquake)

Pull request description:

  This was added in 386efb7695 to address spammy Clang warnings when building with ccache.

  The issue was addressed in [ccache 3.2](https://bugzilla.samba.org/show_bug.cgi?id=8118), and from a look at most major distros, it's only Debian Jessie that has a version of ccache older than that ([3.1](https://packages.debian.org/jessie/ccache)).

  Therefore I think it's acceptable to drop this workaround, and re-enable warnings for unused driver arguments (when compiling using Clang and ccache).

ACKs for top commit:
  hebasto:
    ACK a029805f57.
  vasild:
    utACK a029805f57

Tree-SHA512: f887b9bd12f9c1c8d209943b86e8dafe33cfd1572912f2cafabe08ffe403973e48f0f7289280a8c6db9263c57aad43fbd4bb72f42db762eb090f3b1ef0538f43
2020-05-07 15:41:59 +08:00
Wladimir J. van der Laan c6b15ec0ee
Merge #17874: build: make linker checks more robust
03da4c7781 build: make linker checks more robust (Cory Fields)

Pull request description:

  Check for a flag to turn linker warnings into errors. When flags are passed to
  linkers via the compiler driver using a -Wl,-foo flag, linker warnings may be
  swallowed rather than bubbling up.

  This is one of [Corys commits](b9acd3d33e) that I've modified to also add `-Wl,-fatal_warnings`
  for darwin.

ACKs for top commit:
  vasild:
    re-ACK 03da4c778

Tree-SHA512: 212031d619ed88e52aaae30cf3b711681d72c4d670884406403605d1d86c784c84cb07e2e0d6c30926e659db8f14f8dabd5af3de5291637f8080d6dfee358248
2020-05-06 15:09:55 +02:00
Wladimir J. van der Laan 6621be5351
Merge #18843: build: warn on potentially uninitialized reads
71f183a49b build: warn on potentially uninitialized reads (Vasil Dimov)

Pull request description:

  * Enable `conditional-uninitialized` warning class to show potentially uninitialized
  reads.

  * Fix the sole such warning in Bitcoin Core in `GetRdRand()`: `r1` would be
  set to `0` on `rdrand` failure, so initializing it to `0` is a non-functional
  change.

ACKs for top commit:
  practicalswift:
    ACK 71f183a49b
  laanwj:
    ACK 71f183a49b

Tree-SHA512: 2c1d8caacd86424b16a9d92e5df19e0bedb51ae111eecad7e3bfa46447bc88e5fff1f32dacf6c4a28257ebb3d87e79f80f074ce2c523ce08b1a0c0a67ab44204
2020-05-06 13:49:49 +02:00
Cory Fields 03da4c7781
build: make linker checks more robust
Check for a flag to turn linker warnings into errors. When flags are passed to
linkers via the compiler driver using a -Wl,-foo flag, linker warnings may be
swallowed rather than bubbling up.

Co-authored-by: fanquake <fanquake@gmail.com>
2020-05-06 17:39:07 +08:00
Vasil Dimov a30b0a24e9
build: enable -Werror=gnu
Stop the build if a warning is emitted due to `-Wgnu` and
`--enable-werror` has been used. As usual - this would help notice such
a warning that is about to be introduced in new code.

This is a followup to
https://github.com/bitcoin/bitcoin/pull/18088
build: ensure we aren't using GNU extensions
2020-05-05 14:47:59 +02:00
Hennadii Stepanov 0c63f80854
build: Suppress -Wdeprecated-copy warnings 2020-05-05 06:21:52 +03:00
Vasil Dimov 71f183a49b
build: warn on potentially uninitialized reads
Enable -Wconditional-uninitialized to warn on potentially uninitialized
reads.

Fix the sole such warning in Bitcoin Core in GetRdRand(): r1 would be
set to 0 on rdrand failure, so initializing it to 0 is a non-functional
change.

From "Intel 64 and IA-32 ArchitecturesSoftware Developer's Manual" [1],
page 1711: "CF=1 indicates that the data in the destination is valid.
Otherwise CF=0 and the data in the destination operand will be returned
as zeros for the specified width."

[1] https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf
2020-05-03 17:21:45 +02:00
fanquake 0ae8f18dfe
build: add -Wgnu to compile flags
When compiling with Clang, this will warn when GNU extensions are
used.

Info: https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu
2020-04-30 18:02:04 +08:00
Wladimir J. van der Laan 35ef3c15ef
Merge #18591: Add C++17 build to Travis
c31cbe7cfe Add C++17 test to Travis (Pieter Wuille)
7829685e27 Add configure option for c++17 (Pieter Wuille)
0fbde488b2 Support conversion between Spans of compatible types (Pieter Wuille)
7cbfebbf3d Update ax_cxx_compile_stdcxx.m4 (Pieter Wuille)

Pull request description:

  This adds a `--enable-c++17` option to the configure script, fixes the only C++17 incompatibility (with a commit taken from #18468), and adds a Travis test for it.

  This is all off by default, and release builds remain C++11.

  It implements the first step of the plan in https://github.com/bitcoin/bitcoin/issues/16684.

ACKs for top commit:
  elichai:
    tACK c31cbe7cfe
  practicalswift:
    Tested ACK c31cbe7cfe
  hebasto:
    ACK c31cbe7cfe, tested on Linux Mint 19.3 both C++11 and C++17 modes. Compiled and passed tests locally.

Tree-SHA512: a4b00776dbceef9c12abbb404c6bcd48f7916ce24c8c7a14116355f64e817578b7fcddbedd5ce435322319d1e4de43429b68553f4d96d970c308fe3e3e59b9d1
2020-04-30 11:16:56 +02:00
Wladimir J. van der Laan 63d5ed2fc4
Merge #18437: util: Detect posix_fallocate() instead of assuming
182dbdf0f4 util: Detect posix_fallocate() instead of assuming (Vasil Dimov)

Pull request description:

  Don't assume that `posix_fallocate()` is available on Linux and not
  available on other operating systems. At least FreeBSD has it and we
  are not using it.

  Properly check whether `posix_fallocate()` is present and use it if it
  is.

ACKs for top commit:
  laanwj:
    ACK 182dbdf0f4

Tree-SHA512: f9ed4bd661f33ff6b2b1150591e860b3c1f44e12b87c35e870d06a7013c4e841ed2bf17b41ad6b18fe471b0b23a4b5e42cf1400637180888e0bc56c254fe0766
2020-04-30 10:45:17 +02:00
fanquake cd24f37ea9
doc: Better explain GNU ld's dislike of ld64's options
There's also now more than a single option being special cased for
darwin.
2020-04-27 11:08:51 +08:00
fanquake 7d1a3bda21
Merge #18709: doc: note why we can't use thread_local with glibc back compat
b155fcda51 doc: fix typo in configure.ac (fanquake)
20a30922fb doc: note why we can't use thread_local with glibc back compat (fanquake)

Pull request description:

  Given that we went through a [gitian build](https://github.com/bitcoin/bitcoin/pull/18681) to remember why this is the case, we might as well make a note of it in configure.ac.

  [From #18681](https://github.com/bitcoin/bitcoin/pull/18681#issuecomment-615526634):

  Looking at the Linux build log, this has failed with:
  ```bash
  Checking glibc back compat...
  bitcoind: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  bitcoind: failed IMPORTED_SYMBOLS
  bitcoin-cli: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  bitcoin-cli: failed IMPORTED_SYMBOLS
  bitcoin-tx: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  bitcoin-tx: failed IMPORTED_SYMBOLS
  bitcoin-wallet: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  bitcoin-wallet: failed IMPORTED_SYMBOLS
  test/test_bitcoin: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  test/test_bitcoin: failed IMPORTED_SYMBOLS
  bench/bench_bitcoin: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  bench/bench_bitcoin: failed IMPORTED_SYMBOLS
  qt/bitcoin-qt: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  ```

  `__cxa_thread_atexit_impl` is used for [thread_local variable destruction](https://sourceware.org/glibc/wiki/Destructor%20support%20for%20thread_local%20variables):

  > To implement this support, glibc defines __cxa_thread_atexit_impl exclusively for use by libstdc++ (which has the __cxa_thread_atexit to wrap around it), that registers destructors for thread_local variables in a list. Upon thread or process exit, the destructors are called in reverse order in which they were added.

  As suggested, this only became available in glibc 2.18. From the [2.18 release notes](https://sourceware.org/legacy-ml/libc-alpha/2013-08/msg00160.html):

  > * Add support for calling C++11 thread_local object destructors on thread
    and program exit.  This needs compiler support for offloading C++11
    destructor calls to glibc.

ACKs for top commit:
  hebasto:
    ACK b155fcda51

Tree-SHA512: 5b9567e4a70598a4b0b91956f44ae0d93091db17c84cbf9817dac6cfa992c97d3438a8b1bb66644c74891f2149e44984daed445d22de93ca8858c5b0eabefb40
2020-04-22 14:46:19 +08:00
fanquake b155fcda51
doc: fix typo in configure.ac
s/liner/linker/
2020-04-21 10:05:43 +08:00
fanquake 20a30922fb
doc: note why we can't use thread_local with glibc back compat 2020-04-20 16:58:54 +08:00
Hennadii Stepanov b68e717967
build: Set libevent minimum version to 2.0.21 2020-04-17 13:53:34 +03:00
Vasil Dimov 182dbdf0f4
util: Detect posix_fallocate() instead of assuming
Don't assume that `posix_fallocate()` is available on Linux and not
available on other operating systems. At least FreeBSD has it and we
are not using it.

Properly check whether `posix_fallocate()` is present and use it if it
is.
2020-04-14 09:49:45 +02:00
Pieter Wuille 7829685e27 Add configure option for c++17 2020-04-11 02:15:25 -07:00
Pieter Wuille 7cbfebbf3d Update ax_cxx_compile_stdcxx.m4 2020-04-11 02:15:20 -07:00
MarcoFalke f29bd546ec Revert "Merge #16367: Multiprocess build support"
This reverts the changes made in merge commit
1b30761360:

This reverts commit b919efadff.
This reverts commit d54f64c6c7.
This reverts commit 787f40668d.
This reverts commit d630646662.
This reverts commit e6e44eedd5.
2020-04-10 19:38:21 -04:00
MarcoFalke 1b30761360
Merge #16367: Multiprocess build support
b919efadff depends: Use default macos clang compiler (Russell Yanofsky)
d54f64c6c7 Add multiprocess travis configuration (Russell Yanofsky)
787f40668d Set LD_LIBRARY_PATH consistently in travis tests (Russell Yanofsky)
d630646662 libmultiprocess depends build (Russell Yanofsky)
e6e44eedd5 Multiprocess build changes (Russell Yanofsky)

Pull request description:

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).

  This splits autotools, depends build, and travis changes out of #10102, so code changes and build system changes can be reviewed separately.

ACKs for top commit:
  hebasto:
    re-ACK b919efadff, rebased only since my [previous](https://github.com/bitcoin/bitcoin/pull/16367#issuecomment-605514556) review.

Tree-SHA512: ebc5e403cc99a0d9629ed7fe1595e01d57e6d1255cbf03968a3196ff6f528f734c78060fdc065724ee1f923bcc5aa2b29470fcb36a7f15957eb57c76d58178a4
2020-04-10 12:55:12 -04:00
MarcoFalke 4eb1eeb02c
Merge #18504: build: Drop bitcoin-tx and bitcoin-wallet dependencies on libevent
01a3392b1b Drop bitcoin-wallet dependency on libevent (Russell Yanofsky)
0660119ac3 Drop unintended bitcoin-tx dependency on libevent (Russell Yanofsky)

Pull request description:

  This fixes compile errors trying to build bitcoin-tx and bitcoin-wallet without libevent, which were reported by Luke Dashjr in https://github.com/bitcoin/bitcoin/issues/18465

  The fix avoiding `bitcoin-tx` dependency on libevent just adds a conditional build rule. This is implemented in the first commit (more details in commit description).

  The fix avoiding `bitcoin-wallet` dependency on libevent requires minor code changes, because `bitcoin-wallet` (unlike `bitcoin-tx`) links against code that calls `urlDecode` / `evhttp_uridecode`. This fix is implemented in the second commit (again details in the commit description).

ACKs for top commit:
  jonasschnelli:
    utACK 01a3392b1b.

Tree-SHA512: d2245e912ab494cccceeb427a1eca8e55b01a0006ff93eebcfb5461ae7cecd1083ac2de443d9db036b18bdc6f0fb615546caaa20c585046f66d234937f74870a
2020-04-10 12:52:37 -04:00
Wladimir J. van der Laan d84c9aa25d
build: Bump version to 0.20.99
Now that 0.20 branch has been split off, master is 0.20.99 (pre-0.21).

Also clean out release notes.

Tree-SHA512: bba6133ae9708f75206c8934901b9f9909a233330f4dfefb3c24175bf8e11631cdc89a5d24a22421a73083f7eb743e977db8020b87dfbd3c1e6043929a19a285
2020-04-10 18:01:01 +02:00
fanquake a029805f57
build: remove -Qunused-arguments workaround for clang + ccache
This was added in 386efb7695 to
address spammy Clang warnings when building with ccache.

The issue was addressed in ccache 3.2
(https://bugzilla.samba.org/show_bug.cgi?id=8118, Nov 2014),
and from a look at all major distros, it's only Debian Jessie
that has a version of ccache older than that (3.1).

Therefore I think it's acceptable to drop this workaround, and
re-enable warnings for unused driver arguments (when compiling
using Clang).
2020-04-06 14:08:56 +08:00
Russell Yanofsky e6e44eedd5 Multiprocess build changes
autotools and automake changes to support multiprocess execution.

This adds a new --enable-multiprocess flag, and build configuration code to
detect libraries needed for multiprocess support. The --enable-multiprocess
flag builds new bitcoin-node and bitcoin-gui executables, which are updated in
https://github.com/bitcoin/bitcoin/pull/10102 to communicate across processes.
But for now they are functionally equivalent to existing bitcoind and
bitcoin-qt executables.
2020-04-05 21:48:21 -04:00
fanquake 9e071b0089
test: remove rapidcheck integration and tests 2020-04-03 22:47:59 +08:00
Russell Yanofsky 0660119ac3 Drop unintended bitcoin-tx dependency on libevent
Don't include util/url.cpp to libbitcoin_util.a when libevent isn't available.
This fixes a compile error trying to build bitcoin-tx without libevent reported
by Luke Dashjr in https://github.com/bitcoin/bitcoin/issues/18465

Fixes #18465
2020-04-02 08:28:11 -04:00
Ben Woosley a46484c8b3
build: Detect gmtime_* definitions via configure
This improves the portability of the codebase and fixes compilation
with mingw-w64 7.0+.

Co-authored-by: fanquake <fanquake@gmail.com>
2020-04-02 12:31:54 +08:00