diff --git a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp index f0694aefb..797663804 100644 --- a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp +++ b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp @@ -1363,7 +1363,7 @@ class OwnedReturnConsumptionPass mlir::OpBuilder builder(&getContext()); for (auto *op : discarded) { - if (!allUsesReleasableInOwnBlock(op)) + if (!allUsesReleasableInOwnBlock(op) || storedIntoUnownedLocalOutside(op)) { continue; } @@ -1515,6 +1515,42 @@ class OwnedReturnConsumptionPass return true; } + // Is the value stored into a local that owns nothing, declared outside the value's block? Such + // a local - one declared with no initializer (`let n: N;`), or in a catch or finally clause + // (MLIRGenImpl::localTakesOwnership) - takes no count for what it is given, so the value's own + // reference is all that keeps it, and the local holds it until its own scope ends: later than + // the end of the value's block. `let n: N; if (c) { n = mk(); } use(n)` gave the new value back + // at the end of the `if`, and `finally { let n = mk(); for (..) { use(n); n = mk(); } }` at the + // end of the loop body; the local then read a freed block. Such a value is left with no one to + // give it back - a leak - which is the side the whole arc keeps the uncertain case on. + static bool storedIntoUnownedLocalOutside(mlir::Operation *op) + { + llvm::SmallVector values{op->getResult(0)}; + while (!values.empty()) + { + auto value = values.pop_back_val(); + for (auto *user : value.getUsers()) + { + if (auto castOp = mlir::dyn_cast(user)) + { + values.push_back(castOp.getRes()); + continue; + } + + auto storeOp = mlir::dyn_cast(user); + auto varOp = storeOp && storeOp.getValue() == value + ? storeOp.getReference().getDefiningOp() + : mlir_ts::VariableOp(); + if (varOp && !varOp->hasAttr(OWNED_LOCAL_ATTR_NAME) && varOp->getBlock() != op->getBlock()) + { + return true; + } + } + } + + return false; + } + // Can a release at the end of this value's own block give its reference back safely? See // releaseDiscardedTemporaries for what disqualifies a use and why. // diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 800f7b235..7f76feed5 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -405,6 +405,7 @@ tslang_add_test(NAME test-compile-00-const-element-read COMMAND test-runner "${P tslang_add_test(NAME test-compile-00-const-global-read COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_global_read.ts") 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-unowned-local-store COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00unowned_local_store.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-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") @@ -985,6 +986,7 @@ tslang_add_test(NAME test-jit-00-const-element-read COMMAND test-runner -jit "${ tslang_add_test(NAME test-jit-00-const-global-read COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_global_read.ts") 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-unowned-local-store COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00unowned_local_store.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-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") @@ -1955,6 +1957,7 @@ set(TSLANG_CORPUS 00const_global_read.ts 00global_argument_reassigned.ts 00global_argument_view.ts + 00unowned_local_store.ts 00array_argument_held.ts 00array_data_string_alias.ts 00array7.ts diff --git a/tslang/test/tester/tests/00unowned_local_store.ts b/tslang/test/tester/tests/00unowned_local_store.ts new file mode 100644 index 000000000..3d835cd65 --- /dev/null +++ b/tslang/test/tester/tests/00unowned_local_store.ts @@ -0,0 +1,102 @@ +// A value stored into a local that owns nothing - one declared with no initializer, or in a +// finally or catch clause - stays alive while the local holds it. Such a local takes no count for +// what it is given, and under rc the value's own reference was given back at the end of the block +// that made it: `let n: N; if (c) { n = mk(); } use(n)`, or a loop in a finally assigning a local +// declared outside the loop, then read a freed block. 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 N { + v: V; + constructor(x: number) { + this.v = new V(x); + } +} + +interface IX { + v: V; +} + +function mk(x: number) { + return new N(x); +} + +function churn() { + const junk: N[] = []; + for (let i = 0; i < 64; i++) { + junk.push(new N(999)); + } +} + +function noInitializer(c: boolean) { + let n: N; + if (c) { + n = mk(5); + } else { + n = mk(6); + } + + churn(); + return n.v.x == (c ? 5 : 6) && n.v.tag == (c ? "v5" : "v6"); +} + +function noInitializerInLoop() { + let n: N; + n = mk(0); + for (let i = 1; i < 4; i++) { + n = mk(i); + } + + churn(); + return n.v.x == 3; +} + +function loopInFinally(ix: IX) { + try { + } finally { + let n = mk(5); + for (let j = 0; j < 3; j++) { + ix.v = n.v; + n = mk(j + 10); + churn(); + } + + assert(n.v.x == 12 && n.v.tag == "v12", "a local of a finally, assigned in a loop"); + } + + churn(); + return ix.v.x == 11; +} + +function ifInCatch() { + let r: number = 0; + try { + throw 1; + } catch (e) { + let n = mk(1); + if (r == 0) { + n = mk(7); + } + + churn(); + r = n.v.x; + } + + return r == 7; +} + +function main() { + for (let i = 0; i < 3; i++) { + assert(noInitializer(i % 2 == 0), "a local with no initializer, assigned in an if"); + assert(noInitializerInLoop(), "a local with no initializer, assigned in a loop"); + assert(loopInFinally({ v: new V(0) }), "a local of a finally"); + assert(ifInCatch(), "a local of a catch clause, assigned in an if"); + } + + print("done."); +}