Copy owning elements by assignment, not memcpy; String.repeat stops one copy short

src/generics/lib.generics.ts

@@ -71,7 +71,17 @@ namespace __Array {
             end = this.length;
         }
 
-        memmove(Ref(this[target]), Ref(this[start]), sizeof<T>() * (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<T>() * 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<T>() * item.length);
-            index += item.length;
+            for (const v of item) newArray[index++] = v;
         }
 
         return newArray;
@@ -131,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;
@@ -340,7 +350,8 @@ namespace __Array {
 
         let newArray: T[] = [];
         newArray.length = end - start;
-        memcpy(Ref(newArray[0]), Ref(this[start]), sizeof<T>() * (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<K = any, V = any> {
         entries.length = newSize;
 
         const count = this.count;
-        memcpy(Ref(entries[0]), Ref(this._entries[0]), sizeof<typeof entries[0]>() * 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<V = any> {
         entries.length = newSize;
 
         const count = this.count;
-        memcpy(Ref(entries[0]), Ref(this._entries[0]), sizeof<typeof entries[0]>() * 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);

src/lib.ts

@@ -842,7 +842,9 @@ namespace __String {
         const newString = this.clone().resize(newSize);        
         let index = this.length;
         const byteSize = sizeof<char>() * 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;
         }

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

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<string>();
+    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<string, string>();
+    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");
+}

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