From 14ca94827874b4ebff37ef0974cdef4a1cf4701f Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Wed, 30 Sep 2026 13:24:19 +0100 Subject: [PATCH 1/2] Copy owning elements by assignment, not memcpy; String.repeat stops one copy short Array.concat, slice and copyWithin, and Set and Map growing their entries, copied elements with memcpy/memmove. Under -mm=rc an element can own what it holds (a string built at run time, an `any` box, a Set entry's value), and a raw copy takes no reference to it: the copy and the original then both release it, which freed it early and corrupted the heap - set_union and set_symmetricDifference crashed at JIT teardown, and concat of non-literal strings double-freed. The elements are now copied one assignment at a time, which takes and gives up references as every store does and is the same copy under gc. copyWithin copies backwards when the ranges overlap that way, as memmove did, and no longer copies past the end of the array. String.repeat wrote `count` copies after the one clone() had already placed, running one copy past the end of the string it had sized; it now writes count - 1. New test owning_copies.ts (non-literal strings through concat, slice, copyWithin, and a growing Set and Map; a literal is immortal and would hide the bug), and string_repeat.ts now asserts its results. Needs the compiler changes for `s.length = n`, rest-parameter packing and grown array slots (TypeScriptCompiler PR) for the rc tests to pass. Co-Authored-By: Claude Opus 5.5 --- src/generics/lib.generics.ts | 33 ++++++++++++++++----- src/lib.ts | 4 ++- tests/owning_copies.ts | 57 ++++++++++++++++++++++++++++++++++++ tests/string_repeat.ts | 7 ++++- 4 files changed, 91 insertions(+), 10 deletions(-) create mode 100644 tests/owning_copies.ts diff --git a/src/generics/lib.generics.ts b/src/generics/lib.generics.ts index 1f87e4c..4376a2d 100644 --- a/src/generics/lib.generics.ts +++ b/src/generics/lib.generics.ts @@ -71,7 +71,17 @@ namespace __Array { end = this.length; } - memmove(Ref(this[target]), Ref(this[start]), sizeof() * (end - start)); + // element by element, not memmove: under rc an element may own what it holds, and an + // assignment is what takes a reference to the copy and gives up the one overwritten. + // Backwards when the ranges overlap that way, as memmove would. + let count = end - start; + if (count > this.length - target) count = this.length - target; + if (target > start) { + for (let i = count - 1; i >= 0; i--) this[target + i] = this[start + i]; + } else { + for (let i = 0; i < count; i++) this[target + i] = this[start + i]; + } + return this; } @@ -82,12 +92,12 @@ namespace __Array { count += item.length; let newArray: T[] = []; newArray.length = count; + // element by element, not memcpy: under rc an element may own what it holds, and the new + // array's store is what takes the reference its release will give back let index = 0; - memcpy(Ref(newArray[index]), Ref(this[0]), sizeof() * this.length); - index += this.length; + for (const v of this) newArray[index++] = v; for (const item of other) { - memcpy(Ref(newArray[index]), Ref(item[0]), sizeof() * item.length); - index += item.length; + for (const v of item) newArray[index++] = v; } return newArray; @@ -340,7 +350,8 @@ namespace __Array { let newArray: T[] = []; newArray.length = end - start; - memcpy(Ref(newArray[0]), Ref(this[start]), sizeof() * (end - start)); + // element by element, not memcpy (see concat) + for (let i = start; i < end; i++) newArray[i - start] = this[i]; return newArray; } @@ -821,7 +832,10 @@ class Map { entries.length = newSize; const count = this.count; - memcpy(Ref(entries[0]), Ref(this._entries[0]), sizeof() * count); + // entry by entry, not memcpy: under rc an entry owns its key and value, and the old array + // gives them up when it is replaced below - the new one has to have taken its own + const oldEntries = this._entries; + for (let i = 0; i < count; i++) entries[i] = oldEntries[i]; if (forceNewHashCodes) { this.newHashCodes(entries); @@ -1243,7 +1257,10 @@ class Set { entries.length = newSize; const count = this.count; - memcpy(Ref(entries[0]), Ref(this._entries[0]), sizeof() * count); + // entry by entry, not memcpy: under rc an entry owns its key and value, and the old array + // gives them up when it is replaced below - the new one has to have taken its own + const oldEntries = this._entries; + for (let i = 0; i < count; i++) entries[i] = oldEntries[i]; if (forceNewHashCodes) { this.newHashCodes(entries); diff --git a/src/lib.ts b/src/lib.ts index d0c91f5..4e19ade 100644 --- a/src/lib.ts +++ b/src/lib.ts @@ -842,7 +842,9 @@ namespace __String { const newString = this.clone().resize(newSize); let index = this.length; const byteSize = sizeof() * this.length; - for (let i = 0; i < count; i++) { + // the clone holds the first copy already: count - 1 more fill the string, and one more + // wrote past its end + for (let i = 1; i < count; i++) { memcpy(Ref(newString[index]), this, byteSize); index += this.length; } diff --git a/tests/owning_copies.ts b/tests/owning_copies.ts new file mode 100644 index 0000000..9ba5ade --- /dev/null +++ b/tests/owning_copies.ts @@ -0,0 +1,57 @@ +// Copies of elements that own what they hold. Array.concat, slice and copyWithin, and Set and Map +// growing their entries, copied elements with memcpy - under rc that duplicated references nobody +// took, so the copy and the original released the same strings and the heap was corrupted. Every +// string here is built at run time: a literal is immortal and would hide it. + +function made(prefix: string, n: int): string[] { + const result: string[] = []; + for (let i = 0; i < n; i++) result.push(prefix + i); + return result; +} + +function concatHoldsItsOwn() { + const a = made("a", 3); + const joined = a.concat(made("b", 2), made("c", 1)); + return joined.join() == "a0,a1,a2,b0,b1,c0" && a.join() == "a0,a1,a2"; +} + +function sliceHoldsItsOwn() { + const a = made("s", 5); + const part = a.slice(1, 4); + return part.join() == "s1,s2,s3" && a.join() == "s0,s1,s2,s3,s4"; +} + +function copyWithinKeepsEveryElement() { + const a = made("w", 5); + a.copyWithin(0, 3); + const b = made("v", 5); + b.copyWithin(2, 0); + return a.join() == "w3,w4,w2,w3,w4" && b.join() == "v0,v1,v0,v1,v2"; +} + +function setGrowsWithItsStrings() { + const s = new Set(); + for (const v of made("e", 20)) s.add(v); + let all = ""; + for (const v of s) all += v; + return s.size == 20 && s.has("e7") && s.has("e19") && all.length == 50; +} + +function mapGrowsWithItsStrings() { + const m = new Map(); + for (let i = 0; i < 20; i++) m.set("k" + i, "v" + i); + return m.size == 20 && m.get("k7") == "v7" && m.get("k19") == "v19"; +} + +function main() { + // twice over, so the second round allocates over whatever the first freed + for (let round = 0; round < 2; round++) { + assert(concatHoldsItsOwn(), "concat"); + assert(sliceHoldsItsOwn(), "slice"); + assert(copyWithinKeepsEveryElement(), "copyWithin"); + assert(setGrowsWithItsStrings(), "Set growth"); + assert(mapGrowsWithItsStrings(), "Map growth"); + } + + console.log("ALL DONE"); +} diff --git a/tests/string_repeat.ts b/tests/string_repeat.ts index 4b8315b..33c4d5c 100644 --- a/tests/string_repeat.ts +++ b/tests/string_repeat.ts @@ -6,6 +6,11 @@ function main() { console.log("abc".repeat(0)); // '' console.log("abc".repeat(1)); // 'abc' - console.log("abc".repeat(2)); // 'abcabc' + console.log("abc".repeat(2)); // 'abcabc' + + assert(mood.repeat(3) == "Happy! Happy! Happy! "); + assert("abc".repeat(0) == ""); + assert("abc".repeat(1) == "abc"); + assert("abc".repeat(2) == "abcabc"); console.log("ALL DONE"); } From ee419b976f2de5048d02f40fd731112d7f40e81f Mon Sep 17 00:00:00 2001 From: Alex D Date: Wed, 30 Sep 2026 15:07:25 +0100 Subject: [PATCH 2/2] Array.fill stops before `end` (#15) fill's loop ran `i <= end`, so it filled one element past the range, and with the default `end = this.length` it wrote one element past the end of the array it had just copied: a heap overrun, usually hidden by the allocator rounding the block up, and the likely cause of array_fill's intermittent 0xC0000374 under -mm=rc. A guard-page heap catches the write every run. It now runs `i < end`. array_fill.ts now asserts its results and adds `[1, 2, 3].fill(9, 1, 2)`, which gave [1, 9, 9] before. Co-authored-by: Claude Opus 5.5 --- src/generics/lib.generics.ts | 2 +- tests/array_fill.ts | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/generics/lib.generics.ts b/src/generics/lib.generics.ts index 4376a2d..f59534c 100644 --- a/src/generics/lib.generics.ts +++ b/src/generics/lib.generics.ts @@ -141,7 +141,7 @@ namespace __Array { end = this.length; } - for (let i = start; i <= end; i++) + for (let i = start; i < end; i++) newArray[i] = value; return newArray; diff --git a/tests/array_fill.ts b/tests/array_fill.ts index 4d9986d..bdef201 100644 --- a/tests/array_fill.ts +++ b/tests/array_fill.ts @@ -2,14 +2,27 @@ function main() { const array1 = [1, 2, 3, 4]; // Fill with 0 from position 2 until position 4 - console.log(array1.fill(0, 2, 4)); + const zeros = array1.fill(0, 2, 4); + console.log(zeros); // Expected output: Array [1, 2, 0, 0] + assert(zeros.length == 4 && zeros[0] == 1 && zeros[1] == 2 && zeros[2] == 0 && zeros[3] == 0); // Fill with 5 from position 1 - console.log(array1.fill(5, 1)); + const fives = array1.fill(5, 1); + console.log(fives); // Expected output: Array [1, 5, 5, 5] + assert(fives.length == 4 && fives[0] == 1 && fives[1] == 5 && fives[2] == 5 && fives[3] == 5); - console.log(array1.fill(6)); + const sixes = array1.fill(6); + console.log(sixes); // Expected output: Array [6, 6, 6, 6] + assert(sixes.length == 4 && sixes[0] == 6 && sixes[1] == 6 && sixes[2] == 6 && sixes[3] == 6); + + // `end` is not filled: this wrote one past it, and past the end of the array when `end` is its length + const inner = [1, 2, 3].fill(9, 1, 2); + console.log(inner); + // Expected output: Array [1, 9, 3] + assert(inner.length == 3 && inner[0] == 1 && inner[1] == 9 && inner[2] == 3); + console.log("ALL DONE"); }