From c958ad0c9203d30c0aea44038263c3e91f7d40a6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 02:47:03 +0000 Subject: [PATCH 1/3] feat(d5.4): extractor emits alias_join for verified ctor-adopt (step 2, first slice) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit D5.4 step 2 — the Roslyn extractor learns to EMIT the `alias_join` flow op (steps 0/1 built the core primitive + bridge; this fires it on real C#). First slice: constructor adoption at the construction site — `var w = new W(x)` where W is a first-party wrapper that adopts the tracked local x into an owning field. The adopt is PROVEN, never guessed (precision-first / §11 must-only): - DisposedOwningFields(W): the set of owning fields W disposes UNCONDITIONALLY in its Dispose() — a top-level `_f.Dispose()` / `_f?.Dispose()`; conditional or nested disposes are excluded (a sometimes-dispose can't justify an alias). - TryAdoptedArgIndex(new W(...)): an adopt iff W disposes EXACTLY ONE owning field and a ctor assigns that field DIRECTLY from a single parameter (`_f = p;`), with the call positional up to that slot. Any ambiguity -> no claim (false adopt would fabricate a double-dispose, the one thing we must never do). Escape-pass interaction (the hazard): today x passed to `new W(x)` escapes (untracked). IsAdoptedArgOfBoundedWrapper keeps the adopted arg tracked ONLY when the wrapper is a non-`using` local candidate that does not itself escape (LocalEscapesSyntactically, deliberately over-approximating so we DECLINE rather than fabricate). This avoids the false-OWN001 traps (`using var w`, returned/ stored wrapper). Then the construction site emits `alias_join var=w src=x` instead of an acquire, and the per-RID core does the rest. Validated end-to-end on real C# by FactoryAdoptSample.cs (new CI step): adopt- clean and dispose-inner-clean stay silent, dropping both leaks the inner ONCE (OWN001), disposing both is OWN003, and a NON-adopting holder makes no claim so disposing both is not a false double-dispose. The bridge half + the exact rendered findings were pre-verified locally against simulated facts; the extractor emission is validated only in CI (no local dotnet). Deferred to a later slice: shape (a) return-alias / caller-side propagation, the field-store-to-`this` adopt (Polly BulkheadPolicy), and the declined using/ escaping-wrapper cases. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Fmwk5ruExKy5fQ77CJ2NtF --- .github/workflows/ci.yml | 23 +++ docs/notes/d5-ownership-transfer.md | 30 +++- frontend/roslyn/OwnSharp.Extractor/Program.cs | 170 +++++++++++++++++- frontend/roslyn/samples/FactoryAdoptSample.cs | 76 ++++++++ 4 files changed, 291 insertions(+), 8 deletions(-) create mode 100644 frontend/roslyn/samples/FactoryAdoptSample.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 176011d5..36d0b584 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -833,6 +833,29 @@ jobs: if echo "$out" | grep -q "'$ok'"; then echo "FAIL: D5.2 silent 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)" + - name: P-005 D5.4 T4 wrap/adopt (--flow-locals) + run: | + # The extractor recognises a first-party wrapper that ADOPTS a disposable arg into an + # owning field (its Dispose disposes that field; its ctor stores the arg into it) and + # emits an `alias_join`, so the wrapper and the inner share ONE obligation: disposing + # either is clean, dropping both leaks the resource ONCE, disposing both is OWN003. A + # non-adopting holder makes NO alias claim (precision-first: no false double-dispose). + dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \ + frontend/roslyn/samples/FactoryAdoptSample.cs --flow-locals -o "$RUNNER_TEMP/adopt.json" + out=$(python -m ownlang ownir "$RUNNER_TEMP/adopt.json" || true) + echo "$out" + # dropping both aliases leaks the ONE underlying resource once, on the inner local. + echo "$out" | grep -qE "FactoryAdoptSample\.cs:[0-9]+:.*\[OWN001\].*'adoptInnerLeak'" \ + || { echo "FAIL: expected the per-RID OWN001 on the dropped adopted inner (adoptInnerLeak)"; exit 1; } + # disposing both aliases is a double-dispose through the shared obligation. + echo "$out" | grep -q "OWN003" \ + || { echo "FAIL: expected OWN003 (double-dispose through the alias)"; exit 1; } + # clean / inner-only / non-adopt cases stay silent (no finding on their locals); the + # leaked WRAPPER alias must also be silent (one finding, attributed to the inner). + for ok in adoptInnerClean adoptWrapClean adoptInnerDirect adoptWrapDirect holdInner holdWrap adoptWrapLeak; do + if echo "$out" | grep -q "'$ok'"; then echo "FAIL: D5.4 case '$ok' must be silent"; exit 1; fi + done + echo "OK: D5.4 T4 alias_join on real C# — adopt verified (Dispose-field + ctor-param), double-dispose caught, non-adopt makes no claim" - name: Opt-in body-throw-edges tier (--body-throw-edges, P-016 throw firehose) run: | # The opt-in tier: body-level "any call may throw" dispose-not-called-on-throw (CodeQL diff --git a/docs/notes/d5-ownership-transfer.md b/docs/notes/d5-ownership-transfer.md index a0bc477d..cc907778 100644 --- a/docs/notes/d5-ownership-transfer.md +++ b/docs/notes/d5-ownership-transfer.md @@ -387,13 +387,29 @@ escape-without-transfer and all `unknown`/`may` lower to **silence** in the defa alias minted inside one branch of an `if` that merges with a non-aliasing path is out of v1 scope (the bridge emits it straight-line at the wrapper site); the conflicting-merge raise in `_join_handle_rid` keeps that loud rather than silently wrong. - - **Step 2 — extractor emission (next).** Turn on the Roslyn recogniser branches that emit - the `alias_join` flow op for *verified* wrapper / factory / ctor-adopt sites: **(a)** the arg - is forwarded to an `aliasOf` position and the method returns that call's result, or **(b)** - the arg is stored into a single owning field whose `Dispose()` releases it (§11). Result: - **Dapper / Polly** (`BulkheadSemaphoreFactory` → owning fields) modelled explicitly and added - as oracle regression anchors that resolve *with a recorded reason* (cross-link - `field-notes-patterns.md`). + - **Step 2 — extractor emission, ctor-adopt at the construction site (shipped, first slice).** + The Roslyn extractor now emits the `alias_join` flow op for a *verified* constructor adopt: + `var w = new W(x)` where `W` is first-party and adopts `x` into an owning field. The adopt is + proven, never guessed (§11 must-only): `TryAdoptedArgIndex` requires `W` to dispose **exactly + one** owning field **unconditionally** in its `Dispose()` (`DisposedOwningFields` — a top-level + `_f.Dispose()`/`_f?.Dispose()`, conditional/nested excluded) **and** that field to be assigned + **directly from a single ctor parameter** (`_f = p;`); the call must be positional up to that + slot. Any ambiguity → no claim. The escape pass gains a matching exception + (`IsAdoptedArgOfBoundedWrapper`): the adopted arg stays tracked **only** when the wrapper is a + non-`using` local candidate that does not itself escape (`LocalEscapesSyntactically`, + deliberately over-approximating so we *decline* rather than fabricate). Then the construction + site emits `alias_join var=w src=x` instead of an acquire, so the per-RID core (steps 0/1) does + the rest: dispose either → clean, dispose both → OWN003, drop both → one OWN001 on the inner. + Validated end-to-end on real C# by `FactoryAdoptSample.cs` in CI (adopt-clean / dispose-inner- + clean silent, drop-both OWN001 on the inner once, dispose-both OWN003, and a **non-adopting + holder** making no claim so disposing both is not a false double-dispose). + - **Step 2 remainder (deferred).** Shape **(a)** — return-alias / caller-side propagation + (`var r = Create(reader)` where `Create` returns `aliasOf:reader`) — and the field-store-to- + `this` form of (b) (the wrapper adopts into *its own* field across the boundary, e.g. Polly's + `BulkheadPolicy(factory())`), plus the cases the v1 gate declines (a `using` or escaping + wrapper). These need return-skeleton `aliasOf:i` inference and/or cross-method field analysis; + they ride a later slice. Once shape (a) lands, **Dapper / Polly** become oracle regression + anchors that resolve *with a recorded reason* (cross-link `field-notes-patterns.md`). - **D5.5 — Tier C annotations** (`[OwnTransfers]` / `[MustCallAlias]` + external file). - **D5.x — advisory** `OWN051` for `may`/`unknown`, and the strict/pessimistic mode. diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 8282df99..aae732d6 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -596,6 +596,157 @@ static string FlowFunctionName(BaseMethodDeclarationSyntax method, string fallba ? $"{ms.ContainingType.ToDisplayString()}.{ms.Name}" : $"{fallbackType}.{MethodName(method)}"; +// P-005 D5.4 (T4 wrap/adopt): the set of OWNING fields a first-party type disposes +// UNCONDITIONALLY in its `Dispose()` — i.e. a `_f.Dispose()` / `_f?.Dispose()` / +// `this._f.Dispose()` that is a TOP-LEVEL statement of the Dispose body (not nested in an +// if/try/loop), where `_f` resolves to an instance field of `w`. Conditional or nested +// disposes are excluded: if the field is only *sometimes* disposed, claiming the arg is +// adopted could fabricate a false double-dispose, so precision-first declines it (§11 +// must-only). Used to verify a ctor-adopt before emitting an `alias_join`. +static HashSet DisposedOwningFields(INamedTypeSymbol w, SemanticModel model) +{ + var result = new HashSet(SymbolEqualityComparer.Default); + var dispose = w.GetMembers("Dispose").OfType() + .FirstOrDefault(m => m.Parameters.Length == 0 && m.ReturnsVoid + && m.DeclaringSyntaxReferences.Length > 0); + if (dispose is null) + return result; + if (dispose.DeclaringSyntaxReferences[0].GetSyntax() is not MethodDeclarationSyntax mds + || mds.Body is not { } body) + return result; + var dm = model.Compilation.GetSemanticModel(mds.SyntaxTree); + foreach (var st in body.Statements) // TOP-LEVEL statements only + { + if (st is not ExpressionStatementSyntax es) + continue; + // unwrap the disposed receiver from `recv.Dispose()` or `recv?.Dispose()` — the + // latter parses as a ConditionalAccess at the statement level, not an Invocation. + ExpressionSyntax? recv = es.Expression switch + { + InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax ma } + when ma.Name.Identifier.Text is "Dispose" => ma.Expression, + ConditionalAccessExpressionSyntax + { WhenNotNull: InvocationExpressionSyntax + { Expression: MemberBindingExpressionSyntax mb } } ca + when mb.Name.Identifier.Text is "Dispose" => ca.Expression, + _ => null, + }; + if (recv is null) + continue; + if (dm.GetSymbolInfo(recv).Symbol is IFieldSymbol f + && SymbolEqualityComparer.Default.Equals(f.ContainingType, w)) + result.Add(f); + } + return result; +} + +// P-005 D5.4: does `new W(args)` ADOPT one of its arguments into an owning field — i.e. is +// the constructed object a wrapper that takes responsibility for disposing that arg? True +// iff (a) the ctor is first-party, (b) its type disposes exactly ONE owning field +// unconditionally (DisposedOwningFields), and (c) that field is assigned DIRECTLY from +// exactly one constructor parameter (`_f = p;` as a top-level ctor statement). Returns that +// parameter's positional index. Single-source by construction (§11): any ambiguity — no +// visible Dispose, 0/2+ disposed fields, the field not assigned from a single param, a +// non-positional call — yields no claim (false, idx -1). This is the only gate that lets the +// extractor emit an `alias_join`, so it must never over-claim (a false adopt would fabricate +// a double-dispose), only under-claim. +static bool TryAdoptedArgIndex(ObjectCreationExpressionSyntax oce, SemanticModel model, + out int idx) +{ + idx = -1; + if (model.GetSymbolInfo(oce).Symbol is not IMethodSymbol ctor + || ctor.MethodKind != MethodKind.Constructor + || ctor.DeclaringSyntaxReferences.Length == 0) + return false; + var w = ctor.ContainingType; + var disposed = DisposedOwningFields(w, model); + if (disposed.Count != 1) + return false; + var field = disposed.First(); + if (ctor.DeclaringSyntaxReferences[0].GetSyntax() is not ConstructorDeclarationSyntax cds + || cds.Body is not { } cbody) + return false; + var cm = model.Compilation.GetSemanticModel(cds.SyntaxTree); + var matched = -1; + foreach (var st in cbody.Statements) // TOP-LEVEL `_f = p;` only + { + if (st is not ExpressionStatementSyntax es + || es.Expression is not AssignmentExpressionSyntax asg + || !asg.IsKind(SyntaxKind.SimpleAssignmentExpression)) + continue; + if (cm.GetSymbolInfo(asg.Left).Symbol is not IFieldSymbol lf + || !SymbolEqualityComparer.Default.Equals(lf, field)) + continue; + if (cm.GetSymbolInfo(asg.Right).Symbol is not IParameterSymbol p + || !SymbolEqualityComparer.Default.Equals(p.ContainingSymbol, ctor)) + return false; // field set from a non-parameter — bail + if (matched >= 0 && matched != p.Ordinal) + return false; // the field assigned from two params — bail + matched = p.Ordinal; + } + if (matched < 0) + return false; + // the call must be POSITIONAL up to the adopted slot, so the arg index == the param + // ordinal (a named/reordered call would mis-attribute). Require enough positional args. + if (oce.ArgumentList is not { } al || al.Arguments.Count <= matched + || al.Arguments.Take(matched + 1).Any(a => a.NameColon is not null)) + return false; + idx = matched; + return true; +} + +// P-005 D5.4: the adopted-argument expression of `new W(x)`, or null if W does not adopt an +// argument (TryAdoptedArgIndex). Wraps the index lookup so callers work with the syntax. +static ExpressionSyntax? AdoptedArg(ObjectCreationExpressionSyntax oce, SemanticModel model) => + TryAdoptedArgIndex(oce, model, out var i) ? oce.ArgumentList!.Arguments[i].Expression : null; + +// P-005 D5.4: does the local `name` ESCAPE this method (returned, stored as an assignment +// RHS, passed on as an argument, or captured by a closure)? Deliberately OVER-approximates +// — any arg-pass counts, even a borrow — because it only gates the *adopt* exception below: +// over-escaping the wrapper makes us DECLINE the alias (under-claim), never fabricate one. +// `w.Dispose()` (a member access) is not an escape, so a disposed wrapper still qualifies. +static bool LocalEscapesSyntactically(string name, BlockSyntax mbody) +{ + foreach (var idn in mbody.DescendantNodes().OfType()) + { + if (idn.Identifier.Text != name) + continue; + if (idn.Parent is ReturnStatementSyntax) + return true; + if (idn.Parent is AssignmentExpressionSyntax a && a.Right == idn) + return true; + if (idn.Parent is ArgumentSyntax) + return true; + for (var p = idn.Parent; p is not null && p != mbody; p = p.Parent) + if (p is AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax) + return true; + } + return false; +} + +// P-005 D5.4: is `idn` the adopted argument of `new W(idn)` whose wrapper is a method-bounded +// owner — i.e. should this arg occurrence NOT count as an escape? Gated hard to preserve +// own-only-0: the wrapper must be a NON-`using` local `var w = new W(idn)`, `w` must be a +// tracked disposable candidate, and `w` must not itself escape (else keeping the arg tracked +// with nothing to discharge it would fabricate an OWN001). When all hold, the arg's +// obligation is adopted by `w` and modelled as an `alias_join`; the arg stays tracked. +static bool IsAdoptedArgOfBoundedWrapper(IdentifierNameSyntax idn, SemanticModel model, + HashSet candidates, BlockSyntax mbody) +{ + if (idn.Parent is not ArgumentSyntax { Parent: ArgumentListSyntax + { Parent: ObjectCreationExpressionSyntax oce } }) + return false; + if (AdoptedArg(oce, model) != idn) // idn must BE the adopted arg + return false; + if (oce.Parent is not EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax wv } + || wv.Parent is not VariableDeclarationSyntax + || wv.Parent.Parent is not LocalDeclarationStatementSyntax wld + || wld.UsingKeyword != default) + return false; + var w = wv.Identifier.Text; + return candidates.Contains(w) && !LocalEscapesSyntactically(w, mbody); +} + // A `Dispose()`/`Close()`/`DisposeAsync()` call — through member access (`x.Dispose()`) // or member binding (`x?.Dispose()`), and seen through a trailing `.ConfigureAwait(false)` // (the idiomatic `await x.DisposeAsync().ConfigureAwait(false)` is the release, not a @@ -783,7 +934,18 @@ static bool LowerFlowStmt(StatementSyntax st, HashSet tracked, SemanticM if (ld.UsingKeyword == default) foreach (var v in ld.Declaration.Variables) { + // P-005 D5.4 (T4 adopt): `var w = new W(x)` where W is a verified wrapper + // that adopts the tracked local `x` into an owning field (TryAdoptedArgIndex) + // — emit `alias_join` instead of an acquire, so `w` joins `x`'s obligation: + // disposing either discharges the one resource, disposing both is OWN003. The + // arg `x` is kept tracked by the matching escape exception above. if (tracked.Contains(v.Identifier.Text) + && v.Initializer?.Value is ObjectCreationExpressionSyntax adoptOce + && AdoptedArg(adoptOce, model) is IdentifierNameSyntax adoptedId + && tracked.Contains(adoptedId.Identifier.Text)) + nodes.Add(new { op = "alias_join", var = v.Identifier.Text, + src = adoptedId.Identifier.Text, line = LineOf(v) }); + else if (tracked.Contains(v.Identifier.Text) && (v.Initializer?.Value is ObjectCreationExpressionSyntax or ImplicitObjectCreationExpressionSyntax || IsPoolRent(v.Initializer?.Value, model) // ArrayPool Rent @@ -3355,7 +3517,13 @@ or ImplicitObjectCreationExpressionSyntax } init // 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 + // P-005 D5.4: a disposable passed to an ADOPTING ctor arg of a + // method-bounded wrapper is not an escape — its obligation is + // adopted by the wrapper (modelled as an alias_join) and stays + // tracked. Gated to a non-using, non-escaping local wrapper so we + // never keep an arg tracked with nothing to discharge it (FP). + && !IsAdoptedArgOfBoundedWrapper(idn, model, candidates, mbody)) // 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 — diff --git a/frontend/roslyn/samples/FactoryAdoptSample.cs b/frontend/roslyn/samples/FactoryAdoptSample.cs new file mode 100644 index 00000000..6915ea35 --- /dev/null +++ b/frontend/roslyn/samples/FactoryAdoptSample.cs @@ -0,0 +1,76 @@ +using System; +using System.IO; + +// P-005 D5.4 step 2 (T4 wrap/adopt): the Roslyn extractor recognises a first-party wrapper +// that ADOPTS a disposable argument into an owning field (its Dispose disposes that field, its +// ctor stores the arg into it) and emits an `alias_join` — so the wrapper and the inner share +// ONE obligation. Disposing either discharges it; disposing both is a double-dispose; dropping +// both leaks the one resource ONCE. A non-adopting wrapper makes NO claim (precision-first). +namespace OwnSharp.Samples +{ + // VERIFIED ADOPTER: Dispose disposes the field unconditionally; the ctor assigns the field + // directly from its parameter. -> `new StreamAdopter(s)` adopts `s`. + internal sealed class StreamAdopter : IDisposable + { + private readonly MemoryStream _inner; + public StreamAdopter(MemoryStream inner) { _inner = inner; } + public void Dispose() { _inner.Dispose(); } + public void Poke() { } + } + + // NON-ADOPTER: holds the disposable but its Dispose does NOT dispose it. The extractor must + // make no alias claim, so disposing both the holder and the inner is NOT a double-dispose. + internal sealed class StreamHolder : IDisposable + { + private readonly MemoryStream _held; + public StreamHolder(MemoryStream held) { _held = held; } + public void Dispose() { } + } + + internal static class AdoptConsumers + { + // Disposing the wrapper alone discharges the shared obligation -> CLEAN (silent). + public static void AdoptClean() + { + var adoptInnerClean = new MemoryStream(); + var adoptWrapClean = new StreamAdopter(adoptInnerClean); + adoptWrapClean.Dispose(); + } + + // Dropping BOTH leaks the one underlying resource ONCE (OWN001 on the inner). + public static void AdoptLeak() + { + var adoptInnerLeak = new MemoryStream(); + var adoptWrapLeak = new StreamAdopter(adoptInnerLeak); + adoptWrapLeak.Poke(); + } + + // Disposing BOTH aliases is a double-dispose (OWN003). + public static void AdoptDouble() + { + var adoptInnerDbl = new MemoryStream(); + var adoptWrapDbl = new StreamAdopter(adoptInnerDbl); + adoptInnerDbl.Dispose(); + adoptWrapDbl.Dispose(); + } + + // Disposing the inner directly (the Dapper "dispose-the-inner" path) also discharges + // the shared obligation -> CLEAN (silent). + public static void AdoptInnerOnly() + { + var adoptInnerDirect = new MemoryStream(); + var adoptWrapDirect = new StreamAdopter(adoptInnerDirect); + adoptInnerDirect.Dispose(); + } + + // PRECISION CONTROL: a NON-adopting holder. No alias is claimed, so the inner escapes as + // an argument (silent) and disposing both must NOT be a false double-dispose. + public static void NonAdoptDoubleOk() + { + var holdInner = new MemoryStream(); + var holdWrap = new StreamHolder(holdInner); + holdInner.Dispose(); + holdWrap.Dispose(); + } + } +} From e86e3807e200070bd039ac085746f0aa849f36f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 02:52:56 +0000 Subject: [PATCH 2/3] fix(d5.4): recognise target-typed `new(...)` adopters too (Codex) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P2 on #137: the adopt check and its escape exception were typed to ObjectCreationExpressionSyntax, so target-typed `StreamAdopter w = new(inner)` (an ImplicitObjectCreationExpressionSyntax) was missed — the wrapper fell through to an independent acquire and disposing the inner directly produced a false OWN001 on the wrapper, contradicting the adopt model (either alias should discharge the shared obligation). Fix: widen TryAdoptedArgIndex / AdoptedArg / IsAdoptedArgOfBoundedWrapper and the construction-site check from ObjectCreationExpressionSyntax to the common base BaseObjectCreationExpressionSyntax, which covers both `new W(x)` and `new(x)` and still resolves the ctor + ArgumentList from the symbol. The candidate detection already handled ImplicitObjectCreationExpressionSyntax. Sample/CI: FactoryAdoptSample gains AdoptTargetTyped (`StreamAdopter w = new(s); s.Dispose();`) and its locals are added to the silence assertion — dispose-inner on a target-typed adopter must stay clean. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Fmwk5ruExKy5fQ77CJ2NtF --- .github/workflows/ci.yml | 3 ++- frontend/roslyn/OwnSharp.Extractor/Program.cs | 10 ++++++---- frontend/roslyn/samples/FactoryAdoptSample.cs | 9 +++++++++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 36d0b584..20e801ad 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -852,7 +852,8 @@ jobs: || { echo "FAIL: expected OWN003 (double-dispose through the alias)"; exit 1; } # clean / inner-only / non-adopt cases stay silent (no finding on their locals); the # leaked WRAPPER alias must also be silent (one finding, attributed to the inner). - for ok in adoptInnerClean adoptWrapClean adoptInnerDirect adoptWrapDirect holdInner holdWrap adoptWrapLeak; do + for ok in adoptInnerClean adoptWrapClean adoptInnerDirect adoptWrapDirect \ + adoptInnerTt adoptWrapTt holdInner holdWrap adoptWrapLeak; do if echo "$out" | grep -q "'$ok'"; then echo "FAIL: D5.4 case '$ok' must be silent"; exit 1; fi done echo "OK: D5.4 T4 alias_join on real C# — adopt verified (Dispose-field + ctor-param), double-dispose caught, non-adopt makes no claim" diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index aae732d6..4bcb8c99 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -650,7 +650,7 @@ static HashSet DisposedOwningFields(INamedTypeSymbol w, SemanticModel m // non-positional call — yields no claim (false, idx -1). This is the only gate that lets the // extractor emit an `alias_join`, so it must never over-claim (a false adopt would fabricate // a double-dispose), only under-claim. -static bool TryAdoptedArgIndex(ObjectCreationExpressionSyntax oce, SemanticModel model, +static bool TryAdoptedArgIndex(BaseObjectCreationExpressionSyntax oce, SemanticModel model, out int idx) { idx = -1; @@ -697,7 +697,9 @@ static bool TryAdoptedArgIndex(ObjectCreationExpressionSyntax oce, SemanticModel // P-005 D5.4: the adopted-argument expression of `new W(x)`, or null if W does not adopt an // argument (TryAdoptedArgIndex). Wraps the index lookup so callers work with the syntax. -static ExpressionSyntax? AdoptedArg(ObjectCreationExpressionSyntax oce, SemanticModel model) => +// Covers both `new W(x)` and target-typed `new(x)` (BaseObjectCreationExpressionSyntax) — +// the adopt verification resolves the ctor from the symbol either way (Codex). +static ExpressionSyntax? AdoptedArg(BaseObjectCreationExpressionSyntax oce, SemanticModel model) => TryAdoptedArgIndex(oce, model, out var i) ? oce.ArgumentList!.Arguments[i].Expression : null; // P-005 D5.4: does the local `name` ESCAPE this method (returned, stored as an assignment @@ -734,7 +736,7 @@ static bool IsAdoptedArgOfBoundedWrapper(IdentifierNameSyntax idn, SemanticModel HashSet candidates, BlockSyntax mbody) { if (idn.Parent is not ArgumentSyntax { Parent: ArgumentListSyntax - { Parent: ObjectCreationExpressionSyntax oce } }) + { Parent: BaseObjectCreationExpressionSyntax oce } }) return false; if (AdoptedArg(oce, model) != idn) // idn must BE the adopted arg return false; @@ -940,7 +942,7 @@ static bool LowerFlowStmt(StatementSyntax st, HashSet tracked, SemanticM // disposing either discharges the one resource, disposing both is OWN003. The // arg `x` is kept tracked by the matching escape exception above. if (tracked.Contains(v.Identifier.Text) - && v.Initializer?.Value is ObjectCreationExpressionSyntax adoptOce + && v.Initializer?.Value is BaseObjectCreationExpressionSyntax adoptOce && AdoptedArg(adoptOce, model) is IdentifierNameSyntax adoptedId && tracked.Contains(adoptedId.Identifier.Text)) nodes.Add(new { op = "alias_join", var = v.Identifier.Text, diff --git a/frontend/roslyn/samples/FactoryAdoptSample.cs b/frontend/roslyn/samples/FactoryAdoptSample.cs index 6915ea35..60233494 100644 --- a/frontend/roslyn/samples/FactoryAdoptSample.cs +++ b/frontend/roslyn/samples/FactoryAdoptSample.cs @@ -63,6 +63,15 @@ public static void AdoptInnerOnly() adoptInnerDirect.Dispose(); } + // TARGET-TYPED `new(...)`: the adopt must be recognised for an implicit object creation + // too (Codex P2). Disposing the inner directly discharges the shared obligation -> CLEAN. + public static void AdoptTargetTyped() + { + var adoptInnerTt = new MemoryStream(); + StreamAdopter adoptWrapTt = new(adoptInnerTt); + adoptInnerTt.Dispose(); + } + // PRECISION CONTROL: a NON-adopting holder. No alias is claimed, so the inner escapes as // an argument (silent) and disposing both must NOT be a false double-dispose. public static void NonAdoptDoubleOk() From 699e1eafd8b84e61a9ee865234428e7a9be7c5e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 02:57:13 +0000 Subject: [PATCH 3/3] =?UTF-8?q?fix(d5.4):=20harden=20adopt=20proof=20?= =?UTF-8?q?=E2=80=94=20no=20static=20fields,=20pure-ctor=20only,=20ancesto?= =?UTF-8?q?r=20escape=20walk=20(CodeRabbit)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three CodeRabbit Major findings on the adopt verification (a false adopt would fabricate a double-dispose, so the gate must only ever under-claim): 1. Reject static fields. DisposedOwningFields and the ctor LHS check now require the owning field to be an INSTANCE field (!IsStatic) — a static field is shared across all instances, not per-wrapper ownership, so aliasing on it could fake an OWN003 across unrelated wrappers. 2. Decline a ctor that does more than the direct `_f = p`. The scan accepted a matching assignment even amid sibling statements that could rebind the field or mutate the param (`_f = p; Rebind(other);`). v1 now accepts ONLY a pure single-assignment adopter ctor `{ _f = p; }`; any multi-statement ctor is declined (deferred with the rest of step 2's remainder). 3. Walk ancestors when classifying wrapper escapes. LocalEscapesSyntactically only inspected the direct parent, so `return (IDisposable)w;`, `x = w ?? f;`, `Foo((object)w)` slipped through and kept the adopted arg tracked even though the wrapper escaped — a potential false OWN001. It now climbs value-wrapping expressions (casts/coalesce/parens/conditionals) to where the value lands (a return, an argument, or an assignment RHS), while a bare `w.Dispose()` receiver stays a use, not an escape. Sample unaffected (StreamAdopter is a single-statement, instance-field, non- escaping wrapper); validated on CI. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Fmwk5ruExKy5fQ77CJ2NtF --- frontend/roslyn/OwnSharp.Extractor/Program.cs | 78 ++++++++++++------- 1 file changed, 51 insertions(+), 27 deletions(-) diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 4bcb8c99..c97879cf 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -634,7 +634,8 @@ static HashSet DisposedOwningFields(INamedTypeSymbol w, SemanticModel m if (recv is null) continue; if (dm.GetSymbolInfo(recv).Symbol is IFieldSymbol f - && SymbolEqualityComparer.Default.Equals(f.ContainingType, w)) + && !f.IsStatic // a static field is shared across instances, + && SymbolEqualityComparer.Default.Equals(f.ContainingType, w)) // not per-wrapper ownership result.Add(f); } return result; @@ -667,25 +668,23 @@ static bool TryAdoptedArgIndex(BaseObjectCreationExpressionSyntax oce, SemanticM || cds.Body is not { } cbody) return false; var cm = model.Compilation.GetSemanticModel(cds.SyntaxTree); - var matched = -1; - foreach (var st in cbody.Statements) // TOP-LEVEL `_f = p;` only - { - if (st is not ExpressionStatementSyntax es - || es.Expression is not AssignmentExpressionSyntax asg - || !asg.IsKind(SyntaxKind.SimpleAssignmentExpression)) - continue; - if (cm.GetSymbolInfo(asg.Left).Symbol is not IFieldSymbol lf - || !SymbolEqualityComparer.Default.Equals(lf, field)) - continue; - if (cm.GetSymbolInfo(asg.Right).Symbol is not IParameterSymbol p - || !SymbolEqualityComparer.Default.Equals(p.ContainingSymbol, ctor)) - return false; // field set from a non-parameter — bail - if (matched >= 0 && matched != p.Ordinal) - return false; // the field assigned from two params — bail - matched = p.Ordinal; - } - if (matched < 0) + // v1 accepts only a PURE single-assignment adopter ctor `{ _f = p; }`. Any sibling + // statement could rebind the field or mutate the param (`_f = p; Rebind(other);`), which + // would over-claim adoption — so a multi-statement ctor is DECLINED (deferred to a later + // slice). Precision-first: under-claim, never fabricate a false double-dispose (CodeRabbit). + if (cbody.Statements.Count != 1 + || cbody.Statements[0] is not ExpressionStatementSyntax es + || es.Expression is not AssignmentExpressionSyntax asg + || !asg.IsKind(SyntaxKind.SimpleAssignmentExpression)) + return false; + if (cm.GetSymbolInfo(asg.Left).Symbol is not IFieldSymbol lf + || lf.IsStatic // shared storage, not per-instance ownership + || !SymbolEqualityComparer.Default.Equals(lf, field)) return false; + if (cm.GetSymbolInfo(asg.Right).Symbol is not IParameterSymbol p + || !SymbolEqualityComparer.Default.Equals(p.ContainingSymbol, ctor)) + return false; // field set from a non-parameter — bail + var matched = p.Ordinal; // the call must be POSITIONAL up to the adopted slot, so the arg index == the param // ordinal (a named/reordered call would mis-attribute). Require enough positional args. if (oce.ArgumentList is not { } al || al.Arguments.Count <= matched @@ -706,22 +705,47 @@ static bool TryAdoptedArgIndex(BaseObjectCreationExpressionSyntax oce, SemanticM // RHS, passed on as an argument, or captured by a closure)? Deliberately OVER-approximates // — any arg-pass counts, even a borrow — because it only gates the *adopt* exception below: // over-escaping the wrapper makes us DECLINE the alias (under-claim), never fabricate one. -// `w.Dispose()` (a member access) is not an escape, so a disposed wrapper still qualifies. +// `w.Dispose()` (a member access where `w` is the receiver) is a use, not an escape, so a +// disposed wrapper still qualifies. The escape test WALKS ANCESTORS through value-wrapping +// expressions (casts, `??`, parens, conditionals) so `return (IDisposable)w;` / `x = w ?? f;` +// / `Foo((object)w)` are all caught, not just a direct-parent return/arg/assignment-RHS +// (CodeRabbit). static bool LocalEscapesSyntactically(string name, BlockSyntax mbody) { foreach (var idn in mbody.DescendantNodes().OfType()) { if (idn.Identifier.Text != name) continue; - if (idn.Parent is ReturnStatementSyntax) - return true; - if (idn.Parent is AssignmentExpressionSyntax a && a.Right == idn) - return true; - if (idn.Parent is ArgumentSyntax) - return true; + // a bare receiver `name.Member(...)` / `name?.Member` is a use/release, not an escape. + if (idn.Parent is MemberAccessExpressionSyntax ma && ma.Expression == idn) + continue; + // captured by a closure (lambda / local function) — outlives the method. + var captured = false; for (var p = idn.Parent; p is not null && p != mbody; p = p.Parent) if (p is AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax) - return true; + { + captured = true; + break; + } + if (captured) + return true; + // climb the value's wrapping expressions to where it lands: a return, an argument, or + // an assignment RHS is an escape; reaching the enclosing statement first is not. + SyntaxNode child = idn; + var escapes = false; + for (var p = idn.Parent; p is not null; child = p, p = p.Parent) + { + if (p is ReturnStatementSyntax or ArgumentSyntax + || (p is AssignmentExpressionSyntax asg && asg.Right == child)) + { + escapes = true; + break; + } + if (p is StatementSyntax) // reached the enclosing statement, no escape + break; + } + if (escapes) + return true; } return false; }