From 735d59fba674d16605cb3f4a59ad997b629f58c8 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 22 Jun 2026 08:56:34 +0000 Subject: [PATCH 1/2] fix(extractor): a pooled buffer passed to an escaping constructor is an ownership transfer, not a leak (mined FP) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second false positive from mining Pipelines.Sockets.Unofficial: ArrayPoolBufferWriter.CreateNewSegment does `var array = pool.Rent(size); return new ArrayPoolRefCountedSegment(pool, array, prev);`. The flow-locals escape analysis treats a pooled buffer passed as an argument as a BORROW (so `pool.Return(buf); Work(buf)` still trips use-after-return) and expects a same-method Return — missing that the buffer's ownership is TRANSFERRED to the constructed object (which Returns it on its own teardown) and leaves the method inside that returned object. Result: a false OWN001 "never disposed/returned". New PassedToEscapingCtor: a pooled buffer handed to a `new` whose result is the direct return value or a field-assignment RHS escapes (untracked), like a returned local. One level only — a `new` buried in a local or another expression stays a borrow (an honest limitation). Purely syntactic, so it holds for unresolved wrapper types. Only the --flow-locals path is affected (the syntactic D1 path already escapes all arg-passed locals); a plain borrow `Work(buf)` still leaks if unreturned. Regression guard: FlowLocalsSample.PooledIntoReturnedCtor (`ctorMoved` handed to a returned PooledHolder) added to the --flow-locals job's must-stay-silent list. No corpus case has this shape, so the recall floor is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 5 +++- frontend/roslyn/OwnSharp.Extractor/Program.cs | 23 ++++++++++++++++++- frontend/roslyn/samples/FlowLocalsSample.cs | 22 ++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 670df2a3..ffd0c863 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -510,7 +510,10 @@ 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 defer; do + # `ctorMoved`: a pooled buffer handed to a constructor whose result is RETURNED transfers + # ownership to the returned wrapper -> escaped -> silent (mined FP on + # Pipelines.Sockets.Unofficial: ArrayPoolBufferWriter.CreateNewSegment). + 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 ctorMoved; 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 44a48903..4d6e4908 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -1037,6 +1037,19 @@ e is InvocationExpressionSyntax i && i.ArgumentList.Arguments[0].Expression is IdentifierNameSyntax buf ? buf.Identifier.Text : null; +// Is `idn` a pooled buffer passed as an argument to a CONSTRUCTOR whose result ESCAPES this method — +// `return new Wrapper(…, buf, …)` or `_field = new Wrapper(…, buf, …)`? Such a `new` hands the buffer +// to the constructed object, which becomes responsible for Return, so the buffer leaves the method +// inside the escaping object (an ownership transfer, not a borrow). One level only: the `new` must be +// the direct return value or a field-assignment RHS (a `new` buried in a local or another expression +// stays a borrow — an honest limitation). Purely syntactic, so it holds even when the wrapper type +// does not resolve. +static bool PassedToEscapingCtor(IdentifierNameSyntax idn) => + idn.Parent is ArgumentSyntax { Parent: ArgumentListSyntax + { Parent: BaseObjectCreationExpressionSyntax oce } } + && (oce.Parent is ReturnStatementSyntax + || (oce.Parent is AssignmentExpressionSyntax a && a.Right == oce && FieldName(a.Left) is not null)); + // Is `t` the System.Buffers.MemoryPool type — the Dispose-based pool. Mirrors IsArrayPoolType // (checked on the resolved symbol, so an aliased/injected `MemoryPool` receiver binds and a // look-alike does not). @@ -2394,8 +2407,16 @@ or ImplicitObjectCreationExpressionSyntax } init if (!usingMemoryOwners.Contains(nm)) escapedLocals.Add(nm); } + // A pooled buffer handed as an argument is normally a BORROW (the renter Returns it), + // NOT an escape — so `pool.Return(buf); Work(buf)` still trips use-after-return. But a + // pooled buffer passed to a CONSTRUCTOR whose result ESCAPES this method (`return new + // Wrapper(buf)` / `_field = new Wrapper(buf)`) transfers ownership to that object, which + // becomes responsible for Return — so the buffer is NOT leaked here. Treat that as an + // escape (mined FP on Pipelines.Sockets.Unofficial: ArrayPoolBufferWriter.CreateNewSegment + // returns `new ArrayPoolRefCountedSegment(pool, array, prev)`). else if ((idn.Parent is AssignmentExpressionSyntax asg && asg.Right == idn) - || (idn.Parent is ArgumentSyntax && !poolBuffers.Contains(nm) && !consumedArg)) + || (idn.Parent is ArgumentSyntax && !poolBuffers.Contains(nm) && !consumedArg) + || (poolBuffers.Contains(nm) && PassedToEscapingCtor(idn))) escapedLocals.Add(nm); } var tracked = new HashSet(candidates); diff --git a/frontend/roslyn/samples/FlowLocalsSample.cs b/frontend/roslyn/samples/FlowLocalsSample.cs index 9dbe0132..464f9658 100644 --- a/frontend/roslyn/samples/FlowLocalsSample.cs +++ b/frontend/roslyn/samples/FlowLocalsSample.cs @@ -1,4 +1,5 @@ using System; +using System.Buffers; using System.Collections.Generic; using System.IO; using System.Net; @@ -504,6 +505,27 @@ public void DeferredHandoffNoFalsePositive() defer.WriteByte(1); // must NOT trip OWN002 (it would, without the fix) defer.Dispose(); // disposed here -> balanced -> silent } + + // NOT a leak (mined FP on Pipelines.Sockets.Unofficial — ArrayPoolBufferWriter.CreateNewSegment): + // a pooled buffer handed to a constructor whose result is RETURNED transfers ownership to the + // returned wrapper (which Returns the buffer on its own teardown), so this method does not leak it + // even though it never calls Return -> silent ('ctorMoved'). A plain borrow `Work(buf)` still + // leaks if not returned, so this is specifically the escaping-constructor transfer. + public PooledHolder PooledIntoReturnedCtor(int n) + { + var ctorMoved = ArrayPool.Shared.Rent(n); + return new PooledHolder(ctorMoved); + } +} + +// Takes ownership of a pooled buffer (Returns it on teardown) — the wrapper that +// PooledIntoReturnedCtor hands its rented buffer to. Models the ownership transfer through a +// constructor argument that the escape analysis must recognise. +internal sealed class PooledHolder +{ + private readonly byte[] _buffer; + public PooledHolder(byte[] buffer) => _buffer = buffer; + public void Release() => ArrayPool.Shared.Return(_buffer); } // A domain exception type literally named `Exception`, in a non-System namespace — the From 56fc7dfbbdd56701432abc8ba431cc85d7fed755 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 22 Jun 2026 09:00:52 +0000 Subject: [PATCH 2/2] fix(extractor): escaping-ctor transfer only for return or a REAL field assignment (Codex) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex: FieldName(a.Left) returns a name for any bare identifier, so a constructor result assigned to a LOCAL (`PooledHolder h; h = new PooledHolder(buf);`) was mistaken for a field-store and escaped the buffer — silencing a real missing-Return leak when that local never leaves the method. Gate the field-assignment case on the LHS resolving to an IFieldSymbol (symbol-based); the return-value case (the mined FP and the FlowLocalsSample regression) is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- frontend/roslyn/OwnSharp.Extractor/Program.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 4d6e4908..902d039e 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -1041,14 +1041,16 @@ e is InvocationExpressionSyntax i // `return new Wrapper(…, buf, …)` or `_field = new Wrapper(…, buf, …)`? Such a `new` hands the buffer // to the constructed object, which becomes responsible for Return, so the buffer leaves the method // inside the escaping object (an ownership transfer, not a borrow). One level only: the `new` must be -// the direct return value or a field-assignment RHS (a `new` buried in a local or another expression -// stays a borrow — an honest limitation). Purely syntactic, so it holds even when the wrapper type -// does not resolve. -static bool PassedToEscapingCtor(IdentifierNameSyntax idn) => +// the direct return value or assigned to a real FIELD — a `new` stored in a LOCAL stays a borrow (the +// local is method-scoped; a wrapper that never leaves the method and never Returns the buffer is a +// real leak, Codex), as does a `new` buried in another expression. The field check is symbol-based +// (`IFieldSymbol`) so an assignment to a same-named LOCAL is not mistaken for a field. +static bool PassedToEscapingCtor(IdentifierNameSyntax idn, SemanticModel model) => idn.Parent is ArgumentSyntax { Parent: ArgumentListSyntax { Parent: BaseObjectCreationExpressionSyntax oce } } && (oce.Parent is ReturnStatementSyntax - || (oce.Parent is AssignmentExpressionSyntax a && a.Right == oce && FieldName(a.Left) is not null)); + || (oce.Parent is AssignmentExpressionSyntax a && a.Right == oce + && model.GetSymbolInfo(a.Left).Symbol is IFieldSymbol)); // Is `t` the System.Buffers.MemoryPool type — the Dispose-based pool. Mirrors IsArrayPoolType // (checked on the resolved symbol, so an aliased/injected `MemoryPool` receiver binds and a @@ -2416,7 +2418,7 @@ or ImplicitObjectCreationExpressionSyntax } init // returns `new ArrayPoolRefCountedSegment(pool, array, prev)`). else if ((idn.Parent is AssignmentExpressionSyntax asg && asg.Right == idn) || (idn.Parent is ArgumentSyntax && !poolBuffers.Contains(nm) && !consumedArg) - || (poolBuffers.Contains(nm) && PassedToEscapingCtor(idn))) + || (poolBuffers.Contains(nm) && PassedToEscapingCtor(idn, model))) escapedLocals.Add(nm); } var tracked = new HashSet(candidates);