rc: hold an argument whose place a later argument overwrites under another name

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<mlir_ts::PropertyRefOp>();
+        auto fieldB = b.getDefiningOp<mlir_ts::PropertyRefOp>();
+        if (fieldA && fieldB)
+        {
+            auto type = stripCasts(fieldA->getOperand(0)).getType();
+            return fieldA.getPosition() == fieldB.getPosition() && mlir::isa<mlir_ts::ClassType>(type) &&
+                   type == stripCasts(fieldB->getOperand(0)).getType();
+        }
+
+        auto elementA = a.getDefiningOp<mlir_ts::ElementRefOp>();
+        auto elementB = b.getDefiningOp<mlir_ts::ElementRefOp>();
+        if (elementA && elementB)
+        {
+            auto type = stripCasts(elementA->getOperand(0)).getType();
+            return mlir::isa<mlir_ts::ArrayType>(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<mlir_ts::ReleaseSlotOp>(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())

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

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