From 6d52d26f6963ae106f3f4f1aab485bcde7c94d28 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Thu, 22 Oct 2026 17:11:46 +0100 Subject: [PATCH] rc: a kept bound method counts the object it is bound to `let m = v.get; v = mkv(2); m()` ran on a freed object under rc, and so did a bound method kept in a field or an element. A bound method's value carried a null type tag at CLOSURE_TYPE_INDEX, so its receivers' retains and releases did nothing for `this`. Under rc the lowering now gives a method bound to a class object that class's tag, as an interface over the object gets its tag: a bound value is born with no count, and each receiver that keeps it counts the object. ThisSymbolRef / ThisVirtualSymbolRef cast `this` to an opaque pointer, so they record the object's type on the CreateBoundFunction they build (BOUND_OBJECT_TYPE_ATTR_NAME). own, gc and none keep the null tag. Co-Authored-By: Claude Opus 5.5 --- tslang/include/TypeScript/Defines.h | 10 ++- tslang/lib/TypeScript/LowerToLLVM.cpp | 50 ++++++++++++-- tslang/test/tester/CMakeLists.txt | 3 + .../test/tester/tests/00bound_method_count.ts | 67 +++++++++++++++++++ 4 files changed, 123 insertions(+), 7 deletions(-) create mode 100644 tslang/test/tester/tests/00bound_method_count.ts diff --git a/tslang/include/TypeScript/Defines.h b/tslang/include/TypeScript/Defines.h index e4d933fcc..2db4725d0 100644 --- a/tslang/include/TypeScript/Defines.h +++ b/tslang/include/TypeScript/Defines.h @@ -90,10 +90,16 @@ // Marks a `ts.CreateBoundFunction` whose `this` is a capture box built for it a moment earlier, // rather than a receiver that belongs to somebody else. Only such a closure owns its `this`, and -// only it gets the type tag at CLOSURE_TYPE_INDEX - a bound method must not take ownership of -// the object it is bound to. Set where the closure is built and the difference is known. +// gets the box's type tag at CLOSURE_TYPE_INDEX - a bound method does not own the object it is +// bound to (under rc its receivers count it, see BOUND_OBJECT_TYPE_ATTR_NAME). Set where the +// closure is built and the difference is known. #define OWNS_CAPTURE_ATTR_NAME "__owns_capture" +// On a `ts.CreateBoundFunction` that binds a method to an object: the object's type, which the +// lowering casts `this` away from. Under rc the bound value counts the object through that type's +// tag at CLOSURE_TYPE_INDEX, as an interface over it does. +#define BOUND_OBJECT_TYPE_ATTR_NAME "__bound_object_type" + // Marks the `ts.Variable` that CaptureOpLowering creates for a capture box. It carries // `captured = true` only because that is how a variable asks to be allocated in the heap, and // without this marker it would be indistinguishable from the cell of a captured variable - and diff --git a/tslang/lib/TypeScript/LowerToLLVM.cpp b/tslang/lib/TypeScript/LowerToLLVM.cpp index ca38f6eb0..0b8ef48da 100644 --- a/tslang/lib/TypeScript/LowerToLLVM.cpp +++ b/tslang/lib/TypeScript/LowerToLLVM.cpp @@ -6352,6 +6352,8 @@ struct ThisVirtualSymbolRefOpLowering : public TsLlvmPattern(loc, boundFunc, thisOpaque, methodTyped); + boundFuncVal->setAttr(BOUND_OBJECT_TYPE_ATTR_NAME, + mlir::TypeAttr::get(thisVirtualSymbolRefOp.getThisVal().getType())); rewriter.replaceOp(thisVirtualSymbolRefOp, ValueRange{boundFuncVal}); } else @@ -6390,7 +6392,8 @@ struct ThisSymbolRefOpLowering : public TsLlvmPattern auto method = rewriter.create( loc, mlir_ts::FunctionType::get(rewriter.getContext(), boundFunc.getInputs(), boundFunc.getResults()), thisSymbolRefOp.getIdentifierAttr()); - rewriter.replaceOpWithNewOp(thisSymbolRefOp, boundFunc, thisOpaque, method); + auto boundFuncVal = rewriter.replaceOpWithNewOp(thisSymbolRefOp, boundFunc, thisOpaque, method); + boundFuncVal->setAttr(BOUND_OBJECT_TYPE_ATTR_NAME, mlir::TypeAttr::get(thisSymbolRefOp.getThisVal().getType())); return success(); } }; @@ -6924,9 +6927,9 @@ struct CreateBoundFunctionOpLowering : public TsLlvmPatternhasAttr(OWNS_CAPTURE_ATTR_NAME)) { - return nullTag(); + return getBoundObjectTypeTag(createBoundFunctionOp, rewriter); } auto captureRefType = dyn_cast(createBoundFunctionOp.getThisVal().getType()); @@ -6984,6 +6987,43 @@ struct CreateBoundFunctionOpLowering : public TsLlvmPatterngetAttrOfType(BOUND_OBJECT_TYPE_ATTR_NAME); + auto thisType = boundType ? boundType.getValue() : createBoundFunctionOp.getThisVal().getType(); + if (!tsLlvmContext->compileOptions.isRefCounted() || !isa(thisType)) + { + return rewriter.create(loc, th.getPtrType()); + } + + TypeOfOpHelper toh(rewriter); + auto name = toh.typeOfAsString(thisType); + if (name.empty()) + { + return rewriter.create(loc, th.getPtrType()); + } + + // generated first: the descriptor's initializer takes their addresses + OwnershipRoutineLogic orl(createBoundFunctionOp, rewriter, getTypeConverter(), tsLlvmContext->compileOptions); + auto releaseRoutineName = orl.getOrCreateReleaseRoutine(thisType); + auto retainRoutineName = orl.getOrCreateRetainRoutine(thisType); + + LLVMCodeHelper ch(createBoundFunctionOp, rewriter, getTypeConverter(), tsLlvmContext->compileOptions); + return ch.getOrCreateTypeDescriptorName(thisType, name, TypeOfOpHelper::typeKindFromName(name), + releaseRoutineName, retainRoutineName); + } + LogicalResult matchAndRewrite(mlir_ts::CreateBoundFunctionOp createBoundFunctionOp, Adaptor transformed, ConversionPatternRewriter &rewriter) const final { diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 2f61af6f5..a611bb063 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -388,6 +388,7 @@ tslang_add_test(NAME test-compile-00-string-empty-falsy COMMAND test-runner "${P 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-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-bound-method-count COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00bound_method_count.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") tslang_add_test(NAME test-compile-00-array-length-valid COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_valid.ts") @@ -957,6 +958,7 @@ tslang_add_test(NAME test-jit-00-string-empty-falsy COMMAND test-runner -jit "${ 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-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-bound-method-count COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00bound_method_count.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") tslang_add_test(NAME test-jit-00-array-length-valid COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_valid.ts") @@ -1916,6 +1918,7 @@ set(TSLANG_CORPUS 00string_concat_bounds.ts 00parse_number_range.ts 00narrowed_assign_other_member.ts + 00bound_method_count.ts 00const_record_owned_fields.ts 00generator_temporaries.ts 00array_length_valid.ts diff --git a/tslang/test/tester/tests/00bound_method_count.ts b/tslang/test/tester/tests/00bound_method_count.ts new file mode 100644 index 000000000..44cc99c31 --- /dev/null +++ b/tslang/test/tester/tests/00bound_method_count.ts @@ -0,0 +1,67 @@ +// A method bound to an object and kept - in a `let`, a field, an element - keeps that object alive +// when the variable it was read from is given a new value. Under rc a bound method carried no type +// tag, so keeping it counted nothing, and `v = mkv(2)` freed the object `m()` then ran on. Each +// call follows allocations of the same size, so a freed object is reused. +class W { + constructor(public x: number) {} +} + +class V { + items: number[] = [1, 2, 3]; + constructor(public w: W) {} + + get() { + return this.w.x; + } +} + +class H { + f: () => number = () => 0; +} + +function mkv(n: number) { + return new V(new W(n)); +} + +function churn(): number { + let s = 0; + const a: V[] = []; + for (let i = 0; i < 64; i++) a.push(mkv(1000 + i)); + for (const e of a) s += e.w.x; + return s; +} + +function inLet(n: number) { + let v = mkv(n); + let m = v.get; + v = mkv(n + 1); + churn(); + return m() == n && v.w.x == n + 1; +} + +function inField(n: number) { + let v = mkv(n); + const h = new H(); + h.f = v.get; + v = mkv(n + 1); + churn(); + return h.f() == n; +} + +function inElement(n: number) { + let v = mkv(n); + const methods = [v.get]; + v = mkv(n + 1); + churn(); + return methods[0]() == n; +} + +function main() { + for (let n = 0; n < 20; n++) { + assert(inLet(n), "a bound method in a let"); + assert(inField(n), "a bound method in a field"); + assert(inElement(n), "a bound method in an element"); + } + + print("done."); +}