These checks were added in #4339, (see also #4081), to test
our back-compat stubs, however, those stubs no-longer exist (#22930),
meaning that these checks are now just testing some specific standard
library behaviour, without a particular rationale, or reason, compared
to any other standard library functions we use.
There has also been some discussion about the sanity checks in the
context of the libbitcoinkernel refactoring, see
https://github.com/bitcoin/bitcoin/pull/25065#discussion_r880668218.
Removing the checks removes the need to worry about atleast the glibcxx
checks.
Also remove the list of check from the doc in init.h, because it is
incomplete, and anyone who wants to know what checks are included can
look at the function.
Add missing includes.
Swap C headers for their C++ counterparts.
Remove pointless / unmaintainable include comments. This is even more the case
when we are actually using IWYU, as if anyone wants to see the comments they can
just get IWYU to generate them.
In glib 2.13 memcpy was changed such that the way it copied bytes was reversed.
This caused all sorts of issues for existing software, which depended on the
existing behavior (when they should have been using memmove). See:
https://sourceware.org/bugzilla/show_bug.cgi?id=12518https://bugzilla.redhat.com/show_bug.cgi?id=638477
Now that we require glibc 2.17+ (#17538), we should be well clear of having to
maintain our memcpy -> memmove aliasing, which was introduced in #4339.
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
```
This was originally added in #9366 to fix the gui build, as
Protobuf would also define these macros. Now that we're no-longer
using Protobuf, remove the additional check.
SmartOS FD_ZERO is implemented in a way that requires
an external declaration of memcpy. We can not simply
include cstring in the existing file because
sanity_test_memcpy is attempting to replace memcpy, but we can do
so here, now that the fdelt test is split out.
While this isn't a supported build configuration, some build
systems need to build without going through our autotools steps,
so defaulting to something sane may make it easier to build.
Specifically, this fixes the inability to build
rust-bitcoinconsensus on some non-x86 platforms. It needs to build
without our autotools/configure steps to ensure correct compile
args are passed from the rust build system to gcc. Converting the
args from the rust build system to gcc would be a lot of
unmaintainable work.
ac1cf8d Trivial: Improve #endif comments (danra)
Pull request description:
Improve the #endif comments for the '#if HAVE_DECL_BSWAP_XX == 0' preprocessor conditions, so each shows the full condition which it closes.
Tree-SHA512: f533311fa48cb2f46f6490b6c965ad5f8861dcfad70c56d70e31fa989b422880c78b2dd6f24f648b19d3a22f767606e0de5cf1cb71445012b42c97ac2149295e
eefc2f3 Move local include to before system includes (danra)
Pull request description:
Prevents accidental missing includes and hidden dependencies in the local file.
Tree-SHA512: 466b9dd53c596980fdbcccf1dfd8f34eb7ec5b32323ccb635e5705efcedc81af8fbe155ac57b9a2fc5c1f516489e940d1762b3508ded1fb54e187219bb9f75e6
Simplify bswap_16 implementation on platforms which don't already have it defined.
This has no effect on the generated assembly; it just simplifies the source code.
Defers to pre-defined version if found (e.g. protobuf). For protobuf case, the definitions are identical and thus include order should not affect results.
Three categories of modifications:
1)
1 instance of 'The Bitcoin Core developers \n',
1 instance of 'the Bitcoin Core developers\n',
3 instances of 'Bitcoin Core Developers\n', and
12 instances of 'The Bitcoin developers\n'
are made uniform with the 443 instances of 'The Bitcoin Core developers\n'
2)
3 instances of 'BitPay, Inc\.\n' are made uniform with the other 6
instances of 'BitPay Inc\.\n'
3)
4 instances where there was no '(c)' between the 'Copyright' and the year
where it deviates from the style of the local directory.
