# Fold-on-fold static-analysis pass (2026-05-23)

Output of running fold against its own source tree:

```
fold --quiet scan \
  --compile-db build/release/compile_commands.json \
  --detectors clone,dead-statement,equiv \
  --clone-type 3 --clone-similarity 0.8 \
  --processes 4 \
  --format json --output docs/internal/perf-audit/fold-on-fold-2026-05-23.json
```

Wall: ~9.7 min over 134 TUs.

**Totals:** 639 findings — 487 clone, 152 dead-statement, 0 equiv.

The equiv-detector slot caught nothing because equiv only fires on
function pairs whose signatures match exactly AND that pass z3 input/
output equivalence; the recently scout-rule-extracted helpers don't
collide on signature.

## Top duplication clusters worth refactoring

### `mixHash` across the security detectors

Sites: `src/detect/sign_conversion.cpp:154`, `div_by_zero.cpp:*`,
`int_truncation.cpp:*`, `int_overflow.cpp:*`, `null_deref.cpp:*`.

5+ identical copies of the FNV-1a-style mix function inside per-
detector anonymous namespaces. **Refactor target:** lift to a single
helper in `src/ir/normalize.cpp` (already has `mix64`) or
`src/symbolic/theory.hpp`. Each detector loses ~6 lines.

### `supportsParam` and `collectConjuncts`

Multiple anonymous-namespace copies across detectors. The Phase-B v2
migration moved some helpers but missed these. **Refactor target:**
extract to `src/symbolic/z3_helpers.cpp` (sister to
`z3_model_support.{hpp,cpp}` that already exists).

### `onFunctionEnd` boilerplate

`DivByZeroObserver::onFunctionEnd`, `IntOverflowObserver::onFunctionEnd`,
`OOBObserver::onFunctionEnd`, etc. all do the same `solver_ = nullptr;
current_fn_ = nullptr;` cleanup. **Refactor target:** a small mixin
helper or a base-class `SafetyObserverBase` providing the default
implementation.

### `ir::*::valid` / `operator bool` / `operator==` boilerplate

`include/fold/ir/call.hpp:23-29` and friends — `Call`, `MemberRef`,
`Symbol`, `Expr` all spell out their own identical `valid()`,
`operator bool`, `operator==`, `operator!=`, `asClangX()`. Five copies
of each. **Refactor target:** common CRTP base or just a macro; the
opaque-handle shape repeats deliberately but the accessor boilerplate
shouldn't.

### Test-fixture duplication

`tests/unit/test_oob_access.cpp:*` (60 hits), `test_null_deref.cpp:*`
(51 hits), `test_int_truncation.cpp:*` (7 hits) all show massive
cross-cluster matches. **Judgment call:** these are parameterised
detector tests — the duplication is the *point* of fixture coverage.
Not a refactor target. Could be reframed as Catch2 GENERATE() tables
if test count keeps growing, but that's stylistic.

## Dead-statement findings (152 → 16 after fix)

**Initial finding:** 152 dead-statement reports. Spot-checks of the
top files (incremental_cache.cpp, call_graph.cpp, def_use.cpp,
r2_backend.cpp, static_ptr_asymmetry.cpp) all showed the same
shape: the flagged statement was inside a range-for body. Plain
`for (int x : v)` was fine; `for (const auto& [k, v] : map)` was
not.

**Root cause:** clang's CFG for a range-for whose loop variable
is a structured binding leaves the body block without an incoming
edge from the loop's condition check. The reachability walk in
`DeadStatementVisitor` then marks every body statement
unreachable. Reproducer in 7 lines of C++.

**Fix:** commit `8895d50` adds a parent-map walk; if the anchor's
ancestor chain contains a `CXXForRangeStmt`, suppress the finding.
Regression test `dead-statement: range-for + structured binding
body is reachable` pins it.

**Result:** 152 → 16 findings on fold's own source (90% reduction).

### Remaining 16 are all "defensive return after exhaustive enum switch"

The 16 surviving findings are the same idiom across 13 files:

```cpp
const char* findingKindName(FindingKind k) {
    switch (k) {
        case FindingKind::Clone: return "clone";
        // ... every enum value covered ...
        case FindingKind::FuzzCrash: return "fuzz-crash";
    }
    return "unknown";   // ← flagged
}
```

When every enum case returns (no default), clang's CFG does not
insert a synthetic edge to the post-switch block, so the trailing
`return "unknown"` is genuinely unreachable. The detector is
*technically correct*; the engineer wrote the return *defensively*
in case a future enum addition forgets a case.

Three resolutions worth considering:

1. **Leave as documented exception.** The defensive return is
   graceful — returns a placeholder rather than UB. Cost is a few
   FPs on tight C++ codebases that use the same idiom.
2. **Replace with `__builtin_unreachable()` / `std::unreachable()`
   at fold's own sites.** Removes the dead code, signals intent,
   improves codegen. Cost: UB at runtime if exhaustiveness breaks
   later.
3. **Add a second filter to dead_statement.cpp** for trailing
   statements after exhaustive enum switches. Cost: complex AST
   analysis (find the surrounding switch, verify it covers every
   enum case, etc.) and risks suppressing real dead code.

Recommendation: **(1)** is fine for fold-as-a-tool; the 16
remaining findings are a known false-positive idiom worth
documenting in user-facing release notes. Sites in fold's own
source: finding.cpp (3), theory.cpp (5), ir_expr_clang.cpp (1),
corpus_writer.cpp (1), harness_emitter.cpp (2), clone.cpp (1),
triage.cpp (1), int_overflow.cpp (1), test_cross_tu_equiv.cpp (1).

## What this confirms vs. denies

**Confirms:**
- The 5 security detectors share substantial duplicated mechanics
  (mixHash, supportsParam, onFunctionEnd, hashQuery cache shape).
  Hypothesised in the perf-audit plan; visible in the data.
- The ir::* opaque types have unnecessary per-type boilerplate.

**Denies:**
- equiv-detector finding nothing means the prior scout-rule
  extractions DID move shared logic out — the per-detector
  remaining duplication is in small helpers, not whole functions.

## Pointer

Full JSON: `docs/internal/perf-audit/fold-on-fold-2026-05-23.json` (639
findings, ~50 KB). Profile: `fold-on-fold-2026-05-23-profile.json`.

## 2026-05-23 late-session pass (after audit + cache + composite-walker)

| Rule | Count |
|---|---|
| fold.clone | 81 |
| fold.dead-statement | 14 |
| fold.equiv | 1 |
| fold.null-deref | 22 (new; not in earlier passes) |

The 22 null-deref findings on fold itself are **mostly the same
shape**: a parameter is null-checked early via a compound
condition (e.g. `if (!rd || rd->isImplicit()) return true;`),
then the parameter is passed to a helper function that
dereferences it. The encoder doesn't propagate the
`!rd → false` arm of the `||` into a non-null fact on the
post-early-return path; downstream the helper sees an unknown
pointer and flags the deref.

Example: `src/detect/class_dup.cpp:186-193`. Parameter `rd` is
null-checked at line 186 (`if (!rd || ...)`), passed to
`eligibleMethodCount(rd)` at line 193 which derefs param 0.

### What this surfaces

The compound-guard pattern (`if (!x || other) return;`) is real
and worth recognising syntactically. `cfg_helpers::nullCheckVar`
now handles `NULLCHECK || other` and `NULLCHECK && other`
(symmetric AND case for `if (x != nullptr && other)` shapes).
Three unit tests under `[m22-extension]` pin the contract. The
existing SMT path predicate already handled most in-function
derefs after these guards; the syntactic flag is mainly defense
in depth + faster precondition discharge at call sites.

The 22 null-deref findings on fold itself are NOT eliminated by
that change. They come from a separate gap: fold analyzes static
file-local helpers as standalone entry points, so
`static int methods(const R* r) { return r->v; }` emits a
nullable-deref finding on `r->v` regardless of whether the only
callers in the TU prove `r` non-null. Two routes to fix that:

1. Skip standalone analysis for file-static helpers whose every
   call site discharges the parameter non-null precondition.
2. Derive a stronger required-precondition summary from the
   callee body and suppress the deref-site finding when every
   call site satisfies it.

Both are detector-side work, larger scope, filed for a future
null-deref precision refresh. Tight well-typed codebases tend to
produce a long tail of these context-sensitive parameter-deref
FPs — they are domain-invariant, not signal.