Skip to content

Commit 4dfaa59

Browse files
rc: hold a global argument from its read, and give a const read of a global a count
Two shapes the memory-model fuzzer (#646) found double-freeing under rc since #639. - holdGlobalArguments retained the argument right before the call. A later argument of the same call could give the global a new value first (`f(G, (G = mk(), x))`, `f(H.n, (H.n = mk(), x))`): its release freed the block, and the hold then retained and released a freed block. The retain (and the #645 unwind slot) now follow the read. - `const n = G` was folded into the read, holding no count. Once the global got a new value, here or in a call, `n` dangled (fuzzer R1). #639 made that a crash: `run(n.v)`, with `run` assigning G, held and released a field of the freed block. Under rc a const read out of a global, or an interface or bound method made over one, now gets storage like a const read out of a field or an element (#635, #641, #642, #644). Under own the folded read stays a borrow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 50ef536 commit 4dfaa59

5 files changed

Lines changed: 227 additions & 13 deletions

File tree

‎tslang/lib/TypeScript/MLIRGenImpl.h‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1737,6 +1737,18 @@ class MLIRGenImpl
17371737
return loadOp && loadOp.getReference().getDefiningOp<mlir_ts::PropertyRefOp>();
17381738
}
17391739

1740+
// A value read out of a global variable, through any casts: what `G` is in `const n = G`.
1741+
static bool readsGlobal(mlir::Value value)
1742+
{
1743+
while (auto castOp = value ? value.getDefiningOp<mlir_ts::CastOp>() : mlir_ts::CastOp())
1744+
{
1745+
value = castOp.getIn();
1746+
}
1747+
1748+
auto loadOp = value ? value.getDefiningOp<mlir_ts::LoadOp>() : mlir_ts::LoadOp();
1749+
return loadOp && loadOp.getReference().getDefiningOp<mlir_ts::AddressOfOp>();
1750+
}
1751+
17401752
// A value read out of an element, through any casts: what `a[0]` is in `const c = a[0]`, and
17411753
// what `b` is in `const [b] = a`, read there only when `a` is long enough (the `then` of an if).
17421754
static bool readsElement(mlir::Value value)
@@ -1886,11 +1898,12 @@ class MLIRGenImpl
18861898
// interface or a bound method made over such a read (`const i: I = a`, `const m = a.get`):
18871899
// it holds the block as `b` would.
18881900
//
1889-
// Under rc, so does a const read out of a field (`const c = h.c`) or an element (`const c =
1890-
// a[0]`, `const [c] = a`): folded, `h.c = d` or `a[0] = d` freed what `c` still read. So
1891-
// does an interface or a bound method made over such a read (`const i: I = h.v`,
1892-
// `const m = a[0].get`), which holds the block as `c` would. Under own the folded read is a
1893-
// borrow, and a use after the overwrite is an error there.
1901+
// Under rc, so does a const read out of a field (`const c = h.c`), an element (`const c =
1902+
// a[0]`, `const [c] = a`) or a global (`const c = G`): folded, `h.c = d`, `a[0] = d` or
1903+
// `G = d` - here or in a call - freed what `c` still read. So does an interface or a bound
1904+
// method made over such a read (`const i: I = h.v`, `const m = a[0].get`), which holds the
1905+
// block as `c` would. Under own the folded read is a borrow, and a use after the overwrite
1906+
// is an error there.
18941907
//
18951908
// And a const record whose fields own blocks (`const o = { s: p.child }`): the literal
18961909
// hands its fields over unretained - its receiver is the one that retains them, as a
@@ -1910,8 +1923,9 @@ class MLIRGenImpl
19101923
|| ((isa<mlir_ts::ArrayType>(type) || MLIRTypeHelper::isSharedHandleType(type)
19111924
|| ((readsLocalSlot(initial) || viewsLocalSlot(initial)) && mth.ownsHeapMemory(location, type))
19121925
|| (compileOptions.isRefCounted()
1913-
&& (readsField(initial) || readsElement(initial) || readsField(viewedValue(initial))
1914-
|| readsElement(viewedValue(initial)))
1926+
&& (readsField(initial) || readsElement(initial) || readsGlobal(initial)
1927+
|| readsField(viewedValue(initial)) || readsElement(viewedValue(initial))
1928+
|| readsGlobal(viewedValue(initial)))
19151929
&& mth.ownsHeapMemory(location, type))
19161930
|| (readsLiteralSlot(mth, location, initial) && isa<mlir_ts::TupleType>(type)
19171931
&& mth.ownsHeapMemory(location, type))

‎tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -297,10 +297,10 @@ class OwnedReturnConsumptionPass
297297
// A call's argument read out of a global (`run(G)`, `run(G.c)`, `G.m()`) is borrowed like any
298298
// other argument, and the callee may give the global a new value while it still uses the
299299
// parameter (`function run(h: C) { G = mk(); print(h.x); }`): the global's release freed the
300-
// block the parameter read. Under rc the caller holds a count across the call - retained right
301-
// before it, released right after it - and, when a try around the call takes what it throws,
302-
// released where that try's handler starts (unwindRegionOf). This runs before the affine
303-
// lowering, so a call in a try body is still a plain call here (a `ts.Invoke` later).
300+
// block the parameter read. Under rc the caller holds a count across the call - retained where
301+
// the argument is read, released right after the call - and, when a try around the call takes
302+
// what it throws, released where that try's handler starts (unwindRegionOf). This runs before
303+
// the affine lowering, so a call in a try body is still a plain call here (a `ts.Invoke` later).
304304
//
305305
// A Shared<T> handle (spec 23.1) is counted under rc and own alike, and a program reads its
306306
// count (`Shared.count`) as the number of handles - which a parameter, a borrow, is not one of.
@@ -495,12 +495,22 @@ class OwnedReturnConsumptionPass
495495
for (auto [op, argument] : held)
496496
{
497497
auto location = op->getLoc();
498+
499+
// From the read on, not from the call: an argument after it may give the global a new
500+
// value first (`f(G, (G = mk(), x))`), and its release freed the block this one read
501+
// before a retain at the call could keep it.
498502
mlir::OpBuilder builder(op);
503+
auto *readOp = argument.getDefiningOp();
504+
if (readOp && readOp->getBlock() == op->getBlock())
505+
{
506+
builder.setInsertionPointAfter(readOp);
507+
}
508+
499509
builder.create<mlir_ts::RetainOp>(location, argument);
500510

501511
// A call a try around it takes the exceptions of gives the hold back on that path too,
502-
// through a slot that holds the value only while the call runs: null before and after
503-
// it, so the handler's release does nothing when something else threw.
512+
// through a slot that holds the value only while the hold lasts: null before the read
513+
// and after the call, so the handler's release does nothing when something else threw.
504514
auto *unwindRegion = unwindRegionOf(op);
505515
mlir::Value slot;
506516
if (unwindRegion)

‎tslang/test/tester/CMakeLists.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,8 @@ tslang_add_test(NAME test-compile-00-global-copy-counted COMMAND test-runner "${
402402
tslang_add_test(NAME test-compile-00-c-string-copy COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00c_string_copy.ts")
403403
tslang_add_test(NAME test-compile-00-const-field-read COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_field_read.ts")
404404
tslang_add_test(NAME test-compile-00-const-element-read COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_element_read.ts")
405+
tslang_add_test(NAME test-compile-00-const-global-read COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_global_read.ts")
406+
tslang_add_test(NAME test-compile-00-global-argument-reassigned COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_reassigned.ts")
405407
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")
406408
tslang_add_test(NAME test-compile-00-arrays7 COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array7.ts")
407409
tslang_add_test(NAME test-compile-00-arrays8-tuple-spread COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array8_tuple_spread.ts")
@@ -978,6 +980,8 @@ tslang_add_test(NAME test-jit-00-global-copy-counted COMMAND test-runner -jit "$
978980
tslang_add_test(NAME test-jit-00-c-string-copy COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00c_string_copy.ts")
979981
tslang_add_test(NAME test-jit-00-const-field-read COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_field_read.ts")
980982
tslang_add_test(NAME test-jit-00-const-element-read COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_element_read.ts")
983+
tslang_add_test(NAME test-jit-00-const-global-read COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_global_read.ts")
984+
tslang_add_test(NAME test-jit-00-global-argument-reassigned COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_reassigned.ts")
981985
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")
982986
tslang_add_test(NAME test-jit-00-arrays7 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array7.ts")
983987
tslang_add_test(NAME test-jit-00-arrays8-tuple-spread COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array8_tuple_spread.ts")
@@ -1944,6 +1948,8 @@ set(TSLANG_CORPUS
19441948
00c_string_copy.ts
19451949
00const_field_read.ts
19461950
00const_element_read.ts
1951+
00const_global_read.ts
1952+
00global_argument_reassigned.ts
19471953
00array_data_string_alias.ts
19481954
00array7.ts
19491955
00array8_tuple_spread.ts
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// A `const` read out of a global keeps what it read alive when the global is given a new value,
2+
// here or in a call. Folded, the const was the read itself and held no reference of its own: under
3+
// rc the overwrite freed the block it still read, and a value read through it and lent to a call
4+
// that assigns the global (`run(n.v)`) was retained and released after the block was gone.
5+
// Allocations of the same size reuse anything freed before each read.
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+
class N {
18+
v: V;
19+
constructor(x: number) {
20+
this.v = new V(x);
21+
}
22+
}
23+
24+
interface IV {
25+
x: number;
26+
get(): number;
27+
}
28+
29+
class Junk {
30+
a = -1;
31+
b = -2;
32+
tag = "junk";
33+
}
34+
35+
function churn() {
36+
let keep: Junk[] = [];
37+
for (let i = 0; i < 1000; i++) keep.push(new Junk());
38+
return keep.length;
39+
}
40+
41+
let G = new N(1);
42+
let GV = new V(1);
43+
44+
function assignG(x: number) {
45+
G = new N(x);
46+
churn();
47+
}
48+
49+
// assigns the global its argument was read out of
50+
function run(v: V, x: number) {
51+
assignG(100 + x);
52+
return v.x == x && v.tag == "v" + x;
53+
}
54+
55+
function reassignedHere(i: number) {
56+
G = new N(i);
57+
const n = G;
58+
G = new N(9);
59+
churn();
60+
assert(n.v.x == i && n.v.tag == "v" + i, "a const read of a global, reassigned here");
61+
}
62+
63+
function reassignedInCall(i: number) {
64+
G = new N(i);
65+
const n = G;
66+
assignG(9);
67+
assert(n.v.x == i && n.v.get() == i, "a const read of a global, reassigned in a call");
68+
}
69+
70+
function lentFromConst(i: number) {
71+
G = new N(i);
72+
const n = G;
73+
for (let j = 0; j < 3; j++) {
74+
assert(run(n.v, i), "a field of a const read of a global, lent to a call that assigns it");
75+
try {
76+
churn();
77+
} finally {
78+
assert(n.v.tag == "v" + i, "read in a finally");
79+
}
80+
}
81+
}
82+
83+
function views(i: number) {
84+
GV = new V(i);
85+
const iv: IV = GV;
86+
const m = GV.get;
87+
GV = new V(9);
88+
churn();
89+
assert(iv.x == i && iv.get() == i && m() == i, "an interface and a bound method over a global");
90+
}
91+
92+
function main() {
93+
for (let i = 2; i < 5; i++) {
94+
reassignedHere(i);
95+
reassignedInCall(i);
96+
lentFromConst(i);
97+
views(i);
98+
}
99+
100+
churn();
101+
assert(G.v.x > 100 && GV.x == 9, "the globals' last values");
102+
print("done.");
103+
}
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// An argument read out of a global, or out of a global's field, stays alive when a later argument
2+
// of the same call gives that global a new value: `check(G, (G = mk(2), G), 1)`. Under rc the
3+
// caller holds a count across a call that may assign the global (00global_argument_held.ts), and
4+
// took it at the call - after the later argument had already freed the block, so the hold
5+
// retained and released a freed block. Each read follows allocations of the same size, so a freed
6+
// block is reused.
7+
// @strict-null false
8+
class C {
9+
x = 5;
10+
tag = "";
11+
v: number[] = [];
12+
}
13+
14+
class H {
15+
c: C = mk(3);
16+
}
17+
18+
function mk(x: number): C {
19+
const r = new C();
20+
r.x = x;
21+
r.tag = "c" + x;
22+
r.v.push(x);
23+
return r;
24+
}
25+
26+
function churn() {
27+
const junk: C[] = [];
28+
for (let i = 0; i < 64; i++) {
29+
const c = new C();
30+
c.x = 999;
31+
c.tag = "junk";
32+
c.v.push(99);
33+
junk.push(c);
34+
}
35+
}
36+
37+
let G: C = mk(1);
38+
const GH = new H();
39+
40+
function thrower(): C {
41+
throw "thrown";
42+
}
43+
44+
// assigns both globals, so a caller holds what it reads out of them
45+
function check(c: C, other: C, x: number) {
46+
G = mk(100 + x);
47+
GH.c = mk(200 + x);
48+
churn();
49+
return c.x == x && c.tag == "c" + x && c.v[0] == x && other.x > 0;
50+
}
51+
52+
function inLoop() {
53+
for (let i = 0; i < 3; i++) {
54+
const x = G.x;
55+
assert(check(G, (G = mk(10 + i), G), x), "a global, in a loop");
56+
}
57+
}
58+
59+
function inTryCatch() {
60+
G = mk(30);
61+
let caught = false;
62+
try {
63+
check(G, (G = mk(31), thrower()), 30);
64+
} catch (e) {
65+
caught = true;
66+
}
67+
68+
churn();
69+
assert(caught && G.x == 31 && G.tag == "c31", "a later argument throws after the assignment");
70+
}
71+
72+
function main() {
73+
assert(check(G, (G = mk(2), G), 1), "a global given before an argument that assigns it");
74+
const fx = GH.c.x;
75+
assert(check(GH.c, (GH.c = mk(4), GH.c), fx), "a global's field given before an argument that assigns it");
76+
inLoop();
77+
inTryCatch();
78+
churn();
79+
assert(G.x == 31 && GH.c.x == 701, "the globals' last values");
80+
print("done.");
81+
}

0 commit comments

Comments
 (0)