Add avoidable loop allocation triage shapes

[richlander]

Summary

Adds avoidable-loop-allocation family v1 to Performance Triage:

• unsized-collection-grown: no-capacity growable collection stored to a local and grown in a loop, linked by reaching definitions.
• string-slice-in-loop: trusted System.String Substring / Split / Trim* calls inside loop regions.
• Reusable cold-region classifier: EH handler/filter blocks or unconditionally-throwing bodies demote loop-allocation rows to low with "On an exception/cold path; low steady-state impact."

Evidence

• Build: dotnet build src/dotnet-inspect -c Release ✅
• Tests: dotnet run --project src/ILInspector.Analysis.Tests -c Release ✅ (275 passed)

Unsized collection canaries

• STJ JsonHelpers.TraverseGraphWithTopologicalSort: 4 rows, all medium.
• STJ ThrowHelper StringBuilder rows: low (cold demotion), yielding STJ split 6 low / 5 medium.
• Aspire: 13 rows, all medium/unchanged.

String slice canaries

• Aspire DcpLogParser.FormatSystemLog: Split + Trim rows.
• Dapper SqlMapper.GenerateValueTupleDeserializer: 2 Substring rows.
• AutoMapper TypeDetails.PossibleNames: Substring row.
• Extended Performance Triage: technique discovery — vision + evidence-backed idea catalog #2040-style probe (six pinned + Dapper + Humanizer.Core + RestSharp): 51 string-slice-in-loop rows.

Fixtures

• unsized-collection-grown: no-capacity+loop fires; capacity ctor does not; EnsureCapacity before loop does not; Capacity setter before loop does not; ternary sized/unsized constructor merge does not; lexical-but-not-natural loop does not; non-loop growth does not; static Add helper does not; throw-helper path demotes low; infinite-loop work with conditional throw stays medium.
• string-slice-in-loop: Substring-in-loop fires; Substring outside loop does not; no-op Substring(0) / Substring(0, s.Length) in loop does not; lexical-but-not-natural loop does not; Substring-in-loop throw helper demotes low; Substring-in-finally stays medium.

Corpus favorability

Re-verified after GPT-5.5 and Gemini precision fixes: existing rows remain byte-identical to the 0.16.0 baseline after removing the two new shapes. The only baseline deltas are the two new shapes. The strict natural-loop gate intentionally suppresses a few previously reported new-shape rows that could not be confirmed as natural-loop sites (fail-closed precision).

Library	string-slice-in-loop new rows	Other rows
System.Text.Json@10.0.9	0	existing unchanged
Aspire.Hosting@13.4.6	13	existing unchanged
Newtonsoft.Json@13.0.4	7	existing unchanged
Serilog@4.3.1	2	existing unchanged
Polly@8.7.0	0	existing unchanged
AutoMapper@16.1.1	2	existing unchanged

Aspire before/after demo

Shape	dnx dotnet-inspect@0.16.0	this PR	Examples
unsized-collection-grown	0	13	ResourceLoggerService.GetLogger (List<ILogger>::Add), DrainAppHostStartupEvents (List<AppHostStartupEvent>::Add)
string-slice-in-loop	0	13	DcpLogParser.FormatSystemLog (Split, Trim), ContainerDirectory.GetFileSystemItemsFromPath (Substring, Split)

Adversarial review

• Claude Opus 4.8: initially found static Add and instance-call stack receiver hardening issues; fixed in fcd5d68f. Latest family review found no significant issues.
• GPT-5.5: found three precision issues; fixed in d9823aa8 by suppressing dominating pre-loop sizing (EnsureCapacity / Capacity setter), suppressing provable no-op Substring, and keeping finally handlers warm.
• Gemini 3.1 Pro: found the same finally issue plus lexical loop attribution, branchy constructor-def aliasing, and infinite-loop throw-helper over-demotion; fixed in 042cbd1a with a new-shapes-only natural-loop gate, constructor/store domination and all-reaching-def fail-closed behavior, and throw-helper-name-gated cold demotion. Confirming re-review found recursive Tarjan stack-overflow risk; fixed in eb2c572b by making SCC discovery iterative.
• GPT-5.5 confirming re-review: found zero-capacity pre-size over-suppression; fixed in 0e045ccf so EnsureCapacity(0) / Capacity = 0 still report while non-zero/unknown sizing suppresses.

Latest local build/tests passed after 0e045ccf; CI/review still pending, so not marking Ready to merge yet.

Fixes #2042.

[richlander]

Independent verification — shape is correct and clean; one value-precision refinement

Built the branch and verified against the corpus.

Correct and additive:

• True positives confirmed — JsonHelpers.TraverseGraphWithTopologicalSort
  fires on List<bool[]>::Add and Queue<int>::Enqueue in a loop, exactly the
  no-capacity+grow pattern I hand-verified in the decompiled source earlier.
• Favorability clean — shape-tally diff (0.16.0 vs this PR) shows only
  unsized-collection-grown added (STJ +11, Aspire +13, Newtonsoft +24); every
  existing shape count is byte-identical. Purely additive.
• Precision guards hold — the fixtures (capacity ctor / non-loop / static
  helper do not fire) plus the receiver-matching hardening address the main
  false-positive vectors.

One refinement (recommend before/with ship): the detection is precise on the
pattern but not yet on value. ~5 of STJ's 11 rows are ThrowHelper
exception-message building —
ThrowJsonException_JsonRequiredPropertyMissing / ThrowArgumentException_InvalidUTF8,
a StringBuilder grown in a loop on the cold exception path. Technically the
pattern, but negligible value (only runs when throwing). This is exactly the
cold-region/exception-path case the ControlFlow probe (#2040 / #1939) identified
as the salvageable demotion. Recommend applying an exception-path/cold-region
demotion so these drop to advisory — either here or as an immediate follow-up —
so the shape flags value, not just the mechanical pattern. Aspire's 13 rows do
not have this problem (logger/startup/arg-parsing paths look genuinely warm), so
the demo stands.

Verdict: solid, correct, favorability-clean shape. Ready once (a) the two
adversarial false-positive reviews land, and (b) the cold-path demotion is
applied or explicitly deferred with a tracking note.

[richlander]

Adversarial review reconciliation — 6 findings, all resolved and independently verified

Two-family false-positive review: GPT-5.5 and Gemini 3.1 Pro (both non-Claude, since the build agent is Claude-family). Six real precision issues; none dismissed.

GPT-5.5 (fixed in d9823aa8)

• False positive — pre-sized collections. new List() + EnsureCapacity(len) / Capacity setter before the loop still fired. Fix: a dominating pre-loop capacity setter suppresses the row.
• False positive — no-op Substring. Substring(0) / Substring(0, s.Length) return the same string (no allocation) but fired. Fix: suppress those statically-provable no-op forms.
• False demotion — finally as cold. finally runs on the normal path; demoting its loop allocations hid warm cost. Fix: cold-region = catch/filter/fault (+ throw-helper), not finally.

Gemini 3.1 Pro (fixed in 042cbd1a)

• False positive — lexical loop attribution. Loop regions were lexical [target, branch] spans, so goto-spaghetti code physically-inside-but-logically-outside a loop fired. Fix: the two new shapes now confirm loop membership via dominance/natural-loop (back-edge) info and fail-closed otherwise; shared lexical helpers left untouched so existing shapes are unperturbed.
• False positive — branchy sized/unsized defs. A ternary flag ? new List(100) : new List() merges to one stloc; the analyzer bound the unsized def and fired. Fix: fail-closed — fire only when ALL reaching defs of the receiver are no-capacity.
• False demotion — infinite-loop-as-throw-helper. A while(true){ work; if(fatal) throw; } was demoted because its only reachable exit is throw. Fix: restrict throw-helper demotion to genuine unconditional throw-helpers; real-work-in-unbounded-loop stays warm.

Convergence: both reviewers independently flagged the finally-as-cold demotion — high-signal.

Independent verification (built the fixed branch)

• Repro assembly covering all six: the 4 false positives (capacity ctor, EnsureCapacity, no-op Substring, ternary) → no rows; the 2 false demotions (finally, infinite-loop) → stay medium (warm); a genuine unsized-in-loop → fires medium.
• True positives preserved: STJ TraverseGraphWithTopologicalSort 4 medium; STJ ThrowHelper 6 low (genuine throw-helpers).
• Favorability clean: existing shapes byte-identical. The stricter natural-loop precision correctly removed a lexical false positive on Aspire (string-slice 14→13) and one Serilog row — a net precision gain.

Status: precision-hardened. A confirming re-review round is dispatched to reach formal two-clean.

[richlander]

Status update: confirming re-review found 2 more issues (1 critical) — NOT ready to merge

The confirming re-review round (GPT-5.5 + Gemini 3.1 Pro on the fixed branch) verified the original 6 findings are resolved, but each reviewer found one NEW issue introduced by the fixes:

• CRITICAL (Gemini, verified): CollectNaturalLoops() uses a recursive Tarjan SCC Visit local function that stack-overflows and crashes the entire analyzer process on a method with a very deep CFG. I reproduced it deterministically: a synthesized method with ~25k sequential branch blocks makes <CollectNaturalLoops>g__Visit recurse ~24,865 times → Stack overflow. / abort (exit 134). Impact: any assembly containing one pathologically-large method takes down the whole library command. Fix in progress: rewrite Visit as an iterative DFS with an explicit work stack (preserving exact SCC semantics), plus a deep-CFG regression test.
• Medium (GPT-5.5, verified): HasDominatingPreLoopSizingEvent suppresses on EnsureCapacity(0) / Capacity = 0, which are no-ops on an empty list — hiding genuine unsized growth. Fix in progress: do not treat a provably-zero capacity argument as a dominating pre-size.

Both are being fixed on this branch with regression tests. I'll re-verify and run a second confirming round before this is ready. Holding merge.

[richlander]

Confirming-round findings resolved + independently verified

Both issues from the first confirming round are fixed and I re-verified each on the built branch:

• CRITICAL crash — resolved (eb2c572b). CollectNaturalLoops recursive Tarjan SCC rewritten as iterative (explicit TarjanFrame work stack; child->parent lowlink propagated at frame-pop, exactly mirroring the post-recursion min). My repro that previously aborted the process (25k-block method, ~24,865-deep recursion, SIGABRT) now completes cleanly. Regression test added (HandlesDeepCfgWithoutRecursion). Loop-detection parity confirmed: STJ TraverseGraphWithTopologicalSort still 4 medium, ThrowHelper 6 low, Aspire 13 collection + 13 string all medium — byte-identical.
• Zero-capacity over-suppression — resolved (0e045ccf). A provably-zero capacity arg (EnsureCapacity(0) / Capacity = 0) is no longer treated as a dominating pre-size. Verified on a fixture: ZeroEnsure/ZeroCap now fire medium; genuine EnsureCapacity(len) still suppresses; plain unsized still fires. Regression tests added for both zero cases.

286 analysis tests pass; CI re-running on the new HEAD. A second confirming re-review round (GPT-5.5 + Gemini, focused on iterative-Tarjan semantic equivalence + the zero-cap fix) is running before this is called ready. Still holding merge.

[richlander]

Two-clean reached — second confirming round is CLEAN from both reviewers

The second confirming re-review round (on HEAD 0e045ccf, focused on the iterative-Tarjan rewrite and the zero-capacity fix) came back CLEAN from both models:

• Gemini 3.1 Pro — CLEAN. Byte-for-byte diff of triage output on STJ + Aspire, recursive base 042cbd1a vs iterative 0e045ccf: 0 lines of diff — loop detection perfectly identical. Deep-CFG (25k blocks) completes without crash; the added HandlesDeepCfgWithoutRecursion test pushes 30k block edges. Confirmed VisitIterative mirrors the recursive SCC exactly (parent recorded in the frame; child->parent lowlink propagated at frame-pop; successor cursor advanced before descent; back/cross edges use index while on-stack). Zero-capacity fix verified on a real split-block case.
• GPT-5.5 — CLEAN. Build clean; 286 analysis tests pass. SCC parity vs the recursive base matched for nested, self-loop, sibling, and irreducible multi-entry loop rejection. Deep 25k-block command exits 0, no crash. Zero-sizing probe: only EnsureCapacity(0)/Capacity=0 fire; EnsureCapacity(len), non-zero constants, computed-zero, and unknown args all stay suppressed (fail-safe). Corpus parity exact (STJ TraverseGraph 4 medium, ThrowHelper 6 low, Aspire 13 + 13 medium).

Full adversarial trail (8 findings, all resolved + independently verified)

• Round 1 (FP review): 6 findings — GPT (pre-sized FP, no-op Substring FP, finally-as-cold) + Gemini (lexical-loop FP, branchy-defs FP, infinite-loop-as-throw-helper). Fixed in d9823aa8, 042cbd1a.
• Confirming round 1: 2 new findings the fixes introduced — Gemini CRITICAL recursive-Tarjan process crash + GPT zero-capacity over-suppression. Fixed in eb2c572b, 0e045ccf.
• Confirming round 2: CLEAN from both.

Every actionable finding maps to a resolution commit; none dismissed. This is ready to merge on your nod.