Skip to content

Commit fac948a

Browse files
committed
fix(observability-map): refuse catch evidence from a try a finally cancels
A finally that leaves itself by break or continue cancels the try's completion, so a throw or classifier in that tryBlock never escapes the clause. The catchless-try walk entry credited it anyway: prepending do { try { if (e instanceof Error) { throw e; } } finally { break; } } while (false); to every catch raised 80 routes and took the global from 19 to 27. Entry now requires the finally to contain no escaping jump (containment, since entry grants credit), and containsLiveWhere folds a try dead when its finally provably completes abruptly, so the refused statement cannot blind the classification after it either. dead-throw-in-cancelled-try in the mutation corpus is the tree-scale guard; the unit pins cover the break, continue, switch-hosted and may-break spellings and the no-blinding identity.
1 parent 184f441 commit fac948a

3 files changed

Lines changed: 140 additions & 8 deletions

File tree

internal-packages/observability-map/src/mutations.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -720,6 +720,20 @@ export const MUTATIONS: Mutation[] = [
720720
(e) =>
721721
`if (false) { if (${e} instanceof Error) { return new Response(null, { status: 400 }); } } else { 0; }`
722722
),
723+
// A finally that leaves itself by `break` cancels the try's completion, so nothing hosted in
724+
// that tryBlock ever escapes the clause: the whole statement is a no-op. The walk's
725+
// catchless-try entry credited it anyway, minting a branch from the hosted classifier on 80
726+
// routes and 8 global points when measured. Additive: it plants fake signal. The classifier is
727+
// guarded (`if (e instanceof Error)`) rather than a bare `throw e` so `definitelyExits` cannot
728+
// read the statement as an unconditional exit; the bare spelling trips a separate, pre-existing
729+
// over-cut in `definitelyExits`'s try/finally case that this entry is not about.
730+
prependToEveryCatch(
731+
"dead-throw-in-cancelled-try",
732+
"preserving",
733+
"splice a finally-break try that discards its own throw into every catch",
734+
(e) =>
735+
`do { try { if (${e} instanceof Error) { throw ${e}; } } finally { break; } } while (false);`
736+
),
723737

724738
// The additive class. Everything above either takes signal away or moves it about; these put in
725739
// signal that is not real, which is the direction the corpus was blind to.
@@ -951,6 +965,7 @@ export const ADDITIVE_IDS = [
951965
"wrap-body-in-same-arms-throw-ternary",
952966
"empty-instanceof-if",
953967
"dead-classifier-one-arm",
968+
"dead-throw-in-cancelled-try",
954969
"dead-deciding-map",
955970
"registered-throw",
956971
"fake-require-guard",

internal-packages/observability-map/src/scan.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -960,6 +960,58 @@ describe("scanFile: catch clause evidence", () => {
960960
expect(evidence.rethrows).toBe(false);
961961
});
962962

963+
// A finally that leaves itself by `break` or `continue` cancels the try's completion the same
964+
// way a finally return does, so a throw in that tryBlock never escapes the clause. Crediting
965+
// it made `do { try { throw e; } finally { break; } } while (false);` a no-op that minted
966+
// rethrows, and its classifier-hosting variant minted branches on 80 real routes;
967+
// `dead-throw-in-cancelled-try` in the mutation corpus is the tree-scale twin.
968+
it("reads a throw a finally break discards as no rethrow", () => {
969+
const evidence = clauseEvidence(
970+
"do { try { throw e; } finally { break; } } while (false);\nlogger.error(e);"
971+
);
972+
expect(evidence).toMatchObject({ rethrows: false, throws: false, branches: false });
973+
});
974+
975+
it("reads a throw a finally continue discards as no rethrow", () => {
976+
const evidence = clauseEvidence(
977+
'do { try { throw e; } finally { continue; } } while (false);\nlogger.error("x", { e });'
978+
);
979+
expect(evidence).toMatchObject({ rethrows: false, throws: false, branches: false });
980+
});
981+
982+
it("reads a throw a switch-hosted finally break discards as no rethrow", () => {
983+
const evidence = clauseEvidence(
984+
"switch (0) { default: try { throw e; } finally { break; } }\nlogger.error(e);"
985+
);
986+
expect(evidence).toMatchObject({ rethrows: false, throws: false, branches: false });
987+
});
988+
989+
// The refusal is a containment read: a jump that only MAY run still cancels entry, because
990+
// entry grants credit and a wrong grant pays.
991+
it("refuses the tryBlock when the finally only may break", () => {
992+
const evidence = clauseEvidence(
993+
"do { try { throw e; } finally { if (pick()) { break; } } } while (false);\nlogger.error(e);"
994+
);
995+
expect(evidence).toMatchObject({ rethrows: false, throws: false });
996+
});
997+
998+
// A loop inside the finally captures its own bare jumps, so nothing there leaves the finally
999+
// and the try's completion stands: the rethrow is genuine.
1000+
it("does not refuse a finally whose loop captures its own break", () => {
1001+
const evidence = clauseEvidence("try { throw e; } finally { while (pick()) { break; } }");
1002+
expect(evidence).toMatchObject({ rethrows: true, throws: true });
1003+
});
1004+
1005+
// The cancelled statement contributes nothing, in either direction: no credit from inside it,
1006+
// and no blinding of the real classification after it. Same evidence as the bare clause.
1007+
it("keeps the classification after a finally-break no-op", () => {
1008+
expect(
1009+
clauseEvidence(
1010+
`do { try { if (e instanceof Error) { throw e; } } finally { break; } } while (false);\n${DECIDING}`
1011+
)
1012+
).toEqual(clauseEvidence(DECIDING));
1013+
});
1014+
9631015
// The `definitelyExits` fold: `if (true) { X }` definitely exits iff X does, so the trailing
9641016
// throw is cut rather than read. Without the fold the throw still walks and mints `throws`.
9651017
it("cuts a dead trailing statement after an if true that exits", () => {

internal-packages/observability-map/src/scan.ts

Lines changed: 73 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,43 @@ function reachableStatements(statements: readonly ts.Statement[]): readonly ts.S
571571
return index === -1 ? statements : statements.slice(0, index + 1);
572572
}
573573

574+
/**
575+
* Whether the tree rooted at `node` contains a `break` or `continue` that would leave it, i.e. a
576+
* jump no construct INSIDE `node` captures. What the catch walk asks of a finally block before
577+
* entering the tryBlock beside it: a finally that completes abruptly cancels the try's completion,
578+
* so a throw in that tryBlock never leaves the clause and crediting it minted evidence
579+
* (`reads a throw a finally break discards as no rethrow` and its continue and switch-hosted
580+
* siblings pin the refusal; `dead-throw-in-cancelled-try` in the mutation corpus is the tree-scale
581+
* shape, 80 routes when measured).
582+
*
583+
* A containment read, not a liveness one, on purpose: the caller is deciding whether to GRANT
584+
* credit and a wrong grant pays, so a jump that only may run still refuses
585+
* (`refuses the tryBlock when the finally only may break`). Two over-approximations in the same
586+
* direction: a labelled jump always counts, even when its label sits inside `node`, and a `return`
587+
* is not looked for here because the returns veto already reads it off the whole statement.
588+
*/
589+
function containsEscapingJump(node: ts.Node, jumps: BareJumps = ESCAPES): boolean {
590+
if (ts.isFunctionLike(node)) return false;
591+
if (ts.isBreakStatement(node)) return node.label !== undefined || jumps.break;
592+
if (ts.isContinueStatement(node)) return node.label !== undefined || jumps.continue;
593+
if (ts.isSwitchStatement(node)) {
594+
const inClause: BareJumps = { break: false, continue: jumps.continue };
595+
return node.caseBlock.clauses.some((c) =>
596+
c.statements.some((s) => containsEscapingJump(s, inClause))
597+
);
598+
}
599+
// Any loop captures both bare jumps, so nothing inside one can leave `node`
600+
// (`does not refuse a finally whose loop captures its own break`).
601+
if (ts.isIterationStatement(node, false)) {
602+
return (
603+
ts.forEachChild(node, (child) =>
604+
containsEscapingJump(child, { break: false, continue: false })
605+
) === true
606+
);
607+
}
608+
return ts.forEachChild(node, (child) => containsEscapingJump(child, jumps)) === true;
609+
}
610+
574611
/** Whether the tree rooted at `node` contains a `return` or a `throw` of its own, not counting one
575612
* inside a nested function. What separates an arm that takes the error somewhere from an arm that
576613
* runs and falls back into the clause's single common exit. */
@@ -692,6 +729,16 @@ function containsLiveWhere(root: ts.Node, hit: (n: ts.Node) => boolean): boolean
692729
return clauses.slice(matched).some((c) => c.statements.some(walk));
693730
}
694731
if (ts.isTryStatement(node)) {
732+
// A finally that always completes abruptly (a return, a throw, or a jump out of the block)
733+
// supersedes the try's and the catch's completion: an exit written in either never leaves
734+
// the statement, so only the finally's own statements stay live. Folded only when
735+
// `definitelyExits` can prove it; a conditional jump keeps the containment answer, the
736+
// direction that refuses credit rather than inventing it. Without this fold the
737+
// `dead-throw-in-cancelled-try` prepend blinded the walk to every real classification below
738+
// it (`keeps the classification after a finally-break no-op`).
739+
if (node.finallyBlock !== undefined && definitelyExits(node.finallyBlock)) {
740+
return walk(node.finallyBlock);
741+
}
695742
if (walk(node.tryBlock)) return true;
696743
if (node.finallyBlock !== undefined && walk(node.finallyBlock)) return true;
697744
if (node.catchClause !== undefined && tryBlockMayThrow(node.tryBlock)) {
@@ -749,7 +796,11 @@ function selectsADistinctPath(statement: ts.IfStatement | ts.SwitchStatement): b
749796
* enter a construct exactly where the entered statements are guaranteed to execute whenever the
750797
* clause body runs, so no credit can ever come from code a semantics-preserving edit could have
751798
* added dead. Entered on those terms: a bare nested block, a `do` body, the tryBlock of a `try`
752-
* that has NO catch clause, the sole clause of a single-DefaultClause `switch`, the then-arm of an
799+
* that has NO catch clause and whose finally (if any) contains no jump out of itself (a finally
800+
* that completes abruptly cancels the try's completion, so nothing in that tryBlock ever escapes
801+
* the clause; `reads a throw a finally break discards as no rethrow` and
802+
* `dead-throw-in-cancelled-try` in the mutation corpus hold it), the sole clause of a
803+
* single-DefaultClause `switch`, the then-arm of an
753804
* `if` whose condition is exactly the literal `true` keyword, and both arms of an `if`/`else` with
754805
* per-arm states merged by intersection (evidence in both arms is unconditional; evidence in one
755806
* is not). Each entry is pinned by `reads a clause wrapped in a single-default switch as the bare
@@ -878,13 +929,27 @@ function catchClauseEvidence(clause: ts.CatchClause): {
878929
// try whose finally returns as swallowing, not rethrowing` is the pin.
879930
//
880931
// A `try` WITHOUT a catch clause: its tryBlock always runs when the clause body does, and a
881-
// throw there escapes the clause, so rethrow credit is genuine. The finallyBlock is NOT
882-
// walked (classification living only in a finally block is under-credited; the tree has no
883-
// such clause). A `try` WITH a catch clause is not entered at all: a throw in that tryBlock
884-
// is intercepted by the nested catch, so crediting it would launder a returnless swallow
885-
// into not-applicable. `does not read the tryBlock of a caught try as this clause's rethrow`
886-
// is the pin, and the nested clause is judged separately as its own `ep.catches` entry.
887-
if (ts.isTryStatement(statement) && statement.catchClause === undefined) {
932+
// throw there escapes the clause, so rethrow credit is genuine — unless the finally can
933+
// complete abruptly. A `finally` holding a `return` is covered by the explicit
934+
// `containsLiveReturn` read below; a `finally` holding a `break` or `continue` that leaves
935+
// it cancels the try's completion the same way, so the throw never escapes and the tryBlock
936+
// must not be entered (`reads a throw a finally break discards as no rethrow`, its continue
937+
// and switch-hosted siblings, and `refuses the tryBlock when the finally only may break`;
938+
// `dead-throw-in-cancelled-try` in the mutation corpus is the tree-scale shape). The refusal
939+
// is a containment read and entry requires its absence, because entry GRANTS credit; the
940+
// matching liveness fold in `containsLiveWhere` then keeps the refused statement from
941+
// blinding what follows it (`keeps the classification after a finally-break no-op`). The
942+
// finallyBlock itself is NOT walked (classification living only in a finally block is
943+
// under-credited; the tree has no such clause). A `try` WITH a catch clause is not entered
944+
// at all: a throw in that tryBlock is intercepted by the nested catch, so crediting it would
945+
// launder a returnless swallow into not-applicable. `does not read the tryBlock of a caught
946+
// try as this clause's rethrow` is the pin, and the nested clause is judged separately as
947+
// its own `ep.catches` entry.
948+
if (
949+
ts.isTryStatement(statement) &&
950+
statement.catchClause === undefined &&
951+
(statement.finallyBlock === undefined || !containsEscapingJump(statement.finallyBlock))
952+
) {
888953
walk(statement.tryBlock.statements, state);
889954
if (state.vetoReturns && containsLiveReturn(statement)) returns = true;
890955
if (containsLiveExit(statement)) state.exited = true;

0 commit comments

Comments
 (0)