Fix memmove sizes in array shift, unshift and splice

tslang/lib/TypeScript/LowerToLLVM.cpp

@@ -2982,8 +2982,11 @@ struct ArrayUnshiftOpLowering : public TsLlvmPattern<mlir_ts::ArrayUnshiftOp>
         auto offset0 = allocated;
         auto offsetN = rewriter.create<LLVM::GEPOp>(loc, th.getPtrType(), llvmElementType, allocated, ValueRange{incSize});
 
-        auto newCountAsIndexTypeAdapt = rewriter.create<mlir_ts::DialectCastOp>(loc, th.getIndexType(), newCountAsIndexType);
-        rewriter.create<mlir_ts::MemoryMoveOp>(loc, offsetN, offset0, newCountAsIndexTypeAdapt);
+        // shift the existing elements up: only the old count is there to move - moving the new count wrote
+        // `incSize` elements past the end of the block (caught by the debug heap under -mm=rc)
+        auto moveBytes = rewriter.create<LLVM::MulOp>(loc, llvmIndexType, ValueRange{sizeOfTypeValue, countAsIndexType});
+        auto moveBytesAdapt = rewriter.create<mlir_ts::DialectCastOp>(loc, th.getIndexType(), moveBytes);
+        rewriter.create<mlir_ts::MemoryMoveOp>(loc, offsetN, offset0, moveBytesAdapt);
 
         mlir::Value index = clh.createIndexConstantOf(llvmIndexType, 0);
         auto next = false;
@@ -3079,8 +3082,8 @@ struct ArrayShiftOpLowering : public TsLlvmPattern<mlir_ts::ArrayShiftOp>
         auto multSizeOfTypeValue =
             rewriter.create<LLVM::MulOp>(loc, llvmIndexType, ValueRange{sizeOfTypeValue, newCountAsIndexType});
 
-        auto newCountAsIndexTypeAdapt = rewriter.create<mlir_ts::DialectCastOp>(loc, th.getIndexType(), newCountAsIndexType);
-        rewriter.create<mlir_ts::MemoryMoveOp>(loc, offset0, offset1, newCountAsIndexTypeAdapt);
+        auto multSizeOfTypeValueAdapt = rewriter.create<mlir_ts::DialectCastOp>(loc, th.getIndexType(), multSizeOfTypeValue);
+        rewriter.create<mlir_ts::MemoryMoveOp>(loc, offset0, offset1, multSizeOfTypeValueAdapt);
 
         auto allocated = ch.MemoryRealloc(currentPtr, multSizeOfTypeValue);
 
@@ -3187,8 +3190,9 @@ struct ArraySpliceOpLowering : public TsLlvmPattern<mlir_ts::ArraySpliceOp>
             auto offsetStart = rewriter.create<LLVM::GEPOp>(loc, ptrType, llvmElementType, allocated, ValueRange{startIndexAsLLVMType});
             auto offsetFrom = rewriter.create<LLVM::GEPOp>(loc, ptrType, llvmElementType, offsetStart, ValueRange{decSizeAsLLVMType});
             auto offsetTo = rewriter.create<LLVM::GEPOp>(loc, ptrType, llvmElementType, offsetStart, ValueRange{incSizeAsLLVMType});
-            auto moveCountAsIndexTypeAdapt = rewriter.create<mlir_ts::DialectCastOp>(loc, indexType, moveCountAsLLVMType);
-            rewriter.create<mlir_ts::MemoryMoveOp>(loc, offsetTo, offsetFrom, moveCountAsIndexTypeAdapt);
+            auto moveBytes = rewriter.create<LLVM::MulOp>(loc, llvmIndexType, ValueRange{sizeOfTypeValue, moveCountAsLLVMType});
+            auto moveBytesAdapt = rewriter.create<mlir_ts::DialectCastOp>(loc, indexType, moveBytes);
+            rewriter.create<mlir_ts::MemoryMoveOp>(loc, offsetTo, offsetFrom, moveBytesAdapt);
 
             return allocated;
         };
@@ -3205,8 +3209,9 @@ struct ArraySpliceOpLowering : public TsLlvmPattern<mlir_ts::ArraySpliceOp>
             auto offsetFrom = rewriter.create<LLVM::GEPOp>(loc, ptrType, llvmElementType, offsetStart, ValueRange{decSizeAsLLVMType});
             auto offsetTo = rewriter.create<LLVM::GEPOp>(loc, ptrType, llvmElementType, offsetStart, ValueRange{incSizeAsLLVMType});
 
-            auto moveCountAsIndexTypeAdapt = rewriter.create<mlir_ts::DialectCastOp>(loc, indexType, moveCountAsLLVMType);
-            rewriter.create<mlir_ts::MemoryMoveOp>(loc, offsetTo, offsetFrom, moveCountAsIndexTypeAdapt);
+            auto moveBytes = rewriter.create<LLVM::MulOp>(loc, llvmIndexType, ValueRange{sizeOfTypeValue, moveCountAsLLVMType});
+            auto moveBytesAdapt = rewriter.create<mlir_ts::DialectCastOp>(loc, indexType, moveBytes);
+            rewriter.create<mlir_ts::MemoryMoveOp>(loc, offsetTo, offsetFrom, moveBytesAdapt);
 
             auto allocated = ch.MemoryRealloc(currentPtr, multSizeOfTypeValue);
             return allocated;
@@ -4606,16 +4611,10 @@ struct MemoryMoveOpLowering : public TsLlvmPattern<mlir_ts::MemoryMoveOp>
         values.push_back(transformed.getDst());
         values.push_back(transformed.getSrc());
 
-        auto countAsIndexType = memoryMoveOp.getCount();
-
-        auto llvmSrcType = tch.convertType(memoryMoveOp.getSrc().getType());
-        auto srcSizeMLIR = rewriter.create<mlir_ts::SizeOfOp>(loc, th.getIndexType(), transformed.getSrc().getType());
-        auto srcSize = rewriter.create<mlir_ts::DialectCastOp>(loc, llvmIndexType, srcSizeMLIR);
-        auto countAsIndexLLVMType = rewriter.create<mlir_ts::DialectCastOp>(loc, llvmIndexType, countAsIndexType);
-        auto multSizeOfTypeValue =
-            rewriter.create<LLVM::MulOp>(loc, llvmIndexType, ValueRange{srcSize, countAsIndexLLVMType});
-
-        values.push_back(multSizeOfTypeValue);
+        // the count is in bytes, as for MemoryCopyOp: the operands are plain pointers, so the element
+        // size is only known to the caller. Scaling by the size of the source's type scaled by a
+        // pointer's size, which moved half of every 16-byte element (e.g. a tuple) in shift/unshift/splice.
+        values.push_back(transformed.getCount());
 
         auto immarg = clh.createI1ConstantOf(false);
         values.push_back(immarg);

tslang/test/tester/CMakeLists.txt

@@ -171,6 +171,7 @@ add_test(NAME test-compile-00-funcs-nesting-generic COMMAND test-runner "${PROJE
 add_test(NAME test-compile-00-funcs-nesting-capture COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_nesting_capture.ts")
 add_test(NAME test-compile-00-funcs-hybrid-null-this COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_hybrid_null_this.ts")
 add_test(NAME test-compile-00-typeof-static-fold COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00typeof_static_fold.ts")
+add_test(NAME test-compile-00-array-move-wide-elements COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_move_wide_elements.ts")
 add_test(NAME test-compile-00-funcs-expression-generic COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_expression_generic.ts")
 add_test(NAME test-compile-00-funcs-expression-iterator COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_expression_iterator.ts")
 add_test(NAME test-compile-00-arrow-generic COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00arrow_generic.ts")
@@ -587,6 +588,7 @@ add_test(NAME test-jit-00-funcs-nesting-generic COMMAND test-runner -jit "${PROJ
 add_test(NAME test-jit-00-funcs-nesting-capture COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_nesting_capture.ts")
 add_test(NAME test-jit-00-funcs-hybrid-null-this COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_hybrid_null_this.ts")
 add_test(NAME test-jit-00-typeof-static-fold COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00typeof_static_fold.ts")
+add_test(NAME test-jit-00-array-move-wide-elements COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_move_wide_elements.ts")
 add_test(NAME test-jit-00-funcs-expression-generic COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_expression_generic.ts")
 add_test(NAME test-jit-00-funcs-expression-iterator COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00funcs_expression_iterator.ts")
 add_test(NAME test-jit-00-arrow-generic COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00arrow_generic.ts")
@@ -1322,6 +1324,7 @@ set(TSLANG_CORPUS
     00any.ts
     00array_assignment5.ts
     00array_cond_access.ts
+    00array_move_wide_elements.ts
     00array_of.ts
     00array_shift.ts
     00array_splice.ts

tslang/test/tester/tests/00array_move_wide_elements.ts

@@ -0,0 +1,30 @@
+// shift, unshift and splice move the elements after the changed position with memmove. The move
+// was sized by a pointer rather than by the element, so for elements wider than 8 bytes (a tuple
+// here) only part of each element moved, and unshift moved one element count too many, writing
+// past the end of the block (under -mm=rc the debug heap caught that on free).
+function main() {
+    let s: [number, string][] = [[1, "a"], [2, "b"], [3, "c"]];
+    s.shift();
+    assert(s.length == 2, "shift length");
+    assert(s[0][0] == 2 && s[0][1] == "b", "shift first");
+    assert(s[1][0] == 3 && s[1][1] == "c", "shift second");
+
+    let u: [number, string][] = [[2, "b"], [3, "c"]];
+    u.unshift([1, "a"]);
+    assert(u.length == 3, "unshift length");
+    assert(u[0][0] == 1 && u[0][1] == "a", "unshift first");
+    assert(u[1][0] == 2 && u[1][1] == "b", "unshift second");
+    assert(u[2][0] == 3 && u[2][1] == "c", "unshift third");
+
+    let g: [number, string][] = [[1, "a"], [4, "d"]];
+    g.splice(1, 0, [2, "b"], [3, "c"]);
+    assert(g.length == 4, "splice grow length");
+    assert(g[1][0] == 2 && g[2][1] == "c" && g[3][0] == 4 && g[3][1] == "d", "splice grow");
+
+    let k: [number, string][] = [[1, "a"], [2, "b"], [3, "c"], [4, "d"]];
+    k.splice(1, 2);
+    assert(k.length == 2, "splice shrink length");
+    assert(k[0][0] == 1 && k[1][0] == 4 && k[1][1] == "d", "splice shrink");
+
+    print("done.");
+}