Commit graph

460 commits

Author SHA1 Message Date
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
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
1b307613604883daea4913a65da30ae073c9dc4d:

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
Wladimir J. van der Laan bdc2644b72
Merge #18107: build: Add cov_fuzz target
faf7d4fa86 build: Add cov_fuzz target (MarcoFalke)
fac71e364e build: link fuzz/test_runner.py for out-of-tree builds (MarcoFalke)
faf2c5aca0 build: Remove unused USE_COVERAGE (MarcoFalke)

Pull request description:

  Only libFuzzer is supported right now, so clang is required. Thus, this needs a workaround such as https://github.com/bitcoin/bitcoin/issues/12602#issuecomment-562788247

  Can be tested with:

  ```
  mkdir build && cd build
  ../configure --enable-fuzz --with-sanitizers=fuzzer --enable-lcov --enable-lcov-branch-coverage CC=clang CXX=clang++
  make $MAKEJOBS
  make cov_fuzz

ACKs for top commit:
  practicalswift:
    ACK faf7d4fa86

Tree-SHA512: 6828f8f81d95f6781713d0b09d7eba2ffdb50217e09ca839db61791a4ed70024859c7a0cb01d9eede79166d574dd57ece01f9d9fe2610d4a72a4ca4a4ce0b838
2020-03-27 14:29:48 +01:00
Hennadii Stepanov f709ad0c90
build: Fix libevent linking for bench_bitcoin binary 2020-03-21 14:18:07 +02:00
fanquake e90e3e684f
build: fix sysctl() detection on macOS
sysctl() on *BSD takes a "const int *name", whereas sysctl() on macOS
it takes an "int *name". So our configure check and sysctl() detection on
macOS currently fails:

```bash
/usr/include/sys/sysctl.h:759:9: note: candidate function not viable:
	no known conversion from 'const int [2]' to 'int *' for 1st argument
int     sysctl(int *, u_int, void *, size_t *, void *, size_t);
```

This change removes the name argument from the sysctl() detection check,
meaning we will detect correctly on macOS and *BSD.

For consistency we also switch to using the more generic, non-const
version of the name parameter in the rest of our usage.
2020-03-19 11:36:38 +08:00
Wladimir J. van der Laan 3b21a78634
Merge #18290: build: Set minimum Automake version to 1.13
ddc7e42d60 build: Set minimum Automake version to 1.13 (Hennadii Stepanov)

Pull request description:

  This PR suggests to set the required minimum Automake version to `1.13` explicitly for the following reasons:
  - it guarantees that [CVE-2012-3386](https://lists.gnu.org/archive/html/automake/2012-07/msg00023.html) has been fixed
  - `AC_CONFIG_MACRO_DIR` macro support, which we already use; from the [release notes](https://lists.gnu.org/archive/html/automake/2012-12/msg00038.html):
  > Improvements to aclocal and related rebuilds rules:
  >    - Autoconf-provided macros AC_CONFIG_MACRO_DIR and AC_CONFIG_MACRO_DIRS are now traced by aclocal, and can be used to declare the local m4 include directories.  Formerly, one had to specify it with an explicit '-I' option to the 'aclocal' invocation.
  - `AM_SILENT_RULES` macro support (since version `1.11`)

  Automake `1.13` requires Autoconf `2.65` or greater. We already have `2.69` since #17769.

  ---

  For reference, Automake `1.13` was released in [December of 2012](https://lists.gnu.org/archive/html/automake/2012-12/msg00038.html).

  CentOS 7 uses Automake [`1.13.4`](https://centos.pkgs.org/7/centos-x86_64/automake-1.13.4-3.el7.noarch.rpm.html)

  See the Automake docs for more info:
  - [`AM_INIT_AUTOMAKE`](https://www.gnu.org/software/automake/manual/automake.html#Public-Macros)
  - [List of Automake options](https://www.gnu.org/software/automake/manual/automake.html#List-of-Automake-options)

ACKs for top commit:
  laanwj:
    so also ACK ddc7e42d60
  fanquake:
    ACK ddc7e42d60 - I think adding a minimum required version here is fine. I'd be surprised if someone who is currently building Bitcoin Core was unable to after this change.

Tree-SHA512: a1f97864bc3a513450c03d041498f28e823e6f8cd9710d81df081435d72bd4b6cd2f3deb997dbf902f950215a859e48a2ee7ca1f8ebf4271778dd951ab78abf4
2020-03-11 16:31:38 +01:00
Hennadii Stepanov ddc7e42d60
build: Set minimum Automake version to 1.13 2020-03-07 17:38:06 +02:00
fanquake bd37f2bc26
build: remove Boost Chrono detection from build system 2020-03-07 08:34:31 +08:00
fanquake 97aadf98d0
Merge #16117: util: Replace boost sleep with std sleep
fae86c38bc util: Remove unused MilliSleep (MarcoFalke)
fa9af06d91 scripted-diff: Replace MilliSleep with UninterruptibleSleep (MarcoFalke)
fa4620be78 util: Add UnintrruptibleSleep (MarcoFalke)

Pull request description:

  We don't use the interruptible feature of boost's sleep anywhere, so replace it with the sleep in `std::thread`

ACKs for top commit:
  ajtowns:
    ACK fae86c38bc quick code review
  practicalswift:
    ACK fae86c38bc -- patch looks correct
  sipa:
    Concept and code review ACK fae86c38bc
  fanquake:
    ACK fae86c38bc - note that an instance of `DHAVE_WORKING_BOOST_SLEEP_FOR` was missed in the [linter](https://github.com/bitcoin/bitcoin/blob/master/test/lint/extended-lint-cppcheck.sh#L69), but that can be cleaned up later.

Tree-SHA512: 7c0f8eb197664b9f7d9fe6c472c77d384f11c797c913afc31de4b532e3b4fd9ea6dd174f92062ff9d1ec39b25e0900ca7c597435add87f0f2477d9557204848c
2020-03-06 15:41:00 +08:00
fanquake 3d9b41ecc0
build: add --enable-determinism configure flag
If used, this will enable additional compile / link time flags
that will make subsequent builds of bitcoind determinisitic.
2020-02-27 12:12:18 +08:00