From 101ce568279c96125bafbcebc0aae3b3dccbba66 Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Sun, 4 Oct 2026 14:24:42 +0100 Subject: [PATCH] rc: a string made over an array's memory is a view, copied where it is kept (#481) `Ref(buffer[i])` makes a string that points into an array's data block. Under rc a receiver retained it like any string, counting the word in front of the pointer: for `Ref(buffer[0])` that is the data block's count, so the block outlived the array, and once the array grew the string's release decremented the count of the freed old block (heap corruption) while the grown block leaked; for `Ref(buffer[8])` the "count" was elements 0-7, which the retain incremented. Now, as under own, such a string is a view under rc: it keeps no count. OwnedReturnConsumptionPass (rc only) turns each retain of a view into a copy for the receiver it was made for - a return, a store into a field, an element or a global, a push, a capture - made right there, after the bytes were written through the view (`sprintf_s(s, ...)`, then `return s`, as the default library's convertNumber does). A `let` that only ever holds a view borrows it; one assigned again starts from a copy, since the assignment releases what it held. A view still points into the array: used after the array grows or dies, it reads freed memory in every model, as a C pointer would. A view kept inside a tuple or record literal, or by a function it is passed to, is not copied. Closes #481 Co-Authored-By: Claude Opus 5.5 --- .../2026-10-03-array-reference-design.md | 3 + .../TypeScript/OwnedReturnConsumptionPass.cpp | 138 ++++++++++++++++++ tslang/test/tester/CMakeLists.txt | 3 + .../test/tester/tests/00string_view_copy.ts | 86 +++++++++++ 4 files changed, 230 insertions(+) create mode 100644 tslang/test/tester/tests/00string_view_copy.ts diff --git a/tslang/docs/superpowers/specs/2026-10-03-array-reference-design.md b/tslang/docs/superpowers/specs/2026-10-03-array-reference-design.md index ceadbedd8..b0354a2f3 100644 --- a/tslang/docs/superpowers/specs/2026-10-03-array-reference-design.md +++ b/tslang/docs/superpowers/specs/2026-10-03-array-reference-design.md @@ -111,6 +111,9 @@ unchanged. under it (five default-library tests failed ahead of time). Counting the header's reference restores what main does. A reallocating change (`push`, and PR 3's growth) still moves the block under such a string, as it does on main (#481). + - Since #481 such a string is a view under rc, as under own: it takes no reference to the block, + and where it is kept (returned, stored, pushed, captured) the keeper takes a copy. The count + the header holds on its data block stays; nothing else adds one now. - Slots `[length, capacity)` are never released (and, from PR 3, hold zero). - `pop` and `shift` hand the removed element's reference to the caller, as today. - **own.** The header is a single-owned block, like a class instance. diff --git a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp index de4770954..c2280f9ce 100644 --- a/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp +++ b/tslang/lib/TypeScript/OwnedReturnConsumptionPass.cpp @@ -8,6 +8,8 @@ #include "TypeScript/MLIRLogic/MLIRTypeHelper.h" #include "TypeScript/MLIRLogic/MLIROwnedReference.h" +#include "OwnershipFacts.h" + #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/SmallVector.h" @@ -60,6 +62,8 @@ class OwnedReturnConsumptionPass auto module = getOperation(); MLIRTypeHelper mth(module->getContext(), compileOptions); + copyEscapingStringViews(module); + llvm::DenseSet returnsOwned; // Every method in the module, grouped by the name after the last dot: the candidate set // for a virtual call, see virtualCallReturnsOwned. @@ -411,6 +415,140 @@ class OwnedReturnConsumptionPass }); } + // A string made over memory it does not own - `Ref(buffer[0])`, a cast out of + // `!ts.opaque` - is a view of that memory, not a block of its own (#481). Under rc a receiver + // used to retain it, counting whatever word sits in front of the pointer: the array's data + // block for `Ref(buffer[0])`, which then outlived the array and leaked once the array grew, + // and element data or another block's tail for `Ref(buffer[1])`. Now, as under own, a view + // keeps no count: where a receiver retains it (a return, a store into a field, an element or + // a global, a push, a capture), the receiver takes a copy of the string instead, made right + // there - after the bytes were written through the view (`sprintf_s(s, ...)`). A `let` that + // only ever holds a view borrows it; one that is assigned again starts from a copy, since the + // assignment releases what it held. + // + // A view still points into the array: used after the array grows or dies, it reads freed + // memory in every model, as a C pointer would. A view kept inside a tuple or record literal, + // or by a function it is passed to, is not copied. + void copyEscapingStringViews(mlir::ModuleOp module) + { + if (!compileOptions.isRefCounted()) + { + return; + } + + llvm::DenseSet viewLocals; + auto isView = [&](mlir::Value value) { + for (auto depth = 0; depth < 16 && value; depth++) + { + auto *definingOp = value.getDefiningOp(); + if (auto castOp = mlir::dyn_cast_or_null(definingOp)) + { + if (mlir::isa(castOp.getIn().getType())) + { + return true; + } + + value = own_facts::isCopyableString(castOp.getIn().getType()) ? castOp.getIn() : mlir::Value(); + } + else if (auto loadOp = mlir::dyn_cast_or_null(definingOp)) + { + auto *slot = loadOp.getReference().getDefiningOp(); + value = slot && viewLocals.contains(slot) + ? mlir::cast(slot).getInitializer() + : mlir::Value(); + } + else + { + return false; + } + } + + return false; + }; + + // `let l = view`: borrowed when nothing assigns the variable again, a copy when something does. + // Collected first: the walk has already stepped to the op after a variable - its RetainSlot, + // which is erased below. + llvm::SmallVector variables; + module.walk([&](mlir_ts::VariableOp varOp) { variables.push_back(varOp); }); + for (auto varOp : variables) + { + auto init = varOp.getInitializer(); + if (!init || !own_facts::isCopyableString(init.getType()) || !isView(init) || varOp.getCaptured().value_or(false)) + { + continue; + } + + auto assignedAgain = llvm::any_of(varOp->getUsers(), [&](mlir::Operation *user) { + auto storeOp = mlir::dyn_cast(user); + return storeOp && storeOp.getReference() == varOp.getResult(); + }); + + if (assignedAgain) + { + mlir::OpBuilder builder(varOp); + auto copy = builder.create(varOp.getLoc(), init.getType(), init); + varOp.getInitializerMutable().assign(copy); + continue; + } + + llvm::SmallVector slotOps; + for (auto *user : varOp->getUsers()) + { + if (mlir::isa(user)) + { + slotOps.push_back(user); + } + } + + for (auto *slotOp : slotOps) + { + slotOp->erase(); + } + + varOp->removeAttr(OWNED_LOCAL_ATTR_NAME); + viewLocals.insert(varOp); + } + + // a receiver's retain: MLIRGen puts it right in front of the receiver, with nothing but + // other counting between them (`ts.Retain(v); ts.ReleaseSlot(field); ts.Store(v, field)`) + llvm::SmallVector retains; + module.walk([&](mlir_ts::RetainOp retainOp) { + auto value = retainOp.getReference(); + if (own_facts::isCopyableString(value.getType()) && isView(value)) + { + retains.push_back(retainOp); + } + }); + + for (auto retainOp : retains) + { + auto value = retainOp.getReference(); + auto *receiver = retainOp->getNextNode(); + while (receiver && mlir::isa(receiver)) + { + receiver = receiver->getNextNode(); + } + + if (!receiver || !llvm::is_contained(receiver->getOperands(), value)) + { + continue; + } + + mlir::OpBuilder builder(retainOp); + auto copy = builder.create(retainOp->getLoc(), value.getType(), value); + retainOp->setOperand(0, copy); + for (auto &operand : receiver->getOpOperands()) + { + if (operand.get() == value) + { + operand.set(copy); + } + } + } + } + // An array or tuple built in a global's region (`const a = [1, 2]`, `const t = [1, [6, 7]]`, or // a field of `const o = { items: [1, 2] }`) is born at count 0 under rc: a local takes its first // count with RetainSlot, and nothing took one for the global, so the first local that read it diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 8c41a7592..4b82368fa 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -348,6 +348,7 @@ tslang_add_test(NAME test-compile-00-array-rest-copy COMMAND test-runner "${PROJ tslang_add_test(NAME test-compile-00-arrays6 COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array6.ts") tslang_add_test(NAME test-compile-00-array-static-nested COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_static_nested.ts") tslang_add_test(NAME test-compile-00-array-literal-copy COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_literal_copy.ts") +tslang_add_test(NAME test-compile-00-string-view-copy COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_view_copy.ts") tslang_add_test(NAME test-compile-00-array-data-string-alias COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_data_string_alias.ts") tslang_add_test(NAME test-compile-00-arrays7 COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array7.ts") tslang_add_test(NAME test-compile-00-arrays8-tuple-spread COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array8_tuple_spread.ts") @@ -850,6 +851,7 @@ tslang_add_test(NAME test-jit-00-array-rest-copy COMMAND test-runner -jit "${PRO tslang_add_test(NAME test-jit-00-arrays6 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array6.ts") tslang_add_test(NAME test-jit-00-array-static-nested COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_static_nested.ts") tslang_add_test(NAME test-jit-00-array-literal-copy COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_literal_copy.ts") +tslang_add_test(NAME test-jit-00-string-view-copy COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_view_copy.ts") tslang_add_test(NAME test-jit-00-array-data-string-alias COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_data_string_alias.ts") tslang_add_test(NAME test-jit-00-arrays7 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array7.ts") tslang_add_test(NAME test-jit-00-arrays8-tuple-spread COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array8_tuple_spread.ts") @@ -1681,6 +1683,7 @@ set(TSLANG_CORPUS 00array6.ts 00array_static_nested.ts 00array_literal_copy.ts + 00string_view_copy.ts 00array_data_string_alias.ts 00array7.ts 00array8_tuple_spread.ts diff --git a/tslang/test/tester/tests/00string_view_copy.ts b/tslang/test/tester/tests/00string_view_copy.ts new file mode 100644 index 000000000..777bbb217 --- /dev/null +++ b/tslang/test/tester/tests/00string_view_copy.ts @@ -0,0 +1,86 @@ +// A string made over an array's memory with `Ref(buffer[i])` is a view of that +// memory (#481). Under rc it keeps no count; where it is kept - returned, stored, pushed, +// captured - the keeper takes a copy, made after the bytes were written through the view. +declare function strcpy(dst: string, src: string): string; + +class Box { + s: string = ""; +} + +let g = ""; + +function conv(value: string): string { + let buffer: char[] = []; + buffer.length = 50; + const s = Ref(buffer[0]); + strcpy(s, value); + return s; +} + +function convLet(value: string): string { + let buffer: char[] = []; + buffer.length = 50; + let s = Ref(buffer[0]); + strcpy(s, value); + return s; +} + +function tail(): string { + let buffer: char[] = []; + buffer.length = 50; + strcpy(Ref(buffer[0]), "xyz"); + const s = Ref(buffer[1]); + return s; +} + +function keep(b: Box, arr: string[]) { + let buffer: char[] = []; + buffer.length = 50; + const s = Ref(buffer[0]); + strcpy(s, "kept"); + b.s = s; + g = s; + arr.push(s); +} + +// a view is not a block: the word in front of it is element data, not a count +function frontBytes() { + let buffer: char[] = []; + buffer.length = 32; + strcpy(Ref(buffer[0]), "abcdefghijklmnop"); + g = Ref(buffer[8]); + const whole = Ref(buffer[0]); + assert(whole == "abcdefghijklmnop", "the bytes in front of a view are not a count"); + assert(g == "ijklmnop", "a copy of the view"); +} + +// the array grows after its view was kept: the keeper's copy does not hold the old block +function grows() { + let buffer: char[] = []; + buffer.length = 16; + const s = Ref(buffer[0]); + strcpy(s, "grows"); + g = s; + for (let j = 0; j < 64; j++) buffer.push(65); +} + +function main() { + const a = conv("1.5"); + const b = convLet("2.25"); + const t = tail(); + const box = new Box(); + const arr: string[] = []; + keep(box, arr); + + // reuse freed memory: a string left pointing at a freed buffer would show it + const fill: string[] = []; + for (let i = 0; i < 4096; i++) fill.push("filler-" + i); + + assert(a == "1.5" && b == "2.25" && t == "yz", "returned"); + assert(box.s == "kept" && g == "kept" && arr[0] == "kept", "kept"); + + frontBytes(); + for (let i = 0; i < 3; i++) grows(); + + print("done."); +}