From a072f882209d5a5e2a19f60951196217869be5cb Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Sat, 3 Oct 2026 18:49:58 +0100 Subject: [PATCH] rc: a ternary branch yielding a fresh object widened to a union hands its reference out (#465) `cond ? new C() : null` freed the new object under rc. The then-branch yields `new C()` cast to `C | null`; OwnedReturnConsumptionPass looked for a branch that hands an unclaimed reference out by asking about the yielded value itself, which was the cast, not the `new`. Seeing no branch that carries one, it left the ts.If alone, and the `new`'s reference was released as a discarded temporary at the end of its branch, before the merge: the result pointed at freed memory (and was released a second time by its receiver). The branch's yield is now seen through the casts that only widen it (to a union or an optional, OptionalValue, CreateUnionInstance): the producer's reference goes out with the ts.If result, as it already did when a branch yielded the `new` itself. A cast that makes a new value (a box to `any`) carries its own reference and is not looked through. Test: 00ternary_new_or_null (compile and JIT, and in the rc/none corpus): a local and a returned `cond ? new C() : null`, an optional `cond ? n : undefined`, and a loop, each read after allocations of the same size have reused anything freed. It failed under rc before the change. The own corpus is unchanged (453 / 593 plain, 451 / 593 with --opt). Fixes #465. Co-Authored-By: Claude Opus 5.5 --- .../TypeScript/OwnedReturnConsumptionPass.cpp | 43 +++++++++++++-- tslang/test/tester/CMakeLists.txt | 3 ++ .../tester/tests/00ternary_new_or_null.ts | 53 +++++++++++++++++++ 3 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 tslang/test/tester/tests/00ternary_new_or_null.ts diff --git a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp index 908fe00b6..fb9c40ad1 100644 --- a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp +++ b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp @@ -389,9 +389,43 @@ class OwnedReturnConsumptionPass // Only worth doing when some branch actually carries an unclaimed +1. Where every // branch borrows, the receiver's retain is already the right and only answer. + // What a branch yields, seen through the casts that only widen it: `new C()` yielded as + // `C | null` or `C | undefined` is still that block, so its reference goes out with it + // (#465). A cast that makes a new value (a box) carries its own reference instead. + auto producerOf = [](mlir::Value value) { + while (auto *definingOp = value.getDefiningOp()) + { + if (definingOp->hasAttr(OWNED_RESULT_ATTR_NAME)) + { + break; + } + + if (auto castOp = mlir::dyn_cast(definingOp); + castOp && mlir::isa(castOp.getType())) + { + value = castOp.getIn(); + } + else if (auto optionalValueOp = mlir::dyn_cast(definingOp)) + { + value = optionalValueOp.getIn(); + } + else if (auto createUnionInstanceOp = mlir::dyn_cast(definingOp)) + { + value = createUnionInstanceOp.getIn(); + } + else + { + break; + } + } + + return value; + }; + auto carriesOwned = [&](mlir_ts::ResultOp resultOp) { - auto *definingOp = resultOp->getOperand(0).getDefiningOp(); - return ::typescript::mayTakeOverReference(resultOp->getOperand(0)) && + auto produced = producerOf(resultOp->getOperand(0)); + auto *definingOp = produced.getDefiningOp(); + return ::typescript::mayTakeOverReference(produced) && definingOp->getParentRegion() == resultOp->getParentRegion(); }; @@ -406,8 +440,9 @@ class OwnedReturnConsumptionPass { // The `ts.If` result now stands for this reference, so the producer's +1 has // an owner and must not also be released as a discarded temporary. - resultOp->getOperand(0).getDefiningOp()->setAttr(OWNED_RESULT_CONSUMED_ATTR_NAME, - mlir::UnitAttr::get(&getContext())); + producerOf(resultOp->getOperand(0)) + .getDefiningOp() + ->setAttr(OWNED_RESULT_CONSUMED_ATTR_NAME, mlir::UnitAttr::get(&getContext())); continue; } diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 2ba3fadfa..316f19464 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -469,6 +469,7 @@ tslang_add_test(NAME test-compile-00-in-method-names COMMAND test-runner "${PROJ tslang_add_test(NAME test-compile-00-class-structural-extends COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_structural_extends.ts") tslang_add_test(NAME test-compile-00-instanceof COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof.ts") tslang_add_test(NAME test-compile-00-instanceof-union COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof_union.ts") +tslang_add_test(NAME test-compile-00-ternary-new-or-null COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00ternary_new_or_null.ts") tslang_add_test(NAME test-compile-00-class COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts") tslang_add_test(NAME test-compile-00-class-new COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_new.ts") tslang_add_test(NAME test-compile-00-capture-in-new-arguments COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00capture_in_new_arguments.ts") @@ -952,6 +953,7 @@ tslang_add_test(NAME test-jit-00-in-method-names COMMAND test-runner -jit "${PRO tslang_add_test(NAME test-jit-00-class-structural-extends COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_structural_extends.ts") tslang_add_test(NAME test-jit-00-instanceof COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof.ts") tslang_add_test(NAME test-jit-00-instanceof-union COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00instanceof_union.ts") +tslang_add_test(NAME test-jit-00-ternary-new-or-null COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00ternary_new_or_null.ts") tslang_add_test(NAME test-jit-00-class COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts") tslang_add_test(NAME test-jit-00-class-new COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_new.ts") tslang_add_test(NAME test-jit-00-capture-in-new-arguments COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00capture_in_new_arguments.ts") @@ -1767,6 +1769,7 @@ set(TSLANG_CORPUS 00instanceof.ts 00instanceof_namespace_narrowing.ts 00instanceof_union.ts + 00ternary_new_or_null.ts 00interface_captures.ts 00interface_conjunction.ts 00interface_function_typed_field.ts diff --git a/tslang/test/tester/tests/00ternary_new_or_null.ts b/tslang/test/tester/tests/00ternary_new_or_null.ts new file mode 100644 index 000000000..676b55180 --- /dev/null +++ b/tslang/test/tester/tests/00ternary_new_or_null.ts @@ -0,0 +1,53 @@ +// `cond ? new C() : null` hands the new object to whoever takes the result (#465). Under rc the +// branch released it as a discarded temporary before the merge, so the result pointed at freed +// memory. Each result is read after other allocations have reused whatever was freed. +class Node { + v = 5; + name = ""; +} + +function churn() { + let keep: Node[] = []; + for (let i = 0; i < 1000; i++) { + const n = new Node(); + n.v = -i; + n.name = "junk" + i; + keep.push(n); + } + + return keep.length; +} + +function nodeOrNull(set: boolean): Node | null { + return set ? new Node() : null; +} + +function named(set: boolean, name: string): Node | undefined { + const n = new Node(); + n.name = name; + return set ? n : undefined; +} + +function main() { + const set = true; + const n: Node | null = set ? new Node() : null; + assert(churn() == 1000); + assert(n !== null && n.v == 5, "a local from a ternary keeps its node"); + + const m = nodeOrNull(true); + assert(churn() == 1000); + assert(m !== null && m.v == 5, "a returned ternary keeps its node"); + + assert(nodeOrNull(false) === null, "the null side is null"); + + const k = named(true, "kept" + 1); + assert(churn() == 1000); + assert(k !== undefined && k.name == "kept1", "an optional ternary keeps its node"); + + for (let i = 0; i < 1000; i++) { + const t = i % 2 == 0 ? new Node() : null; + if (t) t.v = i; + } + + print("done."); +}