diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e7fc72ed..b2017c7b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -187,6 +187,57 @@ jobs: echo "FAIL: a static-handler subscription was wrongly reported"; exit 1 fi echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) at the C# location" + - name: Flow-sensitive local IDisposables (--flow-locals, P-016 B0b/B2) + run: | + # 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" + 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; } + echo "$out" | grep -q "OWN001" || { echo "FAIL: expected OWN001 (leak on a path)"; exit 1; } + echo "$out" | grep -q "OWN003" || { echo "FAIL: expected OWN003 (double-dispose)"; exit 1; } + # a real Timer leak the flat curated allowlist misses but the semantic path catches: + echo "$out" | grep -q "OWN001.*'realTimer'" || { echo "FAIL: expected OWN001 on the leaked Timer"; exit 1; } + # the OWN001 wording splits on whether the local was released anywhere: the + # Timer is released on no path -> "is never disposed"; LeakOnElse's `leak` is + # released on the then-branch only -> "may not be disposed on every path". + echo "$out" | grep -qE "'realTimer' is never disposed" \ + || { echo "FAIL: expected the never-disposed wording for the 0-release Timer"; exit 1; } + echo "$out" | grep -qE "'leak' may not be disposed on every path" \ + || { echo "FAIL: expected the partial-path wording for LeakOnElse"; exit 1; } + # dispose-optional (Task) and disposed/loopy/escaping locals must stay silent: + for ok in clean looped esc exemptTask; 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, never-vs-every-path wording, dispose-optional exempt, beyond flat)" + - name: Escape-via-projection leak — GTM UnitOfWork (--flow-locals, P-016 B0b/B2) + run: | + # A real GTM leak the flat detector misses: a UnitOfWork (IDisposable) used + # ONLY through member access to build a returned DEFERRED IQueryable. The + # bare `uow` never escapes, so it stays tracked and is disposed on no path + # -> OWN001. Crucially NOT fixable by a naive `using` (the deferred query + # would run after dispose) — the `using var`+materialize fix (uowFixed) is + # the one that must stay silent. + dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \ + frontend/roslyn/samples/UnitOfWorkFlowSample.cs --flow-locals -o "$RUNNER_TEMP/uow.json" + out=$(python -m ownlang ownir "$RUNNER_TEMP/uow.json" || true) + echo "$out" + # `uow` is released on no path, so the OWN001 reads "is never disposed". + echo "$out" | grep -qE "OWN001.*'uow' is never disposed" \ + || { echo "FAIL: expected OWN001 'is never disposed' on UnitOfWork 'uow'"; exit 1; } + echo "$out" | grep -q "UnitOfWorkFlowSample.cs" \ + || { echo "FAIL: expected the finding at the C# sample location"; exit 1; } + # the three correct fixes must stay silent: materialize inside `using` + # (uowFixed), and ownership TRANSFERRED to the caller — returned (uowOwned) + # or moved out as an argument (uowMoved). + for ok in uowFixed uowOwned uowMoved; do + if echo "$out" | grep -q "'$ok'"; then + echo "FAIL: a correct fix ('$ok') was wrongly reported as a leak"; exit 1 + fi + done + echo "OK: escape-via-projection UnitOfWork leak -> OWN001 'never disposed'; materialize + ownership-transfer fixes stay silent" # The distribution surface (Уровень 1): the own-check.sh orchestrator walks a # directory of real C# and prints findings in the host-parseable formats the diff --git a/docs/proposals/P-016-deep-fact-extraction.md b/docs/proposals/P-016-deep-fact-extraction.md index 2a3ac20e..98199ca5 100644 --- a/docs/proposals/P-016-deep-fact-extraction.md +++ b/docs/proposals/P-016-deep-fact-extraction.md @@ -1,6 +1,16 @@ # P-016 — Deep C# fact extraction: CFG + flow lowering -- **Status:** draft (P1 — the "make the core actually bite real C#" track) +- **Status:** draft (P1 — the "make the core actually bite real C#" track). **B0a + done** (direct `Module`, no re-parse). **B0b+B2 spike landed** (experimental + `--flow-locals`): real C# → CFG flow facts → core → path-sensitive OWN001/002/003 + on local IDisposables — proven on `samples/FlowLocalsSample.cs`, run clean over + GTM. **GTM findings triaged:** 9 real leaks the flat name-based detector misses + (UnitOfWork, `System.Threading.Timer`, StreamReader/Writer) + 14 dispose-optional + FPs (Task/DataTable) now excluded by a CA2000-style exemption → 100% precision on + the sample. The own-check wrappers (`own-check.ps1`/`.sh`) now **default to + `--flow-locals`** (`-Legacy`/`--legacy` opts back to the flat detector); the raw + extractor flag stays default-off pending A1 + an `OWNIR_VERSION` bump. Next: A1 + (loops), escape-via-projection hardening, then full graduation. - **Depends on:** - [P-014](P-014-semantic-resolution.md) Tier A — the `SemanticModel` (**DONE**). The hard prerequisite: typed ownership facts are impossible without binding. diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 9466b91b..45e85ae7 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -35,10 +35,16 @@ // (e.g. to run only the disposable/pool detectors); it is the first instance of // the broader check-selection surface tracked in P-015. bool emitEvents = true; +// --flow-locals (P-016 B0b/B2, EXPERIMENTAL, default off): emit per-method flow +// facts for non-escaping local IDisposables (acquire/use/release/if/return over a +// CFG) so the core checks them path-sensitively (OWN001/002/003). Supersedes the +// flat D1 local-disposable detector when on. Default off keeps the shipped surface. +bool flowLocals = false; for (int i = 0; i < args.Length; i++) { if (args[i] == "-o" && i + 1 < args.Length) outPath = args[++i]; else if (args[i] == "--no-event-leaks") emitEvents = false; + else if (args[i] == "--flow-locals") flowLocals = true; else rawInputs.Add(args[i]); } @@ -144,6 +150,114 @@ static bool IsStaticHandler(ExpressionSyntax right, SemanticModel model) => IsHandler(right) && model.GetSymbolInfo(right).Symbol is IMethodSymbol { IsStatic: true }; +// --- P-016 B0b/B2: flow lowering for local IDisposables (experimental) --- + +// A type that implements System.IDisposable (semantic) — the flow lowering tracks +// locals of such types. +static bool ImplementsIDisposable(ITypeSymbol? t) => + t is not null + && ((t.Name == "IDisposable" && t.ContainingNamespace?.ToString() == "System") + || t.AllInterfaces.Any(i => i.Name == "IDisposable" + && i.ContainingNamespace?.ToString() == "System")); + +// Types that implement IDisposable but whose disposal is conventionally OPTIONAL — +// the .NET guidance / Roslyn CA2000 exempt them: Task/ValueTask only hold a +// lazily-allocated wait handle, and the System.Data containers' Dispose() is a +// no-op. The flow detector must not flag an undisposed local of these (this is the +// curated exemption the flat D1 detector gets for free via IsDisposableType, which +// is exactly why D1 never flagged Task/DataTable and the semantic path did). +static bool IsDisposeOptional(ITypeSymbol t) +{ + var ns = t.ContainingNamespace?.ToString(); + return (ns == "System.Threading.Tasks" && t.Name is "Task" or "ValueTask") + || (ns == "System.Data" && t.Name is "DataTable" or "DataSet" or "DataView"); +} + +static string MethodName(BaseMethodDeclarationSyntax m) => m switch +{ + MethodDeclarationSyntax md => md.Identifier.Text, + ConstructorDeclarationSyntax => ".ctor", + _ => "?", +}; + +// Lower a method block to OwnIR flow nodes (acquire/use/release/if/return) for the +// `tracked` local IDisposables. Returns null on any UNMODELLED statement +// (loop/try/switch/...): the method is then honestly skipped, not guessed. +static List? LowerFlowBody(BlockSyntax block, HashSet tracked) +{ + var nodes = new List(); + foreach (var st in block.Statements) + if (!LowerFlowStmt(st, tracked, nodes)) + return null; + return nodes; +} + +static bool LowerFlowStmt(StatementSyntax st, HashSet tracked, List nodes) +{ + switch (st) + { + case BlockSyntax b: + foreach (var s2 in b.Statements) + if (!LowerFlowStmt(s2, tracked, nodes)) + return false; + return true; + case LocalDeclarationStatementSyntax ld: + if (ld.UsingKeyword == default) + foreach (var v in ld.Declaration.Variables) + if (tracked.Contains(v.Identifier.Text) + && v.Initializer?.Value is ObjectCreationExpressionSyntax + or ImplicitObjectCreationExpressionSyntax) + nodes.Add(new { op = "acquire", var = v.Identifier.Text, line = LineOf(v) }); + return true; + case ExpressionStatementSyntax es: + EmitFlowExpr(es.Expression, tracked, nodes); + return true; + case IfStatementSyntax ifs: + { + var thenNodes = new List(); + if (!LowerFlowStmt(ifs.Statement, tracked, thenNodes)) + return false; + var elseNodes = new List(); + if (ifs.Else is { } e && !LowerFlowStmt(e.Statement, tracked, elseNodes)) + return false; + nodes.Add(new { op = "if", line = LineOf(ifs), then = thenNodes, @else = elseNodes }); + return true; + } + case UsingStatementSyntax us: + // using(...) {body}: the using local is auto-disposed (untracked); still + // lower the body so a tracked plain local used inside is seen. + return us.Statement is null || LowerFlowStmt(us.Statement, tracked, nodes); + case ReturnStatementSyntax rs: + // a tracked local never escapes (excluded), so a returned value is not a + // tracked resource — model it as a bare return (a CFG exit edge). + nodes.Add(new { op = "return", var = (string?)null, line = LineOf(rs) }); + return true; + default: + return false; // unmodelled (loop/try/switch/...) -> bail the method + } +} + +static void EmitFlowExpr(ExpressionSyntax expr, HashSet tracked, List nodes) +{ + // x.Dispose()/x.Close() on a tracked local -> release. + if (expr is InvocationExpressionSyntax inv + && inv.Expression is MemberAccessExpressionSyntax ma + && ma.Name.Identifier.Text is "Dispose" or "Close" + && ma.Expression is IdentifierNameSyntax rid + && tracked.Contains(rid.Identifier.Text)) + { + nodes.Add(new { op = "release", var = rid.Identifier.Text, line = LineOf(inv) }); + return; + } + // any other reference to a tracked local -> use (once per local in this expr). + var used = new SortedSet(StringComparer.Ordinal); + foreach (var idn in expr.DescendantNodesAndSelf().OfType()) + if (tracked.Contains(idn.Identifier.Text)) + used.Add(idn.Identifier.Text); + foreach (var u in used) + nodes.Add(new { op = "use", var = u, line = LineOf(expr) }); +} + // The field name an expression refers to: "_f" for `_f` or `this._f`, else null. static string? FieldName(ExpressionSyntax expr) => expr switch { @@ -165,6 +279,8 @@ t is "IDisposable" or "IAsyncDisposable" or "CancellationTokenSource" || t.EndsWith("Subscription"); var components = new List(); +// P-016 B0b/B2: per-method flow bodies (only when --flow-locals). +var flowFunctions = new List(); // Parse every input into a syntax tree first (keeping the file path we report // it under), then build ONE compilation over all of them so the SemanticModel @@ -385,7 +501,9 @@ or ImplicitObjectCreationExpressionSyntax // D1 (P-005): a local IDisposable the method `new`s but never disposes, // not guarded by `using`, and not handed out (returned / passed as an // argument / assigned out) — ownership transfer is ambiguous syntactically - // (P-005 D5), so those are conservatively excluded. Per member. + // (P-005 D5), so those are conservatively excluded. Per member. Suppressed + // under --flow-locals, where the path-sensitive flow detector supersedes it. + if (!flowLocals) foreach (var member in cls.Members) { var usingGuarded = new HashSet(); @@ -436,6 +554,51 @@ or ImplicitObjectCreationExpressionSyntax } } + // P-016 B0b/B2 (--flow-locals): per-method flow facts for non-escaping local + // IDisposables. The core checks them path-sensitively (OWN001/002/003). + // Methods with an unmodelled construct (loop/try/switch) are honestly skipped. + if (flowLocals) + foreach (var method in cls.Members.OfType()) + { + if (method.Body is not { } mbody) + continue; + var candidates = new HashSet(); + foreach (var ld in mbody.DescendantNodes().OfType()) + { + if (ld.UsingKeyword != default) + continue; + foreach (var v in ld.Declaration.Variables) + if (v.Initializer is { Value: ObjectCreationExpressionSyntax + or ImplicitObjectCreationExpressionSyntax } init + && model.GetTypeInfo(init.Value).Type is { } dt + && ImplementsIDisposable(dt) && !IsDisposeOptional(dt)) + candidates.Add(v.Identifier.Text); + } + if (candidates.Count == 0) + continue; + // a local that escapes (returned / passed as arg / assigned out) is + // conservatively not tracked — its disposal may be the callee's job. + var escapedLocals = new HashSet(); + foreach (var idn in mbody.DescendantNodes().OfType()) + if (candidates.Contains(idn.Identifier.Text) + && (idn.Parent is ReturnStatementSyntax or ArgumentSyntax + || (idn.Parent is AssignmentExpressionSyntax asg && asg.Right == idn))) + escapedLocals.Add(idn.Identifier.Text); + var tracked = new HashSet(candidates); + tracked.ExceptWith(escapedLocals); + if (tracked.Count == 0) + continue; + var fbody = LowerFlowBody(mbody, tracked); + if (fbody is null || fbody.Count == 0) + continue; + flowFunctions.Add(new + { + name = $"{cls.Identifier.Text}.{MethodName(method)}", + file, + body = fbody, + }); + } + if (subs.Count > 0) components.Add(new { name = cls.Identifier.Text, file, subscriptions = subs }); } @@ -443,7 +606,7 @@ or ImplicitObjectCreationExpressionSyntax // ownir_version stamps the fact-schema vocabulary; the Python core rejects a // mismatch loudly (ownlang/ownir.py OWNIR_VERSION) rather than mis-reading facts. -var facts = new { ownir_version = 0, module = "Extracted", components }; +var facts = new { ownir_version = 0, module = "Extracted", components, functions = flowFunctions }; var json = JsonSerializer.Serialize(facts, new JsonSerializerOptions { WriteIndented = true }); if (outPath is null) Console.WriteLine(json); diff --git a/frontend/roslyn/samples/FlowLocalsSample.cs b/frontend/roslyn/samples/FlowLocalsSample.cs new file mode 100644 index 00000000..72b7efe9 --- /dev/null +++ b/frontend/roslyn/samples/FlowLocalsSample.cs @@ -0,0 +1,78 @@ +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +// P-016 B0b/B2 (experimental, --flow-locals): path-sensitive flow analysis of +// local IDisposables. These are bugs the flat D1 detector cannot catch (it only +// asks "disposed anywhere?"). Distinct local names let CI assert each verdict. +public class FlowLocalsSample +{ + // OWN002: used after Dispose() + public void UseAfterDispose() + { + var uad = new MemoryStream(); + uad.WriteByte(1); + uad.Dispose(); + uad.WriteByte(2); + } + + // OWN001: disposed only on the `then` path -> leaks on the else path + public void LeakOnElse(bool c) + { + var leak = new MemoryStream(); + if (c) + { + leak.Dispose(); + } + } + + // OWN003: disposed twice + public void DoubleDispose() + { + var dbl = new MemoryStream(); + dbl.Dispose(); + dbl.Dispose(); + } + + // clean: disposed on all paths -> silent + public void Clean() + { + var clean = new MemoryStream(); + clean.WriteByte(1); + clean.Dispose(); + } + + // has a loop -> method honestly skipped (no flow finding) + public void HasLoop() + { + var looped = new MemoryStream(); + for (int i = 0; i < 3; i++) { looped.WriteByte((byte)i); } + looped.Dispose(); + } + + // escapes (returned) -> not tracked + public Stream Escapes() + { + var esc = new MemoryStream(); + return esc; + } + + // dispose-optional: Task is IDisposable but disposing it is unnecessary + // (CA2000-exempt) -> silent, not a leak. + public void TaskIsExempt() + { + var exemptTask = new Task(() => { }); + exemptTask.Start(); + } + + // real leak: System.Threading.Timer owns an unmanaged timer-queue handle and + // MUST be disposed (not dispose-optional) -> OWN001. The flat detector misses + // this (Timer is absent from its curated allowlist); the semantic flow path + // catches it. + public void TimerLeaks() + { + var realTimer = new Timer(_ => { }); + realTimer.Change(0, 1000); + } +} diff --git a/frontend/roslyn/samples/UnitOfWorkFlowSample.cs b/frontend/roslyn/samples/UnitOfWorkFlowSample.cs new file mode 100644 index 00000000..249f5b91 --- /dev/null +++ b/frontend/roslyn/samples/UnitOfWorkFlowSample.cs @@ -0,0 +1,145 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +// P-016 B0b/B2 (--flow-locals): a real leak distilled from GTM's +// CatalogService.GetProductsFromCatalogWODocuments. A UnitOfWork (IDisposable, +// owning a DbContext) is created as a local and used ONLY through member access +// (uow.ProductCatalogs / uow.TempProducts / uow.GenericRepository<>()) to build a +// DEFERRED IQueryable that is then returned. The bare `uow` never escapes (only +// `uow.Member` does), so the escape filter keeps it tracked; disposed on no path +// -> OWN001. This is the escape-via-projection case the flat D1 detector misses, +// and it is NOT fixable by a naive `using`: that would dispose the context before +// the deferred query runs (ObjectDisposedException). The real fix is to MATERIALIZE +// inside the using (GetProductsFromCatalogWODocumentsFixed below). +namespace Catalog +{ + public class CatalogService + { + // OWN001: 'uow' is created, captured into the returned deferred query via + // member access, and disposed on no path. It is never returned or passed as + // the bare identifier, so it is not exempted as an escaping local. + public IQueryable GetProductsFromCatalogWODocuments(DocHeader header) + { + Guid sessionId = Guid.NewGuid(); + IQueryable data; + IUnitOfWork uow = new UnitOfWork(true); + IQueryable productCatalogs = uow.ProductCatalogs.Where(p => p.DocumentID == null); + IQueryable productDocuments = uow.GenericRepository().GetAll(); + + IQueryable pcwodoc = from pc in productCatalogs + join pd in productDocuments on pc.ID equals pd.ProductID into a + from x in a.DefaultIfEmpty() + where x == null + select pc; + if (header == null) + { + data = pcwodoc; + } + else + { + Guid headerid = header.ID; + data = from p in pcwodoc + join tp in uow.TempProducts on new { p.Article, p.ManufacturerID } equals + new { tp.Article, tp.ManufacturerID } + where p.DocumentID == null && tp.SessionID.Equals(sessionId) && tp.DocumentID.Equals(headerid) + select p; + } + return data; + } + + // The correct fix: MATERIALIZE inside the `using` (a naive `using` would + // dispose before the deferred query executes). `using var` disposes on all + // paths -> the flow detector skips it -> silent. + public List GetProductsFromCatalogWODocumentsFixed(DocHeader header) + { + using var uowFixed = new UnitOfWork(true); + IQueryable productCatalogs = + uowFixed.ProductCatalogs.Where(p => p.DocumentID == null); + return productCatalogs.ToList(); // materialized before dispose + } + + // Option B — ownership TRANSFERRED to the caller (the composable fix): the + // unit of work is `new`'d here but RETURNED, so the caller keeps the context + // alive across the deferred query's enumeration and disposes it itself: + // using var uow = svc.CreateUnitOfWork(); + // foreach (var p in svc.QueryProducts(uow, header)) { ... } // ctx alive + // The bare `uowOwned` escapes via `return`, so the flow detector does not + // track it -> silent. Unlike the materialize fix, the IQueryable stays + // composable (the caller can still add .Where()/paging translated to SQL). + public IUnitOfWork CreateUnitOfWork() + { + var uowOwned = new UnitOfWork(true); + return uowOwned; + } + + // Option B, the argument form: the unit of work is `new`'d here but handed to + // a callee that takes ownership (disposal becomes the callee's contract). The + // bare `uowMoved` escapes via the ARGUMENT, so it is not tracked -> silent. + public void ImportProducts(DocHeader header) + { + var uowMoved = new UnitOfWork(true); + ConsumeUnitOfWork(uowMoved); + } + + private static void ConsumeUnitOfWork(IUnitOfWork uow) => uow.Dispose(); + } + + // --- Minimal stand-ins for the GTM domain types: self-contained so the sample + // compiles standalone and `new UnitOfWork(...)` binds to an IDisposable. --- + + public interface IUnitOfWork : IDisposable + { + IQueryable ProductCatalogs { get; } + IQueryable TempProducts { get; } + IGenericRepository GenericRepository(); + } + + // Owns a DbContext-like resource, so Dispose() is meaningful (NOT dispose- + // optional) — which is what makes a leaked local matter. + public sealed class UnitOfWork : IUnitOfWork + { + public UnitOfWork(bool isCatalogs) { } + public IQueryable ProductCatalogs => Enumerable.Empty().AsQueryable(); + public IQueryable TempProducts => Enumerable.Empty().AsQueryable(); + public IGenericRepository GenericRepository() => new GenericRepository(); + public void Dispose() { } + } + + public interface IGenericRepository + { + IQueryable GetAll(); + } + + public sealed class GenericRepository : IGenericRepository + { + public IQueryable GetAll() => Enumerable.Empty().AsQueryable(); + } + + public class ProductCatalog + { + public int ID { get; set; } + public int? DocumentID { get; set; } + public string Article { get; set; } = ""; + public int ManufacturerID { get; set; } + } + + public class ProductDocuments + { + public int ID { get; set; } + public int ProductID { get; set; } + } + + public class TempProduct + { + public string Article { get; set; } = ""; + public int ManufacturerID { get; set; } + public Guid SessionID { get; set; } + public Guid DocumentID { get; set; } + } + + public class DocHeader + { + public Guid ID { get; set; } + } +} diff --git a/ownlang/ownir.py b/ownlang/ownir.py index 6d224eff..b50fee93 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -77,12 +77,15 @@ from .ast_nodes import ( Acquire, FnDecl, + If, Let, Module, Release, ResourceDecl, ResourceMember, + Return, Stmt, + Use, ) from .di import LIFETIMES as DI_LIFETIMES from .di import Service, find_captive_dependencies @@ -261,6 +264,11 @@ def load(path: str) -> dict[str, Any]: ln = s.get("line", 0) if not isinstance(ln, int) or isinstance(ln, bool): raise OwnIRError("service 'line' must be an integer") + # Optional per-method flow bodies (P-016 B0b/B2 — local IDisposable + # acquire/use/release over a CFG). Additive/optional; an older core ignores it. + fns = result.get("functions", []) + if not isinstance(fns, list) or not all(isinstance(f, dict) for f in fns): + raise OwnIRError("OwnIR 'functions' must be a JSON array of objects") return result @@ -371,11 +379,101 @@ def to_module(facts: dict[str, Any]) -> tuple[Module, dict[str, dict[str, Any]]] if sub.get("released"): body.append(Release(handle, line)) functions.append(FnDecl(cname, [], None, body, 0)) + # P-016 B0b/B2: per-method flow bodies for local IDisposables (acquire / use / + # release / if / return over a CFG). The core checks them path-sensitively + # (OWN001 not-released-on-all-paths, OWN002 use-after-release, OWN003 double- + # release) — beyond the flat detectors. Experimental/additive at v0; graduation + # bumps OWNIR_VERSION. + loc = [0] + raw_fns = facts.get("functions", []) + if isinstance(raw_fns, list): + for fn in raw_fns: + if not isinstance(fn, dict): + continue + fname = str(fn.get("name", f"Fn{loc[0]}")) + ffile = str(fn.get("file", "?")) + nodes = fn.get("body", []) + nodes = nodes if isinstance(nodes, list) else [] + # which locals have ANY release in the body (any branch) — lets the OWN001 + # wording distinguish "never disposed" (no release at all) from "not + # disposed on every path" (released on some branch, leaked on another). + released = _released_vars(nodes) + fbody = _lower_flow(nodes, ffile, fname, handles, loc, {}, released) + functions.append(FnDecl(fname, [], None, fbody, 0)) return (Module(str(facts.get("module", "Extracted")), resources=_prelude_resources(), functions=functions), handles) +def _released_vars(nodes: list[Any]) -> set[str]: + """The set of local names with at least one `release` op anywhere in a flow body + (recursing into `if` branches). Used to word an OWN001 as "never disposed" (the + name is absent here, so it was released on no path) vs "not on every path" (the + name is present, but the core still found a leaking path).""" + out: set[str] = set() + for n in nodes: + if not isinstance(n, dict): + continue + op = n.get("op") + if op == "release": + v = n.get("var") + if isinstance(v, str): + out.add(v) + elif op == "if": + for branch in ("then", "else"): + b = n.get(branch, []) + if isinstance(b, list): + out |= _released_vars(b) + return out + + +def _lower_flow(nodes: list[Any], ffile: str, fname: str, + handles: dict[str, dict[str, Any]], loc: list[int], + localmap: dict[str, str], + released_vars: set[str]) -> list[Stmt]: + """Lower one OwnIR flow body (B0b/B2) into core statements. acquire/use/release/ + return reference a C# local by name (`var`); `if` carries `then`/`else` + sub-bodies. Each acquire gets a globally-unique handle `loc_` (so a finding + maps back to the C# local); `localmap` resolves later references within the same + function and its branches.""" + body: list[Stmt] = [] + for n in nodes: + if not isinstance(n, dict): + continue + op = n.get("op") + line = _as_int(n.get("line", 0)) + if op == "acquire": + handle = f"loc_{loc[0]}" + loc[0] += 1 + name = str(n.get("var", "?")) + localmap[name] = handle + handles[handle] = {"file": ffile, "line": line, "event": name, + "component": fname, "resource": "flow-local", + "ever_released": name in released_vars} + body.append(Let(handle, Acquire("Disposable", [], line), line)) + elif op == "use": + h = localmap.get(str(n.get("var"))) + if h is not None: + body.append(Use(h, line)) + elif op == "release": + h = localmap.get(str(n.get("var"))) + if h is not None: + body.append(Release(h, line)) + elif op == "return": + v = n.get("var") + h = localmap.get(str(v)) if v is not None else None + body.append(Return(h, line)) + elif op == "if": + tn = n.get("then", []) + en = n.get("else", []) + then_b = _lower_flow(tn if isinstance(tn, list) else [], + ffile, fname, handles, loc, localmap, released_vars) + else_b = _lower_flow(en if isinstance(en, list) else [], + ffile, fname, handles, loc, localmap, released_vars) + body.append(If("?", then_b, else_b, line)) + return body + + def _handle_of(diag: object) -> str | None: """The synthetic handle (`sub_N`) a diagnostic is about, recovered from its structured `subject` (`name#line`) — NOT by scraping the human message. Each @@ -423,6 +521,30 @@ def check_facts(facts: dict[str, Any]) -> list[Finding]: handler = sub.get("handler", "?") component = sub["component"] rkind = sub.get("resource", "subscription") + if rkind == "flow-local": + # P-016 B0b/B2: path-sensitive local-IDisposable verdicts. The code is + # the core's (OWN001/002/003/009); phrase it for the C# local. + name = event + 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)") + else: + msg = { + "OWN002": f"IDisposable local '{name}' is used after it is disposed", + "OWN003": f"IDisposable local '{name}' is disposed more than once", + "OWN009": f"IDisposable local '{name}' may be used after disposal on some path", + }.get(d.code, f"IDisposable local '{name}': {d.message}") + findings.append(Finding( + file=sub["file"], line=int(sub.get("line", 0)), code=d.code, + component=component, event=name, handler="", message=msg, + kind="disposable")) + continue _, kind = _RESOURCES.get(rkind, _RESOURCES["subscription"]) if rkind == "timer": message = (f"timer '{event}' (handler '{handler}') is started but " diff --git a/scripts/own-check.ps1 b/scripts/own-check.ps1 index 54b1dff6..544674e2 100644 --- a/scripts/own-check.ps1 +++ b/scripts/own-check.ps1 @@ -26,6 +26,13 @@ analysis skipped" notes, P-014 Tier A), normal (default), or verbose (also a per-code breakdown). +.PARAMETER Legacy + Use the legacy flat local-IDisposable detector instead of the default + path-sensitive flow analysis (--flow-locals). The flow analysis is more precise + (no Task/DataTable false positives; catches use-after-dispose / double-dispose / + leak-on-a-path, and any IDisposable type) but honestly skips methods with loops / + try until P-016 A1 lands. -Legacy is the broad, name-based fallback. + .PARAMETER FailOnFinding Exit non-zero (the core's code) when any leak is found. @@ -44,6 +51,7 @@ param( [string]$Severity = "error", [ValidateSet("quiet", "normal", "verbose")] [string]$Verbosity = "normal", + [switch]$Legacy, [switch]$FailOnFinding, [Parameter(ValueFromRemainingArguments = $true)] [string[]]$Paths @@ -65,8 +73,11 @@ $facts = New-TemporaryFile try { # Stage 1: extract facts. dotnet's build chatter is sent to the host (not # stdout) so stdout stays clean for the host-parseable findings; -o writes - # the facts to a file. - & dotnet run --project $extractor -- @Paths -o $facts.FullName 1>$null + # the facts to a file. Default: the path-sensitive flow detector for local + # IDisposables (--flow-locals); -Legacy keeps the flat name-based detector. + $exArgs = @($Paths) + @("-o", $facts.FullName) + if (-not $Legacy) { $exArgs += "--flow-locals" } + & dotnet run --project $extractor -- @exArgs 1>$null if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } # Stage 2: the one checker produces the verdict at the C# location. diff --git a/scripts/own-check.sh b/scripts/own-check.sh index c392073a..55369c82 100755 --- a/scripts/own-check.sh +++ b/scripts/own-check.sh @@ -12,7 +12,7 @@ # # Usage: # scripts/own-check.sh [--format human|github|msbuild] [--severity error|warning] -# [--fail-on-finding] [--root ] +# [--fail-on-finding] [--legacy] [--root ] # [--] [more ...] # # Defaults: --format human, --severity error, scans ".", does not fail the shell @@ -20,6 +20,12 @@ # host shows findings (warning = advisory). With --fail-on-finding the exit code # is the core's (1 = leaks found). A hard error (bad facts) always exits non-zero. # +# Local IDisposables are checked by default with the path-sensitive flow analysis +# (--flow-locals): more precise (no Task/DataTable false positives; catches +# use-after-dispose / double-dispose / leak-on-a-path, any IDisposable type) but it +# honestly skips methods with loops / try until P-016 A1 lands. --legacy falls back +# to the broad, name-based flat detector. +# # Requirements: a .NET SDK (`dotnet`) and Python 3.11+ on PATH. set -euo pipefail @@ -28,6 +34,7 @@ root="" format="human" severity="error" fail_on_finding=0 +legacy=0 paths=() while [[ $# -gt 0 ]]; do @@ -42,6 +49,7 @@ while [[ $# -gt 0 ]]; do [[ $# -ge 2 ]] || { echo "own-check: --severity requires a value" >&2; exit 2; } severity="$2"; shift 2 ;; --fail-on-finding) fail_on_finding=1; shift ;; + --legacy) legacy=1; shift ;; --) shift; while [[ $# -gt 0 ]]; do paths+=("$1"); shift; done ;; -h|--help) sed -n '2,30p' "$0"; exit 0 ;; *) paths+=("$1"); shift ;; @@ -62,7 +70,11 @@ trap 'rm -f "$facts"' EXIT # Stage 1: extract facts. dotnet's build/run chatter goes to stderr so stdout # stays clean for the host-parseable findings (-o writes the facts to a file). -dotnet run --project "$extractor" -- "${paths[@]}" -o "$facts" 1>&2 +# Default: the path-sensitive flow detector for local IDisposables (--flow-locals); +# --legacy keeps the flat name-based detector. +extractor_args=("${paths[@]}" -o "$facts") +[[ "$legacy" -eq 0 ]] && extractor_args+=(--flow-locals) +dotnet run --project "$extractor" -- "${extractor_args[@]}" 1>&2 # Stage 2: the one checker produces the verdict at the C# location. set +e diff --git a/tests/fixtures/ownir/flow_leak_on_else.facts.json b/tests/fixtures/ownir/flow_leak_on_else.facts.json new file mode 100644 index 00000000..e67c533c --- /dev/null +++ b/tests/fixtures/ownir/flow_leak_on_else.facts.json @@ -0,0 +1,17 @@ +{ + "ownir_version": 0, + "module": "Extracted", + "components": [], + "functions": [ + { + "name": "FlowLocalsSample.LeakOnElse", + "file": "FlowLocalsSample.cs", + "body": [ + {"op": "acquire", "var": "leak", "line": 23}, + {"op": "if", "line": 24, "then": [ + {"op": "release", "var": "leak", "line": 26} + ], "else": []} + ] + } + ] +} diff --git a/tests/fixtures/ownir/unitofwork_flow.facts.json b/tests/fixtures/ownir/unitofwork_flow.facts.json new file mode 100644 index 00000000..2a79f5d7 --- /dev/null +++ b/tests/fixtures/ownir/unitofwork_flow.facts.json @@ -0,0 +1,18 @@ +{ + "ownir_version": 0, + "module": "Extracted", + "components": [], + "functions": [ + { + "name": "CatalogService.GetProductsFromCatalogWODocuments", + "file": "UnitOfWorkFlowSample.cs", + "body": [ + {"op": "acquire", "var": "uow", "line": 26}, + {"op": "if", "line": 35, "then": [], "else": [ + {"op": "use", "var": "uow", "line": 42} + ]}, + {"op": "return", "var": null, "line": 48} + ] + } + ] +} diff --git a/tests/test_ownir.py b/tests/test_ownir.py index 1d0ce382..7438b1b2 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -48,6 +48,10 @@ "ownir", "pool.facts.json") _LOCAL_FIXTURE = os.path.join(os.path.dirname(__file__), "fixtures", "ownir", "local_disposable.facts.json") +_UOW_FLOW_FIXTURE = os.path.join(os.path.dirname(__file__), "fixtures", + "ownir", "unitofwork_flow.facts.json") +_LEAK_ON_ELSE_FIXTURE = os.path.join(os.path.dirname(__file__), "fixtures", + "ownir", "flow_leak_on_else.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", @@ -238,6 +242,62 @@ def run() -> int: if "[resource: disposable]" not in l0.render(): fails.append(f"local finding missing kind tag: {l0.render()!r}") + # --- P-016 B0b/B2 escape-via-projection (the GTM UnitOfWork case): a local + # IDisposable created and captured ONLY through member access into a returned + # DEFERRED query is still a leak — the bare handle never escapes, so the flow + # detector keeps it tracked and it is disposed on no path -> OWN001 on the + # local. Distilled from CatalogService.GetProductsFromCatalogWODocuments; pins + # that the projection capture does NOT mask the leak (a naive `using` cannot + # fix it, so the find must survive). `uow` is released on no path, so the + # OWN001 reads "is never disposed" (not the partial-path wording). Tag + # [resource: disposable]. + with open(_UOW_FLOW_FIXTURE, encoding="utf-8") as f: + wfacts = json.load(f) + wfindings = check_facts(wfacts) + checks += 1 + if len(wfindings) != 1 or wfindings[0].code != "OWN001": + fails.append(f"expected 1 flow OWN001 (UnitOfWork 'uow'), got " + f"{[(x.event, x.code) for x in wfindings]}") + else: + w0 = wfindings[0] + checks += 1 + if (w0.file, w0.line, w0.code) != ("UnitOfWorkFlowSample.cs", 26, "OWN001"): + fails.append(f"wrong uow location/code: {w0.file}:{w0.line} {w0.code}") + if w0.event != "uow" or \ + w0.component != "CatalogService.GetProductsFromCatalogWODocuments": + fails.append(f"wrong uow local/component: {w0.event!r}/{w0.component!r}") + if "is never disposed" not in w0.message or "uow" not in w0.message: + fails.append(f"uow message wrong (want 'is never disposed'): {w0.message!r}") + # a 0-release leak must NOT borrow the partial-path wording. + if "every path" in w0.message: + fails.append(f"uow (0 releases) wrongly used the partial-path wording: " + f"{w0.message!r}") + if "[resource: disposable]" not in w0.render(): + fails.append(f"uow finding missing kind tag: {w0.render()!r}") + + # --- the other OWN001 wording: a local released on SOME branch but leaked on + # another (the LeakOnElse shape) reads "may not be disposed on every path", + # NOT "never disposed" — the everReleased split (extractor flow body has a + # release of this local somewhere) chooses between the two phrasings. + with open(_LEAK_ON_ELSE_FIXTURE, encoding="utf-8") as f: + efacts = json.load(f) + efindings = check_facts(efacts) + checks += 1 + if len(efindings) != 1 or efindings[0].code != "OWN001": + fails.append(f"expected 1 flow OWN001 (LeakOnElse 'leak'), got " + f"{[(x.event, x.code) for x in efindings]}") + else: + e0 = efindings[0] + checks += 1 + if (e0.file, e0.line) != ("FlowLocalsSample.cs", 23): + fails.append(f"wrong leak-on-else location: {e0.file}:{e0.line}") + if "may not be disposed on every path" not in e0.message: + fails.append(f"leak-on-else message wrong (want partial-path): " + f"{e0.message!r}") + if "is never disposed" in e0.message: + fails.append(f"partial-release leak wrongly used the never-disposed " + f"wording: {e0.message!r}") + # --- DI001 captive dependency (P-006): a singleton capturing a scoped # service (directly or through a transient) is flagged at its # registration site; safe registrations stay silent.