Skip to content

Commit 4c18e7f

Browse files
ASDAlexander77claude
ASDAlexander77
and
claude
authored
A temporary a finally reads is released after the finally (#650)
* rc: a temporary a finally reads is released after the finally `const v = mv(); try { if (c) continue; } finally { s += v.get(); }`: a folded const (a call's result named by a const) is released where its block ends, and in front of each break, continue or return that leaves the block early. That release ran before the finally the jump goes through, and the finally then read a freed block. #600 fixed the same for locals in slots; a folded const is released by releaseDiscardedTemporaries instead. When a finally between the jump and the temporary's block reads it, the release now goes at the end of the outermost such finally, in a ts.If tagged with the jump (JUMP_ONLY_ATTR_NAME, RETURN_ONLY_ATTR_NAME), so TryOpLowering keeps it in that jump's copy of the finally alone - the scheme MLIRGen uses for the scopes the jump leaves. Jumps with one key share one copy and get one release. Fuzzer (#646) R6, rc. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * rc: a jump through a finally that jumps itself releases a temporary once `for (..) { const v = mv(); try { try { continue; } finally { break; } } finally {} }`: the temporary was released in front of the `continue`, and again in front of the `break` its finally then took. The release for a jump that passes through a finally now always goes at the end of the outermost such finally, tagged with the jump, not only when the finally reads the temporary: a finally that leaves by a jump of its own never reaches it, and the jump's own release is then the only one. Fuzzer (#646), found past the first 1000 programs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent f0f1ca6 commit 4c18e7f

3 files changed

Lines changed: 264 additions & 1 deletion

File tree

‎tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp‎

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
#include "llvm/ADT/TypeSwitch.h"
1818
#include "llvm/Support/Debug.h"
1919

20+
#include <set>
21+
#include <tuple>
22+
2023
#define DEBUG_TYPE "pass"
2124

2225
namespace mlir_ts = mlir::typescript;
@@ -1361,6 +1364,7 @@ class OwnedReturnConsumptionPass
13611364
});
13621365

13631366
mlir::OpBuilder builder(&getContext());
1367+
std::set<std::tuple<mlir::Operation *, mlir::Operation *, std::string>> placedAfterFinally;
13641368
for (auto *op : discarded)
13651369
{
13661370
if (!allUsesReleasableInOwnBlock(op) || storedIntoUnownedLocalOutside(op))
@@ -1414,7 +1418,7 @@ class OwnedReturnConsumptionPass
14141418
auto leavesBlock = mlir::isa<mlir_ts::ReturnValOp, mlir_ts::ReturnOp>(nested) ||
14151419
(mlir::isa<mlir_ts::BreakOp, mlir_ts::ContinueOp>(nested) &&
14161420
jumpLeavesBlock(nested, block));
1417-
if (leavesBlock)
1421+
if (leavesBlock && !releaseAfterFinally(builder, op, nested, block, placedAfterFinally))
14181422
{
14191423
builder.setInsertionPoint(nested);
14201424
builder.create<mlir_ts::ReleaseOp>(op->getLoc(), op->getResult(0));
@@ -1446,6 +1450,85 @@ class OwnedReturnConsumptionPass
14461450
}
14471451
}
14481452

1453+
// A jump out of a `try` body or catch clause runs the `finally` first, and the finally may read
1454+
// the temporary: `const v = mv(); try { if (c) continue; } finally { s += v.get(); }` gave `v`
1455+
// back at the `continue`, before the finally read it. Or the finally may leave by a jump of its
1456+
// own, which releases the temporary in front of it too: `try { continue; } finally { break; }`
1457+
// gave it back twice. When a jump passes through a finally between it and the temporary's block,
1458+
// the release goes at the end of the outermost such finally instead - the last one the jump
1459+
// runs - inside a `ts.If` tagged with the jump, as MLIRGen does for the scopes the jump leaves
1460+
// (mlirGenScopeExitAfterFinally), so that TryOpLowering keeps it in the copy of the finally that
1461+
// jump runs and in no other. A finally that leaves on its own never reaches it. False when the
1462+
// jump passes through no finally.
1463+
//
1464+
// Every jump with one key runs one copy, so the release goes in once for each (`placed`).
1465+
static bool releaseAfterFinally(mlir::OpBuilder &builder, mlir::Operation *op, mlir::Operation *jump,
1466+
mlir::Block *block,
1467+
std::set<std::tuple<mlir::Operation *, mlir::Operation *, std::string>> &placed)
1468+
{
1469+
mlir_ts::TryOp lastTry;
1470+
for (auto *child = jump; child->getBlock() != block; child = child->getParentOp())
1471+
{
1472+
auto tryOp = mlir::dyn_cast<mlir_ts::TryOp>(child->getParentOp());
1473+
if (!tryOp || child->getParentRegion() == &tryOp.getFinally() ||
1474+
child->getParentRegion() == &tryOp.getCleanup())
1475+
{
1476+
continue;
1477+
}
1478+
1479+
auto &finally = tryOp.getFinally();
1480+
if (finally.empty() || &finally.front().front() == finally.front().getTerminator())
1481+
{
1482+
continue;
1483+
}
1484+
1485+
lastTry = tryOp;
1486+
}
1487+
1488+
if (!lastTry)
1489+
{
1490+
return false;
1491+
}
1492+
1493+
auto value = op->getResult(0);
1494+
1495+
auto isReturn = mlir::isa<mlir_ts::ReturnValOp, mlir_ts::ReturnOp>(jump);
1496+
std::string key;
1497+
if (!isReturn)
1498+
{
1499+
auto isBreak = mlir::isa<mlir_ts::BreakOp>(jump);
1500+
auto label = isBreak ? mlir::cast<mlir_ts::BreakOp>(jump).getLabelAttr()
1501+
: mlir::cast<mlir_ts::ContinueOp>(jump).getLabelAttr();
1502+
key = std::string(isBreak ? JUMP_ONLY_BREAK_KEY : JUMP_ONLY_CONTINUE_KEY) +
1503+
(label ? label.getValue().str() : std::string());
1504+
}
1505+
1506+
if (!placed.insert({op, lastTry.getOperation(), isReturn ? std::string(RETURN_ONLY_ATTR_NAME) : key}).second)
1507+
{
1508+
return true;
1509+
}
1510+
1511+
auto location = op->getLoc();
1512+
auto &finallyEnd = lastTry.getFinally().back();
1513+
builder.setInsertionPoint(finallyEnd.getTerminator());
1514+
auto alwaysTrue = builder.create<mlir_ts::ConstantOp>(location, mlir_ts::BooleanType::get(builder.getContext()),
1515+
builder.getBoolAttr(true));
1516+
auto ifOp = builder.create<mlir_ts::IfOp>(location, alwaysTrue, /*withElseRegion=*/false);
1517+
if (isReturn)
1518+
{
1519+
ifOp->setAttr(RETURN_ONLY_ATTR_NAME, builder.getUnitAttr());
1520+
}
1521+
else
1522+
{
1523+
ifOp->setAttr(JUMP_ONLY_ATTR_NAME, builder.getStringAttr(key));
1524+
}
1525+
1526+
mlir::OpBuilder::InsertionGuard guard(builder);
1527+
builder.setInsertionPointToStart(&ifOp.getThenRegion().front());
1528+
builder.create<mlir_ts::ReleaseOp>(location, value);
1529+
return true;
1530+
}
1531+
14491532
// Does `op` catch this jump - is it the loop, switch or labelled statement the jump names?
14501533
//
14511534
// An unlabelled `break` is caught by the nearest enclosing loop or `switch`; an unlabelled

‎tslang/test/tester/CMakeLists.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,7 @@ tslang_add_test(NAME test-compile-00-const-element-read COMMAND test-runner "${P
405405
tslang_add_test(NAME test-compile-00-const-global-read COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_global_read.ts")
406406
tslang_add_test(NAME test-compile-00-global-argument-reassigned COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_reassigned.ts")
407407
tslang_add_test(NAME test-compile-00-global-argument-view COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_view.ts")
408+
tslang_add_test(NAME test-compile-00-temporary-read-in-finally COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00temporary_read_in_finally.ts")
408409
tslang_add_test(NAME test-compile-00-unowned-local-store COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00unowned_local_store.ts")
409410
tslang_add_test(NAME test-compile-00-array-argument-held COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_argument_held.ts")
410411
tslang_add_test(NAME test-compile-00-array-data-string-alias COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_data_string_alias.ts")
@@ -986,6 +987,7 @@ tslang_add_test(NAME test-jit-00-const-element-read COMMAND test-runner -jit "${
986987
tslang_add_test(NAME test-jit-00-const-global-read COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_global_read.ts")
987988
tslang_add_test(NAME test-jit-00-global-argument-reassigned COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_reassigned.ts")
988989
tslang_add_test(NAME test-jit-00-global-argument-view COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_view.ts")
990+
tslang_add_test(NAME test-jit-00-temporary-read-in-finally COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00temporary_read_in_finally.ts")
989991
tslang_add_test(NAME test-jit-00-unowned-local-store COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00unowned_local_store.ts")
990992
tslang_add_test(NAME test-jit-00-array-argument-held COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_argument_held.ts")
991993
tslang_add_test(NAME test-jit-00-array-data-string-alias COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_data_string_alias.ts")
@@ -1957,6 +1959,7 @@ set(TSLANG_CORPUS
19571959
00const_global_read.ts
19581960
00global_argument_reassigned.ts
19591961
00global_argument_view.ts
1962+
00temporary_read_in_finally.ts
19601963
00unowned_local_store.ts
19611964
00array_argument_held.ts
19621965
00array_data_string_alias.ts

‎tslang/test/tester/tests/00temporary_read_in_finally.ts‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
// A `const` holding a call's result (a folded const, released where its block ends) stays alive
2+
// when a `break`, `continue` or `return` leaves its block through a `try` whose `finally` reads
3+
// it, or leaves by a jump of its own. Under rc the release went in front of the jump, and the
4+
// finally then ran on a freed block, or released it a second time in front of its own jump.
5+
// Each read follows allocations of the same size, so a freed block is reused.
6+
class V {
7+
tag = "";
8+
constructor(public x: number) {
9+
this.tag = "v" + x;
10+
}
11+
12+
get() {
13+
return this.x;
14+
}
15+
}
16+
17+
function mv(x: number) {
18+
return new V(x);
19+
}
20+
21+
function churn() {
22+
const junk: V[] = [];
23+
for (let i = 0; i < 64; i++) {
24+
junk.push(new V(999));
25+
}
26+
}
27+
28+
function viaContinue() {
29+
let s: number = 0;
30+
for (let i = 0; i < 4; i++) {
31+
const v = mv(i);
32+
try {
33+
churn();
34+
if (i % 2 == 0) {
35+
continue;
36+
}
37+
} finally {
38+
churn();
39+
s += v.get();
40+
}
41+
}
42+
43+
return s;
44+
}
45+
46+
function viaBreak() {
47+
let s: number = 0;
48+
for (let i = 0; i < 4; i++) {
49+
const v = mv(i + 10);
50+
try {
51+
if (i == 1) {
52+
break;
53+
}
54+
} finally {
55+
churn();
56+
s += v.x;
57+
}
58+
}
59+
60+
return s;
61+
}
62+
63+
function viaTwoJumps() {
64+
let s: number = 0;
65+
for (let i = 0; i < 6; i++) {
66+
const v = mv(i);
67+
try {
68+
if (i % 3 == 0) {
69+
continue;
70+
}
71+
72+
if (i % 3 == 1) {
73+
continue;
74+
}
75+
} finally {
76+
churn();
77+
s += v.get();
78+
}
79+
}
80+
81+
return s;
82+
}
83+
84+
function viaLabelledContinue() {
85+
let s: number = 0;
86+
outer: for (let i = 0; i < 3; i++) {
87+
const v = mv(i + 1);
88+
for (let j = 0; j < 3; j++) {
89+
try {
90+
try {
91+
if (j == 1) {
92+
continue outer;
93+
}
94+
} finally {
95+
churn();
96+
s += v.get();
97+
}
98+
} finally {
99+
churn();
100+
s += v.x * 100;
101+
}
102+
}
103+
}
104+
105+
return s;
106+
}
107+
108+
function viaReturn(x: number) {
109+
const v = mv(x);
110+
try {
111+
if (x > 0) {
112+
return x;
113+
}
114+
} finally {
115+
churn();
116+
assert(v.x == x && v.tag == "v" + x, "read in the finally of a return");
117+
}
118+
119+
return -1;
120+
}
121+
122+
function jumpInFinally() {
123+
let s: number = 0;
124+
for (let i = 0; i < 4; i++) {
125+
const v = mv(i);
126+
try {
127+
try {
128+
if (i % 2 == 0) {
129+
continue;
130+
}
131+
} finally {
132+
churn();
133+
if (i == 2) {
134+
break;
135+
}
136+
}
137+
} finally {
138+
}
139+
140+
s += v.x;
141+
}
142+
143+
churn();
144+
return s;
145+
}
146+
147+
function callInFinally(v: V) {
148+
churn();
149+
return v.get();
150+
}
151+
152+
function viaCallInFinally() {
153+
let s: number = 0;
154+
for (let i = 0; i < 3; i++) {
155+
const v = mv(i + 5);
156+
try {
157+
if (v.x > 2) {
158+
continue;
159+
}
160+
} finally {
161+
s += callInFinally(v);
162+
}
163+
}
164+
165+
return s;
166+
}
167+
168+
function main() {
169+
assert(viaContinue() == 6, "continue through a finally");
170+
assert(viaBreak() == 21, "break through a finally");
171+
assert(viaTwoJumps() == 15, "two continues through one finally");
172+
assert(viaLabelledContinue() == 1212, "a labelled continue through two finallys");
173+
assert(viaReturn(3) == 3, "return through a finally");
174+
assert(viaCallInFinally() == 18, "a call in the finally");
175+
assert(jumpInFinally() == 1, "a continue through a finally that breaks");
176+
print("done.");
177+
}

0 commit comments

Comments
 (0)