From 0bdfb472ec5303fbdfaa0f2cc60864397a5b7da7 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Fri, 23 Oct 2026 09:34:13 +0100 Subject: [PATCH 1/3] rc: a local read for an argument is held when a later argument assigns it `g(n, (n = mk(2), 0))`: the arguments are read in order, so the first read `n` and the second gave `n` a new value before the call took the first - and under rc that assignment released the block the callee then read (exit 139, or garbage). The same with a field or an element of a local (`g(h.v, (h.v = mk(), 0))`, also of a folded `const h = new H()`), a conditional argument (its own blocks), a call in a try body, an assigned parameter, and a closure called there that assigns a captured local. Follow-ups 7 and the rc half of 8 (fuzzer A1, local form). holdGlobalArguments now also holds such an argument, as it holds one read out of a global (#647): retained right after the read, released after the call (and in the handler of a try that takes what the call throws). It holds a read of a local when a store into that local, a place in it, or its release - or any call, for a captured local - lies on a path from the read to the call (own_facts::findBetween). The retain goes right after the read also when a conditional argument put the call in a later block of the same region, unless a loop leads back. own still accepts the local forms; that is the own half of follow-up 8. Co-Authored-By: Claude Opus 5.5 --- .../TypeScript/OwnedReturnConsumptionPass.cpp | 109 ++++++++++- tslang/lib/TypeScript/OwnershipFacts.h | 172 ++++++++++++++++++ tslang/test/tester/CMakeLists.txt | 3 + .../tester/tests/00argument_slot_assigned.ts | 147 +++++++++++++++ 4 files changed, 428 insertions(+), 3 deletions(-) create mode 100644 tslang/test/tester/tests/00argument_slot_assigned.ts diff --git a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp index f0694aefb..315e8ebfc 100644 --- a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp +++ b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp @@ -306,6 +306,11 @@ class OwnedReturnConsumptionPass // count (`Shared.count`) as the number of handles - which a parameter, a borrow, is not one of. // So a handle is held, under both, only across a call that may give the global it was read // from a new value, the one case where the hold is what keeps the block alive. + // + // A read of a local is held too, under rc, where the local is given a new value after the read + // and before the call: by a later argument (`f(n, (n = mk(), 0))`, in a conditional one too), + // or, for a captured local, by a closure called there. The assignment released the block the + // argument read. (own rejects these, OwnershipInferencePass.) void holdGlobalArguments(MLIRTypeHelper &mth, mlir::ModuleOp module) { if (!compileOptions.tracksOwnership()) @@ -602,8 +607,18 @@ class OwnedReturnConsumptionPass } auto global = globalOf(argument, false); + if (global.empty()) + { + if (localAssignedBefore(op, argument)) + { + held.push_back({op, argument}); + } + + continue; + } + auto isHandle = MLIRTypeHelper::isSharedHandleType(argument.getType()); - if (global.empty() || (isHandle ? !mayAssign(op, global) : !compileOptions.isRefCounted())) + if (isHandle ? !mayAssign(op, global) : !compileOptions.isRefCounted()) { continue; } @@ -625,10 +640,13 @@ class OwnedReturnConsumptionPass // From the read on, not from the call: an argument after it may give the global a new // value first (`f(G, (G = mk(), x))`), and its release freed the block this one read - // before a retain at the call could keep it. + // before a retain at the call could keep it. A conditional argument between them puts + // the call in a block of its own, run once for each read unless a loop leads back. mlir::OpBuilder builder(op); auto *readOp = argument.getDefiningOp(); - if (readOp && readOp->getBlock() == op->getBlock()) + if (readOp && (readOp->getBlock() == op->getBlock() || + (readOp->getParentRegion() == op->getParentRegion() && + !blockReaches(op->getBlock(), readOp->getBlock())))) { builder.setInsertionPointAfter(readOp); } @@ -685,6 +703,91 @@ class OwnedReturnConsumptionPass } } + // Whether a read of a local, `argument`, is followed by a new value given to that local (or its + // release) before `call` takes it (holdGlobalArguments). Under rc only. + bool localAssignedBefore(mlir::Operation *call, mlir::Value argument) + { + if (!compileOptions.isRefCounted()) + { + return false; + } + + auto *read = argument.getDefiningOp(); + if (!read) + { + return false; + } + + if (auto local = mlir::dyn_cast_or_null(own_facts::slotOf(argument))) + { + auto captured = local.getCaptured().value_or(false); + return own_facts::findBetween(read, call, [&](mlir::Operation *op) { + return (captured && own_facts::isCall(op)) || + own_facts::sameSlot(own_facts::slotWritten(op), local); + }) != nullptr; + } + + // a field or an element of an object no local's slot holds (`const h = new H()`, folded): + // a store into the same field, or any element, of that object (`f(h.v, (h.v = mk(), 0))`) + auto value = argument; + while (auto castOp = value.getDefiningOp()) + { + value = castOp.getIn(); + } + + auto loadOp = value.getDefiningOp(); + if (!loadOp || !own_facts::isPlace(loadOp.getReference())) + { + return false; + } + + auto baseOf = [](mlir::Operation *place) { + auto base = place->getOperand(0); + while (auto castOp = base.getDefiningOp()) + { + base = castOp.getIn(); + } + + return base; + }; + + auto *place = loadOp.getReference().getDefiningOp(); + auto base = baseOf(place); + auto propertyRef = mlir::dyn_cast(place); + return own_facts::findBetween(read, call, [&](mlir::Operation *op) { + auto storeOp = mlir::dyn_cast(op); + auto *written = storeOp ? storeOp.getReference().getDefiningOp() : nullptr; + if (!written || written->getName() != place->getName() || baseOf(written) != base) + { + return false; + } + + auto writtenField = mlir::dyn_cast(written); + return !propertyRef || writtenField.getPosition() == propertyRef.getPosition(); + }) != nullptr; + } + + static bool blockReaches(mlir::Block *from, mlir::Block *to) + { + llvm::SmallPtrSet seen; + llvm::SmallVector work{from}; + while (!work.empty()) + { + auto *block = work.pop_back_val(); + if (block == to) + { + return true; + } + + if (seen.insert(block).second) + { + work.append(block->getSuccessors().begin(), block->getSuccessors().end()); + } + } + + return false; + } + // Where an exception out of `op` is taken first within its function: the region of the try // around it that runs then, in the order its landing pad takes them (TryOpLowering) - the // cleanup, else the catch clause, else the finally, for a call in its body; the finally for a diff --git a/tslang/lib/TypeScript/OwnershipFacts.h b/tslang/lib/TypeScript/OwnershipFacts.h index 9cfb98266..dc4f03651 100644 --- a/tslang/lib/TypeScript/OwnershipFacts.h +++ b/tslang/lib/TypeScript/OwnershipFacts.h @@ -425,6 +425,178 @@ inline bool isPlace(mlir::Value ref) isInterfaceField(ref); } +// The storage a reference addresses - a local's `ts.Variable` or a global's `ts.AddressOf` - itself +// or through the places in what it holds (a field, an element, a handle's value): `n`, `n.v`, +// `G.items[0]`. Null for anything else. +inline mlir::Operation *slotOfRef(mlir::Value ref) +{ + for (auto depth = 0; depth < 16 && ref; depth++) + { + auto *def = ref.getDefiningOp(); + if (mlir::isa_and_nonnull(def)) + { + return def; + } + + if (!isPlace(ref)) + { + return nullptr; + } + + // what the place is in: a read of a slot, or the slot itself (a field of a local tuple) + auto in = def->getOperand(0); + while (auto castOp = in.getDefiningOp()) + { + in = castOp.getIn(); + } + + auto loadOp = in.getDefiningOp(); + ref = loadOp ? loadOp.getReference() : in; + } + + return nullptr; +} + +// The storage a value is read out of (slotOfRef), through casts. +inline mlir::Operation *slotOf(mlir::Value value) +{ + while (auto castOp = value.getDefiningOp()) + { + value = castOp.getIn(); + } + + auto loadOp = value.getDefiningOp(); + return loadOp ? slotOfRef(loadOp.getReference()) : nullptr; +} + +// One storage: a local's slot, or a global - each access of which is a `ts.AddressOf` of its own. +inline bool sameSlot(mlir::Operation *a, mlir::Operation *b) +{ + if (a == b) + { + return true; + } + + auto globalA = mlir::dyn_cast_or_null(a); + auto globalB = mlir::dyn_cast_or_null(b); + return globalA && globalB && globalA.getGlobalName() == globalB.getGlobalName(); +} + +// The storage an op gives a new value to or releases: a store, `ts.ReleaseSlot`. +inline mlir::Operation *slotWritten(mlir::Operation *op) +{ + if (auto storeOp = mlir::dyn_cast(op)) + { + return slotOfRef(storeOp.getReference()); + } + + if (auto releaseSlotOp = mlir::dyn_cast(op)) + { + return slotOfRef(releaseSlotOp.getSlot()); + } + + return nullptr; +} + +// An op for which `found` holds that may run after `read` and before `call`: the ops between them +// in a block, those of every block on a path from one to the other, and what each of them nests. +// The arguments of a call are read in order, so one read early may be freed by a later one before +// the call takes it - `f(n, (n = mk(), 0))` - and a conditional argument puts that in a block of its +// own. Null when there is none. +inline mlir::Operation *findBetween(mlir::Operation *read, mlir::Operation *call, + llvm::function_ref found) +{ + // the two in the innermost region holding both + mlir::Operation *readTop = nullptr; + mlir::Operation *callTop = nullptr; + for (auto *region = call->getParentRegion(); region && !readTop; region = region->getParentRegion()) + { + readTop = region->findAncestorOpInRegion(*read); + callTop = readTop ? region->findAncestorOpInRegion(*call) : nullptr; + } + + if (!readTop || !callTop || readTop == callTop) + { + return nullptr; + } + + mlir::Operation *at = nullptr; + auto scan = [&](mlir::Operation *op) { + op->walk([&](mlir::Operation *inner) { + if (!at && inner != call && found(inner)) + { + at = inner; + } + }); + }; + + // the statement the call is in, when the call is not one itself + if (callTop != call) + { + scan(callTop); + } + + auto *fromBlock = readTop->getBlock(); + auto *toBlock = callTop->getBlock(); + if (fromBlock == toBlock && readTop->isBeforeInBlock(callTop)) + { + for (auto *op = readTop->getNextNode(); op != callTop; op = op->getNextNode()) + { + scan(op); + } + + return at; + } + + for (auto *op = readTop->getNextNode(); op; op = op->getNextNode()) + { + scan(op); + } + + for (auto *op = &toBlock->front(); op != callTop; op = op->getNextNode()) + { + scan(op); + } + + // the blocks on a path: reached from the read's, and reaching the call's + llvm::SmallPtrSet after; + llvm::SmallVector work(fromBlock->getSuccessors().begin(), fromBlock->getSuccessors().end()); + while (!work.empty()) + { + auto *block = work.pop_back_val(); + if (after.insert(block).second) + { + work.append(block->getSuccessors().begin(), block->getSuccessors().end()); + } + } + + llvm::SmallPtrSet before; + work.assign(toBlock->getPredecessors().begin(), toBlock->getPredecessors().end()); + while (!work.empty()) + { + auto *block = work.pop_back_val(); + if (before.insert(block).second) + { + work.append(block->getPredecessors().begin(), block->getPredecessors().end()); + } + } + + for (auto *block : after) + { + if (!before.contains(block)) + { + continue; + } + + for (auto &op : *block) + { + scan(&op); + } + } + + return at; +} + // A handle (spec 23.2): counted in every model that tracks ownership, and left alone by own. inline bool isHandle(mlir::Value value) { diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 800f7b235..312d49ede 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -406,6 +406,7 @@ tslang_add_test(NAME test-compile-00-const-global-read COMMAND test-runner "${PR tslang_add_test(NAME test-compile-00-global-argument-reassigned COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_reassigned.ts") tslang_add_test(NAME test-compile-00-global-argument-view COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_view.ts") tslang_add_test(NAME test-compile-00-array-argument-held COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_argument_held.ts") +tslang_add_test(NAME test-compile-00-argument-slot-assigned COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00argument_slot_assigned.ts") 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") tslang_add_test(NAME test-compile-00-arrays7 COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array7.ts") tslang_add_test(NAME test-compile-00-arrays8-tuple-spread COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array8_tuple_spread.ts") @@ -986,6 +987,7 @@ tslang_add_test(NAME test-jit-00-const-global-read COMMAND test-runner -jit "${P tslang_add_test(NAME test-jit-00-global-argument-reassigned COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_reassigned.ts") tslang_add_test(NAME test-jit-00-global-argument-view COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_view.ts") tslang_add_test(NAME test-jit-00-array-argument-held COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_argument_held.ts") +tslang_add_test(NAME test-jit-00-argument-slot-assigned COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00argument_slot_assigned.ts") 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") tslang_add_test(NAME test-jit-00-arrays7 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array7.ts") tslang_add_test(NAME test-jit-00-arrays8-tuple-spread COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array8_tuple_spread.ts") @@ -1956,6 +1958,7 @@ set(TSLANG_CORPUS 00global_argument_reassigned.ts 00global_argument_view.ts 00array_argument_held.ts + 00argument_slot_assigned.ts 00array_data_string_alias.ts 00array7.ts 00array8_tuple_spread.ts diff --git a/tslang/test/tester/tests/00argument_slot_assigned.ts b/tslang/test/tester/tests/00argument_slot_assigned.ts new file mode 100644 index 000000000..f50ef286c --- /dev/null +++ b/tslang/test/tester/tests/00argument_slot_assigned.ts @@ -0,0 +1,147 @@ +// An argument read out of a local is lent to the call, and a later argument may give that local a +// new value before the call takes it: `g(n, (n = mk(2), 0))`. Under rc the assignment released the +// block the first argument read, and the callee read freed memory. The same with a field or an +// element of a local, a conditional argument, a call in a try body, a parameter, and a closure that +// assigns a captured local. Each read follows allocations of the same size, so a freed block is +// reused. +class V { + tag = ""; + constructor(public x: number) { + this.tag = "v" + x; + } +} + +class H { + v = new V(0); +} + +class Junk { + a = -1; + b = -2; + tag = "junk"; +} + +function churn() { + let keep: Junk[] = []; + for (let i = 0; i < 1000; i++) keep.push(new Junk()); + return keep.length; +} + +function mk(n: number) { + return new V(n); +} + +function g(a: V, b: number) { + churn(); + assert(a.tag == "v" + a.x, "the argument's block"); + return a.x * 10 + b; +} + +function g2(a: V, b: V) { + churn(); + assert(a.tag == "v" + a.x && b.tag == "v" + b.x, "the arguments' blocks"); + return a.x * 10 + b.x; +} + +function gThrows(a: V, b: number): number { + churn(); + assert(a.tag == "v" + a.x, "the argument's block"); + throw a.x * 10 + b; +} + +function local() { + let n = mk(1); + assert(g(n, (n = mk(2), 0)) == 10, "a local"); + assert(n.x == 2, "the local's new value"); +} + +function localTwice() { + let n = mk(1); + assert(g2(n, (n = mk(2), n)) == 12, "a local, then its new value"); +} + +function localAssignment() { + let n = mk(1); + assert(g2(n, n = mk(3)) == 13, "a local, assigned in the next argument"); +} + +function inLoop() { + let s = 0; + for (let i = 0; i < 100; i++) { + let n = mk(i); + s += g(n, (n = mk(i + 1), 0)); + } + + assert(s == 49500, "a local in a loop"); +} + +function field() { + let h = new H(); + h.v = mk(1); + assert(g(h.v, (h.v = mk(2), 0)) == 10, "a field of a local"); + const k = new H(); + k.v = mk(3); + assert(g(k.v, (k.v = mk(4), 0)) == 30, "a field of a const"); +} + +function element() { + let a = [mk(1)]; + assert(g(a[0], (a[0] = mk(2), 0)) == 10, "an element of a local"); +} + +function conditional() { + let n = mk(1); + const flag = n.x > 0; + assert(g(n, flag ? (n = mk(2), 0) : 1) == 10, "a conditional argument"); +} + +function inTry() { + let n = mk(1); + try { + assert(g(n, (n = mk(2), 0)) == 10, "a call in a try body"); + } catch (e) { + assert(false, "nothing throws"); + } +} + +function thrown() { + let n = mk(1); + let caught = 0; + try { + gThrows(n, (n = mk(2), 0)); + } catch (e) { + caught = e; + } + + churn(); + assert(caught == 10 && n.x == 2, "the call throws"); +} + +function param(p: V) { + assert(g(p, (p = mk(2), 0)) == 10, "a parameter"); +} + +function captured() { + let n = mk(1); + const set = () => { + n = mk(2); + return 0; + }; + assert(g(n, set()) == 10, "a captured local, assigned by a closure"); + assert(n.x == 2, "the captured local's new value"); +} + +function main() { + local(); + localTwice(); + localAssignment(); + inLoop(); + field(); + element(); + conditional(); + inTry(); + thrown(); + param(mk(1)); + captured(); + print("done."); +} From 41a7389d785f4a954ff59f4ccc3f49d59e5f0b55 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Fri, 23 Oct 2026 10:14:25 +0100 Subject: [PATCH 2/3] A Shared handle read for an argument is held when a later argument assigns it `g(s, (s = new Shared(new V(2)), 0))` with `s` a local handle: the assignment gave back the count the old handle held, the last one, and `g` read the freed block. Handles are counted under own too, and the previous commit held such a read under rc only, while own's new check leaves handles to the counts - so own read freed memory. The hold now applies to a handle under own as well, as holdGlobalArguments already does for a handle read out of a global. Co-Authored-By: Claude Opus 5.5 --- .../TypeScript/OwnedReturnConsumptionPass.cpp | 7 ++-- tslang/test/tester/CMakeLists.txt | 4 +-- .../own/own_shared_argument_assigned.ts | 36 +++++++++++++++++++ 3 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 tslang/test/tester/own/own_shared_argument_assigned.ts diff --git a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp index 315e8ebfc..9fbe8c098 100644 --- a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp +++ b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp @@ -310,7 +310,8 @@ class OwnedReturnConsumptionPass // A read of a local is held too, under rc, where the local is given a new value after the read // and before the call: by a later argument (`f(n, (n = mk(), 0))`, in a conditional one too), // or, for a captured local, by a closure called there. The assignment released the block the - // argument read. (own rejects these, OwnershipInferencePass.) + // argument read. Under own only a handle is held so, being counted there too; own rejects the + // others (OwnershipInferencePass). void holdGlobalArguments(MLIRTypeHelper &mth, mlir::ModuleOp module) { if (!compileOptions.tracksOwnership()) @@ -704,10 +705,10 @@ class OwnedReturnConsumptionPass } // Whether a read of a local, `argument`, is followed by a new value given to that local (or its - // release) before `call` takes it (holdGlobalArguments). Under rc only. + // release) before `call` takes it (holdGlobalArguments). Under rc, and for a handle under own. bool localAssignedBefore(mlir::Operation *call, mlir::Value argument) { - if (!compileOptions.isRefCounted()) + if (!compileOptions.isRefCounted() && !MLIRTypeHelper::isSharedHandleType(argument.getType())) { return false; } diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 312d49ede..3c5a5d3a4 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -3742,7 +3742,7 @@ set(shared_models gc none rc own) foreach(shared_mm ${shared_models}) foreach(shared_test own_shared_basic own_shared_user_named own_shared_graph own_shared_containers own_shared_cycle own_shared_union own_shared_global own_shared_unlink own_shared_global_init - own_shared_place_two_uses own_shared_global_field own_shared_global_copy + own_shared_place_two_uses own_shared_global_field own_shared_global_copy own_shared_argument_assigned own_shared_global_argument own_shared_global_argument_throw) tslang_add_test(NAME test-jit-${shared_mm}-${shared_test} COMMAND test-runner -jit -mm=${shared_mm} "${PROJECT_SOURCE_DIR}/test/tester/own/${shared_test}.ts") tslang_add_test(NAME test-compile-${shared_mm}-${shared_test} COMMAND test-runner -mm=${shared_mm} "${PROJECT_SOURCE_DIR}/test/tester/own/${shared_test}.ts") @@ -3757,7 +3757,7 @@ endforeach() foreach(shared_verify_mm own rc) foreach(shared_test own_shared_basic own_shared_graph own_shared_count own_shared_containers own_shared_cycle own_shared_union own_shared_global own_shared_unlink own_shared_global_init - own_shared_place_two_uses own_shared_global_field own_shared_global_copy + own_shared_place_two_uses own_shared_global_field own_shared_global_copy own_shared_argument_assigned own_shared_global_argument own_shared_global_argument_throw) add_test(NAME test-${shared_verify_mm}-verify-ownership-${shared_test} COMMAND $ --emit=mlir-affine --no-default-lib -mm=${shared_verify_mm} --verify-ownership diff --git a/tslang/test/tester/own/own_shared_argument_assigned.ts b/tslang/test/tester/own/own_shared_argument_assigned.ts new file mode 100644 index 000000000..58e9dd632 --- /dev/null +++ b/tslang/test/tester/own/own_shared_argument_assigned.ts @@ -0,0 +1,36 @@ +// A Shared handle read for an argument, and given a new value by a later argument before the call +// takes it: the assignment gave back the count the old handle held, the last one, and the callee +// read the freed block. The caller holds a count across the call, in every model that counts. Each +// read follows allocations of the same size, so a freed block is reused. +class V { + tag = ""; + constructor(public x: number) { + this.tag = "v" + x; + } +} + +class Junk { + a = -1; + b = -2; + tag = "junk"; +} + +function churn() { + let keep: Junk[] = []; + for (let i = 0; i < 1000; i++) keep.push(new Junk()); + return keep.length; +} + +function g(s: Shared, b: number) { + churn(); + assert(s.value.tag == "v" + s.value.x, "the handle's block"); + return s.value.x * 10 + b; +} + +function main() { + let s = new Shared(new V(1)); + assert(g(s, (s = new Shared(new V(2)), 0)) == 10, "a handle given a new value in the next argument"); + churn(); + assert(s.value.x == 2 && s.value.tag == "v2", "the handle's new value"); + print("done."); +} From 6e7f8ffbaf277f1d1b47f2d6bdb1110fcc3fa250 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Fri, 23 Oct 2026 09:54:12 +0100 Subject: [PATCH 3/3] -mm=own: a local lent to a call cannot be assigned before the call takes it `g(n, (n = mk(2), 0))`: the first argument borrows `n`'s object, and the second gives `n` a new value first, which drops that object before `g` reads it. own accepted it and read freed memory - also through a conditional argument (`g(n, c ? (n = mk(), 0) : 1)`), in a try body, and with a closure called there that assigns a captured `n`. Follow-up 8, own half (fuzzer A1, local form); rc holds such an argument instead (the parent commit). checkArgumentsAssigned rejects an argument read out of an owning local when a store into that local's slot, or its release - or a call, when a closure built over the local assigns it - lies on a path from the read to the call (own_facts::findBetween). A read of a global is already checkGlobalRead's; a store into a field of the local, the place borrows'. Co-Authored-By: Claude Opus 5.5 --- .../lib/TypeScript/OwnershipInferencePass.cpp | 110 ++++++++++++++++++ tslang/test/tester/CMakeLists.txt | 3 + .../tester/own/own_err_argument_assigned.ts | 18 +++ .../own/own_err_argument_assigned_closure.ts | 22 ++++ .../own_err_argument_assigned_conditional.ts | 19 +++ 5 files changed, 172 insertions(+) create mode 100644 tslang/test/tester/own/own_err_argument_assigned.ts create mode 100644 tslang/test/tester/own/own_err_argument_assigned_closure.ts create mode 100644 tslang/test/tester/own/own_err_argument_assigned_conditional.ts diff --git a/tslang/lib/TypeScript/OwnershipInferencePass.cpp b/tslang/lib/TypeScript/OwnershipInferencePass.cpp index 1dacfa314..07b785390 100644 --- a/tslang/lib/TypeScript/OwnershipInferencePass.cpp +++ b/tslang/lib/TypeScript/OwnershipInferencePass.cpp @@ -303,6 +303,8 @@ class OwnershipInferencePass checkGlobalRead(read); } + checkArgumentsAssigned(); + // What is proven loses its retains; any other retain is a second reference nobody // proved safe - of a loaded value, a parameter, an unknown producer, or of a fresh value // whose own check was reported above. @@ -1926,6 +1928,114 @@ class OwnershipInferencePass }); } + // An argument read out of a local is lent to the call, but the arguments are read in order: a + // later one may give the local a new value before the call takes the first - `g(n, (n = mk(), 0))`, + // in a conditional argument too - or call a closure that assigns it, when it is captured. The + // assignment drops the block the argument is, and the callee reads freed memory. rc holds such an + // argument across the call instead (OwnedReturnConsumptionPass, holdGlobalArguments). A read of a + // global is checkGlobalRead's, and a store into a place in the local the place borrows' (a field + // read `g(n.v, (n.v = mk(), 0))`); a store into the local's slot ends `n.v` too. + void checkArgumentsAssigned() + { + getFunction().walk([&](mlir::Operation *call) { + if (!isCall(call)) + { + return mlir::WalkResult::advance(); + } + + for (auto argument : callArgs(call)) + { + // a method's object: `this` of the method bound to it + if (auto getThisOp = argument.getDefiningOp()) + { + auto *boundOp = getThisOp->getOperand(0).getDefiningOp(); + if (mlir::isa_and_nonnull(boundOp)) + { + argument = boundOp->getOperand(0); + } + } + + auto *read = argument.getDefiningOp(); + auto local = mlir::dyn_cast_or_null(slotOf(argument)); + if (!read || !local || !ownsHeap(argument) || isHandle(argument)) + { + continue; + } + + auto assignedByClosure = closureAssigns(local); + auto *write = findBetween(read, call, [&](mlir::Operation *op) { + if (assignedByClosure && isCall(op)) + { + return true; + } + + auto storeOp = mlir::dyn_cast(op); + auto releaseSlotOp = mlir::dyn_cast(op); + return (storeOp && storeOp.getReference() == local.getResult()) || + (releaseSlotOp && releaseSlotOp.getSlot() == local.getResult()); + }); + if (!write) + { + continue; + } + + if (reporting()) + { + auto name = varName(local); + auto callee = calleeName(call); + auto diag = call->emitError("'") + << name << "' is lent to '" << (callee.empty() ? "a function" : callee) + << "', but is given a new value before the call takes it; -mm=own cannot prove " + "it outlives the call"; + diag.attachNote(write->getLoc()) << "'" << name << "' is given a new value here"; + signalPassFailure(); + } + + return mlir::WalkResult::interrupt(); + } + + return mlir::WalkResult::advance(); + }); + } + + // Does a closure built over this captured local assign it (OWN_ASSIGNS_CAPTURES_ATTR_NAME)? Yes too + // when its cell goes where this does not follow. + bool closureAssigns(mlir_ts::VariableOp local) + { + if (!local.getCaptured().value_or(false)) + { + return false; + } + + for (auto *user : local->getUsers()) + { + auto storeOp = mlir::dyn_cast(user); + if (mlir::isa(user) || + (storeOp && storeOp.getReference() == local.getResult())) + { + continue; + } + + // the cell put into a closure's capture box + auto propertyRefOp = storeOp ? storeOp.getReference().getDefiningOp() + : mlir_ts::PropertyRefOp(); + auto boundOp = propertyRefOp ? closureOfBox(propertyRefOp.getObjectRef()) : mlir_ts::CreateBoundFunctionOp(); + if (!boundOp) + { + return true; + } + + auto assigned = boundOp->getAttrOfType(OWN_ASSIGNS_CAPTURES_ATTR_NAME); + if (assigned && llvm::is_contained(assigned.asArrayRef(), static_cast(propertyRefOp.getPosition()))) + { + return true; + } + } + + return false; + } + struct SlotReceiver { mlir::Operation *retain; diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 3c5a5d3a4..6a1da1526 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -3967,6 +3967,9 @@ set(own_error_cases "own_err_global_argument_assigned|a value read out of the global 'G' is lent to a call that may release or overwrite it" "own_err_global_method_assigns|a value read out of the global 'G' is lent to a call that may release or overwrite it" "own_err_global_argument_transitive|a value read out of the global 'G' is lent to a call that may release or overwrite it" + "own_err_argument_assigned|is lent to 'g', but is given a new value before the call takes it" + "own_err_argument_assigned_conditional|is lent to 'g', but is given a new value before the call takes it" + "own_err_argument_assigned_closure|is lent to 'g', but is given a new value before the call takes it" "own_err_global_method_transitive|a value read out of the global 'G' is lent to a call that may release or overwrite it" "own_err_global_this_field_move|a value read out of the global 'GK' is lent to a call that may release or overwrite it" "own_err_global_argument_field_let|a value read out of the global 'G' is lent to a call that may release or overwrite it" diff --git a/tslang/test/tester/own/own_err_argument_assigned.ts b/tslang/test/tester/own/own_err_argument_assigned.ts new file mode 100644 index 000000000..26088b776 --- /dev/null +++ b/tslang/test/tester/own/own_err_argument_assigned.ts @@ -0,0 +1,18 @@ +// -mm=own rejects: `g` borrows `n`'s object as `a`, but the second argument gives `n` a new value +// first, which frees that object before the call reads it. +class V { + constructor(public x: number) {} +} + +function mk(n: number) { + return new V(n); +} + +function g(a: V, b: number) { + return a.x * 10 + b; +} + +function main() { + let n = mk(1); + print(g(n, (n = mk(2), 0))); +} diff --git a/tslang/test/tester/own/own_err_argument_assigned_closure.ts b/tslang/test/tester/own/own_err_argument_assigned_closure.ts new file mode 100644 index 000000000..16d85f16f --- /dev/null +++ b/tslang/test/tester/own/own_err_argument_assigned_closure.ts @@ -0,0 +1,22 @@ +// -mm=own rejects: the closure called for the second argument gives the captured `n` a new value +// after the first argument read it, which frees what `g` is lent. +class V { + constructor(public x: number) {} +} + +function mk(n: number) { + return new V(n); +} + +function g(a: V, b: number) { + return a.x * 10 + b; +} + +function main() { + let n = mk(1); + const set = () => { + n = mk(2); + return 0; + }; + print(g(n, set())); +} diff --git a/tslang/test/tester/own/own_err_argument_assigned_conditional.ts b/tslang/test/tester/own/own_err_argument_assigned_conditional.ts new file mode 100644 index 000000000..f09779734 --- /dev/null +++ b/tslang/test/tester/own/own_err_argument_assigned_conditional.ts @@ -0,0 +1,19 @@ +// -mm=own rejects: a conditional argument may give `n` a new value after the first argument read +// it, which frees what `g` is lent. +class V { + constructor(public x: number) {} +} + +function mk(n: number) { + return new V(n); +} + +function g(a: V, b: number) { + return a.x * 10 + b; +} + +function main() { + let n = mk(1); + const flag = n.x > 0; + print(g(n, flag ? (n = mk(2), 0) : 1)); +}