String concatenation copies with the lengths it measured, not strcpy/strcat - #520
Merged
Merged
Conversation
…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 <noreply@anthropic.com>
2 of 3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ts.StringConcatmeasured each operand withstrlento size the result, then copied withstrcpy/strcat. Those copies were unbounded, andstrcatrescans the result for every operand. Each operand is now copied withllvm.memcpyat its offset, bounded by the length that sized the buffer, and the result gets one terminator. No C string function is left in the lowering._itoa/_i64toa/_gcvtinConvertLogic.hare removed, along with theUSE_SPRINTFswitch.USE_SPRINTFwas always defined, so number-to-string already went through the boundedsprintf_s/snprintfpath (ts.ConvertF) on every target.These are the compiler half of a bounds-correctness pass. The DefaultLib half is ASDAlexander77/TypeScriptCompilerDefaultLib (branch
string-bounds-checks, fixes ASDAlexander77/TypeScriptCompilerDefaultLib#27). The two are independent.Test plan
00string_concat_bounds.ts(compile + jit, and inTSLANG_CORPUSfor rc/none): empty, null, many and number operands, a result built in a loop--emit=llvmhas nostrcpy/strcatcallsctest -C Release: 3898/3898🤖 Generated with Claude Code