Repository: bitcoin/bitcoin (language: cpp)

Issue:
--with-sanitizers=float-divide-by-zero crash with -debug=bench in Chainstate::ConnectTip
Not sure if this UB causes issues with any compiler. clang is fine, see:

* `-fsanitize=float-divide-by-zero`: Floating point division by zero. This is undefined per the C and C++ standards, but is defined by Clang (and by ISO/IEC/IEEE 60559 / IEEE 754) as producing either an infinity or NaN value, so is not included in `-fsanitize=undefined`.

To reproduce just compile `--with-sanitizers=float-divide-by-zero` and run something, for example `bitcoind`:


```
UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ./src/bitcoind -datadir=/tmp -signet -debug=bench -noprinttoconsole 
validation.cpp:2839:5: runtime error: division by zero
```


`float-divide-by-zero` is enabled by OSS-Fuzz, see https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=58887


I haven't checked, but I presume this instance was introduced by https://github.com/bitcoin/bitcoin/pull/24216


Discussion / hints:
cc @Sjors @jonatack @theStack
I think #24216 should just be reverted. It doesn't make sense to assume that the number of all connected blocks is equal to the number of blocks read from disk. In fact, the most common case, calling ProcessNewBlock from net processing will have the block already in memory (read from the network).

An alternative might be to use a dedicated read counter for it.
The potential divide by zero was introduced here:

https://github.com/bitcoin/bitcoin/pull/24216/files#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98L2607-R2612

Let me see if I remember why…

Fix patch:
diff --git a/src/validation.cpp b/src/validation.cpp
index e536dfb4ebd95..6a6d0b1153aae 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2760,7 +2760,6 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra
     return true;
 }
 
-static SteadyClock::duration time_read_from_disk_total{};
 static SteadyClock::duration time_connect_total{};
 static SteadyClock::duration time_flush{};
 static SteadyClock::duration time_chainstate{};
@@ -2834,12 +2833,11 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew,
     const CBlock& blockConnecting = *pthisBlock;
     // Apply the block atomically to the chain state.
     const auto time_2{SteadyClock::now()};
-    time_read_from_disk_total += time_2 - time_1;
     SteadyClock::time_point time_3;
-    LogPrint(BCLog::BENCH, "  - Load block from disk: %.2fms [%.2fs (%.2fms/blk)]\n",
-             Ticks<MillisecondsDouble>(time_2 - time_1),
-             Ticks<SecondsDouble>(time_read_from_disk_total),
-             Ticks<MillisecondsDouble>(time_read_from_disk_total) / num_blocks_total);
+    // When adding aggregate statistics in the future, keep in mind that
+    // num_blocks_total may be zero until the ConnectBlock() call below.
+    LogPrint(BCLog::BENCH, "  - Load block from disk: %.2fms\n",
+             Ticks<MillisecondsDouble>(time_2 - time_1));
     {
         CCoinsViewCache view(&CoinsTip());
         bool rv = ConnectBlock(blockConnecting, state, pindexNew, view);


Test patch: