diff --git a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp index 419f439df..84a4feb32 100644 --- a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp +++ b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp @@ -747,8 +747,39 @@ class OwnedReturnConsumptionPass return loadA && loadB && samePlace(loadA.getReference(), loadB.getReference()); } + // Might two references name one place under different names? One field of two objects of one + // class (`p.n` and `G.n`, where `p` is `G`), or an element of two arrays of one type: which + // object a value is cannot be known here. + static bool mayBeSamePlace(mlir::Value a, mlir::Value b) + { + if (samePlace(a, b)) + { + return true; + } + + auto fieldA = a.getDefiningOp(); + auto fieldB = b.getDefiningOp(); + if (fieldA && fieldB) + { + auto type = stripCasts(fieldA->getOperand(0)).getType(); + return fieldA.getPosition() == fieldB.getPosition() && mlir::isa(type) && + type == stripCasts(fieldB->getOperand(0)).getType(); + } + + auto elementA = a.getDefiningOp(); + auto elementB = b.getDefiningOp(); + if (elementA && elementB) + { + auto type = stripCasts(elementA->getOperand(0)).getType(); + return mlir::isa(type) && type == stripCasts(elementB->getOperand(0)).getType(); + } + + return false; + } + // Is the argument read out of a slot or a place that something between the read and the call - // overwrites - a later argument (`f(n, (n = mk(), n))`)? + // overwrites - a later argument (`f(n, (n = mk(), n))`), under its own name or another + // (`f((p.n = mk(), p.n), (G.n = mk(), 0))`, with `p` being `G`)? // // So is one read out of a field or an element of a value read so: overwriting `n` in // `f({ v: n.v }, (n = mk(), 0))` gives back the object whose field the argument read. @@ -771,8 +802,8 @@ class OwnedReturnConsumptionPass auto releaseSlotOp = mlir::dyn_cast(nested); auto place = storeOp ? storeOp.getReference() : releaseSlotOp ? releaseSlotOp.getSlot() : mlir::Value(); - return place && samePlace(place, reference) ? mlir::WalkResult::interrupt() - : mlir::WalkResult::advance(); + return place && mayBeSamePlace(place, reference) ? mlir::WalkResult::interrupt() + : mlir::WalkResult::advance(); }); if (overwrites.wasInterrupted()) diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 983a30334..a17c7b73e 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -409,6 +409,7 @@ tslang_add_test(NAME test-compile-00-global-argument-view COMMAND test-runner "$ tslang_add_test(NAME test-compile-00-element-store-after-value COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00element_store_after_value.ts") tslang_add_test(NAME test-compile-00-const-interface-field-read COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_interface_field_read.ts") tslang_add_test(NAME test-compile-00-argument-place-overwritten COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00argument_place_overwritten.ts") +tslang_add_test(NAME test-compile-00-argument-place-aliased COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00argument_place_aliased.ts") tslang_add_test(NAME test-compile-00-temporary-read-in-finally COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00temporary_read_in_finally.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") @@ -995,6 +996,7 @@ tslang_add_test(NAME test-jit-00-global-argument-view COMMAND test-runner -jit " tslang_add_test(NAME test-jit-00-element-store-after-value COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00element_store_after_value.ts") tslang_add_test(NAME test-jit-00-const-interface-field-read COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_interface_field_read.ts") tslang_add_test(NAME test-jit-00-argument-place-overwritten COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00argument_place_overwritten.ts") +tslang_add_test(NAME test-jit-00-argument-place-aliased COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00argument_place_aliased.ts") tslang_add_test(NAME test-jit-00-temporary-read-in-finally COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00temporary_read_in_finally.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") @@ -1971,6 +1973,7 @@ set(TSLANG_CORPUS 00element_store_after_value.ts 00const_interface_field_read.ts 00argument_place_overwritten.ts + 00argument_place_aliased.ts 00temporary_read_in_finally.ts 00unowned_local_store.ts 00array_argument_held.ts diff --git a/tslang/test/tester/tests/00argument_place_aliased.ts b/tslang/test/tester/tests/00argument_place_aliased.ts new file mode 100644 index 000000000..ff0460e98 --- /dev/null +++ b/tslang/test/tester/tests/00argument_place_aliased.ts @@ -0,0 +1,78 @@ +// An argument read out of a place that a later argument overwrites under another name: `p.n` read +// for one argument, and `G.n = mk()` in the next, where `p` is `G`. Under rc the caller holds an +// argument whose place is overwritten before the call (`f(n, (n = mk(), n))`), but it compared the +// places by name, so it missed one reached through another variable, and the overwrite freed the +// block the callee was given. Each read follows allocations of the same size, so a freed block is +// reused. +class V { + x: number; + a: number[]; + + constructor(x: number) { + this.x = x; + this.a = [x]; + } +} + +class N { + x: number; + v: V; + + constructor(x: number) { + this.x = x; + this.v = new V(x); + } +} + +class H { + n: N = new N(0); + items: N[] = [new N(0)]; +} + +@noinline function mk(x: number): N { + return new N(x); +} + +function churn() { + const junk: N[] = []; + for (let i = 0; i < 64; i++) { + junk.push(new N(777)); + } +} + +const HG = new H(); +let G = new H(); + +function read(a: N, b: number) { + churn(); + return a.x + a.v.x; +} + +// a parameter that is the const global +function fieldOfConstGlobal(p: H, x: number) { + return read((p.n = mk(x), p.n), (HG.n = mk(0), 0)); +} + +// a parameter that is the global +function fieldOfGlobal(p: H, x: number) { + return read((p.n = mk(x), p.n), (G.n = mk(0), 0)); +} + +// two parameters that are one object +function fieldOfParameter(p: H, q: H, x: number) { + return read((p.n = mk(x), p.n), (q.n = mk(0), 0)); +} + +// an element, through another parameter that is the same array +function elementOfParameter(a: N[], b: N[], x: number) { + return read((a[0] = mk(x), a[0]), (b[0] = mk(0), 0)); +} + +function main() { + assert(fieldOfConstGlobal(HG, 3) == 6, "a field of the const global, through a parameter"); + assert(fieldOfGlobal(G, 4) == 8, "a field of the global, through a parameter"); + const h = new H(); + assert(fieldOfParameter(h, h, 5) == 10, "a field, through two parameters"); + assert(elementOfParameter(h.items, h.items, 6) == 12, "an element, through two parameters"); + print("done."); +}