diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e0de5280..27e4417a 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,18 @@ 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; } + # 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 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..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); @@ -2578,6 +2582,21 @@ 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 `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 + && 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..38afad02 --- /dev/null +++ b/frontend/roslyn/samples/MemoryOwnerEscapeSample.cs @@ -0,0 +1,49 @@ +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 + } + + // 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 + } +}