diff --git a/tslang/include/TypeScript/TypeScriptOps.h b/tslang/include/TypeScript/TypeScriptOps.h index dfdb44212..de3b0885f 100644 --- a/tslang/include/TypeScript/TypeScriptOps.h +++ b/tslang/include/TypeScript/TypeScriptOps.h @@ -58,6 +58,9 @@ void buildTerminatedBody(OpBuilder &builder, Location loc); bool isTrue(mlir::Region &); bool isEmpty(mlir::Region &); +// Does a cast from this type to `string` build a new string (a number or a char printed into one)? +bool castToStringAllocates(mlir::Type valueType); + namespace detail { struct ObjectStorageTypeStorage : public ::mlir::TypeStorage diff --git a/tslang/lib/TypeScript/MLIRGenCast.cpp b/tslang/lib/TypeScript/MLIRGenCast.cpp index d96512926..0b647d579 100644 --- a/tslang/lib/TypeScript/MLIRGenCast.cpp +++ b/tslang/lib/TypeScript/MLIRGenCast.cpp @@ -739,7 +739,7 @@ namespace mlirgen // not received by anything, so not a receiver's to release, and not marked, so not // §9.30's either. It is the plainest allocation in the language and it leaked every // time (§9.37). - if (isa(type) && castToStringAllocates(valueType)) + if (isa(type) && mlir_ts::castToStringAllocates(valueType)) { markFreshBlockOwned(location, castResult); } diff --git a/tslang/lib/TypeScript/MLIRGenImpl.h b/tslang/lib/TypeScript/MLIRGenImpl.h index 54573c20a..2e34ce712 100644 --- a/tslang/lib/TypeScript/MLIRGenImpl.h +++ b/tslang/lib/TypeScript/MLIRGenImpl.h @@ -1055,23 +1055,6 @@ class MLIRGenImpl } } - // Does building a `string` out of a value of this type allocate a new one? - // - // Only the printing conversions do: `ConvertLogic`'s itoa 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 it to before choosing. - static bool castToStringAllocates(mlir::Type valueType) - { - if (auto literalType = dyn_cast(valueType)) - { - valueType = literalType.getElementType(); - } - - return isa(valueType) || valueType.isIntOrIndex(); - } - // Gives a freshly built string, or a freshly made `any` box, the same standing as every other // producer of a new heap value: the retain makes the reference real, and the mark says a // receiver may take it over rather than adding one of its own - which is also what lets §9.30 diff --git a/tslang/lib/TypeScript/OwnershipInferencePass.cpp b/tslang/lib/TypeScript/OwnershipInferencePass.cpp index ddedc8c46..2842e4adf 100644 --- a/tslang/lib/TypeScript/OwnershipInferencePass.cpp +++ b/tslang/lib/TypeScript/OwnershipInferencePass.cpp @@ -1447,7 +1447,7 @@ class OwnershipInferencePass : public mlir::PassWrapper(user)) + mlir_ts::StringResizeOp, mlir_ts::StringLengthOp>(user)) { return true; } diff --git a/tslang/lib/TypeScript/TypeScriptOps.cpp b/tslang/lib/TypeScript/TypeScriptOps.cpp index d19957ca5..017b81fd2 100644 --- a/tslang/lib/TypeScript/TypeScriptOps.cpp +++ b/tslang/lib/TypeScript/TypeScriptOps.cpp @@ -76,6 +76,21 @@ bool mlir_ts::isEmpty(mlir::Region &condtion) return false; } +// Only the printing conversions do: `ConvertLogic`'s itoa 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 +// it to before choosing. +bool mlir_ts::castToStringAllocates(mlir::Type valueType) +{ + if (auto literalType = dyn_cast(valueType)) + { + valueType = literalType.getElementType(); + } + + return isa(valueType) || valueType.isIntOrIndex(); +} + //===----------------------------------------------------------------------===// // Types //===----------------------------------------------------------------------===// @@ -843,12 +858,17 @@ bool mlir_ts::CastOp::areCastCompatible(TypeRange inputs, TypeRange outputs) // frees it) but it is an allocation being reported as pure either way. See // docs/reference-counting-evaluation.md section 9.48. // +// A number or a char cast to `string` is the third: it prints into a new string. `switch (c)` on +// a char with string cases casts `c` once per case, and merged into one, the first case's release +// freed the string every later case then compared (RegExp's flags loop under `-mm=rc`). +// // All other CastOp shapes are true value-preserving casts with no allocation, so they keep // reporting no effects. void mlir_ts::CastOp::getEffects(SmallVectorImpl> &effects) { auto allocates = (isa(getIn().getType()) && isa(getRes().getType())) || - isa(getRes().getType()); + isa(getRes().getType()) || + (isa(getRes().getType()) && castToStringAllocates(getIn().getType())); if (allocates) { auto result = cast(getRes()); diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 6ccbcb978..a90f0690b 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -432,6 +432,7 @@ tslang_add_test(NAME test-compile-00-owned-strings COMMAND test-runner "${PROJEC tslang_add_test(NAME test-compile-00-string-length-resize COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_length_resize.ts") tslang_add_test(NAME test-compile-00-array-length-grow-zeroed COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_grow_zeroed.ts") tslang_add_test(NAME test-compile-00-rest-args-pack-owned COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00rest_args_pack_owned.ts") +tslang_add_test(NAME test-compile-00-switch-char-string-cases COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00switch_char_string_cases.ts") tslang_add_test(NAME test-compile-00-owned-generators COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_generators.ts") tslang_add_test(NAME test-compile-00-owned-globals COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_globals.ts") tslang_add_test(NAME test-compile-00-owned-generator-locals COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_generator_locals.ts") @@ -891,6 +892,7 @@ tslang_add_test(NAME test-jit-00-owned-strings COMMAND test-runner -jit "${PROJE tslang_add_test(NAME test-jit-00-string-length-resize COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_length_resize.ts") tslang_add_test(NAME test-jit-00-array-length-grow-zeroed COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_grow_zeroed.ts") tslang_add_test(NAME test-jit-00-rest-args-pack-owned COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00rest_args_pack_owned.ts") +tslang_add_test(NAME test-jit-00-switch-char-string-cases COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00switch_char_string_cases.ts") tslang_add_test(NAME test-jit-00-owned-generators COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_generators.ts") tslang_add_test(NAME test-jit-00-owned-globals COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_globals.ts") tslang_add_test(NAME test-jit-00-owned-generator-locals COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00owned_generator_locals.ts") @@ -1830,6 +1832,7 @@ set(TSLANG_CORPUS 00string_length_resize.ts 00strings.ts 00switch.ts + 00switch_char_string_cases.ts 00symbol.ts 00throw_in_catch.ts 00throw_bool_bigint.ts @@ -2463,7 +2466,8 @@ foreach(own_test own_fresh_string own_fresh_array own_return_new own_try_local o own_field_borrow own_element_borrow own_container_loop own_optional_access own_borrow_merge own_field_let_borrow own_element_let_borrow own_borrow_merge_after_call - own_call_no_drops own_getter_borrow own_param_kept own_any_box) + own_call_no_drops own_getter_borrow own_param_kept own_any_box + own_string_length_borrow) tslang_add_test(NAME test-jit-own-${own_test} COMMAND test-runner -jit -mm=own "${PROJECT_SOURCE_DIR}/test/tester/own/${own_test}.ts") tslang_add_test(NAME test-compile-own-${own_test} COMMAND test-runner -mm=own "${PROJECT_SOURCE_DIR}/test/tester/own/${own_test}.ts") # The promise: no counting at all in what own emits. diff --git a/tslang/test/tester/own/own_string_length_borrow.ts b/tslang/test/tester/own/own_string_length_borrow.ts new file mode 100644 index 000000000..8c250da5c --- /dev/null +++ b/tslang/test/tester/own/own_string_length_borrow.ts @@ -0,0 +1,27 @@ +// -mm=own: reading a string's length borrows the string. `box.text.length` - the field's string, +// read and let go - was rejected as the field's borrow escaping, because `ts.StringLength` was not +// among the uses own knows keep nothing. +class Box { + text: string; + constructor(t: string) { + this.text = t; + } +} + +function lengthOf(box: Box) { + return box.text.length; +} + +function main() { + let total = 0; + for (let i = 0; i < 1000; i++) { + const box = new Box("n" + i); + total += lengthOf(box); + const s = "x" + i; + total += s.length; + } + + // "n0".."n999" and "x0".."x999": 1000 prefixes and 10 * 1 + 90 * 2 + 900 * 3 digits, twice + assert(total == 2 * (1000 + 2890)); + print("done."); +} diff --git a/tslang/test/tester/tests/00switch_char_string_cases.ts b/tslang/test/tester/tests/00switch_char_string_cases.ts new file mode 100644 index 000000000..b7d845110 --- /dev/null +++ b/tslang/test/tester/tests/00switch_char_string_cases.ts @@ -0,0 +1,41 @@ +// A `switch` on a char with string cases casts the char to a string once per case. That cast +// allocates, and CastOp reported no effect for it, so CSE merged the casts into one: under -mm=rc +// the first case's release freed the string that every later case then retained, compared and +// released again - a use after free and a double free for each character that missed the first +// case. RegExp's constructor reads its flags this way. +function flagsOf(flags: string) { + let g = 0; + let i = 0; + let m = 0; + let other = 0; + for (const c of flags) { + switch (c) { + case "s": + break; + case "g": + g++; + break; + case "i": + i++; + break; + case "m": + m++; + break; + default: + other++; + break; + } + } + + return g * 1000 + i * 100 + m * 10 + other; +} + +function main() { + let t = 0; + for (let n = 0; n < 10000; n++) { + t += flagsOf("gimxgim"); + } + + assert(t == 10000 * 2221); + print("done."); +}