From 304aaaee67ad3732630db4cb220060b0b8f854b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 21 Jun 2026 11:43:55 +0000 Subject: [PATCH 1/3] =?UTF-8?q?feat(extractor):=20transitive=20consume=20c?= =?UTF-8?q?ontract=20=E2=80=94=20use-after-handoff=20through=20a=20forward?= =?UTF-8?q?ing=20consumer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-method D4: the consume-handoff was modelled only for a callee that disposes a by-value IDisposable parameter DIRECTLY (#55). The inference deliberately stopped one hop short — a parameter merely forwarded to another call was left ambiguous (ownir.py's `passed` branch: "awaiting ... a later transitive pass"). So a consumer that forwards rather than disposes (`Consume(sink) -> Inner(sink) -> sink.Dispose()`) was NOT seen as a consumer, the handoff was not a release, and a later use of the argument was invisible. The extractor's consumer detection is now transitive: `ConsumesParam` recognises a parameter as consumed if the body disposes it directly OR forwards it to another first-party consumer that consumes it — following the chain, guarded against cycles (keyed per parameter), binding each callee's body with its own tree's SemanticModel so cross-file chains resolve. So `Consume(s)` is still a call-site release and a use after it trips OWN002. Conservative: a parameter handed to an unknown or merely-borrowing callee is not treated as consumed (no false release, no false OWN002). The facts are identical to the direct handoff (a release at the call site), so the core is unchanged. Pinned by a new corpus case `ownership-handoff-use-transitive` (before -> OWN002, after silent), which lifts the benchmark recall floor 10 -> 11. The .own reduction already checks the transitive shape (the front-end's signature inference recurses); this brings it to real C#. Validated locally: case.own -> OWN002, corpus 9/9, the constructed extractor facts -> OWN002. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 13 +-- corpus/real-world/README.md | 2 + .../ownership-handoff-use-transitive/after.cs | 28 ++++++ .../before.cs | 35 ++++++++ .../ownership-handoff-use-transitive/case.own | 28 ++++++ .../expected-diagnostics.txt | 1 + .../ownership-handoff-use-transitive/notes.md | 33 +++++++ docs/proposals/P-016-deep-fact-extraction.md | 7 +- frontend/roslyn/OwnSharp.Extractor/Program.cs | 89 ++++++++++++++----- 9 files changed, 209 insertions(+), 27 deletions(-) create mode 100644 corpus/real-world/ownership-handoff-use-transitive/after.cs create mode 100644 corpus/real-world/ownership-handoff-use-transitive/before.cs create mode 100644 corpus/real-world/ownership-handoff-use-transitive/case.own create mode 100644 corpus/real-world/ownership-handoff-use-transitive/expected-diagnostics.txt create mode 100644 corpus/real-world/ownership-handoff-use-transitive/notes.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 381abfcc..16b4574f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -780,9 +780,12 @@ jobs: # (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 10 + # a first-party method that owns a by-value IDisposable param — by disposing it + # directly OR by forwarding it to another first-party consumer (the TRANSITIVE chain, + # `ConsumesParam`) — is a handoff that releases the argument at the call site, so a use + # after the handoff trips OWN002 (the cut is the signature, like Rust's move). Remaining + # backlog: a FIELD-mediated cross-method use-after-dispose (dispose in one method, use in + # another via shared state) and an injected-source region-escape. A drop below the floor + # is a regression. + run: python scripts/benchmark.py --min-recall 11 diff --git a/corpus/real-world/README.md b/corpus/real-world/README.md index 1fc63176..7510991e 100644 --- a/corpus/real-world/README.md +++ b/corpus/real-world/README.md @@ -43,3 +43,5 @@ exception-path (анализ не моделирует исключения), co |------|-----|------------------| | `arraypool-use-after-return` | OWN002 | rented-буфер вернули в пул, потом ещё читали slice | | `arraypool-double-return` | OWN003 | один и тот же массив вернули в ArrayPool дважды ([#33767](https://github.com/dotnet/runtime/issues/33767)) | +| `ownership-handoff-use` | OWN002 | поток отдали потребителю (он его закрыл), потом ещё читали — use-after-handoff | +| `ownership-handoff-use-transitive` | OWN002 | то же, но потребитель не закрывает сам, а **пробрасывает** владение дальше (transitive consume) | diff --git a/corpus/real-world/ownership-handoff-use-transitive/after.cs b/corpus/real-world/ownership-handoff-use-transitive/after.cs new file mode 100644 index 00000000..91134e9b --- /dev/null +++ b/corpus/real-world/ownership-handoff-use-transitive/after.cs @@ -0,0 +1,28 @@ +using System; +using System.IO; + +// FIX: read the stream BEFORE handing ownership to the consumer chain, so nothing touches it +// after the handoff. Same transitive handoff (Consume -> Inner -> Dispose), correct order. The +// handoff still discharges ownership (Inner closes it), so there is no leak and no +// use-after-handoff -- the case must stay SILENT (this is the no-false-positive arm). +static class HandoffUseTransitive +{ + static void Inner(Stream s) + { + s.CopyTo(Stream.Null); + s.Dispose(); + } + + static void Consume(Stream sink) + { + Inner(sink); + } + + static long Run(string path) + { + var s = File.OpenRead(path); + var len = s.Length; // read BEFORE the handoff + Consume(s); // ownership moves to Consume -> Inner (disposed) + return len; // no use after the handoff -> silent + } +} diff --git a/corpus/real-world/ownership-handoff-use-transitive/before.cs b/corpus/real-world/ownership-handoff-use-transitive/before.cs new file mode 100644 index 00000000..2ed2ca15 --- /dev/null +++ b/corpus/real-world/ownership-handoff-use-transitive/before.cs @@ -0,0 +1,35 @@ +using System; +using System.IO; + +// A TRANSITIVE inter-procedural use-after-handoff: the stream is handed to `Consume`, which +// does NOT dispose it directly -- it FORWARDS it to `Inner`, which owns and closes it. So +// `Consume` consumes its parameter *transitively* (one hop further down the chain), and the +// caller's later read is a use-after-handoff. The consume signal travels through the +// forwarding chain: the extractor follows `Consume -> Inner -> Dispose` and models `Consume(s)` +// as a release of the argument at the call site, so a use after it trips OWN002. Unlike +// `ownership-handoff-use` (the callee disposes the param itself), here the callee only forwards. +static class HandoffUseTransitive +{ + // Direct consumer: owns and closes the stream. + static void Inner(Stream s) + { + s.CopyTo(Stream.Null); + s.Dispose(); // Inner owns and closes it + } + + // Transitive consumer: it does NOT close `sink` itself -- it forwards ownership to Inner. + // `sink` is still consumed (its obligation is discharged via Inner), so a caller must not + // touch the argument after the handoff. + static void Consume(Stream sink) + { + Inner(sink); // ownership forwarded to the real consumer + } + + // BUG: ownership moved into Consume (-> Inner, which disposed it), then the stream is read. + static long Run(string path) + { + var s = File.OpenRead(path); + Consume(s); // ownership moves to Consume -> Inner (disposed) + return s.Length; // use-after-handoff (s is disposed) -> OWN002 + } +} diff --git a/corpus/real-world/ownership-handoff-use-transitive/case.own b/corpus/real-world/ownership-handoff-use-transitive/case.own new file mode 100644 index 00000000..8db17844 --- /dev/null +++ b/corpus/real-world/ownership-handoff-use-transitive/case.own @@ -0,0 +1,28 @@ +// OwnLang model of a TRANSITIVE inter-procedural use-after-handoff. `take` does not close the +// stream itself -- it FORWARDS it to `inner`, which owns and closes it. So `take` consumes its +// parameter TRANSITIVELY (the obligation travels one hop further), and `run` touching the +// stream after `take(s)` is a use-after-handoff -> OWN002. The signature is still the cut: the +// caller is checked against `take`'s (transitively inferred) contract, not its body -- no +// whole-program analysis. (`consume` is an OwnLang keyword, so the reduction names the +// consumers `take`/`inner`; the C# consumers are `Consume`/`Inner`.) +module Corpus +resource Stream { + acquire open + release close +} +// direct consumer: it OWNS the stream and closes it. +fn inner(s: Stream) { + use s; + release s; +} +// transitive consumer: it forwards ownership to `inner` (does NOT close it itself), so it +// consumes `s` one hop down the chain. +fn take(s: Stream) { + inner(s); +} +// BUG: the stream is used after ownership moved into `take` (-> inner, which closed it). +fn run() { + let s = acquire Stream(); + take(s); // ownership consumed transitively (take -> inner) + use s; // <-- touched after handoff -> OWN002 +} diff --git a/corpus/real-world/ownership-handoff-use-transitive/expected-diagnostics.txt b/corpus/real-world/ownership-handoff-use-transitive/expected-diagnostics.txt new file mode 100644 index 00000000..3a36fa92 --- /dev/null +++ b/corpus/real-world/ownership-handoff-use-transitive/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN002 diff --git a/corpus/real-world/ownership-handoff-use-transitive/notes.md b/corpus/real-world/ownership-handoff-use-transitive/notes.md new file mode 100644 index 00000000..7055497e --- /dev/null +++ b/corpus/real-world/ownership-handoff-use-transitive/notes.md @@ -0,0 +1,33 @@ +# Inter-procedural use-after-handoff — the TRANSITIVE (forwarded) consumer + +**Pattern:** the same use-after-handoff as `ownership-handoff-use`, but the consumer does **not** +dispose the stream itself — it **forwards** it to another method that owns and closes it +(`Consume(sink) -> Inner(sink) -> sink.Dispose()`). The caller hands ownership to `Consume`, then +touches the stream again. The bug is the use **after** ownership moved; the handoff itself is +correct (the stream is closed, just one hop further down). + +**What the checker says:** using a resource after it was consumed (here, *transitively*) by a +callee is the generic **OWN002** (use after release) — the same code as use-after-dispose. + +**Why this case exists (the transitive consume-contract).** The consume contract was already +modelled for a callee that disposes a by-value `IDisposable` parameter *directly* +(`ownership-handoff-use`). But the inference deliberately **stopped at one hop**: a parameter +merely *handed to another call* was "genuinely ambiguous without that callee's contract, so we do +not infer it" (`ownlang/ownir.py`, the `passed` branch) — and the extractor's +`ConsumeReleaseArgs` only recognised a *direct* disposer. So `Consume` (which forwards rather than +disposes) was **not** seen as a consumer, the handoff was not a release, and the later `s.Length` +was invisible — a **miss**. + +Now the extractor's consumer detection (`ConsumesParam`) is **transitive**: a parameter is +consumed if the body either disposes it directly **or** forwards it to another first-party +consumer that consumes it — following `Consume -> Inner -> Dispose` through the chain, guarded +against cycles. Inspecting each callee's own body keeps it inter-procedural without a cross-call +signature table (and without a dangling-callee crash). Conservative: a parameter handed to an +unknown or merely-borrowing callee is **not** treated as consumed (no false release, no false +OWN002). This fixture is a **miss** before and a **catch** after — the ratchet row that lifts the +use-after-handoff capability across a forwarding hop. The `.own` reduction already checks the +transitive shape (the front-end's signature inference recurses); this brings it to real C#. + +**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/proposals/P-016-deep-fact-extraction.md b/docs/proposals/P-016-deep-fact-extraction.md index 2a4a9085..3fadbf09 100644 --- a/docs/proposals/P-016-deep-fact-extraction.md +++ b/docs/proposals/P-016-deep-fact-extraction.md @@ -121,7 +121,12 @@ a CFG-carrying bridge, and the existing core checks real code. slice; do it before the general case. - **B3 — Move / ownership transfer.** `return disposable`, or passing it to a callee that consumes it → move / escape (P-005 D5). Intraprocedural, driven by declared - signatures, not whole-program tracing. + signatures, not whole-program tracing. *Landed (consume handoff):* a call to a + first-party consumer — a method that disposes a by-value `IDisposable` parameter + **directly, or by forwarding it to another first-party consumer** (the transitive + chain, `ConsumesParam`) — releases the argument at the call site, so a use after the + handoff trips OWN002 (`corpus/real-world/ownership-handoff-use{,-transitive}`). The + signal is each callee's own body, so it is inter-procedural without a signature table. - **B4 — Borrow / Span.** `Rent` → view → `Return`, `Span`/`ref` aliasing (P-007) — the borrow checker's crown jewel, hardest on C# (ref structs, Span lifetimes). - **B5 — Lifetime ordering.** WPF005 escape (`OWN014`) and DI captive (P-006, diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index a9801dca..e0798406 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -860,30 +860,21 @@ static bool IsInNamespace(INamedTypeSymbol? t, params string[] parts) } // 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. +// that takes ownership of the by-value IDisposable parameter the argument binds to and +// discharges it: either by disposing it directly, or by forwarding it to another first-party +// consumer (the transitive case, `ConsumesParam`). 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 each 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 consume the param contributes nothing, and the argument stays an ordinary +// escape. Arguments resolve to parameters by NAME when `name:` is used, else by position. 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) 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 consumed; var args = inv.ArgumentList.Arguments; for (int i = 0; i < args.Count; i++) { @@ -891,14 +882,70 @@ static List ConsumeReleaseArgs(ExpressionSyntax e, SemanticModel model) 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) - && args[i].Expression is IdentifierNameSyntax aid) + if (p is not null + && args[i].Expression is IdentifierNameSyntax aid + && ConsumesParam(sym, p, model, + new HashSet(SymbolEqualityComparer.Default))) consumed.Add(aid.Identifier.Text); } return consumed; } +// The body of a first-party method (block or expression-bodied), scanning partial +// declarations; null for an interface/abstract/extern method (no body to inspect). +static SyntaxNode? ConsumerBody(IMethodSymbol sym) +{ + foreach (var r in sym.DeclaringSyntaxReferences) + if (r.GetSyntax() is BaseMethodDeclarationSyntax d + && ((SyntaxNode?)d.Body ?? d.ExpressionBody) is { } b) + return b; + return null; +} + +// Does `method` CONSUME (take ownership of and discharge) its by-value IDisposable +// parameter `param`? Either (a) the body disposes it directly (`param.Dispose()`), or +// (b) — the transitive step — the body hands it to ANOTHER first-party consumer that +// consumes it (`Inner(param)` where `Inner` consumes its matching parameter). So a +// forwarding chain `Consume(sink) => Inner(sink) => sink.Dispose()` is recognised, and a +// caller's `Consume(s)` is still a handoff (a call-site release). Inspecting each callee's +// own body keeps it inter-procedural without a signature table; `visited` (keyed on the +// parameter) guards recursion on cyclic call graphs. Conservative: a param handed to an +// unknown/borrowing callee yields false (no release, no false OWN002). +static bool ConsumesParam(IMethodSymbol method, IParameterSymbol param, + SemanticModel model, HashSet visited) +{ + if (param.RefKind != RefKind.None || !ImplementsIDisposable(param.Type)) + return false; + if (!visited.Add(param.OriginalDefinition)) + return false; // cycle guard (per parameter) + if (ConsumerBody(method) is not { } body) + return false; // no body -> contributes nothing + var name = param.Name; + if (DisposesLocal(body, name)) // (a) disposes the parameter directly + return true; + // (b) transitive: the parameter is handed to another first-party consumer. The body may + // live in another file, so bind its calls with that tree's OWN model (a SemanticModel only + // resolves nodes in its own tree); the Compilation is shared across all parsed inputs. + var bodyModel = model.Compilation.GetSemanticModel(body.SyntaxTree); + foreach (var inv in body.DescendantNodesAndSelf().OfType()) + { + if (bodyModel.GetSymbolInfo(inv).Symbol is not IMethodSymbol callee) + continue; + var cargs = inv.ArgumentList.Arguments; + for (int i = 0; i < cargs.Count; i++) + { + if (cargs[i].Expression is not IdentifierNameSyntax aid || aid.Identifier.Text != name) + continue; + var cp = cargs[i].NameColon is { } nc + ? callee.Parameters.FirstOrDefault(q => q.Name == nc.Name.Identifier.Text) + : (i < callee.Parameters.Length ? callee.Parameters[i] : null); + if (cp is not null && ConsumesParam(callee, cp, model, visited)) + return true; + } + } + return false; +} + // Does `body` dispose the local/parameter named `name` — a `name.Dispose()` / `.Close()` / // `.DisposeAsync()` call (the consume signal)? static bool DisposesLocal(SyntaxNode body, string name) From c99cee3018c6364bb5146d988071e38b3440b195 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 21 Jun 2026 11:50:39 +0000 Subject: [PATCH 2/3] fix(extractor): do not descend into nested lambda/local-function bodies for consume inference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P2: ConsumesParam's DescendantNodesAndSelf scan (and the pre-existing DisposesLocal) descended into nested lambdas / local functions, so a parameter disposed or forwarded ONLY inside a stored callback — `Register(Stream s) { callbacks.Add(() => Inner(s)); }` — was wrongly treated as consumed at the call site. That would release the caller's argument at `Register(s)` and report a phantom use-after-handoff (false OWN002), even though the dispose runs DEFERRED, not at the call boundary. Both scans now use DescendantNodesAndSelf(descendIntoChildren: not lambda/local-function) — the same deferred-body rule the flow lowering already follows (PR #57/#59). A dispose/forward inside a nested function contributes nothing to the consume inference. Conservative: it can only DROP a consume (a missed handoff = a leak, never a false use-after-release). Pinned by a flow-locals silent control: DeferredHandoffNoFalsePositive's `defer` is passed to a deferred-lambda "consumer", used, then disposed normally — it must stay SILENT (it would trip a false OWN002 without the fix). The transitive corpus case (Inner forwarded directly, not in a lambda) is unchanged. corpus 9/9, ownir 116/116. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 2 +- frontend/roslyn/OwnSharp.Extractor/Program.cs | 18 ++++++++++++---- frontend/roslyn/samples/FlowLocalsSample.cs | 21 +++++++++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 16b4574f..e04c0f3d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -510,7 +510,7 @@ jobs: # case disposes (no default) -> last case is the tail, no phantom no-match leak. # `ncf`: `ncf?.Dispose()` (null-conditional) in a threaded finally IS a release # (member-binding form), so it is disposed on the return path -> silent (Codex review). - for ok in clean looped esc exemptTask whileClean asyncDisposed asyncDisposedCfg tfClean tfCatch tfRet tfNull cda daci cif ctorLater lamPrior other doClean swAll ncf captured shaClean stopped; do + for ok in clean looped esc exemptTask whileClean asyncDisposed asyncDisposedCfg tfClean tfCatch tfRet tfNull cda daci cif ctorLater lamPrior other doClean swAll ncf captured shaClean stopped defer; do if echo "$out" | grep -q "'$ok'"; then echo "FAIL: silent/exempt case '$ok' was reported"; exit 1; fi done echo "OK: flow-sensitive OWN001/002/003 on real C# (path-sensitive, loops via while/foreach/for, try/finally sequential, never-vs-every-path wording, dispose-optional exempt, beyond flat)" diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index e0798406..4035627f 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -925,9 +925,14 @@ static bool ConsumesParam(IMethodSymbol method, IParameterSymbol param, return true; // (b) transitive: the parameter is handed to another first-party consumer. The body may // live in another file, so bind its calls with that tree's OWN model (a SemanticModel only - // resolves nodes in its own tree); the Compilation is shared across all parsed inputs. + // resolves nodes in its own tree); the Compilation is shared across all parsed inputs. Do + // NOT descend into nested lambda / local-function bodies: a forward there runs deferred, + // not at this call site, so it is not a handoff (Codex — same rule as the flow lowering). var bodyModel = model.Compilation.GetSemanticModel(body.SyntaxTree); - foreach (var inv in body.DescendantNodesAndSelf().OfType()) + foreach (var inv in body + .DescendantNodesAndSelf(n => n is not (AnonymousFunctionExpressionSyntax + or LocalFunctionStatementSyntax)) + .OfType()) { if (bodyModel.GetSymbolInfo(inv).Symbol is not IMethodSymbol callee) continue; @@ -947,10 +952,15 @@ static bool ConsumesParam(IMethodSymbol method, IParameterSymbol param, } // Does `body` dispose the local/parameter named `name` — a `name.Dispose()` / `.Close()` / -// `.DisposeAsync()` call (the consume signal)? +// `.DisposeAsync()` call (the consume signal)? Nested lambda / local-function bodies are NOT +// descended into: a `name.Dispose()` inside a stored callback runs deferred, not at this call +// site, so it is not a discharge here (Codex — the same deferred-body rule as the flow lowering). static bool DisposesLocal(SyntaxNode body, string name) { - foreach (var i in body.DescendantNodesAndSelf().OfType()) + foreach (var i in body + .DescendantNodesAndSelf(n => n is not (AnonymousFunctionExpressionSyntax + or LocalFunctionStatementSyntax)) + .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) diff --git a/frontend/roslyn/samples/FlowLocalsSample.cs b/frontend/roslyn/samples/FlowLocalsSample.cs index 0323156a..5b5586e2 100644 --- a/frontend/roslyn/samples/FlowLocalsSample.cs +++ b/frontend/roslyn/samples/FlowLocalsSample.cs @@ -484,6 +484,27 @@ public void TcpListenerNeverStopped() var tcpLeak = new TcpListener(IPAddress.Loopback, 0); tcpLeak.Start(); } + + // a "consumer" whose dispose of the parameter lives ONLY inside a nested lambda — it runs + // DEFERRED (when the delegate is invoked), not at the call site, so the method is NOT a + // consumer of `s` at its boundary (the transitive-consume inference must not descend into + // nested function bodies). + private static void DeferredConsumer(Stream s) + { + Action discharge = () => s.Dispose(); // deferred — not a consume at the call site + discharge(); + } + + // control (Codex review on PR #68): passing a local to DeferredConsumer is NOT a handoff + // (the dispose is deferred in a lambda), so the later use must NOT be a phantom + // use-after-handoff — no false OWN002. `defer` is disposed normally here, so it stays SILENT. + public void DeferredHandoffNoFalsePositive() + { + var defer = new MemoryStream(); + DeferredConsumer(defer); // NOT a release here (deferred lambda dispose) + defer.WriteByte(1); // must NOT trip OWN002 (it would, without the fix) + defer.Dispose(); // disposed here -> balanced -> silent + } } // A domain exception type literally named `Exception`, in a non-System namespace — the From dbbfd653198963c322e92de58d7eea5e1c277516 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 21 Jun 2026 11:56:44 +0000 Subject: [PATCH 3/3] refactor(extractor): DRY ImmediateInvocations, follow local-function callees, fix deferred control MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the CodeRabbit review on the transitive consume slice: - Extract `ImmediateInvocations(body)` — the shared "descendants excluding nested lambda / local-function bodies" scan — and use it in both ConsumesParam and DisposesLocal (the deferred-scope fix from c99cee3, now de-duplicated; CodeRabbit Major, already addressed). - ConsumerBody now also returns a LOCAL FUNCTION's body (LocalFunctionStatementSyntax has the same Body/ExpressionBody): a directly-called local function runs synchronously, so a forwarding chain through one is followed too — closes a transitive false-negative (CodeRabbit). - Fix the deferred-handoff control: DeferredConsumer stored the disposer in a lambda but then INVOKED it synchronously, so the stream was actually disposed at the call (a real bug, not a clean control — CodeRabbit Major). It now only STORES the callback (a registry, not drained), so nothing is disposed at the call site; `defer` is disposed once by its own Dispose() and stays SILENT — the genuine no-false-OWN002 guard. Behaviour unchanged on the catching path: the transitive corpus case (Inner forwarded via a direct method call) still hits recall 11 — c99cee3's benchmark was green at the floor. corpus 9/9, ownir 116/116. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- frontend/roslyn/OwnSharp.Extractor/Program.cs | 50 +++++++++++-------- frontend/roslyn/samples/FlowLocalsSample.cs | 25 +++++----- 2 files changed, 41 insertions(+), 34 deletions(-) diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 4035627f..4d04c897 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -891,17 +891,32 @@ static List ConsumeReleaseArgs(ExpressionSyntax e, SemanticModel model) return consumed; } -// The body of a first-party method (block or expression-bodied), scanning partial -// declarations; null for an interface/abstract/extern method (no body to inspect). +// The body of a first-party method or LOCAL FUNCTION (block or expression-bodied), scanning +// partial declarations; null for an interface/abstract/extern method (no body to inspect). A +// directly-called local function runs synchronously, so a forwarding chain through one must be +// followed too (CodeRabbit) — `LocalFunctionStatementSyntax` carries the same Body/ExpressionBody. static SyntaxNode? ConsumerBody(IMethodSymbol sym) { foreach (var r in sym.DeclaringSyntaxReferences) - if (r.GetSyntax() is BaseMethodDeclarationSyntax d - && ((SyntaxNode?)d.Body ?? d.ExpressionBody) is { } b) - return b; + switch (r.GetSyntax()) + { + case BaseMethodDeclarationSyntax d when ((SyntaxNode?)d.Body ?? d.ExpressionBody) is { } b: + return b; + case LocalFunctionStatementSyntax l when ((SyntaxNode?)l.Body ?? l.ExpressionBody) is { } lb: + return lb; + } return null; } +// The invocations that run IMMEDIATELY when `body` executes — excluding those inside a nested +// lambda / local-function body, which run deferred (when the delegate is later invoked), not at +// this method's call boundary. The same deferred-body rule the flow lowering already uses, so a +// dispose/forward stored in a callback is not mistaken for an immediate discharge (Codex/CodeRabbit). +static IEnumerable ImmediateInvocations(SyntaxNode body) => + body.DescendantNodes(n => n is not (AnonymousFunctionExpressionSyntax + or LocalFunctionStatementSyntax)) + .OfType(); + // Does `method` CONSUME (take ownership of and discharge) its by-value IDisposable // parameter `param`? Either (a) the body disposes it directly (`param.Dispose()`), or // (b) — the transitive step — the body hands it to ANOTHER first-party consumer that @@ -923,16 +938,12 @@ static bool ConsumesParam(IMethodSymbol method, IParameterSymbol param, var name = param.Name; if (DisposesLocal(body, name)) // (a) disposes the parameter directly return true; - // (b) transitive: the parameter is handed to another first-party consumer. The body may - // live in another file, so bind its calls with that tree's OWN model (a SemanticModel only - // resolves nodes in its own tree); the Compilation is shared across all parsed inputs. Do - // NOT descend into nested lambda / local-function bodies: a forward there runs deferred, - // not at this call site, so it is not a handoff (Codex — same rule as the flow lowering). + // (b) transitive: the parameter is handed to another first-party consumer at an IMMEDIATE + // call (not one deferred in a nested lambda/local function). The body may live in another + // file, so bind its calls with that tree's OWN model (a SemanticModel only resolves nodes in + // its own tree); the Compilation is shared across all parsed inputs. var bodyModel = model.Compilation.GetSemanticModel(body.SyntaxTree); - foreach (var inv in body - .DescendantNodesAndSelf(n => n is not (AnonymousFunctionExpressionSyntax - or LocalFunctionStatementSyntax)) - .OfType()) + foreach (var inv in ImmediateInvocations(body)) { if (bodyModel.GetSymbolInfo(inv).Symbol is not IMethodSymbol callee) continue; @@ -952,15 +963,12 @@ static bool ConsumesParam(IMethodSymbol method, IParameterSymbol param, } // Does `body` dispose the local/parameter named `name` — a `name.Dispose()` / `.Close()` / -// `.DisposeAsync()` call (the consume signal)? Nested lambda / local-function bodies are NOT -// descended into: a `name.Dispose()` inside a stored callback runs deferred, not at this call -// site, so it is not a discharge here (Codex — the same deferred-body rule as the flow lowering). +// `.DisposeAsync()` call (the consume signal)? Only IMMEDIATE calls count (`ImmediateInvocations` +// excludes nested lambda / local-function bodies): a `name.Dispose()` inside a stored callback +// runs deferred, not at this call site, so it is not a discharge here. static bool DisposesLocal(SyntaxNode body, string name) { - foreach (var i in body - .DescendantNodesAndSelf(n => n is not (AnonymousFunctionExpressionSyntax - or LocalFunctionStatementSyntax)) - .OfType()) + foreach (var i in ImmediateInvocations(body)) 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) diff --git a/frontend/roslyn/samples/FlowLocalsSample.cs b/frontend/roslyn/samples/FlowLocalsSample.cs index 5b5586e2..9dbe0132 100644 --- a/frontend/roslyn/samples/FlowLocalsSample.cs +++ b/frontend/roslyn/samples/FlowLocalsSample.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.IO; using System.Net; using System.Net.Sockets; @@ -485,23 +486,21 @@ public void TcpListenerNeverStopped() tcpLeak.Start(); } - // a "consumer" whose dispose of the parameter lives ONLY inside a nested lambda — it runs - // DEFERRED (when the delegate is invoked), not at the call site, so the method is NOT a - // consumer of `s` at its boundary (the transitive-consume inference must not descend into - // nested function bodies). - private static void DeferredConsumer(Stream s) - { - Action discharge = () => s.Dispose(); // deferred — not a consume at the call site - discharge(); - } + // a registry of deferred disposers: the parameter's dispose is captured in a STORED callback + // that runs LATER (when the registry is drained), not at this method's call boundary — so + // storing it does NOT consume the parameter here (the transitive-consume inference must not + // descend into nested lambda bodies). Intentionally not drained in this sample, so nothing is + // actually disposed via the callback — `defer` below is disposed only by its own Dispose(). + private static readonly List _deferredDisposers = new(); + private static void DeferredConsumer(Stream s) => _deferredDisposers.Add(() => s.Dispose()); - // control (Codex review on PR #68): passing a local to DeferredConsumer is NOT a handoff - // (the dispose is deferred in a lambda), so the later use must NOT be a phantom - // use-after-handoff — no false OWN002. `defer` is disposed normally here, so it stays SILENT. + // control (Codex/CodeRabbit on PR #68): passing a local to DeferredConsumer only STORES a + // deferred disposer (it does not dispose at the call site), so the later use must NOT be a + // phantom use-after-handoff — no false OWN002. `defer` is disposed normally here -> SILENT. public void DeferredHandoffNoFalsePositive() { var defer = new MemoryStream(); - DeferredConsumer(defer); // NOT a release here (deferred lambda dispose) + DeferredConsumer(defer); // stores a deferred disposer -> NOT a release here defer.WriteByte(1); // must NOT trip OWN002 (it would, without the fix) defer.Dispose(); // disposed here -> balanced -> silent }