Age | Commit message (Collapse) | Author |
|
c4ffcf07af19cd0c600b11dabd94e7e9d31ad072 build: remove BIP70 configure option (fanquake)
Pull request description:
This was left in after #17165, so that anyone who had been compiling
with (already disabled by default) BIP70 would realise that support
had been completely removed in 0.20.0. However we should be able to
remove it for 0.21.0.
ACKs for top commit:
jnewbery:
utACK c4ffcf07af19cd0c600b11dabd94e7e9d31ad072
MarcoFalke:
ACK c4ffcf07af19cd0c600b11dabd94e7e9d31ad072 with or without the "catch-all reject"
Tree-SHA512: a5dd4231ed97c9dd1984fb90d69a8725df2fdda0b963269b0575601c74528e5d820a4a863c428f8ede86eaae2a1606671fe1fcebdeb96b1023f7a5f899270284
|
|
9952242c03fe587b5dff46a9f770e319146103bf build: improve builtin_clz* detection (fanquake)
Pull request description:
Fixes #19402.
The way we currently test for `__builtin_clz*` support with `AC_CHECK_DECLS` does not work with Clang:
```bash
configure:21492: clang++-10 -std=c++11 -c -g -O2 -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS conftest.cpp >&5
conftest.cpp:100:10: error: builtin functions must be directly called
(void) __builtin_clz;
^
1 error generated.
```
This also removes the `__builtin_clz()` check, as we don't actually use it anywhere, and it's trvial to re-add detection if we do start using it at some point. If this is controversial then I'll add a test for it as well.
ACKs for top commit:
sipa:
ACK 9952242c03fe587b5dff46a9f770e319146103bf
laanwj:
ACK 9952242c03fe587b5dff46a9f770e319146103bf
Tree-SHA512: 695abb1a694a01a25aaa483b4fffa7d598842f2ba4fe8630fbed9ce5450b915c33bf34bb16ad16a16b702dd7c91ebf49fe509a2498b9e28254fe0ec5177bbac0
|
|
The way we currently test with AC_CHECK_DECLS do not work with Clang:
```bash
configure:21492: clang++-10 -std=c++11 -c -g -O2 -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS conftest.cpp >&5
conftest.cpp:100:10: error: builtin functions must be directly called
(void) __builtin_clz;
^
1 error generated.
```
This also removes the __builtin_clz() check, as we don't actually use
it anywhere, and it's trvial to re-add detection if we do start using
it at some point.
|
|
On OS X, when searching Homebrew keg-only packages for BDB 4.8, if we find it,
use BDB_CPPFLAGS and BDB_LIBS instead of CFLAGS and LIBS for the result. This
is (1) more correct, and (2) necessary in order to give this location
priority over other directories in the include search path, which may include
system include directories with other versions of BDB.
|
|
Doxygen isn't so important that we need to warn when it is missing. I'd
assume it might even be missing more often than not for most builds.
|
|
|
|
including Windows
8a26848c460160e1279f26bc413f693a34e33b9d build: Fix m4 escaping (Hennadii Stepanov)
9123ec15db104397998f5084afc69403d2f9e4b8 build: Remove extra tokens warning (Hennadii Stepanov)
fded4f48c33742d7c790335c8de59c15b80d94e6 build: Remove duplicated QT_STATICPLUGIN define (Hennadii Stepanov)
05a93d5d96101b45d87571af5b772c7a1e82fd27 build: Fix indentation in bitcoin_qt.m4 (Hennadii Stepanov)
ddbb41931019ed4226af3df37874c7eb7cf570f1 build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts (Hennadii Stepanov)
492971de35bab26346545f68365872211f458b00 build: Fix mingw pkgconfig file and dependency naming (Hennadii Stepanov)
Pull request description:
This PR makes `bitcoin_qt.m4` to use `pkg-config` for all hosts and removes non-pkg-config paths from it. This is a step towards the idea which was clear [stated](https://github.com/bitcoin/bitcoin/pull/8314#issue-76644643) by Cory Fields:
> I believe the consensus is to treat Windows like the others and require pkg-config across the board. We can drop all of the non-pkg-config paths, and simply AC_REQUIRE(PKG_PROG_PKG_CONFIG)
There are two unsolved problems with this PR. If depends is built with `DEBUG=1` the `configure` script fails to pickup Qt:
- for macOS host (similar to, but not the same as #16391)
- for Windows host (regression)
The fix is ~on its way~ submitted in #18298 (as a followup).
Also this PR picks some small improvements from #17820.
ACKs for top commit:
theuni:
Code review ACK 8a26848c460160e1279f26bc413f693a34e33b9d
dongcarl:
Code Review ACK 8a26848c460160e1279f26bc413f693a34e33b9d
laanwj:
Code review ACK 8a26848c460160e1279f26bc413f693a34e33b9d
Tree-SHA512: 3b25990934b939121983df7707997b31d61063b1207d909f539d69494c7cb85212f353092956d09ecffebb9fef28b869914dd1216a596d102fcb9744bb5487f7
|
|
This was left in after #17165, so that anyone who had been compiling
with (already disabled by default) BIP70 would realise that support
had been completely removed in 0.20.0. However we should be able to
remove it for 0.21.0.
|
|
|
|
|
|
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.
|
|
eea81146571480b2acd12c8cd7f36b04d056c47f build: Enable unreachable-code-loop-increment (Jonathan Schoeller)
d15db4b1fc988736b08c092d000ca1d1ff686975 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.
```
https://github.com/bitcoin/bitcoin/blob/aa8d76806c74a51ec66e5004394fe9ea8ff0fac4/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 eea81146571480b2acd12c8cd7f36b04d056c47f
hebasto:
re-ACK eea81146571480b2acd12c8cd7f36b04d056c47f, only suggested changes applied since the [previous](https://github.com/bitcoin/bitcoin/pull/19131#pullrequestreview-421772387) review.
Tree-SHA512: 2aa3ceb7fab581b6ba2580900668388d8eba1c3001c8ff9c11c1f4a9a10fbc37f30e590249862676858446e3f4950140a252953ba1643ba3bfd772f8eae20583
|
|
|
|
Enable unreachable-code-loop-increment and treat as error.
refs: #19015
|
|
facef3d4131f9980a4516282f11731361559509c doc: Explain that anyone can work on good first issues, move text to CONTRIBUTING.md (MarcoFalke)
fae2fb2a196ee864e9a13fffc24a0279cd5d17e6 doc: Expand section on Getting Started (MarcoFalke)
100000d1b2c2e38d7a14a31b0af79e0e4316b04c doc: Add headings to CONTRIBUTING.md (MarcoFalke)
fab893e0caf510d4836a20194892ef9c71426c51 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 facef3d4131f9980a4516282f11731361559509c
fanquake:
ACK facef3d4131f9980a4516282f11731361559509c
Tree-SHA512: 8998e273a76dbf4ca77e79374c14efe4dfcc5c6df6b7d801e1e1e436711dbe6f76b436f9cbc6cacb45a56827babdd6396f3bd376a9426ee7be3bb9b8a3b8e383
|
|
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.
|
|
|
|
|
|
e2bab2aa162ae38b2bf8195b577c982402fbee9d multiprocess: add multiprocess travis configuration (Russell Yanofsky)
603fd6a2e708c04ef6c9880f89d0a4cbaa6fc7c5 depends: add MULTIPROCESS depends option (Russell Yanofsky)
5d1377b52bfcd4edf8553aaf332bfeb92fc554cc 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 e2bab2aa162ae38b2bf8195b577c982402fbee9d
Sjors:
tACK e2bab2aa162ae38b2bf8195b577c982402fbee9d on macOS 10.15.4
hebasto:
ACK e2bab2aa162ae38b2bf8195b577c982402fbee9d, tested on Linux Mint 19.3 (x86_64):
Tree-SHA512: b5a76eab5abf63d9d8b6d628cbdff4cc1888eef15cafa0a5d56369e2f9d02595fed623f4b74b2cf2830c42c05a774f0943e700f9c768a82d9d348cad199e135c
|
|
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.
|
|
a30b0a24e97eae7f6d1428c5bf339a579872f28e 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 a30b0a24e97eae7f6d1428c5bf339a579872f28e
Empact:
ACK https://github.com/bitcoin/bitcoin/pull/18887/commits/a30b0a24e97eae7f6d1428c5bf339a579872f28e
Tree-SHA512: f81b71cf3ee4db88b6f664c571075e0d30800a604f067f44273f256695a1dea533779db2ac859dd0a4cd8b66289c3e45f4aff1cfadfa160a1c354237167b05e2
|
|
df6bde031b24112abf3a94337a2c096698acde6e test: remove glibc fdelt sanity check (fanquake)
8bf1540cc235fb8fb5330a7ae8ab638247ceb177 build: remove fdelt_chk backwards compatibility code (fanquake)
Pull request description:
https://github.com/bitcoin/bitcoin/commit/ae30d40e50e9d63d875d29d54d22147b09fc420c
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.
https://github.com/bitcoin/bitcoin/commit/ab7bce584ae02bf25e2e91aa54f9b0249427127d
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 df6bde031b24112abf3a94337a2c096698acde6e
Tree-SHA512: d8b3af4f4eb2d6c767ca6e72ece51d0ab9042e1bbdfcbbdb7ad713414df21489ba3217662b531b8bfdac0265d2ce5431abfae6e861b6187d182ff26c6e59b32d
|
|
0c63f808542ba02fc41aa90b1d96e9123f16d8ad 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 0c63f808542ba02fc41aa90b1d96e9123f16d8ad 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 0c63f808542ba02fc41aa90b1d96e9123f16d8ad - 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
|
|
839add193b13c17a40f42ff69d973caeb800d3f2 build: Enable -Wsuggest-override (Hennadii Stepanov)
de5e91c3034f320f84ee0308a3c31659635d136a 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 839add193b13c17a40f42ff69d973caeb800d3f2
vasild:
ACK 839add193
practicalswift:
ACK 839add193b13c17a40f42ff69d973caeb800d3f2 assuming Travis is happy: patch looks correct
Tree-SHA512: 1e8cc085da30d41536deff9b181962c1882314ab252c2ad958294087ae1e5a0dfa4886bdbe36f21cf6ae71df776a8420f349f007d4b5b49fd79ba98ce308965a
|
|
|
|
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.
|
|
89fea68ffdbd97394d891177e664f896b3e7d1e6 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 89fea68ffdbd97394d891177e664f896b3e7d1e6
Tree-SHA512: 2b5bdef7fff5c87b28199f5822cab3cdf600c90c01a40db5cd85053eef5dcb5816e2e97ff61a30ff94b4f0c6cb7be22beaef34d82235bdf05ff9da865d40b381
|
|
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.
|
|
Explicitly add -Wsign-compare as well - not required for all compilers, as GCC activates it
under -Wall, but may impact clang, etc.
|
|
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
```
|
|
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.
|
|
a029805f57fa9a4ab9867c0d1e865675d57537c7 build: remove -Qunused-arguments workaround for clang + ccache (fanquake)
Pull request description:
This was added in 386efb7695debaf5f0f328a2e4c9abc342161665 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 a029805f57fa9a4ab9867c0d1e865675d57537c7.
vasild:
utACK a029805f57fa9a4ab9867c0d1e865675d57537c7
Tree-SHA512: f887b9bd12f9c1c8d209943b86e8dafe33cfd1572912f2cafabe08ffe403973e48f0f7289280a8c6db9263c57aad43fbd4bb72f42db762eb090f3b1ef0538f43
|
|
03da4c7781fe52cac48530ca292af4d794ae28e2 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](https://github.com/theuni/bitcoin/commit/b9acd3d33e40de5d95901d591a8703a5974195ee) that I've modified to also add `-Wl,-fatal_warnings`
for darwin.
ACKs for top commit:
vasild:
re-ACK 03da4c778
Tree-SHA512: 212031d619ed88e52aaae30cf3b711681d72c4d670884406403605d1d86c784c84cb07e2e0d6c30926e659db8f14f8dabd5af3de5291637f8080d6dfee358248
|
|
71f183a49b714a28622277fa668d8f9f3dac0aae 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 71f183a49b714a28622277fa668d8f9f3dac0aae
laanwj:
ACK 71f183a49b714a28622277fa668d8f9f3dac0aae
Tree-SHA512: 2c1d8caacd86424b16a9d92e5df19e0bedb51ae111eecad7e3bfa46447bc88e5fff1f32dacf6c4a28257ebb3d87e79f80f074ce2c523ce08b1a0c0a67ab44204
|
|
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>
|
|
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
|
|
|
|
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
|
|
When compiling with Clang, this will warn when GNU extensions are
used.
Info: https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu
|
|
c31cbe7cfefc18123eb85ffb2ce509748435efde Add C++17 test to Travis (Pieter Wuille)
7829685e27aae25efb32e07368175c8f664b2218 Add configure option for c++17 (Pieter Wuille)
0fbde488b24f62b4bbbde216647941dcac65c81a Support conversion between Spans of compatible types (Pieter Wuille)
7cbfebbf3df0d26f518811e0bfb7abf270c83e37 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 c31cbe7cfefc18123eb85ffb2ce509748435efde
practicalswift:
Tested ACK c31cbe7cfefc18123eb85ffb2ce509748435efde
hebasto:
ACK c31cbe7cfefc18123eb85ffb2ce509748435efde, tested on Linux Mint 19.3 both C++11 and C++17 modes. Compiled and passed tests locally.
Tree-SHA512: a4b00776dbceef9c12abbb404c6bcd48f7916ce24c8c7a14116355f64e817578b7fcddbedd5ce435322319d1e4de43429b68553f4d96d970c308fe3e3e59b9d1
|
|
182dbdf0f4b6e6484b0d4588aaefacc75862a99c 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 182dbdf0f4b6e6484b0d4588aaefacc75862a99c
Tree-SHA512: f9ed4bd661f33ff6b2b1150591e860b3c1f44e12b87c35e870d06a7013c4e841ed2bf17b41ad6b18fe471b0b23a4b5e42cf1400637180888e0bc56c254fe0766
|
|
There's also now more than a single option being special cased for
darwin.
|
|
b155fcda5186c59fc4fb2a9eaaf791d132e0ab30 doc: fix typo in configure.ac (fanquake)
20a30922fbf6ba14e250ca649239af115dbbe7b0 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 b155fcda5186c59fc4fb2a9eaaf791d132e0ab30
Tree-SHA512: 5b9567e4a70598a4b0b91956f44ae0d93091db17c84cbf9817dac6cfa992c97d3438a8b1bb66644c74891f2149e44984daed445d22de93ca8858c5b0eabefb40
|
|
s/liner/linker/
|
|
|
|
|
|
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.
|
|
|
|
|
|
This reverts the changes made in merge commit
1b307613604883daea4913a65da30ae073c9dc4d:
This reverts commit b919efadff3d0393f4a8c3c1dc735f7ac5c665bb.
This reverts commit d54f64c6c700be0604190f52c84fc5f1cdd9f02f.
This reverts commit 787f40668dc15980c3d6de028d7950c08175d84a.
This reverts commit d6306466626635e6fee44385e6a688c8dc118eb5.
This reverts commit e6e44eedd56ecaf59f3fabf8e07ab7dee0ddb1b6.
|