diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5f0a4d64..cadee87b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -649,6 +649,12 @@ jobs: || { echo "FAIL: expected OWN001 on the local hidden behind a top-level validation throw"; exit 1; } echo "$out" | grep -qE "OWN001.*'dotNoTry' may not be disposed on every path" \ || { echo "FAIL: expected OWN001 on the body-level dispose-not-called-on-throw (no try)"; exit 1; } + # flow-path pool LABEL: an ArrayPool Rent returned on one path only leaks on the other -> + # the flow path must word it as a "pooled buffer" (Return), NOT the generic "disposable". + # The extractor stamps the acquire kind; the bridge tags [resource: pooled buffer]. Pins + # the mislabel fix surfaced by the --body-throw-edges Npgsql capstone. + echo "$out" | grep -qE "OWN001.*pooled buffer 'partialBuf' may not be returned to the pool on every path" \ + || { echo "FAIL: expected the flow-path pooled-buffer partial-path label on 'partialBuf'"; exit 1; } # closure-capture escape (precision): a SemaphoreSlim captured by a returned async # lambda outlives the method, so it cannot be disposed at method scope -> escaped -> # silent ('captured'). A SemaphoreSlim NOT captured and never disposed STILL leaks -> diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 7547331a..3b508f3b 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -744,7 +744,11 @@ or ImplicitObjectCreationExpressionSyntax || IsPoolRent(v.Initializer?.Value, model) // ArrayPool Rent || IsMemoryPoolRent(v.Initializer?.Value, model) // MemoryPool Rent (IMemoryOwner) || IsOwningFactory(v.Initializer?.Value, model))) // File / crypto Create* factory - nodes.Add(new { op = "acquire", var = v.Identifier.Text, line = LineOf(v) }); + // Tag an ArrayPool rent so the bridge labels a partial-path leak a + // "pooled buffer" (Return not on every path), not the generic "disposable" + // — 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" }); // 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). diff --git a/frontend/roslyn/samples/FlowLocalsSample.cs b/frontend/roslyn/samples/FlowLocalsSample.cs index df3afb0e..b51fcaa6 100644 --- a/frontend/roslyn/samples/FlowLocalsSample.cs +++ b/frontend/roslyn/samples/FlowLocalsSample.cs @@ -571,6 +571,22 @@ public void DeferredHandoffNoFalsePositive() defer.Dispose(); // disposed here -> balanced -> silent } + // flow-path pool LABEL: a rented buffer Returned only on the `then` path leaks on the else + // path -> OWN001 "pooled buffer 'partialBuf' may not be returned to the pool on every path" + // [resource: pooled buffer]. Pins the flow-path pool label end-to-end (the extractor stamps + // the acquire kind 'pool'; the bridge words it as a Return, not a Dispose) — it used to be + // mislabelled the generic "IDisposable local … disposable" (surfaced by the --body-throw-edges + // Npgsql capstone on CompositeBuilder/BitStringConverters ArrayPool rents). + public void PoolReturnedOnOnePath(bool c) + { + var partialBuf = ArrayPool.Shared.Rent(16); + partialBuf[0] = 1; + if (c) + { + ArrayPool.Shared.Return(partialBuf); + } + } + // 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 diff --git a/ownlang/ownir.py b/ownlang/ownir.py index ce918e4a..1ed9791f 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -993,7 +993,10 @@ def _lower_flow(nodes: list[Any], ffile: str, fname: str, localmap[name] = handle handles[handle] = {"file": ffile, "line": line, "event": name, "component": fname, "resource": "flow-local", - "ever_released": name in released_vars} + "ever_released": name in released_vars, + # the extractor stamps an ArrayPool Rent's acquire kind so a + # partial-path leak reads as a "pooled buffer", not "disposable". + "pool": n.get("kind") == "pool"} body.append(Let(handle, Acquire("Disposable", [], line), line)) elif op == "use": h = localmap.get(str(n.get("var"))) @@ -1123,15 +1126,34 @@ def check_facts(facts: dict[str, Any]) -> list[Finding]: f"(over-read / over-clear)"), kind="pooled buffer")) continue + # An ArrayPool Rent is released by Return (a "pooled buffer"), not Dispose (a + # "disposable"); the extractor stamps the acquire's kind so the flow path words + # and tags it correctly — previously a Rent leaked/misused on a flow path was + # mislabelled the generic "disposable" (e.g. a partial-throw-path Return). + pool = sub.get("pool") if d.code == "OWN001": - # OWN001 spans "released on 0 paths" and "released on some but not all" - # — the core's "not on every path". Word it from whether the flow body - # released this local anywhere (ever_released): no release at all reads - # as "never disposed"; a partial release as "not on every path". Same - # leak verdict either way. - msg = (f"IDisposable local '{name}' may not be disposed on every path (leak)" - if sub.get("ever_released") - else f"IDisposable local '{name}' is never disposed (leak)") + # OWN001 spans "released on 0 paths" and "released on some but not all" — + # the core's "not on every path". Word it from whether the flow body + # released this local anywhere (ever_released): no release at all reads as + # "never returned/disposed"; a partial release as "not on every path". + if pool: + if sub.get("ever_released"): + msg = (f"pooled buffer '{name}' may not be returned to the " + f"pool on every path (leak)") + else: + msg = (f"pooled buffer '{name}' is rented but never " + f"returned to the pool (leak)") + else: + msg = (f"IDisposable local '{name}' may not be disposed on every path (leak)" + if sub.get("ever_released") + else f"IDisposable local '{name}' is never disposed (leak)") + elif pool: + msg = { + "OWN002": f"pooled buffer '{name}' is used after it is returned to the pool", + "OWN003": f"pooled buffer '{name}' is returned to the pool more than once", + "OWN009": (f"pooled buffer '{name}' may be used after " + f"being returned on some path"), + }.get(d.code, f"pooled buffer '{name}': {d.message}") else: msg = { "OWN002": f"IDisposable local '{name}' is used after it is disposed", @@ -1141,7 +1163,7 @@ def check_facts(facts: dict[str, Any]) -> list[Finding]: findings.append(Finding( file=sub["file"], line=int(sub.get("line", 0)), code=d.code, component=component, event=name, handler="", message=msg, - kind="disposable")) + kind="pooled buffer" if pool else "disposable")) continue if sub.get("di_source_life"): # OWN014 region escape sourced from the DI graph (P-006 + P-004): the diff --git a/tests/fixtures/ownir/flow_pool_partial.facts.json b/tests/fixtures/ownir/flow_pool_partial.facts.json new file mode 100644 index 00000000..a2b2e5a4 --- /dev/null +++ b/tests/fixtures/ownir/flow_pool_partial.facts.json @@ -0,0 +1,22 @@ +{ + "ownir_version": 0, + "module": "Extracted", + "comment": "Pool buffers on the FLOW path must read as a 'pooled buffer' (Return wording / [resource: pooled buffer]), not the generic 'disposable' — the extractor stamps the acquire kind 'pool' for an ArrayPool Rent so the bridge words it right. (1) PartialPoolLeak: a Rent returned on the fall-through but NOT on the throw edge -> 'may not be returned to the pool on every path'. (2) NeverReturnedPool: a Rent never returned -> 'rented but never returned to the pool'. (3) DisposableControl: a plain disposable on the same partial shape keeps the IDisposable wording. Regression for the flow-path pool mislabel surfaced by the --body-throw-edges Npgsql capstone (CompositeBuilder/BitStringConverters ArrayPool rents).", + "components": [], + "functions": [ + {"name": "PoolFlow.PartialPoolLeak", "file": "PoolFlow.cs", "body": [ + {"op": "acquire", "var": "buf", "line": 10, "kind": "pool"}, + {"op": "if", "line": 11, "then": [{"op": "return", "var": null, "line": 11}], "else": []}, + {"op": "release", "var": "buf", "line": 12} + ]}, + {"name": "PoolFlow.NeverReturnedPool", "file": "PoolFlow.cs", "body": [ + {"op": "acquire", "var": "nbuf", "line": 20, "kind": "pool"}, + {"op": "use", "var": "nbuf", "line": 21} + ]}, + {"name": "PoolFlow.DisposableControl", "file": "PoolFlow.cs", "body": [ + {"op": "acquire", "var": "d", "line": 30, "kind": "disposable"}, + {"op": "if", "line": 31, "then": [{"op": "return", "var": null, "line": 31}], "else": []}, + {"op": "release", "var": "d", "line": 32} + ]} + ] +} diff --git a/tests/test_ownir.py b/tests/test_ownir.py index 6b043086..a7dcc02a 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -62,6 +62,8 @@ "ownir", "flow_nested_throw.facts.json") _FINALLY_SWITCH_FIXTURE = os.path.join(os.path.dirname(__file__), "fixtures", "ownir", "flow_finally_switch.facts.json") +_POOL_PARTIAL_FIXTURE = os.path.join(os.path.dirname(__file__), "fixtures", + "ownir", "flow_pool_partial.facts.json") _DI_FIXTURE = os.path.join(os.path.dirname(__file__), "fixtures", "ownir", "di.facts.json") _UNRESOLVED_FIXTURE = os.path.join(os.path.dirname(__file__), "fixtures", @@ -439,6 +441,41 @@ def _sub(source: str | None) -> list[Finding]: f"else-branch leak) only — clean 'r' (finally before return) and 's' " f"(switch all-dispose) stay silent — got {fsgot}") + # --- flow-path pool labelling: an ArrayPool Rent leaked on the flow path must read as a + # "pooled buffer" (Return wording + [resource: pooled buffer]), not the generic + # "disposable". The extractor stamps the acquire's kind 'pool'; the bridge words a + # partial-path leak ('may not be returned to the pool on every path'), a never-returned + # one ('rented but never returned'), and leaves a plain disposable on the same shape + # ('may not be disposed on every path'). Regression for the mislabel the --body-throw- + # edges Npgsql capstone surfaced (CompositeBuilder/BitStringConverters ArrayPool rents). + with open(_POOL_PARTIAL_FIXTURE, encoding="utf-8") as f: + ppfacts = json.load(f) + ppfindings = check_facts(ppfacts) + by = {x.event: x for x in ppfindings} + checks += 1 + # exactly three findings, one per fixture function — no extra/duplicate findings + # (a regression that adds a spurious pool/disposable finding must fail here, not slip + # past the per-event checks below). CodeRabbit hardening. + if set(by.keys()) != {"buf", "nbuf", "d"} or len(ppfindings) != 3: + fails.append(f"pool-label: expected exactly ['buf','nbuf','d'], got " + f"{[x.event for x in ppfindings]}") + want = [ + ("buf", "pooled buffer", "may not be returned to the pool on every path", "pooled buffer"), + ("nbuf", "pooled buffer", "rented but never returned to the pool", "pooled buffer"), + ("d", "disposable", "may not be disposed on every path", "disposable"), + ] + for ev, kind, msg_sub, tag in want: + fdg = by.get(ev) + if fdg is None or fdg.code != "OWN001": + fails.append(f"pool-label: expected OWN001 on '{ev}', got {fdg!r}") + elif fdg.kind != kind: + fails.append(f"pool-label: '{ev}' kind want {kind!r}, got {fdg.kind!r}") + elif msg_sub not in fdg.message: + fails.append(f"pool-label: '{ev}' message want {msg_sub!r}, " + f"got {fdg.message!r}") + elif f"[resource: {tag}]" not in fdg.render(): + fails.append(f"pool-label: '{ev}' render want [resource: {tag}], got {fdg.render()!r}") + # --- P-016 A1 reaches the frontend: a `while` flow body (the extractor now # lowers loops instead of skipping the method) routes through the core's # worklist fixpoint. A resource acquired before the loop and released INSIDE