From c7a24f431a2745bec3fd5e6b79abeeb08d214883 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 22 Jun 2026 12:24:37 +0000 Subject: [PATCH 1/2] fix(extractor): an IMemoryOwner whose .Memory is handed off escapes the owner (mined ImageSharp; CodeQL-validated) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-tool oracle (scripts/oracle_compare.py) on SixLabors/ImageSharp surfaced this: our flow-locals detector flagged `Image.WrapMemory`'s `memoryManager` (a `ByteMemoryManager : IMemoryOwner`) as an undisposed-local leak, but CodeQL (interprocedural) does NOT — the manager's `.Memory` is handed to `MemoryGroup.Wrap(...)` and lives on in the returned `Image`. A `Memory` keeps its `IMemoryOwner` alive (it IS the backing), so passing `owner.Memory` as an argument transfers the owner's lifetime to the consumer; disposing it at method scope would be premature, and not disposing it here is not a leak. The flow escape-exclusion now untracks a local when its `.Memory` view is passed as an argument, scoped tightly: only `IMemoryOwner.Memory` (not any `local.Member`, so a `FileStream` whose `.Length` is read still leaks), and only for non-pool / non-`using` owners (a MemoryPool/`using` owner keeps its dangling-borrow tracking — OWN002 — intact). Mirrors the existing #80 escaping-ctor transfer and the D5 "ambiguous handoff -> conservatively exclude" rule. Regression sample MemoryOwnerEscapeSample.cs (--flow-locals): an IMemoryOwner whose `.Memory` is handed to a consumer stays silent ('handedOwner'); one whose `.Memory` is only read locally and never disposed still warns ('leakedOwner'). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 11 +++++- frontend/roslyn/OwnSharp.Extractor/Program.cs | 13 +++++++ .../roslyn/samples/MemoryOwnerEscapeSample.cs | 39 +++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 frontend/roslyn/samples/MemoryOwnerEscapeSample.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e0de5280..445f901c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -458,7 +458,8 @@ jobs: # Path-sensitive flow analysis of local IDisposables — bugs the flat D1 # detector cannot catch (use-after-dispose, double-dispose, leak-on-path). dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \ - frontend/roslyn/samples/FlowLocalsSample.cs --flow-locals -o "$RUNNER_TEMP/flow.json" + frontend/roslyn/samples/FlowLocalsSample.cs \ + frontend/roslyn/samples/MemoryOwnerEscapeSample.cs --flow-locals -o "$RUNNER_TEMP/flow.json" out=$(python -m ownlang ownir "$RUNNER_TEMP/flow.json" || true) echo "$out" echo "$out" | grep -q "OWN002" || { echo "FAIL: expected OWN002 (use-after-dispose)"; exit 1; } @@ -564,7 +565,13 @@ jobs: # `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 + # P-016 escape-via-projection (mined: ImageSharp Image.WrapMemory; CodeQL agrees it is + # no leak): an IMemoryOwner whose `.Memory` view is handed to a consumer as an argument + # escapes the owner -> silent ('handedOwner', in the list below); one whose `.Memory` is + # only READ locally and never disposed still leaks ('leakedOwner'). + echo "$out" | grep -qE "OWN001.*'leakedOwner'" \ + || { echo "FAIL: expected OWN001 on the read-only, never-disposed IMemoryOwner"; exit 1; } + 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 handedOwner; 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 aea10637..c550c43e 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -2578,6 +2578,19 @@ 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) + // An IMemoryOwner's `.Memory` view handed off as an ARGUMENT escapes the + // OWNER: the Memory keeps the owner alive (it IS the backing), so a consumer + // that stores it — `MemoryGroup.Wrap(owner.Memory)` -> the returned Image — + // takes over the lifetime; the owner is not leaked here. Scoped to + // IMemoryOwner.`Memory` (not any `local.Member`, so a FileStream whose + // `.Length` is read still leaks) and to non-pool / non-`using` owners (a + // MemoryPool / `using` owner keeps its dangling-borrow tracking). Mined FP on + // ImageSharp Image.WrapMemory; CodeQL agrees it is no leak (interprocedural). + || (!poolBuffers.Contains(nm) && !usingMemoryOwners.Contains(nm) + && idn.Parent is MemberAccessExpressionSyntax { Name.Identifier.Text: "Memory" } projMem + && projMem.Expression == idn + && projMem.Parent is ArgumentSyntax + && IsMemoryOwnerType(model.GetTypeInfo(idn).Type)) || (poolBuffers.Contains(nm) && PassedToEscapingCtor(idn, model))) escapedLocals.Add(nm); } diff --git a/frontend/roslyn/samples/MemoryOwnerEscapeSample.cs b/frontend/roslyn/samples/MemoryOwnerEscapeSample.cs new file mode 100644 index 00000000..d8923139 --- /dev/null +++ b/frontend/roslyn/samples/MemoryOwnerEscapeSample.cs @@ -0,0 +1,39 @@ +using System; +using System.Buffers; + +namespace Own.Samples; + +// P-016 escape-via-projection (mined: ImageSharp Image.WrapMemory; CodeQL agrees — no leak). +// +// An IMemoryOwner's `.Memory` view passed as an ARGUMENT hands the OWNER off: the Memory keeps +// the owner alive (it IS the backing), so a consumer that stores it takes over the lifetime — +// the owner is not leaked at method scope. Contrast: an owner whose `.Memory` is only READ +// locally and never disposed IS a leak. Exercised with --flow-locals. + +internal sealed class PixelOwner : IMemoryOwner +{ + private readonly byte[] data = new byte[16]; + + public Memory Memory => this.data; + + public void Dispose() { } +} + +internal static class MemoryOwnerEscape +{ + private static void Store(Memory m) { } + + // owner.Memory handed to a consumer (ambiguous transfer) -> owner escapes -> SILENT. + public static void Transferred() + { + var handedOwner = new PixelOwner(); + Store(handedOwner.Memory); // .Memory passed as an arg -> ownership handed off + } + + // owner.Memory only READ locally (a length); owner never disposed -> real leak -> must WARN. + public static int ReadOnlyLeak() + { + var leakedOwner = new PixelOwner(); + return leakedOwner.Memory.Length; // a local read, NOT a handoff + } +} From 397395d29a45ad0a0059b211f879eb47c54aba3f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 22 Jun 2026 12:32:22 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix(extractor):=20scope=20projection-escape?= =?UTF-8?q?=20to=20`new`'d=20owners=20only=20=E2=80=94=20keep=20pool-renta?= =?UTF-8?q?l=20tracking=20(Codex/CodeRabbit=20P1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first cut excluded only poolBuffers (ArrayPool) and usingMemoryOwners (using-declared MemoryPool), but a `var` MemoryPool.Rent owner is in neither — so its `.Memory` handoff wrongly untracked it, dropping the dangling-borrow / double-dispose findings (OWN002/OWN003). The corpus benchmark caught it: memorypool-double-dispose regressed (recall 23 -> 22). Scope the projection-escape to a new `newedDisposables` whitelist — locals created via `new` (the WrapMemory `new ByteMemoryManager(...)` shape). A pool rental of ANY kind (ArrayPool or MemoryPool, `var` or `using`) is never `new`'d, so it keeps its full use-after-dispose tracking; the renter owns the Return/Dispose. Stricter and more self-evident than enumerating pool sets. Sample/CI gain a MemoryPool-owner boundary case: `pooled.Dispose(); Store(pooled.Memory)` must still trip OWN002 (not be silenced). handedOwner stays silent, leakedOwner still warns. 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 | 14 ++++++++++---- frontend/roslyn/samples/MemoryOwnerEscapeSample.cs | 10 ++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 445f901c..27e4417a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -571,6 +571,11 @@ jobs: # only READ locally and never disposed still leaks ('leakedOwner'). echo "$out" | grep -qE "OWN001.*'leakedOwner'" \ || { echo "FAIL: expected OWN001 on the read-only, never-disposed IMemoryOwner"; exit 1; } + # boundary: the projection-escape is scoped to `new`'d owners — a MemoryPool RENTAL whose + # .Memory is handed off after Dispose must KEEP its use-after-dispose tracking, NOT be + # silenced (Codex/CodeRabbit P1; benchmark memorypool-double-dispose parity). + echo "$out" | grep -qE "OWN002.*'pooled'" \ + || { echo "FAIL: a MemoryPool owner's .Memory used after Dispose must still trip OWN002"; exit 1; } 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 handedOwner; do if echo "$out" | grep -q "'$ok'"; then echo "FAIL: silent/exempt case '$ok' was reported"; exit 1; fi done diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index c550c43e..c7054a01 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -2461,6 +2461,7 @@ or ImplicitObjectCreationExpressionSyntax var candidates = new HashSet(); var poolBuffers = new HashSet(); // candidates that are ArrayPool buffers var usingMemoryOwners = new HashSet(); // `using`-declared MemoryPool owners + var newedDisposables = new HashSet(); // candidates created via `new` (NOT a pool rental / factory) foreach (var ld in mbody.DescendantNodes().OfType()) { if (ld.UsingKeyword != default) @@ -2481,7 +2482,10 @@ or ImplicitObjectCreationExpressionSyntax or ImplicitObjectCreationExpressionSyntax } init && model.GetTypeInfo(init.Value).Type is { } dt && ImplementsIDisposable(dt) && !IsDisposeOptional(dt)) + { candidates.Add(v.Identifier.Text); + newedDisposables.Add(v.Identifier.Text); + } else if (IsPoolRent(v.Initializer?.Value, model)) // an ArrayPool buffer { candidates.Add(v.Identifier.Text); @@ -2583,10 +2587,12 @@ or ImplicitObjectCreationExpressionSyntax } init // that stores it — `MemoryGroup.Wrap(owner.Memory)` -> the returned Image — // takes over the lifetime; the owner is not leaked here. Scoped to // IMemoryOwner.`Memory` (not any `local.Member`, so a FileStream whose - // `.Length` is read still leaks) and to non-pool / non-`using` owners (a - // MemoryPool / `using` owner keeps its dangling-borrow tracking). Mined FP on - // ImageSharp Image.WrapMemory; CodeQL agrees it is no leak (interprocedural). - || (!poolBuffers.Contains(nm) && !usingMemoryOwners.Contains(nm) + // `.Length` is read still leaks) and to `new`'d owners ONLY: a pool rental — + // ArrayPool or MemoryPool, `var` or `using` — keeps its dangling-borrow / use- + // after-dispose (OWN002/OWN003) tracking through `.Memory` handoffs, since the + // RENTER owns the Return/Dispose (Codex/CodeRabbit; benchmark memorypool-double- + // dispose). Mined FP on ImageSharp Image.WrapMemory; CodeQL agrees — no leak. + || (newedDisposables.Contains(nm) && idn.Parent is MemberAccessExpressionSyntax { Name.Identifier.Text: "Memory" } projMem && projMem.Expression == idn && projMem.Parent is ArgumentSyntax diff --git a/frontend/roslyn/samples/MemoryOwnerEscapeSample.cs b/frontend/roslyn/samples/MemoryOwnerEscapeSample.cs index d8923139..38afad02 100644 --- a/frontend/roslyn/samples/MemoryOwnerEscapeSample.cs +++ b/frontend/roslyn/samples/MemoryOwnerEscapeSample.cs @@ -36,4 +36,14 @@ public static int ReadOnlyLeak() var leakedOwner = new PixelOwner(); return leakedOwner.Memory.Length; // a local read, NOT a handoff } + + // A MemoryPool RENTAL (not a `new`'d owner) keeps its dangling-borrow tracking through a + // `.Memory` handoff: projection-escape is scoped to `new`'d owners only, so using `pooled.Memory` + // after Dispose still trips OWN002 — the rule must NOT silence it (Codex/CodeRabbit P1). + public static void PoolOwnerNotEscaped() + { + var pooled = MemoryPool.Shared.Rent(16); + pooled.Dispose(); + Store(pooled.Memory); // use of .Memory AFTER Dispose -> OWN002, must NOT be silenced + } }