Merge #18862: Remove fdelt_chk back-compat code and sanity check

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

4 files changed, 0 insertions, 48 deletions

diff --git a/src/Makefile.am b/src/Makefile.am
index 627df97cad..13812de0e7 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -496,7 +496,6 @@ libbitcoin_util_a_SOURCES = \
   support/lockedpool.cpp \
   chainparamsbase.cpp \
   clientversion.cpp \
-  compat/glibc_sanity_fdelt.cpp \
   compat/glibc_sanity.cpp \
   compat/glibcxx_sanity.cpp \
   compat/strnlen.cpp \
diff --git a/src/compat/glibc_compat.cpp b/src/compat/glibc_compat.cpp
index 4de4fd7f45..d17de33e86 100644
--- a/src/compat/glibc_compat.cpp
+++ b/src/compat/glibc_compat.cpp
@@ -9,10 +9,6 @@
 #include <cstddef>
 #include <cstdint>
 
-#if defined(HAVE_SYS_SELECT_H)
-#include <sys/select.h>
-#endif
-
 // Prior to GLIBC_2.14, memcpy was aliased to memmove.
 extern "C" void* memmove(void* a, const void* b, size_t c);
 extern "C" void* memcpy(void* a, const void* b, size_t c)
@@ -20,15 +16,6 @@ extern "C" void* memcpy(void* a, const void* b, size_t c)
     return memmove(a, b, c);
 }
 
-extern "C" void __chk_fail(void) __attribute__((__noreturn__));
-extern "C" FDELT_TYPE __fdelt_warn(FDELT_TYPE a)
-{
-    if (a >= FD_SETSIZE)
-        __chk_fail();
-    return a / __NFDBITS;
-}
-extern "C" FDELT_TYPE __fdelt_chk(FDELT_TYPE) __attribute__((weak, alias("__fdelt_warn")));
-
 #if defined(__i386__) || defined(__arm__)
 
 extern "C" int64_t __udivmoddi4(uint64_t u, uint64_t v, uint64_t* rp);
diff --git a/src/compat/glibc_sanity.cpp b/src/compat/glibc_sanity.cpp
index cc74f28899..0367b9a53f 100644
--- a/src/compat/glibc_sanity.cpp
+++ b/src/compat/glibc_sanity.cpp
@@ -8,10 +8,6 @@
 
 #include <cstddef>
 
-#if defined(HAVE_SYS_SELECT_H)
-bool sanity_test_fdelt();
-#endif
-
 extern "C" void* memcpy(void* a, const void* b, size_t c);
 void* memcpy_int(void* a, const void* b, size_t c)
 {
@@ -45,9 +41,5 @@ bool sanity_test_memcpy()
 
 bool glibc_sanity_test()
 {
-#if defined(HAVE_SYS_SELECT_H)
-    if (!sanity_test_fdelt())
-        return false;
-#endif
     return sanity_test_memcpy<1025>();
 }
diff --git a/src/compat/glibc_sanity_fdelt.cpp b/src/compat/glibc_sanity_fdelt.cpp
deleted file mode 100644
index 87140d0c71..0000000000
--- a/src/compat/glibc_sanity_fdelt.cpp
+++ /dev/null
@@ -1,26 +0,0 @@
-// Copyright (c) 2009-2019 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#if defined(HAVE_CONFIG_H)
-#include <config/bitcoin-config.h>
-#endif
-
-#if defined(HAVE_SYS_SELECT_H)
-#ifdef HAVE_CSTRING_DEPENDENT_FD_ZERO
-#include <cstring>
-#endif
-#include <sys/select.h>
-
-// trigger: Call FD_SET to trigger __fdelt_chk. FORTIFY_SOURCE must be defined
-//   as >0 and optimizations must be set to at least -O2.
-// test: Add a file descriptor to an empty fd_set. Verify that it has been
-//   correctly added.
-bool sanity_test_fdelt()
-{
-    fd_set fds;
-    FD_ZERO(&fds);
-    FD_SET(0, &fds);
-    return FD_ISSET(0, &fds);
-}
-#endif