From 564c1511173572ddfc754f4508e4ca3e1f53faab Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Mon, 5 Oct 2026 23:07:45 +0100 Subject: [PATCH] String concatenation copies with the lengths it measured, not strcpy/strcat ts.StringConcat measured every operand with strlen to size the result, then copied with strcpy and strcat: unbounded copies, and strcat rescans the result for every operand. Each operand is now copied with llvm.memcpy at its offset, bounded by the length that sized the buffer, and the result gets one terminator at the end. No C string function is left in the lowering. The _itoa, _i64toa and _gcvt conversions are removed: USE_SPRINTF was always defined, so number-to-string has gone through the bounded sprintf_s/snprintf path (ts.ConvertF) on every target, and the MSVC-only helpers were dead code. Co-Authored-By: Claude Opus 5.5 --- tslang/include/TypeScript/Config.h | 7 --- .../TypeScript/LowerToLLVM/ConvertLogic.h | 60 ------------------- tslang/lib/TypeScript/LowerToLLVM.cpp | 35 +++++------ tslang/lib/TypeScript/TypeScriptOps.cpp | 2 +- tslang/test/tester/CMakeLists.txt | 3 + .../tester/tests/00string_concat_bounds.ts | 45 ++++++++++++++ 6 files changed, 63 insertions(+), 89 deletions(-) create mode 100644 tslang/test/tester/tests/00string_concat_bounds.ts diff --git a/tslang/include/TypeScript/Config.h b/tslang/include/TypeScript/Config.h index 6cab1a570..5f859880a 100644 --- a/tslang/include/TypeScript/Config.h +++ b/tslang/include/TypeScript/Config.h @@ -19,13 +19,6 @@ #define ENABLE_ASYNC 1 #define ENABLE_EXCEPTIONS 1 -#define USE_SPRINTF 1 -#ifndef WIN32 -#ifndef USE_SPRINTF -#define USE_SPRINTF 1 -#endif -#endif - #define NUMBER_F64 1 #define ANY_AS_DEFAULT 1 // somehow it will error if set to true diff --git a/tslang/include/TypeScript/LowerToLLVM/ConvertLogic.h b/tslang/include/TypeScript/LowerToLLVM/ConvertLogic.h index 5d84adc21..abf49986c 100644 --- a/tslang/include/TypeScript/LowerToLLVM/ConvertLogic.h +++ b/tslang/include/TypeScript/LowerToLLVM/ConvertLogic.h @@ -35,58 +35,6 @@ class ConvertLogic { } - mlir::Value itoa(mlir::Value value) - { - auto i8PtrTy = th.getPtrType(); - - auto _itoaFuncOp = ch.getOrInsertFunction( - "_itoa", th.getFunctionType(th.getPtrType(), - ArrayRef{rewriter.getI32Type(), th.getPtrType(), rewriter.getI32Type()}, true)); - - auto bufferSizeValue = clh.createI32ConstantOf(50); - // auto newStringValue = ch.Alloca(i8PtrTy, bufferSizeValue, true); - auto newStringValue = ch.MemoryAlloc(bufferSizeValue, MemoryAllocSet::Atomic); - auto base = clh.createI32ConstantOf(10); - - return rewriter.create(loc, _itoaFuncOp, ValueRange{value, newStringValue, base}).getResult(); - } - - mlir::Value i64toa(mlir::Value value) - { - auto i8PtrTy = th.getPtrType(); - - // 64-bit whatever the target: _i64toa's first parameter is `long long` - // (char *_i64toa(long long value, char *str, int radix)) - a fixed 64-bit integer on every - // target this compiler emits for, not a size_t. - auto _i64toaFuncOp = ch.getOrInsertFunction( - "_i64toa", th.getFunctionType(th.getPtrType(), - ArrayRef{rewriter.getI64Type(), th.getPtrType(), rewriter.getI32Type()}, true)); - - auto bufferSizeValue = clh.createI32ConstantOf(50); - // auto newStringValue = ch.Alloca(i8PtrTy, bufferSizeValue, true); - auto newStringValue = ch.MemoryAlloc(bufferSizeValue, MemoryAllocSet::Atomic); - auto base = clh.createI32ConstantOf(10); - - return rewriter.create(loc, _i64toaFuncOp, ValueRange{value, newStringValue, base}).getResult(); - } - - mlir::Value gcvt(mlir::Value in) - { - auto i8PtrTy = th.getPtrType(); - - auto _gcvtFuncOp = ch.getOrInsertFunction( - "_gcvt", th.getFunctionType(th.getPtrType(), - ArrayRef{rewriter.getF64Type(), rewriter.getI32Type(), th.getPtrType()}, true)); - - auto bufferSizeValue = clh.createI32ConstantOf(50); - // auto newStringValue = ch.Alloca(i8PtrTy, bufferSizeValue, true); - auto newStringValue = ch.MemoryAlloc(bufferSizeValue, MemoryAllocSet::Atomic); - auto doubleValue = rewriter.create(loc, rewriter.getF64Type(), in); - auto precision = clh.createI32ConstantOf(16); - - return rewriter.create(loc, _gcvtFuncOp, ValueRange{doubleValue, precision, newStringValue}).getResult(); - } - mlir::Value sprintf(int buffSize, std::string format, mlir::Value value) { auto i8PtrTy = th.getPtrType(); @@ -167,20 +115,12 @@ class ConvertLogic mlir::Value intToString(mlir::Value value, int width, bool isSigned) { -#ifndef USE_SPRINTF - return itoa(value); -#else return sprintfOfInt(value, width, isSigned); -#endif } mlir::Value f64ToString(mlir::Value value) { -#ifndef USE_SPRINTF - return gcvt(value); -#else return sprintfOfF64(value); -#endif } }; } // namespace typescript diff --git a/tslang/lib/TypeScript/LowerToLLVM.cpp b/tslang/lib/TypeScript/LowerToLLVM.cpp index cf246454e..beb8b1240 100644 --- a/tslang/lib/TypeScript/LowerToLLVM.cpp +++ b/tslang/lib/TypeScript/LowerToLLVM.cpp @@ -837,8 +837,6 @@ class StringConcatOpLowering : public TsLlvmPattern auto llvmIndexType = tch.convertType(th.getIndexType()); auto strlenFuncOp = ch.getOrInsertFunction("strlen", th.getFunctionType(llvmIndexType, {i8PtrTy})); - auto strcpyFuncOp = ch.getOrInsertFunction("strcpy", th.getFunctionType(i8PtrTy, {i8PtrTy, i8PtrTy})); - auto strcatFuncOp = ch.getOrInsertFunction("strcat", th.getFunctionType(i8PtrTy, {i8PtrTy, i8PtrTy})); SmallVector opers; for (auto oper : transformed.getOps()) @@ -846,12 +844,14 @@ class StringConcatOpLowering : public TsLlvmPattern opers.push_back(nullStringAsText(loc, oper, rewriter, tch)); } + // each operand is measured once: the lengths size the buffer and bound every copy into it + SmallVector lengths; mlir::Value size = clh.createIndexConstantOf(llvmIndexType, 1); - // calc size for (auto oper : opers) { - auto size1 = rewriter.create(loc, strlenFuncOp, oper); - size = rewriter.create(loc, llvmIndexType, ValueRange{size, size1.getResult()}); + auto length = rewriter.create(loc, strlenFuncOp, oper).getResult(); + lengths.push_back(length); + size = rewriter.create(loc, llvmIndexType, ValueRange{size, length}); } auto allocInStack = op.getAllocInStack().has_value() && op.getAllocInStack().value() @@ -860,25 +860,18 @@ class StringConcatOpLowering : public TsLlvmPattern mlir::Value newStringValue = allocInStack ? ch.Alloca(th.getI8Type(), size, true) : ch.MemoryAlloc(size); - // copy - auto concat = false; - auto result = newStringValue; - for (auto oper : opers) + // copy: each operand at its offset, without its terminator, then one terminator at the end + mlir::Value offset = clh.createIndexConstantOf(llvmIndexType, 0); + for (auto [oper, length] : llvm::zip(opers, lengths)) { - if (concat) - { - auto callResult = rewriter.create(loc, strcatFuncOp, ValueRange{result, oper}); - result = callResult.getResult(); - } - else - { - auto callResult = rewriter.create(loc, strcpyFuncOp, ValueRange{result, oper}); - result = callResult.getResult(); - } - - concat = true; + auto dest = rewriter.create(loc, i8PtrTy, th.getI8Type(), newStringValue, ValueRange{offset}); + rewriter.create(loc, dest, oper, length, /*isVolatile=*/false); + offset = rewriter.create(loc, llvmIndexType, ValueRange{offset, length}); } + auto end = rewriter.create(loc, i8PtrTy, th.getI8Type(), newStringValue, ValueRange{offset}); + rewriter.create(loc, clh.createI8ConstantOf(0), end); + rewriter.replaceOp(op, ValueRange{newStringValue}); return success(); diff --git a/tslang/lib/TypeScript/TypeScriptOps.cpp b/tslang/lib/TypeScript/TypeScriptOps.cpp index 017b81fd2..8cbd92b37 100644 --- a/tslang/lib/TypeScript/TypeScriptOps.cpp +++ b/tslang/lib/TypeScript/TypeScriptOps.cpp @@ -76,7 +76,7 @@ bool mlir_ts::isEmpty(mlir::Region &condtion) return false; } -// Only the printing conversions do: `ConvertLogic`'s itoa and f64ToString, and `ts.CharToString`, +// Only the printing conversions do: `ConvertLogic`'s intToString and f64ToString, and `ts.CharToString`, // each allocate a buffer and write into it. Everything else that reaches the plain cast - a // boolean, `undefined`, a string literal - hands back a global, which is immortal and owns // nothing. A literal is asked about by its element type, since that is what the lowering unwraps diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index c61c1a9e0..cbcc095b5 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -359,6 +359,7 @@ tslang_add_test(NAME test-compile-00-for-optional-class-condition COMMAND test-r tslang_add_test(NAME test-compile-00-for-condition-narrowing COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00for_condition_narrowing.ts") tslang_add_test(NAME test-compile-00-param-assigned-owned COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00param_assigned_owned.ts") tslang_add_test(NAME test-compile-00-string-empty-falsy COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_empty_falsy.ts") +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-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-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-array-length-valid COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_valid.ts") @@ -876,6 +877,7 @@ tslang_add_test(NAME test-jit-00-for-optional-class-condition COMMAND test-runne tslang_add_test(NAME test-jit-00-for-condition-narrowing COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00for_condition_narrowing.ts") tslang_add_test(NAME test-jit-00-param-assigned-owned COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00param_assigned_owned.ts") tslang_add_test(NAME test-jit-00-string-empty-falsy COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_empty_falsy.ts") +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-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-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-array-length-valid COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_valid.ts") @@ -1722,6 +1724,7 @@ set(TSLANG_CORPUS 00for_condition_narrowing.ts 00param_assigned_owned.ts 00string_empty_falsy.ts + 00string_concat_bounds.ts 00narrowed_assign_other_member.ts 00const_record_owned_fields.ts 00array_length_valid.ts diff --git a/tslang/test/tester/tests/00string_concat_bounds.ts b/tslang/test/tester/tests/00string_concat_bounds.ts new file mode 100644 index 000000000..b9749313a --- /dev/null +++ b/tslang/test/tester/tests/00string_concat_bounds.ts @@ -0,0 +1,45 @@ +// `+` on strings copies each operand at its own offset with the length it measured, then ends the +// result with one terminator: empty and null operands, many operands, a result built in a loop, +// and results kept on the stack or the heap all hold exactly the bytes of their operands +function concat3(a: string, b: string, c: string) { + return a + b + c; +} + +function build(count: number) { + let s = ""; + for (let i = 0; i < count; i++) { + s = s + "ab" + ""; + } + + return s; +} + +function main() { + const empty = ""; + const a = "a"; + const bc = "bc"; + + assert(concat3(a, bc, "def") == "abcdef", "three operands"); + assert(concat3(a, bc, "def").length == 6, "three operands: length"); + assert(concat3(empty, empty, empty) == "", "all empty"); + assert(concat3(empty, empty, empty).length == 0, "all empty: length"); + assert(concat3(empty, bc, empty) == "bc", "empty around a string"); + assert(concat3(a, empty, a) == "aa", "empty between strings"); + + const many = a + bc + a + bc + a + bc + a + bc + a + bc; + assert(many == "abcabcabcabcabc", "ten operands"); + assert(many.length == 15, "ten operands: length"); + + const n: string | null = null; + assert(a + n == "anull", "null operand prints as null"); + assert((n + empty).length == 4, "null then empty: length"); + + const built = build(100); + assert(built.length == 200, "built in a loop: length"); + assert(build(3) == "ababab", "built in a loop: content"); + + const withNumber = a + 12 + bc; + assert(withNumber == "a12bc", "number operand"); + + print("done."); +}