fold/docs/internal/perf-audit/fold-on-fold.md

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:

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.