e36f802fa4 lint: add C++ code linter (fanquake)
c4be50fea3 remove usage of boost::bind (fanquake)
Pull request description:
`boost::bind` usage was removed in #13743. However a new usage snuck in as
part of 2bc4c3eaf9 (#15225).
ACKs for top commit:
hebasto:
ACK e36f802fa4
practicalswift:
ACK e36f802fa4 -- patch looks correct
Tree-SHA512: 2b0387c5443c184bcbf7df4849db1ed1296ff82c7b4ff0aff18334a400e56a472a972d18234d3866531a088d7a8da64688e58dc9f15daaad4048697c759d55ce
3340dbadd3 Remove -zapwallettxes (Andrew Chow)
Pull request description:
It's not clear what use there is to keeping `-zapwallettxes` given that it's intended usage has been superseded by `abandontransaction`. So this removes it outright.
Alternative to #19700
ACKs for top commit:
meshcollider:
utACK 3340dbadd3
fanquake:
ACK 3340dbadd3 - remaining manpage references will get cleaned up pre-release.
Tree-SHA512: 3e58e1ef6f4f94894d012b93e88baba3fb9c2ad75b8349403f9ce95b80b50b0b4f443cb623cf76c355930db109f491b3442be3aa02972e841450ce52cf545fc8
-zapwallettxes is made a hidden option to inform users that it is
removed and they should be using abandontransaction to do the stuck
transaction thing.
ca185cf5a1 doc: Document differences in bitcoind and bitcoin-qt locale handling (practicalswift)
Pull request description:
Document differences in `bitcoind` and `bitcoin-qt` locale handling.
Since this seems to be the root cause to the locale dependency issues we've seen over the years I thought it was worth documenting :)
Note that 1.) `QLocale` (used by Qt), 2.) C locale (used by locale-sensitive C standard library functions/POSIX functions and some parts of the C++ standard library such as `std::to_string`) and 3.) C++ locale (used by the C++ input/output library) are three separate things. This comment is about the perhaps surprising interference with the C locale (2) that takes place as part of the Qt initialization.
ACKs for top commit:
hebasto:
re-ACK ca185cf5a1
Tree-SHA512: e51c32f3072c506b0029a001d8b108125e1acb4f2b6a48a6be721ddadda9da0ae77a9b39ff33f9d9eebabe2244c1db09e8502e3e7012d7a5d40d98e96da0dc44
31cf68a3ad [util] add RunCommandParseJSON (Sjors Provoost)
c17f54ee53 [ci] use boost::process (Sjors Provoost)
32128ba682 [doc] include Doxygen comments for HAVE_BOOST_PROCESS (Sjors Provoost)
3c84d85f7d [build] msvc: add boost::process (Sjors Provoost)
c47e4bbf0b [build] make boost-process opt-in (Sjors Provoost)
929cda5470 configure: add ax_boost_process (Sjors Provoost)
8314c23d7b [depends] boost: patch unused variable in boost_process (Sjors Provoost)
Pull request description:
Prerequisite for external signer support in #16546. Big picture overview in [this gist](https://gist.github.com/Sjors/29d06728c685e6182828c1ce9b74483d).
This adds a new dependency [boost process](https://github.com/boostorg/process/tree/boost-1.64.0). This is part of Boost since 1.64 which is part of `depends`. Because the minimum Boost version is 1.47, this functionality is skipped for older versions of Boost.
Use `./configure --with-boost-process` to opt in, which checks for the presence of Boost::Process.
We add `UniValue runCommandParseJSON(const std::string& strCommand)` to `system.{h,cpp}` which calls an arbitrary command and processes the JSON returned by it. This is currently only called by the test suite.
~For testing purposes this adds a new regtest-only RPC method `runcommand`, as well as `test/mocks/command.py` used by functional tests.~ (this is no longer the case)
TODO:
- [ ] review boost process in #15440
ACKs for top commit:
achow101:
ACK 31cf68a3ad
hebasto:
re-ACK 31cf68a3ad, only rebased (verified with `git range-diff`) and removed an unintentional tab character since the [previous](https://github.com/bitcoin/bitcoin/pull/15382#pullrequestreview-458371035) review.
meshcollider:
Very light utACK 31cf68a3ad, although I am not very confident with build stuff.
promag:
Code review ACK 31cf68a3ad, don't mind the nit.
ryanofsky:
Code review ACK 31cf68a3ad. I left some comments below that could be ignored or followed up later. The current change is clean and comprehensive.
Tree-SHA512: c506e747014b263606e1f538ed4624a8ad7bcf4e025cb700c12cc5739964e254dc04a2bbb848996b170e2ccec3fbfa4fe9e2b3976b191222cfb82fc3e6ab182d
284a969cc0 Linter to check commit message formatting (Amir Ghorbanian)
Pull request description:
Write linter to check that commit messages have a new line before the body or no body at all. fixes issue #19091.
ACKs for top commit:
troygiorshev:
ACK 284a969cc0 Reviewed, manually tested. Works great!
fjahr:
tested ACK 284a969cc0
adamjonas:
utACK 284a969cc0
Tree-SHA512: fa278f090780b54e4fa6e2967a62b4c1a4da55d112ec1ad6dd7e1181ac490c5c1af0165524b5781b463fdd6d0f79fd3d95b5160184e6eca432ccff1189f77390
78c312c983 Replace current benchmarking framework with nanobench (Martin Ankerl)
Pull request description:
Replace current benchmarking framework with nanobench
This replaces the current benchmarking framework with nanobench [1], an
MIT licensed single-header benchmarking library, of which I am the
autor. This has in my opinion several advantages, especially on Linux:
* fast: Running all benchmarks takes ~6 seconds instead of 4m13s on
an Intel i7-8700 CPU @ 3.20GHz.
* accurate: I ran e.g. the benchmark for SipHash_32b 10 times and
calculate standard deviation / mean = coefficient of variation:
* 0.57% CV for old benchmarking framework
* 0.20% CV for nanobench
So the benchmark results with nanobench seem to vary less than with
the old framework.
* It automatically determines runtime based on clock precision, no need
to specify number of evaluations.
* measure instructions, cycles, branches, instructions per cycle,
branch misses (only Linux, when performance counters are available)
* output in markdown table format.
* Warn about unstable environment (frequency scaling, turbo, ...)
* For better profiling, it is possible to set the environment variable
NANOBENCH_ENDLESS to force endless running of a particular benchmark
without the need to recompile. This makes it to e.g. run "perf top"
and look at hotspots.
Here is an example copy & pasted from the terminal output:
| ns/byte | byte/s | err% | ins/byte | cyc/byte | IPC | bra/byte | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 2.52 | 396,529,415.94 | 0.6% | 25.42 | 8.02 | 3.169 | 0.06 | 0.0% | 0.03 | `bench/crypto_hash.cpp RIPEMD160`
| 1.87 | 535,161,444.83 | 0.3% | 21.36 | 5.95 | 3.589 | 0.06 | 0.0% | 0.02 | `bench/crypto_hash.cpp SHA1`
| 3.22 | 310,344,174.79 | 1.1% | 36.80 | 10.22 | 3.601 | 0.09 | 0.0% | 0.04 | `bench/crypto_hash.cpp SHA256`
| 2.01 | 496,375,796.23 | 0.0% | 18.72 | 6.43 | 2.911 | 0.01 | 1.0% | 0.00 | `bench/crypto_hash.cpp SHA256D64_1024`
| 7.23 | 138,263,519.35 | 0.1% | 82.66 | 23.11 | 3.577 | 1.63 | 0.1% | 0.00 | `bench/crypto_hash.cpp SHA256_32b`
| 3.04 | 328,780,166.40 | 0.3% | 35.82 | 9.69 | 3.696 | 0.03 | 0.0% | 0.03 | `bench/crypto_hash.cpp SHA512`
[1] https://github.com/martinus/nanobench
ACKs for top commit:
laanwj:
ACK 78c312c983
Tree-SHA512: 9e18770b18b6f95a7d0105a4a5497d31cf4eb5efe6574f4482f6f1b4c88d7e0946b9a4a1e9e8e6ecbf41a3f2d7571240677dcb45af29a6f0584e89b25f32e49e
Write linter to check that commit messages have a new line before the body or no body at all.
reference: gist.github.com/agnivade/67b42d664ece2d4210c7
Fixes issue #19091.
a4a3fc4cd2 doc: improve subtree check instructions (Sjors Provoost)
Pull request description:
Running `git-subtree-check.sh` requires adding the subtree repository as a remote. I learned that several years ago and then forgot again.
This PR also improves the error message if the subtree commit can't be found.
ACKs for top commit:
laanwj:
ACK a4a3fc4cd2
fanquake:
ACK a4a3fc4cd2 - this looks ok.
Tree-SHA512: 959bd923726c172d17f9f97f8a56988bf2df5a94d3131e5152a66150b941394cee9e82fdc6b86e09c0ba91d123a496599f07ca454212168d8d301738394c12c8
fab80fef61 refactor: Remove unused EnsureChainman (MarcoFalke)
fa34587f1c scripted-diff: Replace EnsureChainman with Assert in unit tests (MarcoFalke)
fa6ef701ad util: Add Assert identity function (MarcoFalke)
fa457fbd33 move-only: Move NDEBUG compile time check to util/check (MarcoFalke)
Pull request description:
The utility function is primarily useful to dereference pointer types, which are known to be not null at that time.
For example, the ArgsManager is known to exist when the wallets are started: https://github.com/bitcoin/bitcoin/pull/18923/files#diff-fdb2a1a1d8bc790fcddeb6cf5a42ac55R503 . Instead of silently relying on that assumption, `Assert` can be used to abort the program and avoid UB should the assumption ever be violated.
ACKs for top commit:
promag:
Tested ACK fab80fef61.
ryanofsky:
Code review ACK fab80fef61
Tree-SHA512: 830fba10152ba17d47c4dd42809c7e26f9fe6d38e17a2d5b3f054fd644a5c4c9841286ac421ec9bb28cea9f5faeb659740fcf00de6cc589d423fee7694c42d16
54b5eb2b14 tests: Add std::locale::global to list of locale dependent functions in lint-locale-dependence.sh (practicalswift)
Pull request description:
Add `std::locale::global` to list of locale dependent functions in `lint-locale-dependence.sh`.
We currently flag `setlocale(...)` as locale dependent, but prior to this commit we didn't flag
`std::locale::global(...)` as such.
In addition to setting the global C++ locale `std::locale::global(...)` also does the equivalent of `std::setlocale(LC_ALL, ...);`.
Thus the functionality of `std::locale::global(...)` is a superset of `setlocale(...)` :)
ACKs for top commit:
MarcoFalke:
ACK 54b5eb2b14, fine with me
Tree-SHA512: bcf2f1c765add6ed09c3debca968b75eeea81602503f109c0f76ec98635911d453f4834a39e741703c3d470f123178e8952191a9b1a3429394b99c07765dcf1f
optional_last_value, which does not throw, has replaced optional_value as
boost's default combiner. Besides being better supported, it also doesn't
trigger gcc's -Wmaybe-unitialized warning, presumably because exceptions no
longer bubble-up out of signals:
```bash
boost/signals2/last_value.hpp:54:36: warning: '*((void*)& value +1)' may be used uninitialized in this function [-Wmaybe-uninitialized]
if(value) return value.get();
```
The change in default happened in Boost 1.39.0 (along with the
introduction of the signals 2 library. More information is available here:
https://www.boost.org/doc/libs/1_73_0/doc/html/signals2/rationale.html#id-1.3.36.9.4
and here:
https://www.boost.org/doc/libs/1_73_0/doc/html/boost/signals2/optional_last_value.html
Co-authored-by: fanquake <fanquake@gmail.com>
Updates Python linters, spellchecking, and ShellCheck versions. The PR links are updated for
the dependency versions in test/README.md. ShellCheck SC2230 removed to align with with new
behaviour in v0.7.1.
Fixes#19346.
The utility is primarily useful to dereference pointer types, which are
known to be not null at that time.
For example, the ArgsManager is known to exist when the wallets are
started. Instead of silently relying on that assumption, Assert can be
used to abort the program and avoid UB should the assumption ever be
violated.
This replaces the current benchmarking framework with nanobench [1], an
MIT licensed single-header benchmarking library, of which I am the
autor. This has in my opinion several advantages, especially on Linux:
* fast: Running all benchmarks takes ~6 seconds instead of 4m13s on
an Intel i7-8700 CPU @ 3.20GHz.
* accurate: I ran e.g. the benchmark for SipHash_32b 10 times and
calculate standard deviation / mean = coefficient of variation:
* 0.57% CV for old benchmarking framework
* 0.20% CV for nanobench
So the benchmark results with nanobench seem to vary less than with
the old framework.
* It automatically determines runtime based on clock precision, no need
to specify number of evaluations.
* measure instructions, cycles, branches, instructions per cycle,
branch misses (only Linux, when performance counters are available)
* output in markdown table format.
* Warn about unstable environment (frequency scaling, turbo, ...)
* For better profiling, it is possible to set the environment variable
NANOBENCH_ENDLESS to force endless running of a particular benchmark
without the need to recompile. This makes it to e.g. run "perf top"
and look at hotspots.
Here is an example copy & pasted from the terminal output:
| ns/byte | byte/s | err% | ins/byte | cyc/byte | IPC | bra/byte | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 2.52 | 396,529,415.94 | 0.6% | 25.42 | 8.02 | 3.169 | 0.06 | 0.0% | 0.03 | `bench/crypto_hash.cpp RIPEMD160`
| 1.87 | 535,161,444.83 | 0.3% | 21.36 | 5.95 | 3.589 | 0.06 | 0.0% | 0.02 | `bench/crypto_hash.cpp SHA1`
| 3.22 | 310,344,174.79 | 1.1% | 36.80 | 10.22 | 3.601 | 0.09 | 0.0% | 0.04 | `bench/crypto_hash.cpp SHA256`
| 2.01 | 496,375,796.23 | 0.0% | 18.72 | 6.43 | 2.911 | 0.01 | 1.0% | 0.00 | `bench/crypto_hash.cpp SHA256D64_1024`
| 7.23 | 138,263,519.35 | 0.1% | 82.66 | 23.11 | 3.577 | 1.63 | 0.1% | 0.00 | `bench/crypto_hash.cpp SHA256_32b`
| 3.04 | 328,780,166.40 | 0.3% | 35.82 | 9.69 | 3.696 | 0.03 | 0.0% | 0.03 | `bench/crypto_hash.cpp SHA512`
[1] https://github.com/martinus/nanobench
* Adds support for asymptotes
This adds support to calculate asymptotic complexity of a benchmark.
This is similar to #17375, but currently only one asymptote is
supported, and I have added support in the benchmark `ComplexMemPool`
as an example.
Usage is e.g. like this:
```
./bench_bitcoin -filter=ComplexMemPool -asymptote=25,50,100,200,400,600,800
```
This runs the benchmark `ComplexMemPool` several times but with
different complexityN settings. The benchmark can extract that number
and use it accordingly. Here, it's used for `childTxs`. The output is
this:
| complexityN | ns/op | op/s | err% | ins/op | cyc/op | IPC | total | benchmark
|------------:|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|----------:|:----------
| 25 | 1,064,241.00 | 939.64 | 1.4% | 3,960,279.00 | 2,829,708.00 | 1.400 | 0.01 | `ComplexMemPool`
| 50 | 1,579,530.00 | 633.10 | 1.0% | 6,231,810.00 | 4,412,674.00 | 1.412 | 0.02 | `ComplexMemPool`
| 100 | 4,022,774.00 | 248.58 | 0.6% | 16,544,406.00 | 11,889,535.00 | 1.392 | 0.04 | `ComplexMemPool`
| 200 | 15,390,986.00 | 64.97 | 0.2% | 63,904,254.00 | 47,731,705.00 | 1.339 | 0.17 | `ComplexMemPool`
| 400 | 69,394,711.00 | 14.41 | 0.1% | 272,602,461.00 | 219,014,691.00 | 1.245 | 0.76 | `ComplexMemPool`
| 600 | 168,977,165.00 | 5.92 | 0.1% | 639,108,082.00 | 535,316,887.00 | 1.194 | 1.86 | `ComplexMemPool`
| 800 | 310,109,077.00 | 3.22 | 0.1% |1,149,134,246.00 | 984,620,812.00 | 1.167 | 3.41 | `ComplexMemPool`
| coefficient | err% | complexity
|--------------:|-------:|------------
| 4.78486e-07 | 4.5% | O(n^2)
| 6.38557e-10 | 21.7% | O(n^3)
| 3.42338e-05 | 38.0% | O(n log n)
| 0.000313914 | 46.9% | O(n)
| 0.0129823 | 114.4% | O(log n)
| 0.0815055 | 133.8% | O(1)
The best fitting curve is O(n^2), so the algorithm seems to scale
quadratic with `childTxs` in the range 25 to 800.
Support for type hints was introduced in Python 3.5. Type hints make it easier to read and review code in my opinion. Also an IDE may discover a potential bug sooner. Yet, as PEP 484 says: "It should also be emphasized that Python will remain a dynamically typed language, and the authors have no desire to ever make type hints mandatory, even by convention."
Mypy is used in lint-python.sh to do the type checking. The package is standard so there is little chance that it will be abandoned. Mypy checks that type hints in source code are correct when they are not, it fails with an error.
Useful resources:
* https://docs.python.org/3.5/library/typing.html
* https://www.python.org/dev/peps/pep-0484/
2aa48edec0 refactor: Drop unused ${WRAP_DIR}/${HOST} directory (Hennadii Stepanov)
1362be0447 build: Drop make dist in gitian builds (Hennadii Stepanov)
Pull request description:
After the merge of #18331, the packaged source tarball is created by `git archive`, but the binaries are built from another one which is made by `make dist`.
With this PR the only source tarball, created by `git archive`, is used both for binaries building and for packaging to users.
Close#16588.
Close#18547.
As a good side-effect, #18349 becomes redundant.
**Change in behavior**
The following variables 1b151e3ffc/configure.ac (L2-L6)
are no longer used for naming of directories and tarballs.
Instead of them the gitian descriptors use a git tag (if available) or a commit hash.
---
Also a small refactor commit picked from #18404.
ACKs for top commit:
dongcarl:
ACK 2aa48edec0
MarcoFalke:
ACK 2aa48edec0
fanquake:
ACK 2aa48edec0 - I've had a quick look over this, and don't want to block merging if this actually gets as closer to finally having this all sorted out. Obviously we've still got #18741, and after speaking to Carl this morning, there will likely be even more changes after that (not Guix specific).
Tree-SHA512: d3b16f87e48d1790a3264940c28acd5d881bfd10f3ce94fb0c8a6af76d8039289d01e0cd4972adac49ae24362857251f6c1e5e09e3e9fbf636c10708b4015a7c
We currently flag `setlocale(...)` as locale dependent, but prior to this commit we didn't flag
`std::locale::global(...)` as such.
In addition to setting the global C++ locale `std::locale::global(...)` also does the equivalent
of `std::setlocale(LC_ALL, ...);`.
Thus the functionality of `std::locale::global(...)` is a superset of `setlocale(...)` :)
70a6b529f3 lint-cppcheck: Remove -DHAVE_WORKING_BOOST_SLEEP_FOR (Anthony Towns)
294937b39d scheduler_tests: re-enable mockforward test (Anthony Towns)
cea19f6859 Drop unused reverselock.h (Anthony Towns)
d0ebd93270 scheduler: switch from boost to std (Anthony Towns)
b9c4260127 sync.h: add REVERSE_LOCK (Anthony Towns)
306f71b4eb scheduler: don't rely on boost interrupt on shutdown (Anthony Towns)
Pull request description:
Replacing boost functionality with C++11 stuff.
Motivated by #18227, but should stand alone. Changing from `boost::condition_var` to `std::condition_var` means `threadGroup.interrupt_all` isn't enough to interrupt `serviceQueue` anymore, so that means calling `stop()` before `join_all()` is needed. And the existing reverselock.h code doesn't work with sync.h's DebugLock code (because the reversed lock won't be removed from `g_lockstack` which then leads to incorrect potential deadlock warnings), so I've replaced that with a dedicated class and macro that's aware of our debug lock behaviour.
Fixes#16027, Fixes#14200, Fixes#18227
ACKs for top commit:
laanwj:
ACK 70a6b529f3
Tree-SHA512: d1da13adeabcf9186d114e2dad9a4fdbe2e440f7afbccde0c13dfbaf464efcd850b69d3371c5bf8b179d7ceb9d81f4af3cc22960b90834e41eaaf6d52ef7d331
Changes from boost::chrono to std::chrono, boost::condition_var to
std::condition_var, boost::mutex to sync.h Mutex, and reverselock.h to
sync.h REVERSE_LOCK. Also adds threadsafety annotations to CScheduler
members.
2a95c7c956 ci: Check for submodules (Emil Engler)
Pull request description:
See #18019.
The current solution looks like this (I also tested with multiple submodules):
```
These submodules were found, delete them:
355a5a310019659d9bf6818d2fd66fbb214dfed7 curl (curl-7_68_0-108-g355a5a310)
```
The submodule example command was `git submodule add https://github.com/curl/curl.git curl`
ACKs for top commit:
laanwj:
ACK 2a95c7c956
Tree-SHA512: 64bf388123f0a88d12e3e41ff29bc190339377a0615c35dc3f2700bb7773470a8fa426e0ff57188a60ed88bded39f75082ff0b73118651ff403b163422395005
677fb8e923 test: Add ubsan surpression for crc32c (Wladimir J. van der Laan)
8e68bb1dde build: Disable msvc warning 4722 for leveldb build (Aaron Clauson)
be23949765 build: MSVC changes for leveldb update (Aaron Clauson)
9ebdf04757 build: CRC32C build system integration (Wladimir J. van der Laan)
402252a808 build: Add LCOV exception for crc32c (Wladimir J. van der Laan)
3a037d0067 test: Add crc32c exception to various linters and generation scripts (Wladimir J. van der Laan)
84ff1b2076 test: Add crc32c to subtree check linter (Wladimir J. van der Laan)
7cf13a5134 doc: Add crc32c subtree to developer notes (Wladimir J. van der Laan)
24d02a9ac0 build: Update build system for new leveldb (Wladimir J. van der Laan)
2e1819311a Squashed 'src/crc32c/' content from commit 224988680f7673cd7c769963d4035cb315aa3388 (Wladimir J. van der Laan)
66480821b3 Squashed 'src/leveldb/' changes from f545dfabff4c2e9836efed094dba99a34fbc6b88..f8ae182c1e5176d12e816fb2217ae33a5472fdd7 (Wladimir J. van der Laan)
Pull request description:
This updates leveldb to currently newest upstream commit 0c40829872:
- CRC32C hardware acceleration is now an external library [crc32c](https://github.com/google/crc32c). This adds acceleration on ARM, and should be faster on x86 because of using prefetch. It also makes it easy to support similar instruction sets on other platforms in the future.
- Thread handling uses C++11, instead of platform specific code.
- Native windows environment was added. No need to maintain our own hacky one, anymore.
- Upstream now builds using CMake. This doesn't mean we need to use that (phew), but internal configuration changed to a a series of checks, instead of OS profiles. This means the blanket error "Cannot build leveldb for $host. Please file a bug report' is removed.
All changes: a53934a3ae...0c40829872
Pretty much all our changes have been subsumed by upstream, so we figured it was cleaner to start over with a new branch from upstream with the still-relevant patches applied: https://github.com/bitcoin-core/leveldb/tree/bitcoin-fork-new
There's quite some testing to be done (see below). See https://github.com/bitcoin-core/leveldb/issues/25 and https://github.com/bitcoin-core/leveldb/pull/26 for more history and context.
TODO:
- [x] Subtree `crc32c`
- [x] Make linters happy about crc32 subtree
- [x] Integrate `crc32c` library into build system
- [x] MSVC build system
ACKs for top commit:
sipa:
ACK 677fb8e923
Tree-SHA512: 37ee92a750e053e924bc4626b12bb3fd81faa9f8c5ebaa343931fee810c45ba05aa6051fdea82535fa351bf2be7297801b98af9469865fc5ead771650a5d6240
cb8a86d9f9 gui: Remove WalletView and BitcoinGUI circular dependency (João Barbosa)
ac3d10777d gui: Add transactionClicked and coinsSent signals to WalletView (João Barbosa)
Pull request description:
Essentially moves the code in `WalletView::setBitcoinGUI` to the only caller. Two new signals are added beforehand in the first commit so that the connections in `WalletFrame` are all from the wallet view.
ACKs for top commit:
hebasto:
ACK cb8a86d9f9, tested on Linux Mint 19.3.
jonasschnelli:
utACK cb8a86d9f9
Tree-SHA512: 250316cd3689e51c8cded9ccd75963c836dcafa6db25d684f2aa691dea9738895f9140793e0f925784909e39f8257f7e1c7d611e8bd6d6634e1a50333f4ddb1e
cc668d06fb tests: Add fuzzing harness for strprintf(...) (practicalswift)
ccc3c76e2b tests: Add fuzzer strprintf to FUZZERS_MISSING_CORPORA (temporarily) (practicalswift)
6ef04912af tests: Update FuzzedDataProvider.h from upstream (LLVM) (practicalswift)
Pull request description:
Add fuzzing harness for `strprintf(…)`.
Update `FuzzedDataProvider.h`.
Avoid hitting some issues in tinyformat (reported upstreams in https://github.com/c42f/tinyformat/issues/70).
---
Found issues in tinyformat:
**Issue 1.** The following causes a signed integer overflow followed by an allocation of 9 GB of RAM (or an OOM in memory constrained environments):
```
strprintf("%.777777700000000$", 1.0);
```
**Issue 2.** The following causes a stack overflow:
```
strprintf("%987654321000000:", 1);
```
**Issue 3.** The following causes a stack overflow:
```
strprintf("%1$*1$*", -11111111);
```
**Issue 4.** The following causes a `NULL` pointer dereference:
```
strprintf("%.1s", (char *)nullptr);
```
**Issue 5.** The following causes a float cast overflow:
```
strprintf("%c", -1000.0);
```
**Issue 6.** The following causes a float cast overflow followed by an invalid integer negation:
```
strprintf("%*", std::numeric_limits<double>::lowest());
```
Top commit has no ACKs.
Tree-SHA512: 9b765559281470f4983eb5aeca94bab1b15ec9837c0ee01a20f4348e9335e4ee4e4fecbd7a1a5a8ac96aabe0f9eeb597b8fc9a2c8faf1bab386e8225d5cdbc18
fac86ac7b3 scripted-diff: Add missed copyright headers (Hennadii Stepanov)
6fde9d5e47 script: Update EXLUDE list in copyright_header.py (Hennadii Stepanov)
1998152f15 script: Add empty line after C++ copyright (Hennadii Stepanov)
071f2fc204 script: Add ability to insert copyright to *.sh (Hennadii Stepanov)
Pull request description:
This PR improves `contrib/devtools/copyright_header.py` script and adds copyright headers to the files in `src` and `test` directories with two exceptions:
- [`src/reverse_iterator.h`](https://github.com/bitcoin/bitcoin/blob/master/src/reverse_iterator.h) (added to exceptions)
- [`src/test/fuzz/FuzzedDataProvider.h`](https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/FuzzedDataProvider.h) (added to exceptions)
On master 5622d8f315:
```
$ ./contrib/devtools/copyright_header.py report . | grep zero
25 with zero copyrights
```
With this PR:
```
$ ./contrib/devtools/copyright_header.py report . | grep zero
2 with zero copyrights
```
~I am uncertain about our copyright policy with `build_msvc` and `contrib` directories content, so they are out of scope of this PR.~
ACKs for top commit:
MarcoFalke:
ACK fac86ac7b3
Tree-SHA512: d7832c4a7a1a3b7806119775b40ec35d7982f49ff0e6199b8cee4c0e0a36e68d51728b6ee9924b1c161df4bc6105bd93391b79d42914357fa522f499cb113fa8
