From 5291fa9f8c58cb40e3ae7bbdc2686aa93580fa86 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 26 Jun 2026 18:02:30 +0000 Subject: [PATCH 1/3] feat(extractor): emit fresh-returning factory facts for D5.2 (real-C# interprocedural leaks) Wire the Roslyn extractor (--flow-locals) to produce the OwnIR the D5.2 fresh-return machinery consumes, so first-party factory leaks are caught on real C# (not just synthetic unit tests): - A new'd IDisposable returned BARE outside a try stays tracked and lowers to acquire + return , so the core classifies the method returnsOwned: fresh (the return discharges it -> the factory itself stays silent). A return inside a try, or a using/pool/owning-factory local, keeps its existing behaviour. - var r = FirstPartyFactory() (a source-visible method returning an owned IDisposable) emits a new call callee=... result=r op. The core mints the acquire only when it proves the callee fresh, so a non-fresh first-party call is never falsely owned. IsFirstPartyDisposableFactory gates on source-declared, non-void, disposable, non-dispose-optional return; overloads resolve to unknown (silent). Adds FactoryLeakSample.cs + a CI assertion: a dropped factory result leaks interprocedurally (OWN001 at the call site), the disposed caller and the factory stay silent. Bridge side already proven by the D5.2 unit tests; this closes the extractor gap so the capability fires on real code. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Fmwk5ruExKy5fQ77CJ2NtF --- .github/workflows/ci.yml | 13 +++- docs/notes/d5-ownership-transfer.md | 16 ++++- frontend/roslyn/OwnSharp.Extractor/Program.cs | 61 ++++++++++++++++++- frontend/roslyn/samples/FactoryLeakSample.cs | 35 +++++++++++ 4 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 frontend/roslyn/samples/FactoryLeakSample.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4db59c4e..8ce89c37 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -642,7 +642,8 @@ jobs: # detector cannot catch (use-after-dispose, double-dispose, leak-on-path). dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \ frontend/roslyn/samples/FlowLocalsSample.cs \ - frontend/roslyn/samples/MemoryOwnerEscapeSample.cs --flow-locals -o "$RUNNER_TEMP/flow.json" + frontend/roslyn/samples/MemoryOwnerEscapeSample.cs \ + frontend/roslyn/samples/FactoryLeakSample.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; } @@ -821,6 +822,16 @@ jobs: 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 tdClean vtc tif; do if echo "$out" | grep -q "'$ok'"; then echo "FAIL: silent/exempt case '$ok' was reported"; exit 1; fi done + # P-005 D5.2 INTERPROCEDURAL fresh-returning factory (FactoryLeakSample.cs): the core + # infers `StreamFactory.Make` returns `fresh` (its `acquire; return ` body), so a + # caller that binds the result and drops it leaks at the CALL SITE — a finding the flat, + # intra-procedural detectors cannot see. The disposed caller and the factory itself stay + # silent (the factory transfers ownership out via its return). + echo "$out" | grep -qE "OWN001.*'factoryLeak'" \ + || { echo "FAIL: expected the interprocedural OWN001 on the dropped factory result"; exit 1; } + for ok in factoryOk made; do + 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: Opt-in body-throw-edges tier (--body-throw-edges, P-016 throw firehose) run: | diff --git a/docs/notes/d5-ownership-transfer.md b/docs/notes/d5-ownership-transfer.md index 41336ec8..e3309ed8 100644 --- a/docs/notes/d5-ownership-transfer.md +++ b/docs/notes/d5-ownership-transfer.md @@ -294,9 +294,19 @@ escape-without-transfer and all `unknown`/`may` lower to **silence** in the defa never `fresh` (that is wrap/alias, T4/D5.4), and a non-fresh / unknown return makes no claim — the result is never falsely owned. Proven by synthetic OwnIR tests: factory-result leak (OWN001 @ the call), disposed-clean, use-after-dispose (OWN002), forward-return propagation, - and the param-return precision guard. **Remaining T1 door:** `out`/`ref`-owned parameters - (another `fresh` source) — extractor-side recognition of an out-assignment as a fresh acquire - — rides into a later slice before async. + and the param-return precision guard. **Extractor emission (shipped).** The Roslyn extractor + now produces the facts that drive this on real C# (`--flow-locals`): a `new`'d local returned + bare outside a `try` stays tracked and emits `acquire …; return ` (so a first-party + factory is classified `fresh`), and `var r = FirstPartyFactory()` — a call to a source-visible + method returning an owned `IDisposable` — emits a `call callee=… result=r` op (the core mints + the acquire only when it proves the callee `fresh`, so a non-fresh call is never falsely owned). + `IsFirstPartyDisposableFactory` gates on a source-declared, non-void, disposable, non-dispose- + optional return; overloads (non-unique names) resolve to `unknown` and stay silent. Validated + end-to-end by `FactoryLeakSample.cs` in CI: a dropped factory result leaks **interprocedurally** + (OWN001 at the call site — beyond the flat detectors), while the disposed caller and the factory + itself stay silent. **Remaining T1 door:** `out`/`ref`-owned parameters (another `fresh` source) + — extractor-side recognition of an out-assignment as a fresh acquire — rides into a later slice + before async. - **Bridge branch-scope fix (shipped — separate from the D5 transfer ladder).** The OwnIR→core bridge uses a *flat* `localmap` but emitted each synthetic `Let` *inside* the branch block it occurred in, so a local `acquire`d in **both** branches of an `if` and released **after** the diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 4c48dc76..410fb1f6 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -560,6 +560,28 @@ oce.ArgumentList is { } args _ => "?", }; +// P-005 D5.2: a FIRST-PARTY method call whose result is an owned IDisposable — the +// caller side of a fresh-returning factory. Because the callee is defined in source (not +// the BCL), the core can see its body and infer whether it returns `fresh`; if so, a +// `var r = Factory()` binding is an acquire and a caller that drops `r` leaks. The emitted +// `callee` matches the `functions[]` key `{TypeName}.{MethodName}`. A null/extern symbol, +// a void/non-disposable return, or a dispose-optional return is rejected (no claim); an +// overload (non-unique name) resolves to `unknown` in the core and is silently safe. +static bool IsFirstPartyDisposableFactory(ExpressionSyntax? expr, SemanticModel model, out string callee) +{ + callee = ""; + if (expr is not InvocationExpressionSyntax inv) + return false; + if (model.GetSymbolInfo(inv).Symbol is not IMethodSymbol m) + return false; + if (m.ReturnsVoid || m.DeclaringSyntaxReferences.Length == 0) + return false; // void, or not first-party (no visible body to infer `fresh` from) + if (!ImplementsIDisposable(m.ReturnType) || IsDisposeOptional(m.ReturnType)) + return false; + callee = $"{m.ContainingType.Name}.{m.Name}"; + return true; +} + // 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 @@ -758,6 +780,13 @@ or ImplicitObjectCreationExpressionSyntax // — the flow path previously mislabelled a pool buffer leaked on a throw edge. nodes.Add(new { op = "acquire", var = v.Identifier.Text, line = LineOf(v), kind = IsPoolRent(v.Initializer?.Value, model) ? "pool" : "disposable" }); + // P-005 D5.2: `var r = FirstPartyFactory()` — emit a `call` op (NOT an + // acquire); the core mints the acquire only if it proves the callee returns + // `fresh`, so a non-fresh first-party call is never falsely owned. + else if (tracked.Contains(v.Identifier.Text) + && IsFirstPartyDisposableFactory(v.Initializer?.Value, model, out var fpCallee)) + nodes.Add(new { op = "call", callee = fpCallee, args = Array.Empty(), + result = v.Identifier.Text, line = LineOf(v) }); // POOL005: a full-length view in the initializer — `var copy = buf.AsSpan().ToArray();` // — over-reads the pooled tail just as `Emit(buf.AsSpan());` does. EmitFlowExpr is not // called on a non-acquire initializer, so scan it here for the overspan (Codex review). @@ -849,7 +878,16 @@ or ImplicitObjectCreationExpressionSyntax nodes.AddRange(chain); } else - nodes.Add(new { op = "return", var = (string?)null, line = LineOf(rs) }); + { + // P-005 D5.2: a tracked local returned BARE (outside any `finally`) is a + // fresh-factory transfer — emit it as the return's `var` so the core models the + // escape (a discharge: ownership moves to the caller) and classifies the method + // `returnsOwned: fresh`. A non-identifier / non-tracked return is a bare CFG exit. + var rvar = rs.Expression is IdentifierNameSyntax rid + && tracked.Contains(rid.Identifier.Text) + ? rid.Identifier.Text : (string?)null; + nodes.Add(new { op = "return", var = rvar, line = LineOf(rs) }); + } return true; } case WhileStatementSyntax ws: @@ -3181,6 +3219,12 @@ or ImplicitObjectCreationExpressionSyntax } init candidates.Add(v.Identifier.Text); else if (IsMemoryPoolRent(v.Initializer?.Value, model)) // MemoryPool IMemoryOwner (Dispose-released, NOT a poolBuffer) candidates.Add(v.Identifier.Text); + else if (IsFirstPartyDisposableFactory(v.Initializer?.Value, model, out _)) + // P-005 D5.2: `var r = FirstPartyFactory()` — a candidate acquire + // IFF the core proves the callee returns `fresh` (it emits a `call` + // op, not an `acquire`; the core decides). Checked last so `new` / + // pool / BCL-factory initializers keep their existing classification. + candidates.Add(v.Identifier.Text); } // `using (IMemoryOwner owner = MemoryPool.Rent(...)) { … }` STATEMENT form: track the owner // too, so its returned view dangles after the scope-exit dispose (the desugar mirrors the @@ -3254,9 +3298,20 @@ or ImplicitObjectCreationExpressionSyntax } init // use of the returned owner trips OWN002 — the bare-owner twin of the returned-view // dangle. A NON-using returned owner stays a genuine transfer (escaped → untracked → // silent), so this never fires on `var o = Rent(); return o;`. - if (idn.Parent is ReturnStatementSyntax) + if (idn.Parent is ReturnStatementSyntax rsp) { - if (!usingMemoryOwners.Contains(nm)) + // P-005 D5.2: a `new`'d IDisposable returned BARE outside any `try` is a + // fresh-returning FACTORY — keep it tracked so the flow body emits + // `acquire …; return `. The core then classifies the method + // `returnsOwned: fresh` (and the `return ` discharges it, so the + // factory itself stays silent), letting a caller that drops the result + // leak. A return INSIDE a try threads `finally` edges the fresh path does + // not model yet, so keep the old transfer (escape) there; a `using` owner + // also stays tracked (its scope-exit dispose dangles the returned value). + var freshFactory = newedDisposables.Contains(nm) + && !rsp.Ancestors().TakeWhile(a => a != mbody) + .OfType().Any(); + if (!usingMemoryOwners.Contains(nm) && !freshFactory) escapedLocals.Add(nm); } // A pooled buffer handed as an argument is normally a BORROW (the renter Returns it), diff --git a/frontend/roslyn/samples/FactoryLeakSample.cs b/frontend/roslyn/samples/FactoryLeakSample.cs new file mode 100644 index 00000000..9fb14588 --- /dev/null +++ b/frontend/roslyn/samples/FactoryLeakSample.cs @@ -0,0 +1,35 @@ +using System.IO; + +namespace Factories; + +// P-005 D5.2: a fresh-returning factory + its callers. The factory `Make` creates and +// hands back a NEW owned stream; the core infers `returnsOwned: fresh` from its +// `acquire; return ` body. A caller that binds the result and drops it (`Leaks`) +// is then charged the leak at the call site — an INTERPROCEDURAL finding the flat, +// intra-procedural detectors cannot see. A caller that disposes the result (`Clean`) +// stays silent, and the factory itself stays silent (it transfers ownership out). +public static class StreamFactory +{ + public static Stream Make() + { + var made = new MemoryStream(); // freshly owned, handed to the caller + return made; + } +} + +public static class FactoryConsumers +{ + // Drops the fresh factory result without disposing -> OWN001 at the call site. + public static void Leaks() + { + var factoryLeak = StreamFactory.Make(); + factoryLeak.WriteByte(1); + } + + // Disposes the fresh factory result -> clean (silent). + public static void Clean() + { + var factoryOk = StreamFactory.Make(); + factoryOk.Dispose(); + } +} From 099e73c2027c1061e2e409e9dd13d5a6b38dc137 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 26 Jun 2026 18:13:47 +0000 Subject: [PATCH 2/3] fix(d5): don't crash on unknown callees; FQ factory keys; preserve call args (CI fix + Codex/CodeRabbit) The extractor's new call ops surfaced calls to callees not in functions[] (BCL / extension methods like GetRequiredService), which crashed check_facts with OWN040. Plus three review items: - (CI crash) Gate the Call emission on a RESOLVABLE callee (in the MOS or a sink extern); an unknown callee is dropped (no claim), never lowered to a Call that raises OWN040. Add OWN040 to the bridge-artifact skip list belt-and-suspenders. Regression test: a call to an unknown callee makes no claim and does not crash. - (Codex P2) Fully-qualify the factory summary key on BOTH sides ({Namespace.Type}.{Method} via FlowFunctionName + ContainingType.ToDisplayString) so two same-named factories across namespaces never alias into a false OWN001. - (CodeRabbit Major) Preserve the call's tracked identifier args instead of Array.Empty, so the core can apply the callee's per-argument effects. - (CodeRabbit Minor) Anchor the CI assertion to FactoryLeakSample.cs: at the call site, not just any match of 'factoryLeak'. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Fmwk5ruExKy5fQ77CJ2NtF --- .github/workflows/ci.yml | 4 +-- frontend/roslyn/OwnSharp.Extractor/Program.cs | 34 +++++++++++++++++-- ownlang/ownir.py | 21 ++++++++++-- tests/test_ownir.py | 15 ++++++++ 4 files changed, 66 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8ce89c37..176011d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -827,8 +827,8 @@ jobs: # caller that binds the result and drops it leaks at the CALL SITE — a finding the flat, # intra-procedural detectors cannot see. The disposed caller and the factory itself stay # silent (the factory transfers ownership out via its return). - echo "$out" | grep -qE "OWN001.*'factoryLeak'" \ - || { echo "FAIL: expected the interprocedural OWN001 on the dropped factory result"; exit 1; } + echo "$out" | grep -qE "FactoryLeakSample\.cs:[0-9]+:.*\[OWN001\].*'factoryLeak'" \ + || { echo "FAIL: expected the interprocedural OWN001 on the dropped factory result at its call site"; exit 1; } for ok in factoryOk made; do if echo "$out" | grep -q "'$ok'"; then echo "FAIL: D5.2 silent 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 410fb1f6..b02546a2 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -578,10 +578,24 @@ static bool IsFirstPartyDisposableFactory(ExpressionSyntax? expr, SemanticModel return false; // void, or not first-party (no visible body to infer `fresh` from) if (!ImplementsIDisposable(m.ReturnType) || IsDisposeOptional(m.ReturnType)) return false; - callee = $"{m.ContainingType.Name}.{m.Name}"; + // Fully-qualified key (namespace + containing-type chain) so the call resolves to the + // RIGHT summary: two `StreamFactory.Make` in different namespaces must not alias, or a + // call to a non-fresh one could pick up a fresh one's summary and fabricate OWN001 + // (Codex). Must match the `functions[]` name built by `FlowFunctionName`. + callee = $"{m.ContainingType.ToDisplayString()}.{m.Name}"; return true; } +// The fully-qualified `functions[]` key for a method — `{Namespace.Containing.Type}.{Name}` — +// used both as the flow-function name and as a D5.2 call callee, so the two always agree (a +// simple `{Type}.{Name}` would alias same-named types across namespaces). Falls back to the +// syntactic class name only if the symbol cannot be resolved. +static string FlowFunctionName(BaseMethodDeclarationSyntax method, string fallbackType, + SemanticModel model) => + model.GetDeclaredSymbol(method) is IMethodSymbol ms + ? $"{ms.ContainingType.ToDisplayString()}.{ms.Name}" + : $"{fallbackType}.{MethodName(method)}"; + // 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 @@ -785,8 +799,22 @@ or ImplicitObjectCreationExpressionSyntax // `fresh`, so a non-fresh first-party call is never falsely owned. else if (tracked.Contains(v.Identifier.Text) && IsFirstPartyDisposableFactory(v.Initializer?.Value, model, out var fpCallee)) - nodes.Add(new { op = "call", callee = fpCallee, args = Array.Empty(), + { + // Preserve the call's TRACKED identifier args (CodeRabbit) so the core + // can apply the callee's per-argument ownership effects (consume/borrow) + // to a `var r = Wrap(stream)` — not just the fresh return. Untracked / + // non-identifier args are dropped (no local to attribute an effect to). + var fpArgs = v.Initializer?.Value is InvocationExpressionSyntax fpInv + ? fpInv.ArgumentList.Arguments + .Select(a => a.Expression) + .OfType() + .Select(id => id.Identifier.Text) + .Where(tracked.Contains) + .ToArray() + : Array.Empty(); + nodes.Add(new { op = "call", callee = fpCallee, args = fpArgs, result = v.Identifier.Text, line = LineOf(v) }); + } // POOL005: a full-length view in the initializer — `var copy = buf.AsSpan().ToArray();` // — over-reads the pooled tail just as `Emit(buf.AsSpan());` does. EmitFlowExpr is not // called on a non-acquire initializer, so scan it here for the overspan (Codex review). @@ -3355,7 +3383,7 @@ or ImplicitObjectCreationExpressionSyntax } init statMethodsAnalysed++; flowFunctions.Add(new { - name = $"{cls.Identifier.Text}.{MethodName(method)}", + name = FlowFunctionName(method, cls.Identifier.Text, model), file, body = fbody, }); diff --git a/ownlang/ownir.py b/ownlang/ownir.py index 506b47a3..1f5905c0 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -1075,6 +1075,11 @@ def _infer_return_skeleton(nodes: Any, param_names: set[str]) -> ReturnSkeleton: ExternDecl("$borrow_mut", [EffectParam(Effect.BORROW_MUT, "Disposable", 0)], None, 0), ) +# The callee names a `call` op may resolve against in `lower_call` WITHOUT being a +# first-party function summary — the fixed sink externs. A call to any other callee +# that is not in the solved MOS is unresolvable (no signature) and must NOT be lowered +# to a `Call` (it would raise OWN040); see the `call` handler in `_lower_flow`. +_SINK_EXTERN_NAMES = frozenset(e.name for e in _OWNERSHIP_SINK_EXTERNS) # A forward to a sink extern is a *known* transfer, so a skeleton can record the # resolved path action directly — `$consume` is ownership leaving (a must-transfer), @@ -1530,7 +1535,14 @@ def _lower_flow(nodes: list[Any], ffile: str, fname: str, # extractor (it is an escape, surfaced separately). callee = str(n.get("callee", "")) raw_args = n.get("args", []) - if callee and isinstance(raw_args, list): + summ = mos.get(callee) if (mos is not None and callee) else None + # Only emit the `Call` when the callee is RESOLVABLE — a first-party function + # with a summary, or a fixed ownership-sink extern. A real extraction surfaces + # calls to callees we did not lower as functions (BCL / extension methods like + # `GetRequiredService`); those have no signature, so `lower_call` would raise + # OWN040. Drop them (no effect, no claim) — precision-safe, never a crash. + if (summ is not None or callee in _SINK_EXTERN_NAMES) \ + and callee and isinstance(raw_args, list): arg_refs: list[Expr] = [VarRef(localmap.get(str(a), str(a)), line) for a in raw_args] body.append(Call(callee, arg_refs, line)) @@ -1540,7 +1552,6 @@ def _lower_flow(nodes: list[Any], ffile: str, fname: str, # above; this models the return). A non-fresh / unknown return makes no # claim, so the result is never falsely owned (precision-first). result = n.get("result") - summ = mos.get(callee) if (mos is not None and callee) else None if (isinstance(result, str) and result and result not in hoisted and summ is not None and getattr(summ, "returns", None) == "fresh"): handle = f"loc_{loc[0]}" @@ -1600,7 +1611,11 @@ def check_facts(facts: dict[str, Any]) -> list[Finding]: # parameter's contract could not be inferred (an ambiguous pass-through # stays plain) -- a "needs annotation / transitive inference" gap, not a # leak. Surfacing these properly (with a subject) is a later step. - if d.code in ("OWN033", "OWN034", "OWN035", "OWN041"): + # - OWN040: a `call` to a callee the bridge did not lower as a function + # (an extension/BCL method surfaced by the extractor). The `call` handler + # already drops unresolvable callees, so this is belt-and-suspenders — a + # synthetic-call artifact, never a real C# bug (C# already binds the call). + if d.code in ("OWN033", "OWN034", "OWN035", "OWN040", "OWN041"): continue sub = handles.get(_handle_of(d) or "") if sub is None: diff --git a/tests/test_ownir.py b/tests/test_ownir.py index e192aeb0..6e50c18a 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -1471,6 +1471,21 @@ def _sub(source: str | None) -> list[Finding]: gotpr = [(x.component, x.code) for x in pr] fails.append("D5.2 T1: returning a parameter is not `fresh` (no false acquire of " f"the result, no consume of the arg), got {gotpr}") + # ROBUSTNESS (real extraction): a `call` to a callee NOT in functions[] — a BCL / + # extension method the extractor surfaced (e.g. `GetRequiredService`) — has no + # signature, so it must be dropped (no Call, no acquire), NEVER raise OWN040. The + # bridge gates the Call on a resolvable callee and skips OWN040 belt-and-suspenders. + checks += 1 + try: + unk = check_facts({"module": "M", "functions": [ + {"name": "Caller.Use", "file": "T1.cs", + "body": [{"op": "call", "callee": "Ext.GetRequiredService", "args": [], + "result": "svc", "line": 5}]}]}) + if unk: + fails.append("D5.2: a call to an unknown callee must make no claim (no finding), " + f"got {[(x.component, x.code) for x in unk]}") + except OwnIRError as e: + fails.append(f"D5.2: a call to an unknown callee must not crash (OWN040), got {e!r}") # the factory acquire must fire inside CONTROL FLOW too: a fresh-returning call in # an `if` branch whose result is never disposed leaks, exactly like a top-level one. # (Codex P2: the recursive _lower_flow calls must thread `mos` into nested bodies, From fb3d5be3c0791742ce2d429bbec60fdf5d8c3a26 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 26 Jun 2026 18:22:01 +0000 Subject: [PATCH 3/3] fix(d5): kill prior binding on call-result overwrite; positional-only call args (CodeRabbit) - (Major, bridge) When a `call` op's result reuses a tracked local, drop the stale localmap binding before any optional fresh acquire. Otherwise `acquire x; x = Unknown(); release x` read as clean while the original x actually leaked (reference lost). A hoisted local keeps its single outer-scope handle. Regression test: the overwritten original now leaks OWN001. - (Minor, extractor) Emit only POSITIONAL identifier args for the factory call op; a named arg (`Wrap(second: s2, first: s1)`) would mis-attribute under the bridge's positional effect application, so named args are dropped (under-claim, never mis-align). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Fmwk5ruExKy5fQ77CJ2NtF --- frontend/roslyn/OwnSharp.Extractor/Program.cs | 5 +++++ ownlang/ownir.py | 8 ++++++++ tests/test_ownir.py | 14 ++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index b02546a2..8282df99 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -804,8 +804,13 @@ or ImplicitObjectCreationExpressionSyntax // can apply the callee's per-argument ownership effects (consume/borrow) // to a `var r = Wrap(stream)` — not just the fresh return. Untracked / // non-identifier args are dropped (no local to attribute an effect to). + // Positional args only: the bridge applies the callee's effects by + // POSITION, so a NAMED argument (`Wrap(second: s2, first: s1)`) would + // mis-attribute if kept in syntactic order. Dropping named args + // under-claims (no effect on them) but never mis-aligns (CodeRabbit). var fpArgs = v.Initializer?.Value is InvocationExpressionSyntax fpInv ? fpInv.ArgumentList.Arguments + .Where(a => a.NameColon is null) .Select(a => a.Expression) .OfType() .Select(id => id.Identifier.Text) diff --git a/ownlang/ownir.py b/ownlang/ownir.py index 1f5905c0..567d7c2b 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -1552,6 +1552,14 @@ def _lower_flow(nodes: list[Any], ffile: str, fname: str, # above; this models the return). A non-fresh / unknown return makes no # claim, so the result is never falsely owned (precision-first). result = n.get("result") + # Overwriting a tracked local KILLS its previous ownership binding: if the + # old handle was not released before this call, it leaks (the reference is + # lost). Drop the stale mapping before any optional fresh acquire, so + # `acquire x; x = Unknown(); release x` leaks the original x rather than + # reading as clean (CodeRabbit). A hoisted local keeps its single outer-scope + # handle (it is declared once and never re-bound), so leave it alone. + if isinstance(result, str) and result and result not in hoisted: + localmap.pop(result, None) if (isinstance(result, str) and result and result not in hoisted and summ is not None and getattr(summ, "returns", None) == "fresh"): handle = f"loc_{loc[0]}" diff --git a/tests/test_ownir.py b/tests/test_ownir.py index 6e50c18a..a3eb0a99 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -1486,6 +1486,20 @@ def _sub(source: str | None) -> list[Finding]: f"got {[(x.component, x.code) for x in unk]}") except OwnIRError as e: fails.append(f"D5.2: a call to an unknown callee must not crash (OWN040), got {e!r}") + # OVERWRITE kills the prior binding (CodeRabbit): `acquire x; x = Unknown(); release x` + # — the call's result reuses an owned local and the call is dropped (unknown callee), + # so the ORIGINAL x leaks (its reference is lost), not read as clean. The release after + # must not resolve to the dead handle. + checks += 1 + ov = check_facts({"module": "M", "functions": [ + {"name": "C.M", "file": "T1.cs", + "body": [{"op": "acquire", "var": "x", "line": 1}, + {"op": "call", "callee": "Ext.Unknown", "args": [], "result": "x", "line": 2}, + {"op": "release", "var": "x", "line": 3}]}]}) + gotov = [(x.component, x.line, x.code) for x in ov] + if gotov != [("C.M", 1, "OWN001")]: + fails.append("D5.2: a call result overwriting an owned local must leak the original " + f"(OWN001@1), not read as clean, got {gotov}") # the factory acquire must fire inside CONTROL FLOW too: a fresh-returning call in # an `if` branch whose result is never disposed leaks, exactly like a top-level one. # (Codex P2: the recursive _lower_flow calls must thread `mos` into nested bodies,