diff --git a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp index 357c1def0..e652a0289 100644 --- a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp +++ b/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(); auto *place = loadOp ? loadOp.getReference().getDefiningOp() : nullptr; - if (place && MLIRTypeHelper::isSharedHandleType(value.getType()) && - mlir::isa(place)) + if (place && (MLIRTypeHelper::isSharedHandleType(value.getType()) || compileOptions.isRefCounted()) && + mlir::isa(place)) { mlir::OpBuilder builder(loadOp->getContext()); builder.setInsertionPointAfter(loadOp); diff --git a/tslang/lib/TypeScript/OwnershipSignaturePass.cpp b/tslang/lib/TypeScript/OwnershipSignaturePass.cpp index cf2934247..2dc34ad87 100644 --- a/tslang/lib/TypeScript/OwnershipSignaturePass.cpp +++ b/tslang/lib/TypeScript/OwnershipSignaturePass.cpp @@ -175,6 +175,7 @@ class OwnershipSignaturePass : public mlir::PassWrapperempty() ? mlir::dyn_cast(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(); + auto *place = loadOp ? loadOp.getReference().getDefiningOp() : nullptr; + if (!place || !mlir::isa(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. diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 4bc6a05f7..2d3f05175 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/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 $ --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" diff --git a/tslang/test/tester/own/own_err_global_from_field.ts b/tslang/test/tester/own/own_err_global_from_field.ts new file mode 100644 index 000000000..0c40e0a2f --- /dev/null +++ b/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); +} diff --git a/tslang/test/tester/own/own_err_global_from_global.ts b/tslang/test/tester/own/own_err_global_from_global.ts new file mode 100644 index 000000000..3304a2789 --- /dev/null +++ b/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); +} diff --git a/tslang/test/tester/own/own_shared_global_copy.ts b/tslang/test/tester/own/own_shared_global_copy.ts new file mode 100644 index 000000000..00f85fd28 --- /dev/null +++ b/tslang/test/tester/own/own_shared_global_copy.ts @@ -0,0 +1,63 @@ +// Shared (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 = 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, 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."); +} diff --git a/tslang/test/tester/tests/00global_copy_counted.ts b/tslang/test/tester/tests/00global_copy_counted.ts new file mode 100644 index 000000000..2665540d3 --- /dev/null +++ b/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."); +}