A global copied from another global, or from a field, takes a count

tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp

@@ -303,10 +303,9 @@ class OwnedReturnConsumptionPass
     //
     // The producer is anything that carries an owned reference by now: `new Shared(..)` or a fresh
     // string, marked where MLIRGen builds it, or a call, marked above once its callee is classified
-    // - which is why MLIRGen alone cannot see it. A handle read out of a field or an element
-    // carries none, and gets a retain of its own (handles only: under own a retain left in a
-    // global's region is an error for any other type). A read of another global is left alone
-    // (#446: it reads null today in every model).
+    // - which is why MLIRGen alone cannot see it. A read out of a field, an element or another
+    // global carries none, and gets a retain of its own (under own, a handle only: a retain left in
+    // a global's region is an error for any other type).
     void claimGlobalInitializers(MLIRTypeHelper &mth, mlir::ModuleOp module)
     {
         module.walk([&](mlir_ts::GlobalOp globalOp) {
@@ -446,13 +445,16 @@ class OwnedReturnConsumptionPass
 
             // A field or element read (`holder.child`, `new P().child`, `mkP().child`,
             // `mkArr()[0]`) carries no reference: the object or array keeps its count, and a
-            // temporary one gives it back at the end of the region. The global takes a count of its
-            // own, retained right where it is read - before the temporary's release. Erased under
-            // gc and none.
+            // temporary one gives it back at the end of the region. Nor does a read of another
+            // global (`let k = a`), which keeps its own. The global takes a count of its own,
+            // retained right where it is read - before the temporary's release - or the first
+            // overwrite of either global frees what the other still reads. Under own only a handle
+            // is counted; any other read like these is an error there (OwnershipSignaturePass).
+            // Erased under gc and none.
             auto loadOp = value.getDefiningOp<mlir_ts::LoadOp>();
             auto *place = loadOp ? loadOp.getReference().getDefiningOp() : nullptr;
-            if (place && MLIRTypeHelper::isSharedHandleType(value.getType()) &&
-                mlir::isa<mlir_ts::PropertyRefOp, mlir_ts::ElementRefOp>(place))
+            if (place && (MLIRTypeHelper::isSharedHandleType(value.getType()) || compileOptions.isRefCounted()) &&
+                mlir::isa<mlir_ts::PropertyRefOp, mlir_ts::ElementRefOp, mlir_ts::AddressOfOp>(place))
             {
                 mlir::OpBuilder builder(loadOp->getContext());
                 builder.setInsertionPointAfter(loadOp);

tslang/lib/TypeScript/OwnershipSignaturePass.cpp

@@ -175,6 +175,7 @@ class OwnershipSignaturePass : public mlir::PassWrapper<OwnershipSignaturePass,
         }
 
         moveIntoGlobals(module);
+        rejectBorrowedGlobalInitializers(module);
         collectClassVTables(module);
         fieldFunctions.collect(module);
 
@@ -2296,6 +2297,46 @@ class OwnershipSignaturePass : public mlir::PassWrapper<OwnershipSignaturePass,
         return cellLoad && isBoxField(cellLoad.getReference());
     }
 
+    // ---- Globals given a borrow ----
+
+    // A global initialized with a read of another global, a field or an element (`let k = a`,
+    // `let k = h.c`) would be a second owner of a block another owner keeps: the first overwrite
+    // of either frees what the other still reads, and both give it back at exit. rc counts the
+    // global's own reference (OwnedReturnConsumptionPass, claimGlobalInitializers); own cannot
+    // move the value out of its owner, and a global cannot hold a borrow. A handle is counted
+    // under own too.
+    void rejectBorrowedGlobalInitializers(mlir::ModuleOp module)
+    {
+        module.walk([&](mlir_ts::GlobalOp globalOp) {
+            auto *block = globalOp.getInitializerBlock();
+            auto resultOp = block && !block->empty() ? mlir::dyn_cast<mlir_ts::GlobalResultOp>(block->getTerminator())
+                                                     : mlir_ts::GlobalResultOp();
+            if (!resultOp || resultOp->getNumOperands() != 1)
+            {
+                return;
+            }
+
+            auto value = resultOp->getOperand(0);
+            while (value.getDefiningOp() && isView(value.getDefiningOp()))
+            {
+                value = value.getDefiningOp()->getOperand(0);
+            }
+
+            auto loadOp = value.getDefiningOp<mlir_ts::LoadOp>();
+            auto *place = loadOp ? loadOp.getReference().getDefiningOp() : nullptr;
+            if (!place || !mlir::isa<mlir_ts::AddressOfOp, mlir_ts::PropertyRefOp, mlir_ts::ElementRefOp>(place) ||
+                isHandle(value) || isConstantData(value) || !ownsHeap(value.getLoc(), value.getType()))
+            {
+                return;
+            }
+
+            resultOp.emitError("the initializer of global '")
+                << globalOp.getSymName()
+                << "' reads a value another owner keeps; -mm=own cannot make a global a second owner";
+            signalPassFailure();
+        });
+    }
+
     // ---- Drops ----
 
     // Least fixpoint: a function drops nothing until its body or a call it makes shows it may.

tslang/test/tester/CMakeLists.txt

@@ -392,6 +392,7 @@ tslang_add_test(NAME test-compile-00-const-record-owned-fields COMMAND test-runn
 tslang_add_test(NAME test-compile-00-generator-temporaries COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00generator_temporaries.ts")
 tslang_add_test(NAME test-compile-00-array-length-valid COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_valid.ts")
 tslang_add_test(NAME test-compile-00-string-view-copy COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_view_copy.ts")
+tslang_add_test(NAME test-compile-00-global-copy-counted COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_copy_counted.ts")
 tslang_add_test(NAME test-compile-00-c-string-copy COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00c_string_copy.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")
@@ -959,6 +960,7 @@ tslang_add_test(NAME test-jit-00-const-record-owned-fields COMMAND test-runner -
 tslang_add_test(NAME test-jit-00-generator-temporaries COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00generator_temporaries.ts")
 tslang_add_test(NAME test-jit-00-array-length-valid COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_valid.ts")
 tslang_add_test(NAME test-jit-00-string-view-copy COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_view_copy.ts")
+tslang_add_test(NAME test-jit-00-global-copy-counted COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_copy_counted.ts")
 tslang_add_test(NAME test-jit-00-c-string-copy COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00c_string_copy.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")
@@ -1916,6 +1918,7 @@ set(TSLANG_CORPUS
     00generator_temporaries.ts
     00array_length_valid.ts
     00string_view_copy.ts
+    00global_copy_counted.ts
     00c_string_copy.ts
     00array_data_string_alias.ts
     00array7.ts
@@ -3559,7 +3562,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_place_two_uses own_shared_global_field own_shared_global_copy)
         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")
     endforeach()
@@ -3573,7 +3576,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_place_two_uses own_shared_global_field own_shared_global_copy)
         add_test(NAME test-${shared_verify_mm}-verify-ownership-${shared_test}
                  COMMAND $<TARGET_FILE:tslang> --emit=mlir-affine --no-default-lib -mm=${shared_verify_mm} --verify-ownership
                          "${PROJECT_SOURCE_DIR}/test/tester/own/${shared_test}.ts")
@@ -3702,6 +3705,8 @@ set(own_error_cases
     "own_err_merged_result|is merged from several branches"
     "own_err_global_returned|borrows the global 'V.def' and cannot be stored, returned or captured"
     "own_err_global_let_call|borrows the global 'names', but -mm=own cannot prove the global keeps its value"
+    "own_err_global_from_global|the initializer of global 'k' reads a value another owner keeps"
+    "own_err_global_from_field|the initializer of global 'k' reads a value another owner keeps"
     "own_err_captured_returned|borrows a captured variable and cannot be stored, returned or captured"
     "own_err_record_literal_given|an object literal that holds a value it owns is moved only into a local or returned as it is"
     "own_err_string_kept_param_known|is used here after its value was moved"

tslang/test/tester/own/own_err_global_from_field.ts

@@ -0,0 +1,17 @@
+// -mm=own rejects: `k` would be a second owner of the block the field `h.c` owns, and the field's
+// overwrite frees what `k` still reads. A global cannot borrow.
+class C {
+    constructor(public x: number) {}
+}
+
+class H {
+    c = new C(1);
+}
+
+const h = new H();
+const k = h.c;
+
+function main() {
+    h.c = new C(2);
+    print(k.x);
+}

tslang/test/tester/own/own_err_global_from_global.ts

@@ -0,0 +1,13 @@
+// -mm=own rejects: `k` would be a second owner of the block `a` owns, and `a = new C(2)` frees
+// what `k` still reads. A global cannot borrow.
+class C {
+    constructor(public x: number) {}
+}
+
+let a = new C(1);
+let k = a;
+
+function main() {
+    a = new C(2);
+    print(k.x);
+}

tslang/test/tester/own/own_shared_global_copy.ts

@@ -0,0 +1,63 @@
+// Shared<T> (spec 23.1): a global that copies another global's handle, in its initializer
+// (`let k = a`) or later (`k = a`), or that copies a handle out of another global's field, is a
+// second handle and holds a count of its own. Without it, giving back the global it was copied
+// from freed the payload the copy still read. Each read follows allocations of the same size, so
+// a freed payload is reused. Counts are checked where there is one (rc, own); gc and none report -1.
+class Node {
+    v = 0;
+    tag = "";
+}
+
+class Holder {
+    s: Shared<Node> = new Shared(mk("field", 3));
+}
+
+function churn() {
+    let keep: Node[] = [];
+    for (let i = 0; i < 1000; i++) {
+        const n = new Node();
+        n.v = -i;
+        n.tag = "junk" + i;
+        keep.push(n);
+    }
+
+    return keep.length;
+}
+
+function countIs(s: Shared<Node>, expected: number) {
+    const c = Shared.count(s);
+    return c < 0 || c == expected;
+}
+
+function mk(tag: string, v: number): Node {
+    const n = new Node();
+    n.tag = tag;
+    n.v = v;
+    return n;
+}
+
+let a = new Shared(mk("a", 1));
+let k = a;
+let b = new Shared(mk("b", 2));
+let kb = b;
+const h = new Holder();
+let kf = h.s;
+
+function main() {
+    assert(countIs(a, 2) && countIs(k, 2), "a copied global counts twice");
+    a = new Shared(mk("a2", 10));
+    churn();
+    assert(k.value.v == 1 && k.value.tag == "a", "the copy outlives the global it was copied from");
+    assert(countIs(k, 1), "the copy holds the last count");
+
+    kb = b;
+    b = new Shared(mk("b2", 20));
+    churn();
+    assert(kb.value.v == 2 && kb.value.tag == "b", "a copy assigned again outlives its source");
+
+    h.s = new Shared(mk("field2", 30));
+    churn();
+    assert(kf.value.v == 3 && kf.value.tag == "field", "a copy of a field outlives the field's overwrite");
+
+    print("done.");
+}

tslang/test/tester/tests/00global_copy_counted.ts

@@ -0,0 +1,45 @@
+// A global initialized with another global's value (`let k = a`), or with a field of one
+// (`let kf = h.c`), keeps that block alive when the global it was read from is given a new value.
+// Under rc the read took no count of its own: the overwrite freed what the copy still read. Each
+// read follows allocations of the same size, so a freed block is reused.
+class C {
+    tag = "";
+    constructor(public x: number) {
+        this.tag = "c" + x;
+    }
+}
+
+class H {
+    c = new C(3);
+}
+
+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;
+}
+
+let a = new C(1);
+let k = a;
+const h = new H();
+let kf = h.c;
+let s = "s" + a.x;
+let ks = s;
+
+function main() {
+    a = new C(10);
+    h.c = new C(30);
+    s = "t" + a.x;
+    churn();
+    assert(k.x == 1 && k.tag == "c1", "a copy of a global");
+    assert(kf.x == 3 && kf.tag == "c3", "a copy of a global's field");
+    assert(ks == "s1", "a copy of a global string");
+    assert(a.x == 10 && h.c.x == 30 && s == "t10", "the globals' new values");
+    print("done.");
+}