Fix MicroStats bin/range overflow at the top of the 64-bit domain

calcbin() maps any value with bit 63 set into octave (64-NDB), whose
highest index is ((64-NDB)<<NDB)+MASK. NUMBINS was sized (64-NDB)<<NDB,
one octave short, so those samples wrote past bins[] in add(). Size it
(65-NDB)<<NDB.

calcrange() held the sub-bin partition in a uint32_t and shifted it left
by up to (numbits-NDB-1), which exceeds 31 for high octaves and overflows,
corrupting the reconstructed range (and thus percentiles) for very large
samples. Hold partition in 64 bits.

Add testMicroStatsRange: a dependency-free regression test covering NDB
0..6 across the full 64-bit domain (top octave and 2^64-1 included),
asserting no out-of-bounds index, calcrange(calcbin(v)) contains v, and
monotonic bins. Returns nonzero on failure so ctest catches it; wired in
via add_test + enable_testing().

Author: HFTrader

CMakeLists.txt

@@ -60,6 +60,7 @@ include_directories( libs )
 set( TARGETS  tinyperfstats )
 
 if ( TINYPERF_BUILD_TESTS AND Armadillo_FOUND )
+  enable_testing()
   add_subdirectory( datasets )
   add_subdirectory( tests )
 endif()

libs/MicroStats.h

@@ -32,8 +32,12 @@ inline uint32_t msb(uint64_t value) {
 template <uint32_t NDB>
 class MicroStats {
 public:
-    //! The number of available bins possible in a 64-bit range
-    static constexpr uint32_t NUMBINS = (64 - NDB) << NDB;
+    //! The number of available bins possible in a 64-bit range.
+    //! A value with bit 63 set has msb()==64 and lands in octave (64-NDB),
+    //! so the highest bin index is ((64-NDB)<<NDB)+MASK. Sizing the array at
+    //! (64-NDB)<<NDB is one octave short and overflows on those samples;
+    //! (65-NDB)<<NDB gives the top octave its own slots.
+    static constexpr uint32_t NUMBINS = (65 - NDB) << NDB;
 
     //! Mask used extensively in the calculations
     static constexpr uint64_t MASK = (1ULL << NDB) - 1;
@@ -117,7 +121,10 @@ class MicroStats {
     //! Computes the range of a bin given its ID
     static Range calcrange(uint32_t bin) {
         if (bin < (1 << NDB)) return Range{bin, bin};
-        uint32_t partition = bin & MASK;
+        // partition is shifted left by up to (numbits - NDB - 1), which exceeds
+        // 31 for high octaves; a uint32_t shift overflows there and corrupts the
+        // range. Hold it in 64 bits so the shift stays valid across the range.
+        uint64_t partition = bin & MASK;
         uint32_t numbits = (bin >> NDB) + NDB;
         uint64_t base = 1ULL << (numbits - 1);
         uint64_t offset = partition << (numbits - (NDB + 1));

tests/microstats/CMakeLists.txt

@@ -2,4 +2,9 @@
 add_executable( testMicroStats testMicroStats.cpp )
 target_link_libraries( testMicroStats ${REQUIRED_LIBS} )
 
-list( APPEND TARGETS testMicroStats )
+# Assertion-based regression test for the bin/range arithmetic. Header-only,
+# no external dependencies, returns nonzero on failure so ctest catches it.
+add_executable( testMicroStatsRange testMicroStatsRange.cpp )
+add_test( NAME testMicroStatsRange COMMAND testMicroStatsRange )
+
+list( APPEND TARGETS testMicroStats testMicroStatsRange )

tests/microstats/testMicroStatsRange.cpp

@@ -0,0 +1,62 @@
+// Regression test for MicroStats bin/range arithmetic across the full 64-bit
+// domain and for NDB = 0..6. It guards two corner cases that the demo in
+// testMicroStats.cpp never exercises:
+//
+//   1) calcbin() must never return an index >= NUMBINS. A value with bit 63
+//      set lands in the top octave; if NUMBINS is one octave short the index
+//      runs past the bins[] array (out-of-bounds write in add()).
+//
+//   2) calcrange(calcbin(v)) must contain v. For high octaves the sub-bin
+//      partition is shifted left by more than 31 bits, so holding it in a
+//      32-bit value overflows and corrupts the reconstructed range.
+//
+// Uses explicit checks and a nonzero exit code rather than assert(), which is
+// compiled out in Release builds.
+
+#include "MicroStats.h"
+#include <cstdint>
+#include <cstdio>
+
+template <uint32_t NDB>
+static long check() {
+    using M = MicroStats<NDB>;
+    long oob = 0, inversion = 0, monotonic = 0;
+
+    auto probe = [&](uint64_t v) {
+        uint32_t b = M::calcbin(v);
+        if (b >= M::NUMBINS) { ++oob; return; }
+        auto r = M::calcrange(b);
+        if (!(v >= r.from && v <= r.to)) ++inversion;
+    };
+
+    // Dense low range, also checking that the bin index is non-decreasing in v.
+    uint32_t prev = 0;
+    for (uint64_t v = 1; v < (1ull << 24); ++v) {
+        uint32_t b = M::calcbin(v);
+        if (b < prev) ++monotonic;
+        prev = b;
+        probe(v);
+    }
+    // Sparse sweep across every high octave, including the top one (bit 63),
+    // landing on each sub-bin boundary.
+    for (int e = 20; e < 64; ++e)
+        for (uint64_t d = 0; d < (1u << NDB); ++d)
+            probe((1ull << e) + (d << (e > (int)NDB ? e - (int)NDB : 0)));
+    probe(~0ull);  // 2^64 - 1, the very top
+
+    long total = oob + inversion + monotonic;
+    std::printf("  NDB=%u NUMBINS=%-5u out-of-bounds=%ld inversion=%ld monotonicity=%ld %s\n",
+                NDB, M::NUMBINS, oob, inversion, monotonic, total ? "FAIL" : "ok");
+    return total;
+}
+
+int main() {
+    long failures = check<0>() + check<1>() + check<2>() + check<3>() +
+                    check<4>() + check<5>() + check<6>();
+    if (failures) {
+        std::printf("FAILED: %ld total violations\n", failures);
+        return 1;
+    }
+    std::printf("PASSED: bin/range arithmetic valid across NDB 0..6 and the full 64-bit domain\n");
+    return 0;
+}