From 22d9eaf9329d951b94a743cb08fe36f6ad3cb79e Mon Sep 17 00:00:00 2001 From: Bonobo Date: Wed, 2 Sep 2026 13:51:12 +0200 Subject: [PATCH] fix(dataflow): route abrupt completions through finally --- src/dataflow/cfg.ts | 104 +++++++++++++++++++------ test/dataflow.test.ts | 39 ++++++++++ test/fixtures/dataflow-app/src/flow.ts | 23 ++++++ 3 files changed, 141 insertions(+), 25 deletions(-) diff --git a/src/dataflow/cfg.ts b/src/dataflow/cfg.ts index a8200d0..7c14064 100644 --- a/src/dataflow/cfg.ts +++ b/src/dataflow/cfg.ts @@ -48,14 +48,29 @@ interface Lowered { exits: Dangling[]; } +type CompletionTarget = + | { type: "edge"; target: number; kind: CfgEdgeKind } + | { type: "sink"; sink: Dangling[]; kind: CfgEdgeKind }; + +interface FinallyRouter { + entry: number; + continuations: CompletionTarget[]; + abruptParent: FinallyRouter | null; + exceptionParent: FinallyRouter | null; +} + interface LoopLabel { breaks: Dangling[]; continueHeader: number | null; } interface LowerCtx { - /** Nearest enclosing handler node (catch node / finally entry) or EXIT. */ + /** Nearest enclosing handler node or EXIT when no finally interception is required. */ exceptionTarget: number; + /** Abrupt completions entering an enclosing finally before reaching their real destination. */ + finallyRouter: FinallyRouter | null; + /** Exceptional completions use a separate router because a try's catch runs before finally. */ + exceptionFinallyRouter: FinallyRouter | null; /** Break/continue sinks of the nearest enclosing loop/switch. */ nearestBreaks: Dangling[] | null; nearestContinueHeader: number | null; @@ -97,6 +112,8 @@ export function buildCfg(signature: string, fn: Node): FunctionCfgBuild | null { const lower = new Lowerer(idOf, addEdge, exitId); const ctx: LowerCtx = { exceptionTarget: exitId, + finallyRouter: null, + exceptionFinallyRouter: null, nearestBreaks: null, nearestContinueHeader: null, labels: new Map(), @@ -217,26 +234,32 @@ class Lowerer { private leaf(s: Node, ctx: LowerCtx): Lowered { const id = this.idOf.get(s) as number; - this.exceptionEdgeIfThrows(s, id, ctx); + if (!Node.isThrowStatement(s)) this.exceptionEdgeIfThrows(s, id, ctx); if (Node.isReturnStatement(s)) { - this.addEdge(id, this.exitId, "return"); + this.complete(id, { type: "edge", target: this.exitId, kind: "return" }, ctx.finallyRouter); return { entry: id, exits: [] }; } if (Node.isThrowStatement(s)) { - this.addEdge(id, ctx.exceptionTarget, "exception"); + this.complete( + id, + { type: "edge", target: ctx.exceptionTarget, kind: "exception" }, + ctx.exceptionFinallyRouter, + ); return { entry: id, exits: [] }; } if (Node.isBreakStatement(s)) { const lbl = s.getLabel()?.getText(); const sink = lbl ? ctx.labels.get(lbl)?.breaks : ctx.nearestBreaks; - sink?.push({ from: id, kind: "break" }); + if (sink) this.complete(id, { type: "sink", sink, kind: "break" }, ctx.finallyRouter); return { entry: id, exits: [] }; } if (Node.isContinueStatement(s)) { const lbl = s.getLabel()?.getText(); const header = lbl ? (ctx.labels.get(lbl)?.continueHeader ?? null) : ctx.nearestContinueHeader; - if (header !== null) this.addEdge(id, header, "continue"); + if (header !== null) { + this.complete(id, { type: "edge", target: header, kind: "continue" }, ctx.finallyRouter); + } return { entry: id, exits: [] }; } // Plain statement: the outgoing normal edge carries the suspend/resume kind when the @@ -362,24 +385,27 @@ class Lowerer { const cc = s.getCatchClause(); const fin = s.getFinallyBlock(); - // Lower the finally region first so try/catch know their exceptional continuation. - let finLowered: Lowered | null = null; - if (fin) { - finLowered = this.statements(fin.getStatements(), ctx); - // A finally region may re-raise (it runs on the exceptional path too): over-approximate by - // edging every finally exit to the outer handler as well. - if (finLowered.entry !== null) { - for (const d of finLowered.exits) this.addEdge(d.from, ctx.exceptionTarget, "exception"); - } - } - const afterCatchTarget = finLowered?.entry ?? ctx.exceptionTarget; + // Lower the finally region first so try/catch can route every completion through its entry. + const finLowered = fin ? this.statements(fin.getStatements(), ctx) : null; + const router: FinallyRouter | null = finLowered?.entry != null + ? { + entry: finLowered.entry, + continuations: [], + abruptParent: ctx.finallyRouter, + exceptionParent: ctx.exceptionFinallyRouter, + } + : null; const exits: Dangling[] = []; let catchEntry: number | null = null; if (cc) { const catchId = this.idOf.get(cc) as number; // binds the exception variable (a def, stage 3) catchEntry = catchId; - const catchCtx: LowerCtx = { ...ctx, exceptionTarget: afterCatchTarget }; + const catchCtx: LowerCtx = { + ...ctx, + finallyRouter: router, + exceptionFinallyRouter: router, + }; const catchBody = this.statements(cc.getBlock().getStatements(), catchCtx); if (catchBody.entry !== null) { this.addEdge(catchId, catchBody.entry, "fallthrough"); @@ -389,16 +415,18 @@ class Lowerer { } } - const tryCtx: LowerCtx = { ...ctx, exceptionTarget: catchEntry ?? afterCatchTarget }; + const tryCtx: LowerCtx = { + ...ctx, + exceptionTarget: catchEntry ?? ctx.exceptionTarget, + finallyRouter: router, + exceptionFinallyRouter: catchEntry === null ? router : null, + }; const tryBody = this.statements(s.getTryBlock().getStatements(), tryCtx); if (tryBody.entry !== null) this.routeThroughFinally(tryBody.exits, finLowered, exits); - else if (finLowered?.entry != null) this.routeThroughFinally([], finLowered, exits); + if (router && finLowered) this.flushFinally(router, finLowered.exits); const entry = tryBody.entry ?? catchEntry ?? finLowered?.entry ?? null; - if (tryBody.entry === null && finLowered?.entry !== null && finLowered) { - // Empty try block: control passes straight to finally. - exits.push(...finLowered.exits); - } + if (tryBody.entry === null && finLowered?.entry != null) exits.push(...finLowered.exits); return { entry, exits }; } @@ -412,9 +440,35 @@ class Lowerer { } } + private complete(from: number, target: CompletionTarget, router: FinallyRouter | null): void { + if (router) { + this.addEdge(from, router.entry, target.kind); + router.continuations.push(target); + return; + } + this.deliver(from, target); + } + + private flushFinally(router: FinallyRouter, exits: Dangling[]): void { + for (const completion of router.continuations) { + const parent = completion.kind === "exception" ? router.exceptionParent : router.abruptParent; + for (const exit of exits) this.complete(exit.from, completion, parent); + } + } + + private deliver(from: number, target: CompletionTarget): void { + if (target.type === "edge") this.addEdge(from, target.target, target.kind); + else target.sink.push({ from, kind: target.kind }); + } + /** Over-approximate exceptional flow: calls / new / await / tagged templates may throw. */ exceptionEdgeIfThrows(expr: Node, nodeId: number, ctx: LowerCtx): void { - if (mayThrow(expr)) this.addEdge(nodeId, ctx.exceptionTarget, "exception"); + if (!mayThrow(expr)) return; + this.complete( + nodeId, + { type: "edge", target: ctx.exceptionTarget, kind: "exception" }, + ctx.exceptionFinallyRouter, + ); } } diff --git a/test/dataflow.test.ts b/test/dataflow.test.ts index 7e5a686..39c7d4d 100644 --- a/test/dataflow.test.ts +++ b/test/dataflow.test.ts @@ -51,6 +51,16 @@ async function run(level: 1 | 2 | 3, jobs = 1): Promise node.kind === "statement" && flowSource.slice(node.start_offset, node.end_offset).includes(text)) + .sort((a, b) => (a.end_offset - a.start_offset) - (b.end_offset - b.start_offset) || a.id - b.id); + const node = nodes[occurrence]; + if (!node) throw new Error(`no ${signature} node containing ${text}`); + return node.id; +} const cfgOf = (sig: string): FunctionCfg => { const g = pg.functions[sig]?.cfg; if (!g) throw new Error(`no cfg for ${sig}`); @@ -137,6 +147,35 @@ describe("CFG gate", () => { expect(e).toContainEqual({ source: 6, target: 8, kind: "exception" }); // finally → outward (EXIT) }); + test("return, throw, break, and continue complete only after finally", () => { + const signature = "src/flow.abruptFinally"; + const edges = kinds(signature); + const finallyNode = nodeContaining(signature, "seen += 1"); + const abrupt = [ + ["continue outer", "continue"], + ["break outer", "break"], + ["return seen", "return"], + ['throw new Error("abrupt")', "exception"], + ] as const; + for (const [text, kind] of abrupt) { + const source = nodeContaining(signature, text); + expect(edges).toContainEqual({ source, target: finallyNode, kind }); + } + + const continuations = edges.filter((edge) => edge.source === finallyNode).map((edge) => edge.kind); + expect(continuations).toEqual(expect.arrayContaining(["continue", "break", "return", "exception"])); + }); + + test("an abrupt completion in finally overrides the pending return", () => { + const signature = "src/flow.overrideFinally"; + const cfg = cfgOf(signature); + const pending = nodeContaining(signature, "return 1"); + const overriding = nodeContaining(signature, "return 2"); + expect(cfg.edges).toContainEqual({ source: pending, target: overriding, kind: "return" }); + expect(cfg.edges).toContainEqual({ source: overriding, target: cfg.nodes.length - 1, kind: "return" }); + expect(cfg.edges).not.toContainEqual({ source: pending, target: cfg.nodes.length - 1, kind: "return" }); + }); + test("throw with no handler edges to EXIT (parse)", () => { const cfg = cfgOf("src/flow.parse"); const exit = cfg.nodes.length - 1; diff --git a/test/fixtures/dataflow-app/src/flow.ts b/test/fixtures/dataflow-app/src/flow.ts index b57b19b..bf89055 100644 --- a/test/fixtures/dataflow-app/src/flow.ts +++ b/test/fixtures/dataflow-app/src/flow.ts @@ -84,3 +84,26 @@ export function shadow(): number { } return x; } + +export function abruptFinally(values: number[]): number { + let seen = 0; + outer: for (const value of values) { + try { + if (value < 0) continue outer; + if (value === 0) break outer; + if (value === 1) return seen; + throw new Error("abrupt"); + } finally { + seen += 1; + } + } + return seen; +} + +export function overrideFinally(): number { + try { + return 1; + } finally { + return 2; + } +}