rc: a value stored into a local that owns nothing is not freed under it

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<mlir::Value> values{op->getResult(0)};
+        while (!values.empty())
+        {
+            auto value = values.pop_back_val();
+            for (auto *user : value.getUsers())
+            {
+                if (auto castOp = mlir::dyn_cast<mlir_ts::CastOp>(user))
+                {
+                    values.push_back(castOp.getRes());
+                    continue;
+                }
+
+                auto storeOp = mlir::dyn_cast<mlir_ts::StoreOp>(user);
+                auto varOp = storeOp && storeOp.getValue() == value
+                                 ? storeOp.getReference().getDefiningOp<mlir_ts::VariableOp>()
+                                 : 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.
     //

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

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.");
+}