diff --git a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp index e652a0289..9638639b3 100644 --- a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp +++ b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp @@ -67,6 +67,8 @@ class OwnedReturnConsumptionPass retainYieldedRecords(mth, module); + holdGlobalArguments(mth, module); + own_facts::FieldFunctions fieldFunctionsOfModule; const own_facts::FieldFunctions *fieldFunctions = nullptr; if (compileOptions.isRefCounted()) @@ -291,6 +293,81 @@ class OwnedReturnConsumptionPass }); } + // A call's argument read out of a global (`run(G)`, `run(G.c)`, `G.m()`) is borrowed like any + // other argument, and the callee may give the global a new value while it still uses the + // parameter (`function run(h: C) { G = mk(); print(h.x); }`): the global's release freed the + // block the parameter read. Under rc the caller holds a count across the call - retained right + // before it, released right after it. A call in a try body (`ts.Invoke`) ends its block, and is + // left as it was; so is a Shared handle, whose count a program can read (`Shared.count`). + void holdGlobalArguments(MLIRTypeHelper &mth, mlir::ModuleOp module) + { + if (!compileOptions.isRefCounted()) + { + return; + } + + // a read out of a global, or out of a field or an element of one, through casts + auto readsGlobal = [](mlir::Value value) { + for (auto depth = 0; depth < 16 && value; depth++) + { + while (auto castOp = value.getDefiningOp()) + { + value = castOp.getIn(); + } + + auto loadOp = value.getDefiningOp(); + auto *place = loadOp ? loadOp.getReference().getDefiningOp() : nullptr; + if (mlir::isa_and_nonnull(place)) + { + return true; + } + + value = mlir::isa_and_nonnull(place) ? place->getOperand(0) + : mlir::Value(); + } + + return false; + }; + + llvm::SmallVector> held; + module.walk([&](mlir::Operation *op) { + if (!own_facts::isCall(op) || op->hasTrait() || + !op->getParentOfType()) + { + return; + } + + llvm::SmallPtrSet seen; + for (auto argument : own_facts::callArgs(op)) + { + // 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); + } + } + + if (mth.ownsHeapMemory(op->getLoc(), argument.getType()) && + !MLIRTypeHelper::isSharedHandleType(argument.getType()) && readsGlobal(argument) && + seen.insert(argument).second) + { + held.push_back({op, argument}); + } + } + }); + + for (auto [op, argument] : held) + { + mlir::OpBuilder builder(op); + builder.create(op->getLoc(), argument); + builder.setInsertionPointAfter(op); + builder.create(op->getLoc(), argument); + } + } + // A global is born holding the reference its initializer carries. // // The initializer region ends in `ts.GlobalResult`, a terminator, so a value given to it diff --git a/tslang/lib/TypeScript/OwnershipSignaturePass.cpp b/tslang/lib/TypeScript/OwnershipSignaturePass.cpp index 8d7f67b1c..b99ede9e5 100644 --- a/tslang/lib/TypeScript/OwnershipSignaturePass.cpp +++ b/tslang/lib/TypeScript/OwnershipSignaturePass.cpp @@ -190,6 +190,7 @@ class OwnershipSignaturePass : public mlir::PassWrapper()) + { + auto *boundOp = getThisOp->getOperand(0).getDefiningOp(); + if (mlir::isa_and_nonnull(boundOp)) + { + argument = boundOp->getOperand(0); + } + } + + if (isHandle(argument) || !ownsHeap(call.op->getLoc(), argument.getType())) + { + continue; + } + + auto loadOp = rootOf(argument).getDefiningOp(); + auto global = loadOp ? loadOp.getReference().getDefiningOp() : mlir_ts::AddressOfOp(); + if (!global || !writtenGlobals.contains(global.getGlobalName())) + { + continue; + } + + call.op->emitError("a value read out of the global '") + << global.getGlobalName() + << "' is lent to a call that may release or overwrite it; -mm=own cannot prove it outlives the call"; + signalPassFailure(); + } + } + } + // ---- Calls in a global's initializer ---- // A global's region is no function: its calls get no facts pinned, and the inference never sees diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 17f21ec4d..f364e82ad 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -387,6 +387,7 @@ tslang_add_test(NAME test-compile-00-param-assigned-owned COMMAND test-runner "$ tslang_add_test(NAME test-compile-00-string-empty-falsy COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_empty_falsy.ts") tslang_add_test(NAME test-compile-00-string-concat-bounds COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_concat_bounds.ts") tslang_add_test(NAME test-compile-00-parse-number-range COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00parse_number_range.ts") +tslang_add_test(NAME test-compile-00-global-argument-held COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_held.ts") tslang_add_test(NAME test-compile-00-narrowed-assign-other-member COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00narrowed_assign_other_member.ts") tslang_add_test(NAME test-compile-00-const-record-owned-fields COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_record_owned_fields.ts") tslang_add_test(NAME test-compile-00-generator-temporaries COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00generator_temporaries.ts") @@ -956,6 +957,7 @@ tslang_add_test(NAME test-jit-00-param-assigned-owned COMMAND test-runner -jit " tslang_add_test(NAME test-jit-00-string-empty-falsy COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_empty_falsy.ts") tslang_add_test(NAME test-jit-00-string-concat-bounds COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_concat_bounds.ts") tslang_add_test(NAME test-jit-00-parse-number-range COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00parse_number_range.ts") +tslang_add_test(NAME test-jit-00-global-argument-held COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_argument_held.ts") tslang_add_test(NAME test-jit-00-narrowed-assign-other-member COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00narrowed_assign_other_member.ts") tslang_add_test(NAME test-jit-00-const-record-owned-fields COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_record_owned_fields.ts") tslang_add_test(NAME test-jit-00-generator-temporaries COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00generator_temporaries.ts") @@ -1915,6 +1917,7 @@ set(TSLANG_CORPUS 00string_empty_falsy.ts 00string_concat_bounds.ts 00parse_number_range.ts + 00global_argument_held.ts 00narrowed_assign_other_member.ts 00const_record_owned_fields.ts 00generator_temporaries.ts @@ -3717,6 +3720,8 @@ set(own_error_cases "own_err_global_borrowed_temporary|the initializer of global 'c' holds the result of 'id', which borrows its argument" "own_err_global_kept_not_fresh|the initializer of global 'n' gives 'N.constructor' a value it keeps, but -mm=own cannot move it there" "own_err_global_returned|borrows the global 'V.def' and cannot be stored, returned or captured" + "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_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" diff --git a/tslang/test/tester/own/own_err_global_argument_assigned.ts b/tslang/test/tester/own/own_err_global_argument_assigned.ts new file mode 100644 index 000000000..7afd48e29 --- /dev/null +++ b/tslang/test/tester/own/own_err_global_argument_assigned.ts @@ -0,0 +1,20 @@ +// -mm=own rejects: `run` borrows `G`'s object as `h`, then gives `G` a new value, which frees that +// object while `h` still reads it. +class C { + x = 5; +} + +function mk(): C { + return new C(); +} + +let G: C = mk(); + +function run(h: C) { + G = mk(); + print(h.x); +} + +function main() { + run(G); +} diff --git a/tslang/test/tester/own/own_err_global_method_assigns.ts b/tslang/test/tester/own/own_err_global_method_assigns.ts new file mode 100644 index 000000000..1c6e75792 --- /dev/null +++ b/tslang/test/tester/own/own_err_global_method_assigns.ts @@ -0,0 +1,16 @@ +// -mm=own rejects: `G.m()` runs with `this` borrowed from `G`, and `m` gives `G` a new value, which +// frees the object `this` still is. +class C { + x = 5; + + m() { + G = new C(); + return this.x; + } +} + +let G = new C(); + +function main() { + print(G.m()); +} diff --git a/tslang/test/tester/tests/00global_argument_held.ts b/tslang/test/tester/tests/00global_argument_held.ts new file mode 100644 index 000000000..1fb4564fa --- /dev/null +++ b/tslang/test/tester/tests/00global_argument_held.ts @@ -0,0 +1,61 @@ +// An argument read out of a global stays alive while the callee gives the global a new value. The +// argument is borrowed, so under rc the global's release freed the block the parameter still read. +// Each read follows allocations of the same size, so a freed block is reused. +// @strict-null false +class C { + x = 5; + tag = ""; + v: number[] = []; + + m() { + G = mk(7); + churn(); + return this.x; + } +} + +class H { + c: C = mk(3); +} + +function mk(x: number): C { + const r = new C(); + r.x = x; + r.tag = "c" + x; + r.v.push(x); + return r; +} + +function churn() { + const junk: C[] = []; + for (let i = 0; i < 64; i++) { + const c = new C(); + c.x = 999; + c.tag = "junk"; + c.v.push(99); + junk.push(c); + } +} + +let G: C = mk(1); +const GH = new H(); + +function run(h: C) { + G = mk(2); + churn(); + return h.x == 1 && h.tag == "c1" && h.v[0] == 1; +} + +function runField(c: C) { + GH.c = mk(4); + churn(); + return c.x == 3 && c.tag == "c3"; +} + +function main() { + assert(run(G), "a global given to a function that assigns it"); + assert(runField(GH.c), "a global's field given to a function that assigns it"); + G = mk(6); + assert(G.m() == 6, "a method of a global that assigns it"); + print("done."); +}