aboutsummaryrefslogtreecommitdiff
path: root/configure.ac
AgeCommit message (Collapse)Author
2020-08-18Merge #19015: build: Enable some commonly enabled compiler diagnosticsfanquake
2f8a4c9a06ace680b3dff7cd7d5e33204fe45909 build: Enable some commonly enabled compiler diagnostics (practicalswift) Pull request description: Enable some commonly enabled compiler diagnostics as discussed in #17344. | Compiler diagnostic | no# of emitted unique GCC warnings in `master` | no# of emitted unique Clang warnings in `master` | | ------------- | ------------- | ------------- | | `-Wduplicated-branches`: Warn if `if`/`else` branches have duplicated code | 0 | Not supported | | `-Wduplicated-cond`: Warn if `if`/`else` chain has duplicated conditions | 0 | Not supported | | `-Wlogical-op`: Warn about logical operations being used where bitwise were probably wanted | 0 | Not supported | | `-Woverloaded-virtual`: Warn if you overload (not `override`) a virtual function | 0 | 0 | | ~~`-Wunused-member-function`: Warn on unused member function~~ | Not supported | 2 | | ~~`-Wunused-template`: Warn on unused template~~ | Not supported | 1 | There is a large overlap between this list and [Jason Turner's list of recommended compiler diagnostics in the Collaborative Collection of C++ Best Practices (`cppbestpractices`) project](https://github.com/lefticus/cppbestpractices/blob/master/02-Use_the_Tools_Available.md#gcc--clang). There is also an overlap with the recommendations given in the [C++ Core Guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) (with editors Bjarne Stroustrup and Herb Sutter). Closes #17344. ACKs for top commit: jonatack: ACK 2f8a4c9a06ace6 no warnings for me with these locally on debian 5.7.10-1 (2020-07-26) x86_64 with gcc 10 and clang 12 fanquake: ACK 2f8a4c9a06ace680b3dff7cd7d5e33204fe45909 - no-longer seeing any obvious issues with doing this. hebasto: ACK 2f8a4c9a06ace680b3dff7cd7d5e33204fe45909, no new warnings in Travis jobs. Tree-SHA512: f669ea22b31263a555f999eff6a9d65750662e95831b188c3192a2cf0127fb7b5136deb762a6b0b7bbdfb0dc6a40caf48251a62b164fffb81dd562bdd15ec3c8
2020-08-14build: Add Werror=range-loop-analysisMarcoFalke
2020-08-11build: Enable some commonly enabled compiler diagnosticspracticalswift
2020-08-08build, test: Add support for llvm-covHennadii Stepanov
2020-08-06Merge #19667: build: set minimum required Boost to 1.58.0fanquake
70452a070b3b6d4d650a948b3337bc7b8bb2c3c3 build: set minimum required Boost to 1.58 (fanquake) Pull request description: Any systems which only have an older installable Boost can use depends. 1.58.0 retains compatibility with the packages [installable on Ubuntu 16.04](https://packages.ubuntu.com/xenial/libboost-dev). The projects usage of Boost wont be going away any time soon, if ever (i.e #15382), and our usage of the test framework. Fixes: #19506 ACKs for top commit: practicalswift: ACK 70452a070b3b6d4d650a948b3337bc7b8bb2c3c3 -- patch looks correct laanwj: ACK 70452a070b3b6d4d650a948b3337bc7b8bb2c3c3 hebasto: ACK 70452a070b3b6d4d650a948b3337bc7b8bb2c3c3, tested on Linux Mint 20 (x86_64). Tree-SHA512: d290415e3c70a394b3d7659c0480a35b4082bdce8d48b1c64a0025f7ad6e21567b4dc85813869513ad246d27f950706930410587c11c1aa3693ae6245084765c
2020-08-05build: set minimum required Boost to 1.58fanquake
Any systems which only have an older install-able Boost can use depends. Fixes: #19506
2020-07-31[build] make boost-process opt-inSjors Provoost
2020-07-31configure: add ax_boost_processSjors Provoost
Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
2020-07-28build: add -Wl,-z,separate-code to hardening flagsfanquake
This flag was added to binutils/ld in the 2.30 release, see commit c11c786f0b45617bb8807ab6a57220d5ff50e414: > The new "-z separate-code" option will generate separate code LOAD segment which must be in wholly disjoint pages from any other data. It was made the default for Linux/x86 targets in the 2.31 release, see commit f6aec96dce1ddbd8961a3aa8a2925db2021719bb: > This patch adds --enable-separate-code to ld configure to turn on -z separate-code by default and enables it by default for Linux/x86. This avoids mixing code pages with data to improve cache performance as well as security. > To reduce x86-64 executable and shared object sizes, the maximum page size is reduced from 2MB to 4KB when -z separate-code is turned on by default. Note: -z max-page-size= can be used to set the maximum page size. > We compared SPEC CPU 2017 performance before and after this change on Skylake server. There are no any significant performance changes. Everything is mostly below +/-1%. Support was also added to LLVMs lld: https://reviews.llvm.org/D64903, however there is remains off by default. There were concerns about an increase in binary size, however in our case, the increase (1 page worth of bytes) would seem negligible, given we are shipping a multi-megabyte binary, which then downloads 100's of GBs of data. Also note that most recent versions of distros are shipping a new enough version of binutils that this is available and/or on by default (assuming the distro has not turned it off, I haven't checked everywhere): CentOS 8: 2.30 Debian Buster 2.31.1 Fedora 29: 2.31.1 FreeBSD: 2.33 GNU Guix: 2.33 / 2.34 Ubuntu 18.04: 2.30 Related threads / discussion: https://bugzilla.redhat.com/show_bug.cgi?id=1623218
2020-07-22build: call AC_PATH_TOOL for dsymutil in macOS cross-compilefanquake
While testing #19530 I noticed that we couldn't call dsymutil after LTO: ```bash ../libtool: line 10643: x86_64-apple-darwin16-dsymutil: command not found ``` This updates configure to call `AC_PATH_TOOL` so that we end up with the full path to dsymutil, similar to `otool` and `install_name_tool`, ie: `/bitcoin/depends/x86_64-apple-darwin16/share/../native/bin/x86_64-apple-darwin16-otool`.
2020-07-15build: fix -Wformat-security check when compiling with GCCfanquake
Debian GCC ignores -Wformat-security, without -Wformat, which means when we test for it, it currently fails: ```bash checking whether C++ compiler accepts -Wformat-security... no ... configure:15907: checking whether C++ compiler accepts -Wformat-security configure:15926: g++ -std=c++11 -c -g -O2 -Werror -Wformat-security conftest.cpp >&5 cc1plus: error: '-Wformat-security' ignored without '-Wformat' [-Werror=format-security] cc1plus: all warnings being treated as errors ``` Fix this by just combining the -Wformat and -Wformat-security checks together.
2020-07-03Merge #18307: build: Require pkg-config for all of the hostsfanquake
92bc268e4af4ebcbde08567ea00e019ac509a769 build: Detect missed pkg-config early (Hennadii Stepanov) 1739eb23d8a6d272e70f95342323b6fe48b8eb6c build: Drop unused use_pkgconfig variable (Hennadii Stepanov) a661449a2eeaf88efda36b6a84084dcbfe5b24eb build: Drop use_pkgconfig check for libmultiprocess check (Hennadii Stepanov) 90b95e7929463d6127c1b24fe1bf457d750a045c build: Drop dead non-pkg-config code for libevent check (Hennadii Stepanov) 44a14afbb889633a6c9a322a5aeca2e1b2cbdbd8 build: Drop dead non-pkg-config code for qrencode check (Hennadii Stepanov) 10cbae0c399302b0f8b1aa847c4246ba60bf25ee build: Drop dead non-pkg-config code for ZMQ check (Hennadii Stepanov) 06cfc9cadf7c5dd43147e6525a348d5f2d299422 build: Fix indentation in UNIVALUE check (Hennadii Stepanov) 6fd2118e777d11cbc81a45313d1a7d6400e34f3f build: Drop dead non-pkg-config code for UNIVALUE check (Hennadii Stepanov) e9edbe4dbd8c24a779de7d92e5f10c870aab5511 build: Always use pkg-config (Hennadii Stepanov) 9e2e753b0605c8cd826381a362f0c7de56eea81f build: Always define ZMQ_STATIC for MinGW (Hennadii Stepanov) Pull request description: This PR: - is based on #18297 (already merged) - drops all of the non-pkg-config paths from the `configure` script Ref: #17768 ACKs for top commit: fanquake: ACK 92bc268e4af4ebcbde08567ea00e019ac509a769. I re-gitian-built. There are a couple follow-ups that I'll PR shortly. Thanks for addressing my feedback above. I took too long to get back to this. laanwj: ACK 92bc268e4af4ebcbde08567ea00e019ac509a769 Tree-SHA512: 83c2d9cf03518867a1ebf7e26a8fc5b6dd8962ef983fe0d84e0c7eb74717f4c36a834da02faf0e503ffd87167005351671cf040c0d4ddae57ee152a6ff84012b
2020-07-01Merge #19257: build: remove BIP70 configure optionWladimir J. van der Laan
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
2020-06-29Merge #19403: build: improve __builtin_clz* detectionWladimir J. van der Laan
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
2020-06-29build: improve builtin_clz* detectionfanquake
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.
2020-06-23build: Fix search for brew-installed BDB 4 on OS XGlenn Willen
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.
2020-06-23build: Detect missed pkg-config earlyHennadii Stepanov
2020-06-23build: Drop unused use_pkgconfig variableHennadii Stepanov
2020-06-23build: Drop use_pkgconfig check for libmultiprocess checkHennadii Stepanov
2020-06-22build: Drop dead non-pkg-config code for libevent checkHennadii Stepanov
2020-06-17build: don't warn when doxygen isn't foundfanquake
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.
2020-06-16tests: run test-security-check.py in CIfanquake
2020-06-13build: Drop dead non-pkg-config code for qrencode checkHennadii Stepanov
2020-06-13build: Drop dead non-pkg-config code for ZMQ checkHennadii Stepanov
2020-06-13build: Fix indentation in UNIVALUE checkHennadii Stepanov
2020-06-13build: Drop dead non-pkg-config code for UNIVALUE checkHennadii Stepanov
2020-06-13build: Always use pkg-configHennadii Stepanov
2020-06-13build: Always define ZMQ_STATIC for MinGWHennadii Stepanov
2020-06-13Merge #18297: build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts ↵fanquake
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
2020-06-12build: remove BIP70 configure optionfanquake
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.
2020-06-10Drop --disable-jni from libsecp256k1 configure optionsPieter Wuille
2020-06-06build: Add -Wshadow-fieldMarcoFalke
2020-06-05build: turn on --enable-c++17 by --enable-fuzzVasil Dimov
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-04Merge #19131: refactor: Fix unreachable code in init arg checksWladimir J. van der Laan
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
2020-06-03build: improved output of configure for build OSsachinkm77
2020-06-02build: Enable unreachable-code-loop-incrementJonathan Schoeller
Enable unreachable-code-loop-increment and treat as error. refs: #19015
2020-06-01Merge #19072: doc: Expand section on Getting Startedfanquake
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
2020-05-28build: Only allow ASCII identifiersWladimir J. van der Laan
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-28build: Replace -Wthread-safety-analysis with broader -Wthread-safetyHennadii Stepanov
2020-05-27doc: Fix unrelated typos reported by codespellMarcoFalke
2020-05-21Merge #18677: Multiprocess build supportfanquake
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
2020-05-14build: enforce minimum required Windows version (7)fanquake
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-13Merge #18887: build: enable -Werror=gnuWladimir J. van der Laan
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
2020-05-13Merge #18862: Remove fdelt_chk back-compat code and sanity checkWladimir J. van der Laan
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
2020-05-13Merge #18738: build: Suppress -Wdeprecated-copy warningsfanquake
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
2020-05-13Merge #16710: build: Enable -Wsuggest-override if availablefanquake
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
2020-05-12build: Enable -Wsuggest-overrideHennadii Stepanov
2020-05-12build: multiprocess autotools changesRussell Yanofsky
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-12Merge #18928: build: don't pass -w when building for Windowsfanquake
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
2020-05-10build: don't pass -w when building for Windowsfanquake
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.