From 89fee9eff4408ac35b872f488d2bcfa1ec1d9271 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 21 Jun 2026 02:28:25 +0000 Subject: [PATCH 1/3] extractor: inter-procedural consume contract -> use-after-handoff OWN002 (recall 8/10 -> 9/11) The extractor treated every argument-pass as an ESCAPE (untracked), so a stream handed to a consumer that owns it (`Archive(s)`) vanished and a later touch of `s` was invisible -- the use-after-handoff arm of ownership-handoff-consume was a miss. The OwnIR bridge already models consume contracts end-to-end (params with an inferred `effect`, a `call` op that moves ownership across the call, pinned by tests/fixtures/ownir/handoff_contract.facts.json + test_ownir). This teaches the extractor to emit those facts: - ContractedCallee: a call whose first-party target owns a by-value IDisposable parameter carries an ownership contract. Passing a tracked local to it is a handoff, lowered to a `{op:"call", callee, args}` op (not an escape); the bridge moves ownership per the callee's inferred contract (a body that releases its param => consume), so a use after the move is OWN002. The cut is the SIGNATURE, not whole-program points-to -- like Rust's move. - By-value IDisposable parameters are tracked as owned obligations and emitted in the function's `params`, so the consumer's body (use + Dispose of the param) lets the bridge infer the contract and discharge it (no false leak on the consumer). - A resource local passed to a contracted call is exempted from the escape set. New fixture ownership-handoff-use (a pure use-after-handoff, no leak arm) is a miss before and a catch (OWN002) after: recall 8/10 -> 9/11. ownership-handoff-consume now fires both arms (OWN001+OWN002). Specificity 11/11, 0 FP, CI floor --min-recall 9. Precision: the consumer shape (a by-value IDisposable param) appears nowhere else in the corpus or samples, so the machinery engages only on the two handoff fixtures -- no other after.cs or dog-food scan can newly fire, and a `call` op is emitted only for callees the extractor also lowers as functions (callee name == function name, so no OWN040). The bridge half was validated locally (hand-built facts -> check_facts -> the exact OWN001/OWN002 verdicts); case.own pins the logic (test_corpus 7/7, test_ownir 88/88). The C# emission is validated by the corpus-benchmark + wpf-extractor CI jobs (no local .NET SDK). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 18 +++--- .../real-world/ownership-handoff-use/after.cs | 20 ++++++ .../ownership-handoff-use/before.cs | 26 ++++++++ .../real-world/ownership-handoff-use/case.own | 21 +++++++ .../expected-diagnostics.txt | 1 + .../real-world/ownership-handoff-use/notes.md | 25 ++++++++ docs/notes/corpus-benchmark.md | 33 +++++++--- docs/proposals/P-012-bug-corpus-mining.md | 12 ++-- frontend/roslyn/OwnSharp.Extractor/Program.cs | 63 ++++++++++++++++++- 9 files changed, 194 insertions(+), 25 deletions(-) create mode 100644 corpus/real-world/ownership-handoff-use/after.cs create mode 100644 corpus/real-world/ownership-handoff-use/before.cs create mode 100644 corpus/real-world/ownership-handoff-use/case.own create mode 100644 corpus/real-world/ownership-handoff-use/expected-diagnostics.txt create mode 100644 corpus/real-world/ownership-handoff-use/notes.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index abc17813..e294e948 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -625,13 +625,13 @@ jobs: - name: Score the corpus on real C# # Precision is gated absolutely (every fix silent, zero false positives); # recall is pinned at the measured floor and ratchets up as the extractor - # improves. Now 8/10 — pooled buffers ride the path-sensitive flow engine - # (Rent = acquire, Return = release: OWN003/OWN002, pool resolved via the Roslyn - # SemanticModel so an ALIASED receiver is caught), and ownership-transferring - # factory acquires (System.IO.File.Open*/Create*) are now recognised alongside - # `new`, so the leak arm of the interprocedural-handoff case fires OWN001. - # Remaining backlog: the use-after-handoff (OWN002) arm of that case, a - # cross-method use-after-dispose, and an injected-source region-escape. A drop - # below the floor is a regression. - run: python scripts/benchmark.py --min-recall 8 + # improves. Now 9/11 — pooled buffers ride the path-sensitive flow engine + # (Rent/Return: OWN003/OWN002, pool resolved via the Roslyn SemanticModel so an + # ALIASED receiver is caught), factory acquires (System.IO.File.Open*/Create*) are + # recognised alongside `new`, and the inter-procedural CONSUME contract is modelled: + # a first-party method owning a by-value IDisposable param is a handoff (a `call` + # op), so a use after the handoff trips OWN002 (the cut is the signature, like + # Rust's move). Remaining backlog: a cross-method use-after-dispose and an + # injected-source region-escape. A drop below the floor is a regression. + run: python scripts/benchmark.py --min-recall 9 diff --git a/corpus/real-world/ownership-handoff-use/after.cs b/corpus/real-world/ownership-handoff-use/after.cs new file mode 100644 index 00000000..8aacb785 --- /dev/null +++ b/corpus/real-world/ownership-handoff-use/after.cs @@ -0,0 +1,20 @@ +using System; +using System.IO; + +// FIX: read everything we need BEFORE handing ownership off, then never touch the stream. +static class HandoffUse +{ + public static void Consume(Stream sink) + { + sink.CopyTo(Stream.Null); + sink.Dispose(); + } + + static long Run(string path) + { + var s = File.OpenRead(path); + long len = s.Length; // read first ... + Consume(s); // ... then move ownership last + return len; + } +} diff --git a/corpus/real-world/ownership-handoff-use/before.cs b/corpus/real-world/ownership-handoff-use/before.cs new file mode 100644 index 00000000..e4a61bf6 --- /dev/null +++ b/corpus/real-world/ownership-handoff-use/before.cs @@ -0,0 +1,26 @@ +using System; +using System.IO; + +// A pure inter-procedural use-after-handoff (no leak arm): a stream is handed to a +// consumer that takes OWNERSHIP (reads it, then disposes it), and the caller then touches +// the stream again. Unlike ownership-handoff-consume there is no leak arm -- the handoff +// itself is correct, so the ONLY bug is the use AFTER ownership moved. Common shape: +// serialize/compress into a stream, hand it to a sink that owns it, then accidentally read +// it once more (an ObjectDisposedException at runtime). +static class HandoffUse +{ + // Consumer: takes ownership of `sink` and closes it. `sink` is `consume Stream`. + public static void Consume(Stream sink) + { + sink.CopyTo(Stream.Null); + sink.Dispose(); // Consume owns and closes it + } + + // BUG: ownership moved into Consume (which disposed it), then the stream is read. -> OWN002 + static long Run(string path) + { + var s = File.OpenRead(path); + Consume(s); // ownership moves to Consume + return s.Length; // use-after-handoff (s is disposed) -> OWN002 + } +} diff --git a/corpus/real-world/ownership-handoff-use/case.own b/corpus/real-world/ownership-handoff-use/case.own new file mode 100644 index 00000000..de4056d6 --- /dev/null +++ b/corpus/real-world/ownership-handoff-use/case.own @@ -0,0 +1,21 @@ +// OwnLang model of a PURE inter-procedural use-after-handoff (no leak arm). `take` takes +// the stream BY VALUE (a resource type => CONSUME): ownership moves in and it closes the +// stream, so a caller must not touch it after the handoff. `run` hands off then reads -> +// OWN002 (use after the resource was consumed by the callee). The fix reads BEFORE the +// handoff. The signature is the cut -- the caller is checked against `take`'s contract, +// not its body; no whole-program analysis. (`consume` is an OwnLang keyword, so the +// reduction names the consumer `take`; the C# consumer is `Consume`.) +module Corpus +resource Stream { + acquire open + release close +} +fn take(s: Stream) { + use s; + release s; +} +fn run() { + let s = acquire Stream(); + take(s); // ownership moves into the consumer + use s; // <-- touched after handoff -> OWN002 +} diff --git a/corpus/real-world/ownership-handoff-use/expected-diagnostics.txt b/corpus/real-world/ownership-handoff-use/expected-diagnostics.txt new file mode 100644 index 00000000..3a36fa92 --- /dev/null +++ b/corpus/real-world/ownership-handoff-use/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN002 diff --git a/corpus/real-world/ownership-handoff-use/notes.md b/corpus/real-world/ownership-handoff-use/notes.md new file mode 100644 index 00000000..22c0be68 --- /dev/null +++ b/corpus/real-world/ownership-handoff-use/notes.md @@ -0,0 +1,25 @@ +# Inter-procedural use-after-handoff (pure) + +**Pattern:** a stream is handed to a consumer that takes **ownership** (reads it, then +`Dispose()`s it), and the caller then touches the stream again. Unlike +`ownership-handoff-consume` there is **no leak arm** — the handoff itself is correct, so the +*only* bug is the use **after** ownership moved. Common shape: serialize/compress into a +stream, hand it to a sink that owns it, then accidentally read it once more (an +`ObjectDisposedException` at runtime). + +**What the checker says:** using a resource after it was consumed by a callee is the generic +**OWN002** (use after release) — the same code `.own` produces for use-after-dispose. + +**Why this case exists (the consume-contract proof).** The extractor used to treat any +argument-passing as an *escape* (untracked), so a stream handed to `Consume(s)` simply +vanished and the later `s.Length` was invisible — a **miss**. With the inter-procedural +**consume contract**, a first-party method owning a by-value `IDisposable` parameter is +recognised, the handoff `Consume(s)` lowers to a `call` op, and the bridge **moves +ownership** across it — the cut is the *signature*, not whole-program points-to, exactly like +Rust's move. The use after the move then trips **OWN002**. This fixture is a *miss* before the +contract and a *catch* after; `ownership-handoff-consume` is caught for its leak arm either +way, so this is the row that makes the use-after-handoff capability a measurable ratchet. + +**Honesty / scope.** `case.own` is a faithful hand reduction of the C# pattern, not C# the +`.own` checker ingested. `before.cs` / `after.cs` are representative of the bug and its fix, +not a verbatim copy of one PR. diff --git a/docs/notes/corpus-benchmark.md b/docs/notes/corpus-benchmark.md index c053cbc3..cf3b9ac4 100644 --- a/docs/notes/corpus-benchmark.md +++ b/docs/notes/corpus-benchmark.md @@ -26,7 +26,7 @@ correct code), and **recall was 3/9** — the three caught are exactly the subscription/region class the extractor is strongest at (`zombie-viewmodel` → OWN001, two static-event escapes → OWN014). -## Ratchet → 8/10 (four ratchets) +## Ratchet → 9/11 (five ratchets) ### → 4/9: a fixture was understating us @@ -89,14 +89,29 @@ handed back by some other API is never mistaken for an acquire). With it the lea 8/10**. The blast radius is exactly one file: nothing else in the corpus or the samples opens a `File.*` stream, so no `after.cs` and no dog-food scan can newly cry wolf. -The remaining gaps are genuine **frontend extraction** islands. The *use-after-handoff* -(`OWN002`) arm of `ownership-handoff-consume` — caught only as the leak today — needs the -inter-procedural **consume** contract (a method that disposes a by-value parameter, checked at -call sites like Rust's move; the cut is the *signature*, no whole-program points-to). A -field/cross-method use-after-dispose needs cross-method field-state. And the injected-source -region-escape (`viewmodel-escapes-to-app`) needs the source's lifetime *proven* — its DI -registration — which the fixture does not even carry. The `.own` reductions catch all three; -the C# extractor does not yet. Each is a real capability the floor will ratchet up to as it lands. +### → 9/11: the inter-procedural consume contract + +The last handoff gap was the *use-after-handoff* arm — a stream handed to a consumer that +disposes it, then touched again (`OWN002`). The extractor treated every argument-pass as an +*escape* (untracked), so the handed-off stream vanished and the later read was invisible. Now +a first-party method owning a by-value `IDisposable` parameter is recognised as carrying an +ownership **contract**: passing a tracked local to it is a **handoff** (lowered to a `call` +op, not an escape), and the OwnIR bridge — whose consume machinery was already built and +tested (`handoff_contract.facts.json`) — **moves ownership** across the call per the callee's +inferred contract (a body that releases its param ⇒ consume). A use after the move then trips +`OWN002`. The cut is the **signature**, not whole-program points-to — the modular handoff the +`.own` reduction proves, like Rust's move. A new fixture `ownership-handoff-use` (a *pure* +use-after-handoff, no leak arm) is a miss before the contract and a catch after, so **recall +is now 9/11**; `ownership-handoff-consume` now fires both its arms (`OWN001`+`OWN002`). The +bridge half was pinned locally (hand-built facts → `check_facts` → the exact verdicts), and +the blast radius is the two handoff fixtures — the consumer shape (a by-value `IDisposable` +parameter) appears nowhere else in the corpus or samples, so nothing else can newly fire. + +The remaining gaps are genuine **frontend extraction** islands: a field/cross-method +use-after-dispose needs cross-method field-state, and the injected-source region-escape +(`viewmodel-escapes-to-app`) needs the source's lifetime *proven* — its DI registration — +which the fixture does not even carry. The `.own` reductions catch both; the C# extractor does +not yet. Each is a real capability the floor will ratchet up to as it lands. ## Why catch/clean, not exact-code match diff --git a/docs/proposals/P-012-bug-corpus-mining.md b/docs/proposals/P-012-bug-corpus-mining.md index 9e7aeab9..951e4d40 100644 --- a/docs/proposals/P-012-bug-corpus-mining.md +++ b/docs/proposals/P-012-bug-corpus-mining.md @@ -6,7 +6,7 @@ (the bug is caught) and specificity (the fix is silent), gated in the `corpus-benchmark` CI job. This is the measurement spine — the defensible number, and the verifiable reward for any future learning loop. First measurement **3/9 - caught · 9/9 clean · 0 FP**, ratcheted to **8/10** over four steps: (1) a *fixture* + caught · 9/9 clean · 0 FP**, ratcheted to **9/11** over five steps: (1) a *fixture* was understating us — `screentogif-loaded-subscription` referenced an undeclared VM type → `OWN050`, fixed by making it self-contained; (2) a real *capability* — pooled buffers are now routed through the path-sensitive flow engine (Rent = @@ -17,10 +17,12 @@ receiver (`var p = ArrayPool.Shared; p.Return(buf)`) — a miss for the text heuristic — is caught (`arraypool-aliased-receiver`, +1 row); (4) ownership-transferring **factory acquires** (`System.IO.File.Open*`/`Create*`) are recognised alongside `new`, - so the leak arm of the interprocedural-handoff case fires `OWN001`. Perfect precision - throughout. The remaining gaps: the use-after-handoff (`OWN002`) arm of that case (needs - the inter-procedural *consume* contract), a cross-method use-after-dispose, and an - injected-source region-escape — the tracked recall backlog the floor ratchets up to. + so the leak arm of the interprocedural-handoff case fires `OWN001`; (5) the inter-procedural + **consume contract** — a first-party method owning a by-value `IDisposable` param is a + handoff (lowered to a `call` op), so the bridge moves ownership across it and a use after the + handoff trips `OWN002` (the cut is the signature, like Rust's move; `ownership-handoff-use`, + +1 row). Perfect precision throughout. The remaining gaps: a cross-method use-after-dispose, + and an injected-source region-escape — the tracked recall backlog the floor ratchets up to. Still ahead: those, GitHub mining at scale (stage 1) and the 50–100-repo prevalence scan (stage 2). See [docs/notes/corpus-benchmark.md](../notes/corpus-benchmark.md). diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 603d9f31..8cc3769a 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -448,6 +448,20 @@ or ImplicitObjectCreationExpressionSyntax return true; case ExpressionStatementSyntax es: InjectThrowEdge(es, nodes, onThrow); + // A call to a first-party method that takes ownership of an IDisposable + // parameter is an inter-procedural HANDOFF: lower it to a `call` op so the + // bridge moves ownership per the callee's contract (consume => the arg is + // released here; a later use is OWN002). Other calls fall through to use/escape. + if (es.Expression is InvocationExpressionSyntax callInv + && ContractedCallee(callInv, model) is { } callee) + { + var callArgs = callInv.ArgumentList.Arguments + .Select(a => a.Expression is IdentifierNameSyntax aid + ? aid.Identifier.Text : a.Expression.ToString()) + .ToList(); + nodes.Add(new { op = "call", callee, args = callArgs, line = LineOf(callInv) }); + return true; + } EmitFlowExpr(es.Expression, tracked, model, nodes); return true; case IfStatementSyntax ifs: @@ -783,6 +797,31 @@ static bool IsOwningFactory(ExpressionSyntax? e, SemanticModel model) return ns is { Name: "System" } && ns.ContainingNamespace is { IsGlobalNamespace: true }; } +// The qualified name (`Class.Method`) of a first-party method this call targets, IF that +// method takes a by-value IDisposable parameter — i.e. it carries an ownership CONTRACT +// the bridge resolves from the callee's own lowered body (release of the param => consume). +// Passing a tracked local to such a method is an inter-procedural HANDOFF (lowered to a +// `call` op), not an escape: the bridge moves ownership for a consume param, so a later use +// is OWN002. Returns null for BCL / uncontracted calls — the extractor emits a `call` op +// only for callees it also lowers as functions (else the bridge has no signature for them). +static string? ContractedCallee(ExpressionSyntax e, SemanticModel model) +{ + if (e is not InvocationExpressionSyntax inv + || model.GetSymbolInfo(inv).Symbol is not IMethodSymbol sym + || sym.DeclaringSyntaxReferences.Length == 0) // first-party (declared in source) + return null; + var owns = false; + foreach (var p in sym.Parameters) + if (p.RefKind == RefKind.None && ImplementsIDisposable(p.Type)) + { + owns = true; + break; + } + if (!owns) + return null; + return sym.ContainingType is { } ct ? $"{ct.Name}.{sym.Name}" : sym.Name; +} + // A field/local type treated as owned-disposable (syntax-only heuristic — no // semantic model): a curated set plus a few suffixes. Gated on the class `new`ing // the value, so injected/borrowed disposables are not flagged. Timer types are @@ -1311,6 +1350,16 @@ or ImplicitObjectCreationExpressionSyntax continue; var candidates = new HashSet(); var poolBuffers = new HashSet(); // candidates that are ArrayPool buffers + // By-value IDisposable PARAMETERS are owned obligations handed in from the + // caller (the consume/borrow side of an inter-procedural handoff). Track them + // so the body's use / Dispose of the param lowers to use / release — the bridge + // infers the param's contract (released => consume) from that body, and a leak + // of an undischarged consume param maps back to the parameter. + var resourceParams = new List<(string Name, int Line)>(); + foreach (var pp in method.ParameterList.Parameters) + if (pp.Modifiers.Count == 0 && pp.Type is { } ppt + && model.GetTypeInfo(ppt).Type is { } ppts && ImplementsIDisposable(ppts)) + resourceParams.Add((pp.Identifier.Text, LineOf(pp))); foreach (var ld in mbody.DescendantNodes().OfType()) { if (ld.UsingKeyword != default) @@ -1329,7 +1378,7 @@ or ImplicitObjectCreationExpressionSyntax } init else if (IsOwningFactory(v.Initializer?.Value, model)) // File.Open*/Create* factory candidates.Add(v.Identifier.Text); } - if (candidates.Count == 0) + if (candidates.Count == 0 && resourceParams.Count == 0) continue; // A local that escapes (returned / assigned out) is conservatively not // tracked — its release may be the caller's job. For an IDisposable, @@ -1343,13 +1392,22 @@ or ImplicitObjectCreationExpressionSyntax } init var nm = idn.Identifier.Text; if (!candidates.Contains(nm)) continue; + // ... unless it is passed to a CONSUME/BORROW contract (a first-party + // method owning an IDisposable param): that is a handoff modelled by a + // `call` op, not an escape (else the use-after-handoff would be hidden). + bool consumedArg = idn.Parent is ArgumentSyntax + && idn.Parent.Parent is ArgumentListSyntax + && idn.Parent.Parent.Parent is InvocationExpressionSyntax cinv + && ContractedCallee(cinv, model) is not null; if (idn.Parent is ReturnStatementSyntax || (idn.Parent is AssignmentExpressionSyntax asg && asg.Right == idn) - || (idn.Parent is ArgumentSyntax && !poolBuffers.Contains(nm))) + || (idn.Parent is ArgumentSyntax && !poolBuffers.Contains(nm) && !consumedArg)) escapedLocals.Add(nm); } var tracked = new HashSet(candidates); tracked.ExceptWith(escapedLocals); + foreach (var (rp, _) in resourceParams) + tracked.Add(rp); // owned obligations handed in from the caller if (tracked.Count == 0) continue; statMethodsWithLocal++; @@ -1364,6 +1422,7 @@ or ImplicitObjectCreationExpressionSyntax } init { name = $"{cls.Identifier.Text}.{MethodName(method)}", file, + @params = resourceParams.Select(rp => new { name = rp.Name, line = rp.Line }).ToList(), body = fbody, }); } From a25625f957ad7ce37a2543cd3c35bbf295c1578a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 21 Jun 2026 02:39:01 +0000 Subject: [PATCH 2/3] fix: model consume-handoff as a call-site release, not a call op (resolves OWN040 + review) The call-op design crashed CI: UnitOfWorkFlowSample's consumer ConsumeUnitOfWork(UnitOfWork uow) is a by-value IDisposable param the type-name blast-radius grep missed, and its body is skipped by the flow pass, so the emitted `call` op referenced an undeclared callee -> the bridge raised an unmapped OWN040 and the extractor exited with an internal facts error. Codex (P1) and CodeRabbit flagged the same, plus arg/param arity (OWN041) and an over-broad escape exemption. Replace the whole call-op / params / signature machinery with the pool-Return pattern: a call to a first-party CONSUMER (a method whose own body disposes a by-value IDisposable parameter) is modelled as a RELEASE of the matching argument at the call site (ConsumeReleaseArg + DisposesLocal). A use of the argument after the call is then a use-after-handoff (OWN002), via the existing flow machinery. This dissolves every review finding by construction: - no `call` op -> no undeclared-callee OWN040, no arg/param OWN041 (P1/P2); - the escape exemption is gated on the SAME ConsumeReleaseArg as the release and on the `Consume(s);` statement form the pass actually lowers, so an argument is exempted iff it is released -- never a tracked-but-unreleased false leak, and only the arg bound to the disposed by-value param is exempted (P3 + CodeRabbit parameter-aware); - a callee with no body (interface/abstract/extern) or that doesn't dispose the param yields null -> the argument stays an ordinary escape (no crash); - no cross-call callee-name key, so no namespace-collision concern. Same verdicts as before: ownership-handoff-consume before -> OWN001+OWN002, after clean; ownership-handoff-use before -> OWN002, after clean. Recall 9/11, 0 FP, floor --min-recall 9. Validated locally: facts -> check_facts -> the exact verdicts; test_corpus 7/7; test_ownir 88/88. Docs updated to the call-site-release framing. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .../real-world/ownership-handoff-use/notes.md | 15 +-- docs/notes/corpus-benchmark.md | 28 +++-- docs/proposals/P-012-bug-corpus-mining.md | 10 +- frontend/roslyn/OwnSharp.Extractor/Program.cs | 103 +++++++++--------- 4 files changed, 83 insertions(+), 73 deletions(-) diff --git a/corpus/real-world/ownership-handoff-use/notes.md b/corpus/real-world/ownership-handoff-use/notes.md index 22c0be68..769a7dc7 100644 --- a/corpus/real-world/ownership-handoff-use/notes.md +++ b/corpus/real-world/ownership-handoff-use/notes.md @@ -12,13 +12,14 @@ stream, hand it to a sink that owns it, then accidentally read it once more (an **Why this case exists (the consume-contract proof).** The extractor used to treat any argument-passing as an *escape* (untracked), so a stream handed to `Consume(s)` simply -vanished and the later `s.Length` was invisible — a **miss**. With the inter-procedural -**consume contract**, a first-party method owning a by-value `IDisposable` parameter is -recognised, the handoff `Consume(s)` lowers to a `call` op, and the bridge **moves -ownership** across it — the cut is the *signature*, not whole-program points-to, exactly like -Rust's move. The use after the move then trips **OWN002**. This fixture is a *miss* before the -contract and a *catch* after; `ownership-handoff-consume` is caught for its leak arm either -way, so this is the row that makes the use-after-handoff capability a measurable ratchet. +vanished and the later `s.Length` was invisible — a **miss**. Now a call to a first-party +**consumer** — a method whose own body disposes a by-value `IDisposable` parameter — is +modelled as a **release of the argument at the call site**, the same shape as pool +`Return(buf)` (the resource leaves the caller's hands right there). The use *after* that +release then trips **OWN002**. The signal is the callee's own body, so it is inter-procedural +without a cross-call signature table (and so without a dangling-callee crash). This fixture is +a *miss* before and a *catch* after; `ownership-handoff-consume` is caught for its leak arm +either way, so this is the row that makes the use-after-handoff capability a measurable ratchet. **Honesty / scope.** `case.own` is a faithful hand reduction of the C# pattern, not C# the `.own` checker ingested. `before.cs` / `after.cs` are representative of the bug and its fix, diff --git a/docs/notes/corpus-benchmark.md b/docs/notes/corpus-benchmark.md index cf3b9ac4..dc58ea6f 100644 --- a/docs/notes/corpus-benchmark.md +++ b/docs/notes/corpus-benchmark.md @@ -94,18 +94,22 @@ a `File.*` stream, so no `after.cs` and no dog-food scan can newly cry wolf. The last handoff gap was the *use-after-handoff* arm — a stream handed to a consumer that disposes it, then touched again (`OWN002`). The extractor treated every argument-pass as an *escape* (untracked), so the handed-off stream vanished and the later read was invisible. Now -a first-party method owning a by-value `IDisposable` parameter is recognised as carrying an -ownership **contract**: passing a tracked local to it is a **handoff** (lowered to a `call` -op, not an escape), and the OwnIR bridge — whose consume machinery was already built and -tested (`handoff_contract.facts.json`) — **moves ownership** across the call per the callee's -inferred contract (a body that releases its param ⇒ consume). A use after the move then trips -`OWN002`. The cut is the **signature**, not whole-program points-to — the modular handoff the -`.own` reduction proves, like Rust's move. A new fixture `ownership-handoff-use` (a *pure* -use-after-handoff, no leak arm) is a miss before the contract and a catch after, so **recall -is now 9/11**; `ownership-handoff-consume` now fires both its arms (`OWN001`+`OWN002`). The -bridge half was pinned locally (hand-built facts → `check_facts` → the exact verdicts), and -the blast radius is the two handoff fixtures — the consumer shape (a by-value `IDisposable` -parameter) appears nowhere else in the corpus or samples, so nothing else can newly fire. +a call to a first-party **consumer** — a method whose own body disposes a by-value +`IDisposable` parameter — is modelled as a **release of the argument at the call site**, the +same shape as pool `Return(buf)` (the resource leaves the caller's hands right there). A use of +the argument *after* that call is then a use-after-release, `OWN002`; the matching argument is +exempted from the escape set so it stays tracked through the handoff. It is inter-procedural — +the signal is the *callee's own body* — but there is **no cross-call signature table**, so a +callee with no body to inspect (interface / abstract / extern) or that doesn't dispose the +parameter yields nothing and the argument stays an ordinary escape. Crucially there is no +dangling `call` op to a method the flow pass never lowered (the early call-op design crashed +the bridge on exactly that — `UnitOfWorkFlowSample`'s consumer, whose body the pass skips), and +the escape exemption is gated on the *same* body-inspection as the release, so an argument is +exempted **iff** it is also released — never a tracked-but-unreleased local that would read as a +false leak. A new fixture `ownership-handoff-use` (a *pure* use-after-handoff, no leak arm) is a +miss before and a catch after, so **recall is now 9/11**; `ownership-handoff-consume` now fires +both its arms (`OWN001`+`OWN002`). The verdicts were pinned locally (hand-built facts → +`check_facts` → the exact `OWN001`/`OWN002` and silence on the fixes). The remaining gaps are genuine **frontend extraction** islands: a field/cross-method use-after-dispose needs cross-method field-state, and the injected-source region-escape diff --git a/docs/proposals/P-012-bug-corpus-mining.md b/docs/proposals/P-012-bug-corpus-mining.md index 951e4d40..7d58562f 100644 --- a/docs/proposals/P-012-bug-corpus-mining.md +++ b/docs/proposals/P-012-bug-corpus-mining.md @@ -18,10 +18,12 @@ heuristic — is caught (`arraypool-aliased-receiver`, +1 row); (4) ownership-transferring **factory acquires** (`System.IO.File.Open*`/`Create*`) are recognised alongside `new`, so the leak arm of the interprocedural-handoff case fires `OWN001`; (5) the inter-procedural - **consume contract** — a first-party method owning a by-value `IDisposable` param is a - handoff (lowered to a `call` op), so the bridge moves ownership across it and a use after the - handoff trips `OWN002` (the cut is the signature, like Rust's move; `ownership-handoff-use`, - +1 row). Perfect precision throughout. The remaining gaps: a cross-method use-after-dispose, + **consume handoff** — a call to a first-party consumer (a method whose own body disposes a + by-value `IDisposable` param) is modelled as a *release* of the argument at the call site (the + pool-`Return` shape), so a use after the handoff trips `OWN002`; the signal is the callee's + own body, so there is no cross-call signature table and no dangling-callee crash + (`ownership-handoff-use`, +1 row). Perfect precision throughout. The remaining gaps: a + cross-method use-after-dispose, and an injected-source region-escape — the tracked recall backlog the floor ratchets up to. Still ahead: those, GitHub mining at scale (stage 1) and the 50–100-repo prevalence scan (stage 2). See diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 8cc3769a..9d21c6d8 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -448,20 +448,6 @@ or ImplicitObjectCreationExpressionSyntax return true; case ExpressionStatementSyntax es: InjectThrowEdge(es, nodes, onThrow); - // A call to a first-party method that takes ownership of an IDisposable - // parameter is an inter-procedural HANDOFF: lower it to a `call` op so the - // bridge moves ownership per the callee's contract (consume => the arg is - // released here; a later use is OWN002). Other calls fall through to use/escape. - if (es.Expression is InvocationExpressionSyntax callInv - && ContractedCallee(callInv, model) is { } callee) - { - var callArgs = callInv.ArgumentList.Arguments - .Select(a => a.Expression is IdentifierNameSyntax aid - ? aid.Identifier.Text : a.Expression.ToString()) - .ToList(); - nodes.Add(new { op = "call", callee, args = callArgs, line = LineOf(callInv) }); - return true; - } EmitFlowExpr(es.Expression, tracked, model, nodes); return true; case IfStatementSyntax ifs: @@ -718,6 +704,15 @@ static void EmitFlowExpr(ExpressionSyntax expr, HashSet tracked, Semanti nodes.Add(new { op = "release", var = pbuf, line = LineOf(expr) }); return; } + // Foo(s) where Foo consumes (disposes) its by-value IDisposable parameter -> the + // handoff RELEASES the argument here (the inter-procedural consume contract, modelled + // at the call site like pool Return). A later use of the argument is then a + // use-after-handoff (OWN002); `return` early so it is not also counted as a use. + if (ConsumeReleaseArg(expr, model) is { } carg && tracked.Contains(carg)) + { + nodes.Add(new { op = "release", var = carg, line = LineOf(expr) }); + return; + } // any other reference to a tracked local -> use (once per local in this expr). var used = new SortedSet(StringComparer.Ordinal); foreach (var idn in expr.DescendantNodesAndSelf().OfType()) @@ -797,29 +792,45 @@ static bool IsOwningFactory(ExpressionSyntax? e, SemanticModel model) return ns is { Name: "System" } && ns.ContainingNamespace is { IsGlobalNamespace: true }; } -// The qualified name (`Class.Method`) of a first-party method this call targets, IF that -// method takes a by-value IDisposable parameter — i.e. it carries an ownership CONTRACT -// the bridge resolves from the callee's own lowered body (release of the param => consume). -// Passing a tracked local to such a method is an inter-procedural HANDOFF (lowered to a -// `call` op), not an escape: the bridge moves ownership for a consume param, so a later use -// is OWN002. Returns null for BCL / uncontracted calls — the extractor emits a `call` op -// only for callees it also lowers as functions (else the bridge has no signature for them). -static string? ContractedCallee(ExpressionSyntax e, SemanticModel model) +// `Foo(s)` where Foo is a first-party method that CONSUMES a by-value IDisposable +// parameter — its own body disposes that parameter — takes ownership of the matching +// argument. The handoff is modelled as a RELEASE of the argument at the call site (the +// same shape as pool `Return(buf)`): a use of the argument after the call is then a +// use-after-handoff (OWN002). The inspection is the callee's OWN body, so there is no +// cross-call signature table and no dangling-callee crash — a method with no body +// (interface / abstract / extern) or that does not dispose the param yields null and the +// argument simply stays an escape. Returns the consumed argument's local name, or null. +static string? ConsumeReleaseArg(ExpressionSyntax e, SemanticModel model) { if (e is not InvocationExpressionSyntax inv || model.GetSymbolInfo(inv).Symbol is not IMethodSymbol sym - || sym.DeclaringSyntaxReferences.Length == 0) // first-party (declared in source) + || sym.DeclaringSyntaxReferences.Length == 0 + || sym.DeclaringSyntaxReferences[0].GetSyntax() is not BaseMethodDeclarationSyntax decl) return null; - var owns = false; - foreach (var p in sym.Parameters) - if (p.RefKind == RefKind.None && ImplementsIDisposable(p.Type)) - { - owns = true; - break; - } - if (!owns) + SyntaxNode? body = decl.Body ?? (SyntaxNode?)decl.ExpressionBody; + if (body is null) return null; - return sym.ContainingType is { } ct ? $"{ct.Name}.{sym.Name}" : sym.Name; + for (int i = 0; i < sym.Parameters.Length && i < inv.ArgumentList.Arguments.Count; i++) + { + var p = sym.Parameters[i]; + if (p.RefKind == RefKind.None && ImplementsIDisposable(p.Type) + && DisposesLocal(body, p.Name) + && inv.ArgumentList.Arguments[i].Expression is IdentifierNameSyntax aid) + return aid.Identifier.Text; + } + return null; +} + +// Does `body` dispose the local/parameter named `name` — a `name.Dispose()` / `.Close()` / +// `.DisposeAsync()` call (the consume signal)? +static bool DisposesLocal(SyntaxNode body, string name) +{ + foreach (var i in body.DescendantNodesAndSelf().OfType()) + if (i.Expression is MemberAccessExpressionSyntax m + && m.Name.Identifier.Text is "Dispose" or "Close" or "DisposeAsync" + && m.Expression is IdentifierNameSyntax id && id.Identifier.Text == name) + return true; + return false; } // A field/local type treated as owned-disposable (syntax-only heuristic — no @@ -1350,16 +1361,6 @@ or ImplicitObjectCreationExpressionSyntax continue; var candidates = new HashSet(); var poolBuffers = new HashSet(); // candidates that are ArrayPool buffers - // By-value IDisposable PARAMETERS are owned obligations handed in from the - // caller (the consume/borrow side of an inter-procedural handoff). Track them - // so the body's use / Dispose of the param lowers to use / release — the bridge - // infers the param's contract (released => consume) from that body, and a leak - // of an undischarged consume param maps back to the parameter. - var resourceParams = new List<(string Name, int Line)>(); - foreach (var pp in method.ParameterList.Parameters) - if (pp.Modifiers.Count == 0 && pp.Type is { } ppt - && model.GetTypeInfo(ppt).Type is { } ppts && ImplementsIDisposable(ppts)) - resourceParams.Add((pp.Identifier.Text, LineOf(pp))); foreach (var ld in mbody.DescendantNodes().OfType()) { if (ld.UsingKeyword != default) @@ -1378,7 +1379,7 @@ or ImplicitObjectCreationExpressionSyntax } init else if (IsOwningFactory(v.Initializer?.Value, model)) // File.Open*/Create* factory candidates.Add(v.Identifier.Text); } - if (candidates.Count == 0 && resourceParams.Count == 0) + if (candidates.Count == 0) continue; // A local that escapes (returned / assigned out) is conservatively not // tracked — its release may be the caller's job. For an IDisposable, @@ -1392,13 +1393,18 @@ or ImplicitObjectCreationExpressionSyntax } init var nm = idn.Identifier.Text; if (!candidates.Contains(nm)) continue; - // ... unless it is passed to a CONSUME/BORROW contract (a first-party - // method owning an IDisposable param): that is a handoff modelled by a - // `call` op, not an escape (else the use-after-handoff would be hidden). + // ... unless it is handed to a CONSUMER (a first-party method that + // disposes a by-value IDisposable param) as a bare `Consume(s);` + // statement: that is a handoff RELEASED at the call site, not an escape + // (else the use-after-handoff would be hidden). Tied to the statement + // form the flow pass lowers, so a local is never exempted without a + // matching release (a `var n = Consume(s)` initializer is NOT lowered + // here, so it stays an escape rather than a false leak). bool consumedArg = idn.Parent is ArgumentSyntax && idn.Parent.Parent is ArgumentListSyntax && idn.Parent.Parent.Parent is InvocationExpressionSyntax cinv - && ContractedCallee(cinv, model) is not null; + && cinv.Parent is ExpressionStatementSyntax + && ConsumeReleaseArg(cinv, model) == nm; if (idn.Parent is ReturnStatementSyntax || (idn.Parent is AssignmentExpressionSyntax asg && asg.Right == idn) || (idn.Parent is ArgumentSyntax && !poolBuffers.Contains(nm) && !consumedArg)) @@ -1406,8 +1412,6 @@ or ImplicitObjectCreationExpressionSyntax } init } var tracked = new HashSet(candidates); tracked.ExceptWith(escapedLocals); - foreach (var (rp, _) in resourceParams) - tracked.Add(rp); // owned obligations handed in from the caller if (tracked.Count == 0) continue; statMethodsWithLocal++; @@ -1422,7 +1426,6 @@ or ImplicitObjectCreationExpressionSyntax } init { name = $"{cls.Identifier.Text}.{MethodName(method)}", file, - @params = resourceParams.Select(rp => new { name = rp.Name, line = rp.Line }).ToList(), body = fbody, }); } From da857016d924c694e9059142f78e2c5c9acbcb97 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 21 Jun 2026 02:49:08 +0000 Subject: [PATCH 3/3] consume-handoff: harden ConsumeReleaseArgs (multi-arg, partial bodies, named args) Three correctness fixes from CodeRabbit's review of the call-site-release design (none hit the current fixtures, but all real): - Multi-arg calls (Major): EmitFlowExpr no longer `return`s after a consume release, so other tracked arguments of the same call (`Consume(s, t)`) still get their `use` -> a use-after-handoff on a co-argument is no longer hidden. The consumed arg is excluded from the use set (it is a release, never also a use). - Partial-method bodies (Minor): scan ALL DeclaringSyntaxReferences for the first with a body instead of only [0], so a consumer split across partial declarations is still recognised. - Named arguments (Major): map argument -> parameter by NameColon when `name:` is used, else by position, so `Consume(sink: s)` / reordered named args resolve to the right (disposed by-value IDisposable) parameter rather than by raw index. The helper now returns the full set of consumed arguments (ConsumeReleaseArgs); EmitFlowExpr releases each and the escape exemption uses .Contains(nm). Verdicts unchanged: ownership-handoff-consume before -> OWN001+OWN002, use before -> OWN002, both after clean (validated via check_facts); test_corpus 7/7. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- frontend/roslyn/OwnSharp.Extractor/Program.cs | 78 +++++++++++-------- 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 9d21c6d8..804135f6 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -704,19 +704,20 @@ static void EmitFlowExpr(ExpressionSyntax expr, HashSet tracked, Semanti nodes.Add(new { op = "release", var = pbuf, line = LineOf(expr) }); return; } - // Foo(s) where Foo consumes (disposes) its by-value IDisposable parameter -> the - // handoff RELEASES the argument here (the inter-procedural consume contract, modelled - // at the call site like pool Return). A later use of the argument is then a - // use-after-handoff (OWN002); `return` early so it is not also counted as a use. - if (ConsumeReleaseArg(expr, model) is { } carg && tracked.Contains(carg)) - { - nodes.Add(new { op = "release", var = carg, line = LineOf(expr) }); - return; - } - // any other reference to a tracked local -> use (once per local in this expr). + // Foo(s) where Foo consumes (disposes) a by-value IDisposable parameter -> the handoff + // RELEASES the matching argument(s) here (the inter-procedural consume contract, + // modelled at the call site like pool Return). A later use of an argument is then a + // use-after-handoff (OWN002). Do NOT return — other tracked arguments of the same call + // (`Consume(s, t)`) still need their `use` below; a consumed arg is excluded from it. + var consumed = ConsumeReleaseArgs(expr, model); + foreach (var c in consumed) + if (tracked.Contains(c)) + nodes.Add(new { op = "release", var = c, line = LineOf(expr) }); + // any other reference to a tracked local -> use (once per local; a consumed arg is a + // release above, never also a use). var used = new SortedSet(StringComparer.Ordinal); foreach (var idn in expr.DescendantNodesAndSelf().OfType()) - if (tracked.Contains(idn.Identifier.Text)) + if (tracked.Contains(idn.Identifier.Text) && !consumed.Contains(idn.Identifier.Text)) used.Add(idn.Identifier.Text); foreach (var u in used) nodes.Add(new { op = "use", var = u, line = LineOf(expr) }); @@ -792,33 +793,44 @@ static bool IsOwningFactory(ExpressionSyntax? e, SemanticModel model) return ns is { Name: "System" } && ns.ContainingNamespace is { IsGlobalNamespace: true }; } -// `Foo(s)` where Foo is a first-party method that CONSUMES a by-value IDisposable -// parameter — its own body disposes that parameter — takes ownership of the matching -// argument. The handoff is modelled as a RELEASE of the argument at the call site (the -// same shape as pool `Return(buf)`): a use of the argument after the call is then a -// use-after-handoff (OWN002). The inspection is the callee's OWN body, so there is no -// cross-call signature table and no dangling-callee crash — a method with no body -// (interface / abstract / extern) or that does not dispose the param yields null and the -// argument simply stays an escape. Returns the consumed argument's local name, or null. -static string? ConsumeReleaseArg(ExpressionSyntax e, SemanticModel model) +// The local names of arguments handed to a first-party CONSUMER at this call — a method +// whose own body disposes the by-value IDisposable parameter the argument binds to. Such +// an argument's ownership moves into the callee and is discharged there, so the handoff is +// modelled as a RELEASE of the argument at the call site (the same shape as pool +// `Return(buf)`); a later use is then a use-after-handoff (OWN002). Inspecting the callee's +// OWN body means no cross-call signature table and no dangling-callee crash — a callee with +// no body (interface / abstract / extern) or that does not dispose the param contributes +// nothing, and the argument stays an ordinary escape. Arguments resolve to parameters by +// NAME when `name:` is used, else by position; partial declarations are scanned for a body. +static List ConsumeReleaseArgs(ExpressionSyntax e, SemanticModel model) { + var consumed = new List(); if (e is not InvocationExpressionSyntax inv - || model.GetSymbolInfo(inv).Symbol is not IMethodSymbol sym - || sym.DeclaringSyntaxReferences.Length == 0 - || sym.DeclaringSyntaxReferences[0].GetSyntax() is not BaseMethodDeclarationSyntax decl) - return null; - SyntaxNode? body = decl.Body ?? (SyntaxNode?)decl.ExpressionBody; + || model.GetSymbolInfo(inv).Symbol is not IMethodSymbol sym) + return consumed; + SyntaxNode? body = null; + foreach (var r in sym.DeclaringSyntaxReferences) + if (r.GetSyntax() is BaseMethodDeclarationSyntax d + && ((SyntaxNode?)d.Body ?? d.ExpressionBody) is { } b) + { + body = b; + break; + } if (body is null) - return null; - for (int i = 0; i < sym.Parameters.Length && i < inv.ArgumentList.Arguments.Count; i++) + return consumed; + var args = inv.ArgumentList.Arguments; + for (int i = 0; i < args.Count; i++) { - var p = sym.Parameters[i]; - if (p.RefKind == RefKind.None && ImplementsIDisposable(p.Type) + // map argument -> parameter: by name for `name: value`, else by position. + var p = args[i].NameColon is { } nc + ? sym.Parameters.FirstOrDefault(q => q.Name == nc.Name.Identifier.Text) + : (i < sym.Parameters.Length ? sym.Parameters[i] : null); + if (p is { RefKind: RefKind.None } && ImplementsIDisposable(p.Type) && DisposesLocal(body, p.Name) - && inv.ArgumentList.Arguments[i].Expression is IdentifierNameSyntax aid) - return aid.Identifier.Text; + && args[i].Expression is IdentifierNameSyntax aid) + consumed.Add(aid.Identifier.Text); } - return null; + return consumed; } // Does `body` dispose the local/parameter named `name` — a `name.Dispose()` / `.Close()` / @@ -1404,7 +1416,7 @@ or ImplicitObjectCreationExpressionSyntax } init && idn.Parent.Parent is ArgumentListSyntax && idn.Parent.Parent.Parent is InvocationExpressionSyntax cinv && cinv.Parent is ExpressionStatementSyntax - && ConsumeReleaseArg(cinv, model) == nm; + && ConsumeReleaseArgs(cinv, model).Contains(nm); if (idn.Parent is ReturnStatementSyntax || (idn.Parent is AssignmentExpressionSyntax asg && asg.Right == idn) || (idn.Parent is ArgumentSyntax && !poolBuffers.Contains(nm) && !consumedArg))