audit-round-1 fixes: drop MISRA_ on bench-local macro; tighten misc

brightprogrammer · brightprogrammer · commit c855e0e44324 · 2026-05-20T16:26:14.000-07:00
Applied findings from the audit-loop round 1:

  Allocator_misra_correct.c:
    MISRA_BENCH_SLAB_MAX_SLOT -&gt; BENCH_SLAB_MAX_SLOT  (MISRA_ prefix
      reserved for FEATURE_/MAGIC/guards; bench-local cap is none).
    bench_realloc(MODE_SLAB) comment: rewritten to admit the soft NULL
      is intentional (avoid aborting the whole bench process on a
      routing mismatch), instead of the contradictory 'Fail loudly'.
    enum BenchMode -&gt; bench_mode (file-local enum; convention uses
      snake_case for private types).

  Allocator_misra.c, Allocator_misra_correct.c:
    Drop dead &lt;stdlib.h&gt; include.

  Allocator_misra_arena.c, Allocator_misra_page.c:
    int g_*_live -&gt; bool.

  Allocator.h:
    Document the reset asymmetry in bench_use_fixed_size/general --
    misra backends tear down and reinit between benches; libc-shape
    backends keep process malloc state. Median absorbs the cold
    iteration; not advantageous to misra.

  Scripts/run.py (render_frag_table):
    Footprint = 0.0 MB rendered as 'n/a' (mimalloc 3.3.0's
    mi_stats_get is broken; old rendering flattered mimalloc by
    making it look like it allocated nothing).

  Source/Misra/Std/Allocator/Slab.c (alloc loop):
    Explain why the pre-redesign 'bitmap word reads back its bit
    already set' LOG_FATAL was dropped -- it was a self-consistency
    probe (defending against a corrupt-with-itself bitmap word, not
    against caller bugs), and the new contiguous-bitmaps layout makes
    it a defense against CTZ64 itself being broken. Free's
    double-free check still defends against the real-world failure
    mode (releasing a slot twice from user code).

Carried forward without change: the alloc-side check is permanently
removed -- not re-added under FEATURE_HEAP_VALIDATE_FULL -- because
it tested platform code, not caller code.

All 104 MisraStdC tests still pass on Linux.
diff --git a/Benchmark/README.md b/Benchmark/README.md
@@ -11,11 +11,11 @@ Single alloc/free pair, 16 B:
 | backend | time |
 |---|---:|
 | tcmalloc           | 3.6 ns |
-| glibc              | 4.6 ns |
-| jemalloc           | 3.5 ns |
+| glibc              | 4.9 ns |
+| jemalloc           | 3.6 ns |
 | mimalloc           | 7.4 ns |
 | misra (Heap only)  | 22.7 ns |
-| misra-correct (Slab) | 15.3 ns |
+| misra-correct (Slab) | 15.0 ns |
 
 ## Timing
 
@@ -25,13 +25,13 @@ One `alloc(size)` immediately followed by `free(ptr)`, repeated. Hot reuse — t
 
 | benchmark | glibc | jemalloc | mimalloc | tcmalloc | misra | misra-correct | misra-arena | misra-page |
 |---|---:|---:|---:|---:|---:|---:|---:|---:|
-| 16 B | 4.6 | 3.5 | 7.4 | 3.6 | 22.7 | 15.3 | 11.5 | 1610.5 |
-| 64 B | 4.6 | 3.7 | 7.5 | 3.6 | 20.8 | 15.0 | 11.5 | 1614.0 |
-| 256 B | 5.0 | 4.1 | 9.4 | 3.6 | 20.6 | 15.0 | 11.6 | 1608.0 |
-| 1 KiB | 4.9 | 5.8 | 11.0 | 3.6 | 20.0 | 15.0 | 11.8 | 1641.1 |
-| 4 KiB | 16.8 | 11.0 | 12.5 | 3.8 | 1636.8 | 15.1 | 11.8 | 1636.1 |
-| 16 KiB | 16.1 | 18.9 | 20.9 | 3.8 | 1250.4 | 1608.0 | 11.9 | 1203.0 |
-| 64 KiB | 16.1 | 270.8 | 21.0 | 3.9 | 1248.9 | 1621.2 | 11.9 | 1198.1 |
+| 16 B | 4.9 | 3.6 | 7.4 | 3.6 | 22.7 | 15.0 | 11.7 | 1620.7 |
+| 64 B | 5.1 | 3.7 | 7.5 | 3.6 | 21.3 | 14.9 | 11.6 | 1600.6 |
+| 256 B | 5.2 | 4.1 | 9.5 | 3.5 | 21.2 | 15.0 | 11.6 | 1625.0 |
+| 1 KiB | 5.1 | 5.8 | 11.0 | 3.5 | 20.7 | 15.0 | 11.6 | 1629.9 |
+| 4 KiB | 17.4 | 11.0 | 12.4 | 3.8 | 1641.7 | 15.0 | 11.9 | 1611.2 |
+| 16 KiB | 18.0 | 18.6 | 20.8 | 3.8 | 1233.8 | 1635.3 | 11.8 | 1214.2 |
+| 64 KiB | 17.5 | 270.5 | 20.5 | 3.8 | 1243.2 | 1659.1 | 11.5 | 1232.6 |
 
 _Values in ns._
 
@@ -41,9 +41,9 @@ _Values in ns._
 
 | benchmark | glibc | jemalloc | mimalloc | tcmalloc | misra | misra-correct | misra-arena | misra-page |
 |---|---:|---:|---:|---:|---:|---:|---:|---:|
-| 128 × 64 B | 1.0 | 1.1 | 0.7 | 0.5 | 3.0 | 2.0 | n/a | 153.9 |
-| 1024 × 64 B | 8.5 | 11.7 | 6.0 | 4.1 | 32.1 | 20.5 | n/a | 1358.0 |
-| 8192 × 64 B | 66.9 | 95.0 | 51.4 | 40.3 | 531.6 | 437.9 | n/a | 17750.9 |
+| 128 × 64 B | 1.2 | 1.0 | 0.7 | 0.5 | 3.0 | 2.0 | n/a | 154.2 |
+| 1024 × 64 B | 9.0 | 11.6 | 5.5 | 4.0 | 31.9 | 20.5 | n/a | 1351.1 |
+| 8192 × 64 B | 81.3 | 94.2 | 51.2 | 39.9 | 537.7 | 437.0 | n/a | 17811.5 |
 
 _Values in us._
 
@@ -53,9 +53,9 @@ Same shape as the pair test but writes every byte of the allocation before freei
 
 | benchmark | glibc | jemalloc | mimalloc | tcmalloc | misra | misra-correct | misra-arena | misra-page |
 |---|---:|---:|---:|---:|---:|---:|---:|---:|
-| 64 B | 4.9 | 4.7 | 8.5 | 4.5 | 33.0 | 18.2 | 14.6 | 2026.2 |
-| 4 KiB | 30.0 | 25.1 | 27.7 | 20.2 | 2197.8 | 48.5 | 67.8 | 2094.7 |
-| 64 KiB | 856.5 | 1060.5 | 866.9 | 841.7 | 13994.6 | 14209.5 | 1136.3 | 13955.8 |
+| 64 B | 8.2 | 4.8 | 8.5 | 4.4 | 32.2 | 17.7 | 14.7 | 2006.2 |
+| 4 KiB | 30.0 | 25.1 | 27.6 | 20.2 | 2177.9 | 48.3 | 67.7 | 2072.5 |
+| 64 KiB | 858.1 | 1067.3 | 862.4 | 841.2 | 13995.5 | 14096.2 | 1148.8 | 13946.5 |
 
 _Values in ns._
 
@@ -65,7 +65,7 @@ _Values in ns._
 
 | benchmark | glibc | jemalloc | mimalloc | tcmalloc | misra | misra-correct | misra-arena | misra-page |
 |---|---:|---:|---:|---:|---:|---:|---:|---:|
-| Pareto(1.16, 24) | 28.8 | 13.1 | 14.0 | 12.2 | 45.8 | 44.1 | n/a | 629.8 |
+| Pareto(1.16, 24) | 29.0 | 11.9 | 14.0 | 13.0 | 45.2 | 44.8 | n/a | 623.8 |
 
 _Values in us._
 
@@ -75,7 +75,7 @@ Geometric realloc ladder from 8 B up to 1 MiB. Time per full ladder, lower is be
 
 | benchmark | glibc | jemalloc | mimalloc | tcmalloc | misra | misra-correct | misra-arena | misra-page |
 |---|---:|---:|---:|---:|---:|---:|---:|---:|
-| 8 B → 1 MiB | 4771.7 | 11580.5 | 16933.1 | 15135.0 | 268603.8 | 258694.1 | 139.9 | 258397.1 |
+| 8 B → 1 MiB | 5735.8 | 4313.3 | 17014.8 | 15119.2 | 261663.3 | 260083.2 | 134.8 | 260357.2 |
 
 _Values in ns._
 
@@ -85,9 +85,9 @@ Allocate N small (32 B) objects, then release them all. Arena does this as one O
 
 | benchmark | glibc | jemalloc | mimalloc | tcmalloc | misra | misra-correct | misra-arena | misra-page |
 |---|---:|---:|---:|---:|---:|---:|---:|---:|
-| 128 × 32 B | 1.0 | 0.5 | 0.7 | 0.5 | 4.2 | 3.7 | 0.8 | 154.0 |
-| 1024 × 32 B | 8.4 | 9.8 | 5.3 | 4.0 | 41.9 | 39.6 | 6.5 | 1222.4 |
-| 8192 × 32 B | 68.5 | 86.2 | 47.2 | 31.8 | 574.3 | 562.4 | 52.4 | 15842.2 |
+| 128 × 32 B | 1.0 | 0.5 | 0.7 | 0.5 | 4.0 | 3.7 | 0.8 | 152.0 |
+| 1024 × 32 B | 8.0 | 9.6 | 5.1 | 3.9 | 40.8 | 39.0 | 6.4 | 1217.0 |
+| 8192 × 32 B | 67.0 | 85.4 | 47.7 | 31.8 | 569.5 | 564.7 | 52.6 | 15765.0 |
 
 _Values in us._
 
@@ -97,11 +97,11 @@ Each allocator's own introspection API reports committed bytes after the workloa
 
 | benchmark | live MB | glibc MB | jemalloc MB | mimalloc MB | tcmalloc MB | misra MB | misra-correct MB | misra-arena MB | misra-page MB |
 |---|---:|---:|---:|---:|---:|---:|---:|---:|---:|
-| Checkerboard (4 K small) | 0.6 | 1.3 | 9.1 | 0.0 | 7.0 | 3.1 | 2.5 | n/a | 16.0 |
-| Checkerboard (16 K small) | 2.5 | 4.1 | 11.7 | 0.0 | 7.0 | 10.1 | 10.0 | n/a | 64.0 |
-| Checkerboard (64 K small) | 10.0 | 15.3 | 21.9 | 0.0 | 16.0 | 40.2 | 40.2 | n/a | 256.0 |
-| Lifetime mix | 4.0 | 5.0 | 20.1 | 0.0 | 16.0 | 45.8 | 45.8 | n/a | 64.0 |
-| Page overhang | 18.2 | 20.5 | 34.0 | 0.0 | 29.0 | 77.0 | 77.0 | n/a | 256.0 |
+| Checkerboard (4 K small) | 0.6 | 2.2 | 9.2 | n/a | 7.0 | 3.1 | 2.5 | n/a | 16.0 |
+| Checkerboard (16 K small) | 2.5 | 4.1 | 11.7 | n/a | 7.0 | 10.1 | 10.0 | n/a | 64.0 |
+| Checkerboard (64 K small) | 10.0 | 15.3 | 22.0 | n/a | 16.0 | 40.2 | 40.2 | n/a | 256.0 |
+| Lifetime mix | 4.0 | 5.0 | 20.4 | n/a | 16.0 | 45.8 | 45.8 | n/a | 64.0 |
+| Page overhang | 18.2 | 20.4 | 36.4 | n/a | 29.0 | 77.0 | 77.0 | n/a | 256.0 |
 
 ## How to read
 
@@ -126,8 +126,8 @@ Regenerates this file with measurements from the host.
 
 | | |
 |---|---|
-| timestamp | 2026-05-20 23:09:43 UTC |
-| git rev   | 4ed3d620fd4f (perf/slab-bitmap-redesign) |
+| timestamp | 2026-05-20 23:26:13 UTC |
+| git rev   | 85f73f49377d (perf/slab-bitmap-redesign) |
 | host CPU  | Intel(R) Core(TM) Ultra 7 165U |
 | kernel    | Linux 6.18.25 |
 | compiler  | gcc 15.2.0 |
diff --git a/Benchmark/Scripts/run.py b/Benchmark/Scripts/run.py
@@ -217,7 +217,16 @@ def render_frag_table(data: dict) -> str:
                 cells.append("n/a")
                 continue
             fp = get_counter(j, bench_name, "footprint_MB")
-            cells.append(f"{fp:.1f}" if fp is not None else "n/a")
+            # Backends without a working stats API (mimalloc 3.3.0's
+            # mi_stats_get is broken; see the long comment in
+            # Allocator_libc.c::bench_footprint_bytes) report 0.0.
+            # That would flatter them in a fragmentation table -- they
+            # didn't allocate zero bytes, we just can't read what they
+            # did. Render those as n/a, not 0.0.
+            if fp is None or fp == 0.0:
+                cells.append("n/a")
+            else:
+                cells.append(f"{fp:.1f}")
         out.append("| " + " | ".join(cells) + " |")
     return "\n".join(out)
 
diff --git a/Benchmark/Source/Allocator.h b/Benchmark/Source/Allocator.h
@@ -43,9 +43,17 @@ void bench_teardown(void);
 // workload shape.
 //
 // Outstanding allocations from before the call are NOT preserved; the
-// previous allocator's state is destroyed. Call before the first
-// bench_alloc of a benchmark; call bench_use_general before any
-// benchmark that uses multiple sizes or realloc.
+// previous allocator's state is destroyed and a fresh one constructed.
+// This is an asymmetry vs the libc-shape backends, which keep
+// per-process malloc state across benchmark runs. The asymmetry does
+// NOT favour the misra backends: a fresh slab/heap has to fault its
+// first page on iteration 1, and Google Benchmark's auto-scaled
+// iteration count (typically 10^6+ to fill --benchmark_min_time)
+// absorbs that cold-start blip in the median.
+//
+// Call bench_use_fixed_size before the first bench_alloc of a
+// fixed-size benchmark; call bench_use_general before any benchmark
+// that uses multiple sizes or realloc.
 void bench_use_fixed_size(size_t slot);
 void bench_use_general(void);
 
diff --git a/Benchmark/Source/Allocator_misra.c b/Benchmark/Source/Allocator_misra.c
@@ -23,8 +23,6 @@
 #include <Misra/Std/Allocator/Heap.h>
 #include <Misra/Std/Allocator/Page.h>
 
-#include <stdlib.h>
-
 static HeapAllocator g_heap;
 static Allocator    *g_alloc = NULL;
 // `g_heap_typed` exists so the bench passes a typed `HeapAllocator *`
diff --git a/Benchmark/Source/Allocator_misra_arena.c b/Benchmark/Source/Allocator_misra_arena.c
@@ -19,7 +19,7 @@
 #include <Misra/Std/Allocator/Page.h>
 
 static ArenaAllocator g_arena;
-static int            g_arena_live = 0;
+static bool           g_arena_live = false;
 
 const char *bench_backend_name(void) {
 #ifdef BENCH_MISRA_VARIANT_NAME
@@ -31,13 +31,13 @@ const char *bench_backend_name(void) {
 
 void bench_init(void) {
     g_arena      = ArenaAllocatorInit();
-    g_arena_live = 1;
+    g_arena_live = true;
 }
 
 void bench_teardown(void) {
     if (g_arena_live) {
         ArenaAllocatorDeinit(&g_arena);
-        g_arena_live = 0;
+        g_arena_live = false;
     }
 }
 
diff --git a/Benchmark/Source/Allocator_misra_correct.c b/Benchmark/Source/Allocator_misra_correct.c
@@ -27,8 +27,6 @@
 #include <Misra/Std/Allocator/Page.h>
 #include <Misra/Std/Allocator/Slab.h>
 
-#include <stdlib.h>
-
 // MODE_NONE = pre-init / post-teardown. MODE_HEAP / MODE_SLAB choose
 // which of the two static allocator instances below is currently
 // live. The two are unioned by usage, not by struct -- only one is
@@ -38,11 +36,11 @@ typedef enum {
     MODE_NONE = 0,
     MODE_HEAP = 1,
     MODE_SLAB = 2,
-} BenchMode;
+} bench_mode;
 
 static HeapAllocator g_heap;
 static SlabAllocator g_slab;
-static BenchMode     g_mode = MODE_NONE;
+static bench_mode     g_mode = MODE_NONE;
 
 const char *bench_backend_name(void) {
 #ifdef BENCH_MISRA_VARIANT_NAME
@@ -80,11 +78,11 @@ void bench_teardown(void) {
 // Matches MisraStdC's slab max (currently 4096 B, the smallest OS page
 // size we target). If the slab grows multi-page support later, raise
 // this threshold to match.
-#define MISRA_BENCH_SLAB_MAX_SLOT 4096u
+#define BENCH_SLAB_MAX_SLOT 4096u
 
 void bench_use_fixed_size(size_t slot) {
     tear_current();
-    if (slot > MISRA_BENCH_SLAB_MAX_SLOT) {
+    if (slot > BENCH_SLAB_MAX_SLOT) {
         // Slab can't hold a slot this big; fall back to Heap and let
         // it route through XL (mmap-per-alloc) the way the upstream
         // "wrong tool" backend does. The bench's
@@ -124,10 +122,13 @@ void *bench_alloc(size_t n) {
 
 void *bench_realloc(void *p, size_t n) {
     // SlabAllocator slot size is fixed at init; realloc is not a
-    // sensible operation in slab mode and would have to copy through
-    // a new slab of a different size. The bench's only realloc
+    // sensible operation in slab mode. The bench's only realloc
     // workload (BM_ReallocGrow) runs in MODE_HEAP, so this branch
-    // should never fire in slab mode. Fail loudly if it does.
+    // should never fire in slab mode. Return NULL (caller treats as
+    // alloc failure -- the bench then reads the size-class check
+    // and moves on); we avoid LOG_FATAL here because aborting the
+    // whole bench process on a routing mismatch would make the
+    // failure mode hostile to interactive debugging.
     if (g_mode == MODE_SLAB) {
         (void)p;
         (void)n;
diff --git a/Benchmark/Source/Allocator_misra_page.c b/Benchmark/Source/Allocator_misra_page.c
@@ -15,7 +15,7 @@
 #include <Misra/Std/Allocator/Page.h>
 
 static PageAllocator g_page;
-static int           g_page_live = 0;
+static bool          g_page_live = false;
 
 const char *bench_backend_name(void) {
 #ifdef BENCH_MISRA_VARIANT_NAME
@@ -27,13 +27,13 @@ const char *bench_backend_name(void) {
 
 void bench_init(void) {
     g_page      = PageAllocatorInit();
-    g_page_live = 1;
+    g_page_live = true;
 }
 
 void bench_teardown(void) {
     if (g_page_live) {
         PageAllocatorDeinit(&g_page);
-        g_page_live = 0;
+        g_page_live = false;
     }
 }
 
diff --git a/Source/Misra/Std/Allocator/Slab.c b/Source/Misra/Std/Allocator/Slab.c
@@ -269,6 +269,16 @@ void *slab_allocator_allocate(Allocator *self, size bytes, i8 zeroed) {
     // free bit. First fit wins. Tail bits in the last word (if any)
     // are pre-set to 1 at slab-insert time, so ctz on the inverted
     // word can never spuriously hit them.
+    //
+    // The pre-redesign code re-read bm[w] here and LOG_FATAL'd if the
+    // freshly-found bit was already set, defending against an
+    // inconsistent-with-itself bitmap word. That check was a
+    // self-consistency probe against rare corruption -- not a useful
+    // safety net under the contiguous-bitmaps layout, where the only
+    // way the bit can be set is if `CTZ64(~bm[w])` is broken (which
+    // is platform code we trust). Free's double-free check (below)
+    // still defends against the more useful real-world failure mode:
+    // releasing a slot twice from user code.
     for (u32 i = 0; i < slab->slabs_len; i++) {
         u64 *bm = &slab->bitmaps[(size)i * (size)bw];
         for (u32 w = 0; w < bw; w++) {
@@ -277,8 +287,6 @@ void *slab_allocator_allocate(Allocator *self, size bytes, i8 zeroed) {
                 continue;
             }
             u32 bit = CTZ64(inv);
-            // No tail-bit guard needed: tail bits are pre-set to 1,
-            // so ctz(~word) never finds them.
             bm[w] |= ((u64)1 << bit);
             u32   slot_idx = w * 64u + bit;
             void *slot     = (char *)slab->slabs[i] + ((size)slot_idx << slab->slot_size_shift);

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@`
`19`	`19`	`#include <Misra/Std/Allocator/Page.h>`
`20`	`20`
`21`	`21`	`static ArenaAllocator g_arena;`
`22`		`-static int g_arena_live = 0;`
	`22`	`+static bool g_arena_live = false;`
`23`	`23`
`24`	`24`	`const char *bench_backend_name(void) {`
`25`	`25`	`#ifdef BENCH_MISRA_VARIANT_NAME`
`@@ -31,13 +31,13 @@ const char *bench_backend_name(void) {`
`31`	`31`
`32`	`32`	`void bench_init(void) {`
`33`	`33`	`g_arena = ArenaAllocatorInit();`
`34`		`- g_arena_live = 1;`
	`34`	`+ g_arena_live = true;`
`35`	`35`	`}`
`36`	`36`
`37`	`37`	`void bench_teardown(void) {`
`38`	`38`	`if (g_arena_live) {`
`39`	`39`	`ArenaAllocatorDeinit(&g_arena);`
`40`		`- g_arena_live = 0;`
	`40`	`+ g_arena_live = false;`
`41`	`41`	`}`
`42`	`42`	`}`
`43`	`43`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@`
`15`	`15`	`#include <Misra/Std/Allocator/Page.h>`
`16`	`16`
`17`	`17`	`static PageAllocator g_page;`
`18`		`-static int g_page_live = 0;`
	`18`	`+static bool g_page_live = false;`
`19`	`19`
`20`	`20`	`const char *bench_backend_name(void) {`
`21`	`21`	`#ifdef BENCH_MISRA_VARIANT_NAME`
`@@ -27,13 +27,13 @@ const char *bench_backend_name(void) {`
`27`	`27`
`28`	`28`	`void bench_init(void) {`
`29`	`29`	`g_page = PageAllocatorInit();`
`30`		`- g_page_live = 1;`
	`30`	`+ g_page_live = true;`
`31`	`31`	`}`
`32`	`32`
`33`	`33`	`void bench_teardown(void) {`
`34`	`34`	`if (g_page_live) {`
`35`	`35`	`PageAllocatorDeinit(&g_page);`
`36`		`- g_page_live = 0;`
	`36`	`+ g_page_live = false;`
`37`	`37`	`}`
`38`	`38`	`}`
`39`	`39`