fix(string): stop mask-casting SSO values to StringHeader* (#6887)

changelog.d/6887-sso-concat-string-index.md

@@ -0,0 +1 @@
+**Fix SIGSEGV indexing or iterating a short concatenated string (#6887):** `("ab" + "c")[0]`, `for (const ch of a + b)`, `Array.from(a + b)`, `[...s]` and `s.split("")` all segfaulted when the concatenation was short enough to be small-string-optimized. A short string is an inline `SHORT_STRING_TAG` JSValue whose payload *is* the characters, but codegen's `s[i]` fast path mask-unboxed the receiver to a `StringHeader*`, and `js_array_from_value` performed the same mask itself behind a `(bits >> 48) >= 0x7FF8` test that an SSO value passes — both then dereferenced the packed characters as an address. String literals, `join()` results and long (heap-backed) concatenations were unaffected, which is why this survived: a 3-char concat crashed while a 64-char one did not, and `typeof`, `.length` and printing the value all worked first. Codegen now passes the receiver still boxed to a new `js_string_index_get_boxed`, which decides by tag, and `js_array_from_value` materializes an SSO receiver before extracting pointers. This was the blocker behind #6872 and the last defect stopping the Milo compiler from building and running Milo programs under Perry.

crates/perry-codegen/src/expr/index_get.rs

@@ -1596,17 +1596,22 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                 let s_box = lower_expr(ctx, object)?;
                 let idx_d = lower_expr(ctx, index)?;
                 let blk = ctx.block();
-                let s_handle = unbox_to_i64(blk, &s_box);
                 // #3987: route through the canonical-index runtime helper (it
                 // takes the raw NaN-boxed key, not an `fptosi`'d i32) so a valid
                 // array index returns its char and every non-canonical key
                 // (`NaN`, `1.5`, negatives, OOB, `"01"`, non-numeric strings)
                 // returns `undefined` — matching ECMAScript / Node — instead of
                 // truncating the index and returning `""` for OOB.
+                // Pass the receiver STILL BOXED. Unboxing here masked off the
+                // low 48 bits, which is only a pointer for a heap STRING_TAG
+                // value — an inline SHORT_STRING_TAG (SSO) value's payload is
+                // the characters themselves, so the mask produced a garbage
+                // pointer and `(a + b)[0]` segfaulted on any short
+                // concatenation. The boxed entry point decides by tag.
                 return Ok(blk.call(
                     DOUBLE,
-                    "js_string_index_get",
-                    &[(I64, &s_handle), (DOUBLE, &idx_d)],
+                    "js_string_index_get_boxed",
+                    &[(DOUBLE, &s_box), (DOUBLE, &idx_d)],
                 ));
             }
             // #6750 follow-up: a masked-window fact (dense range-loop or

crates/perry-codegen/src/runtime_decls/strings.rs

@@ -630,6 +630,10 @@ pub fn declare_phase_b_strings(module: &mut LlModule) {
     // #3987: `s[key]` canonical-index read — returns the char (NaN-boxed string)
     // for a valid array index, else NaN-boxed `undefined`. Takes the raw key.
     module.declare_function("js_string_index_get", DOUBLE, &[I64, DOUBLE]);
+    // SSO-safe variant: takes the receiver NaN-boxed so an inline
+    // SHORT_STRING_TAG value is decoded by tag instead of being mask-cast into
+    // a bogus pointer (which segfaulted on `(a + b)[0]`).
+    module.declare_function("js_string_index_get_boxed", DOUBLE, &[DOUBLE, DOUBLE]);
     // #2787: NaN-safe JS index coercion (undefined/NaN -> 0, trunc, clamp) for
     // the char-access methods, replacing a raw `fptosi` that is UB on a NaN.
     module.declare_function("js_string_index_to_i32", I32, &[DOUBLE]);

crates/perry-runtime/src/array/from_concat.rs

@@ -81,6 +81,19 @@ pub extern "C" fn js_array_from_value(boxed: f64) -> *mut ArrayHeader {
     if bits == TAG_NULL {
         throw_not_iterable("object null");
     }
+    // An inline SHORT_STRING_TAG (SSO) value's payload is the characters
+    // themselves, not an address, but it passes the `>= 0x7FF8` test below and
+    // the mask then yields a bogus pointer — `Array.from("ab" + "c")`
+    // segfaulted. Materialize to a heap StringHeader so every pointer
+    // extraction downstream is valid; the per-codepoint path in
+    // `js_array_clone` then behaves exactly as it does for a literal.
+    let jsval = crate::value::JSValue::from_bits(bits);
+    if jsval.is_short_string() {
+        let hdr = crate::string::js_string_materialize_to_heap(boxed);
+        if !hdr.is_null() {
+            return js_array_from_value(crate::value::js_nanbox_string(hdr as i64));
+        }
+    }
     // #6454: `Array.from(SomeClass)` where the class DECLARATION (an
     // INT32-tagged ClassRef) carries a — possibly inherited, #36/#321 —
     // `[Symbol.iterator]`: drive it. A class WITHOUT one falls through to the

crates/perry-runtime/src/string/char_ops.rs

@@ -118,6 +118,42 @@ fn utf16_unit_at(s: *const StringHeader, idx: usize) -> Option<u16> {
     None
 }
 
+/// SSO-safe `s[key]`: takes the receiver as a **NaN-boxed JSValue** rather than
+/// an already-unboxed `StringHeader*`.
+///
+/// Codegen's `s[i]` fast path used to `unbox_to_i64` the receiver — masking off
+/// the low 48 bits — and hand that to [`js_string_index_get`]. That is only
+/// correct for a heap `STRING_TAG` value. A short string is an inline
+/// `SHORT_STRING_TAG` value whose payload IS the characters, not an address, so
+/// masking produced a garbage pointer and the read segfaulted. Concatenation is
+/// the common way to produce one (`"ab" + "c"`), which made plain
+/// `for (const ch of a + b)` / `(a + b)[0]` crash while the same operations on a
+/// string literal or a `join()` result were fine.
+///
+/// Short receivers are materialized to a heap `StringHeader` and delegated, so
+/// the CanonicalNumericIndexString key semantics below stay in exactly one
+/// place. That costs a small arena allocation per index on an SSO receiver;
+/// worth revisiting if it shows up hot, but the alternative was a crash.
+#[no_mangle]
+pub extern "C" fn js_string_index_get_boxed(value: f64, key: f64) -> f64 {
+    const UNDEFINED: f64 = f64::from_bits(crate::value::TAG_UNDEFINED);
+    let jsval = crate::value::JSValue::from_bits(value.to_bits());
+    if jsval.is_short_string() {
+        let hdr = crate::string::js_string_materialize_to_heap(value);
+        if hdr.is_null() {
+            return UNDEFINED;
+        }
+        return js_string_index_get(hdr, key);
+    }
+    // Heap strings and every non-string receiver keep the existing behavior:
+    // `js_string_index_get` already guards invalid pointers and delegates
+    // non-string heap objects to the polymorphic index path.
+    js_string_index_get(
+        (value.to_bits() & crate::value::POINTER_MASK) as *const StringHeader,
+        key,
+    )
+}
+
 /// `s[key]` indexed read with ECMAScript CanonicalNumericIndexString semantics
 /// (#3987): returns the single-UTF-16-code-unit string at `key` only when `key`
 /// is a canonical array index — a non-negative integer (or a numeric string

test-files/test_gap_sso_concat_string_index.ts

@@ -0,0 +1,65 @@
+// A short string built by concatenation is an inline SHORT_STRING_TAG (SSO)
+// JSValue whose payload IS the characters, not a heap address. Codegen's `s[i]`
+// fast path mask-unboxed the receiver to a `StringHeader*`, and
+// `js_array_from_value` did the same mask itself, so both produced a bogus
+// pointer and segfaulted. `"ab" + "c"` is the ordinary way to make one, which
+// made `(a + b)[0]`, `for (const ch of a + b)` and `Array.from(a + b)` crash
+// while the identical operations on a literal or a `join()` result were fine.
+//
+// Long concatenations exceed the SSO threshold and were always heap-backed —
+// they are covered here so the fix cannot regress the heap path.
+
+const a = "ab";
+const b = "c";
+const short = a + b;
+
+console.log(typeof short, short.length, short);
+
+// --- indexed reads on a short concatenation -------------------------------
+console.log(short[0], short[1], short[2]);
+console.log(String(short[3]));           // undefined, out of range
+console.log(String(short[-1]));          // undefined, negative
+console.log(short.charAt(1), short.charCodeAt(1), short.codePointAt(1));
+console.log(short.at(0), short.at(-1));
+
+// index-loop accumulation (the shape milo's codegen uses)
+let viaIndex = "";
+for (let i = 0; i < short.length; i++) viaIndex += short[i];
+console.log(viaIndex);
+
+// --- iteration protocols --------------------------------------------------
+let viaForOf = 0;
+for (const ch of short) viaForOf++;
+console.log(viaForOf);
+console.log([...short].join("-"));
+console.log(Array.from(short).length, Array.from(short).join("|"));
+console.log(short.split("").join("+"));
+
+// --- concatenation of a join result, exactly milo's shape -----------------
+const parts: string[] = [];
+parts.push("%.*s");
+const fmt = parts.join("") + "\n";
+console.log(fmt.length);
+let fmtChars = 0;
+for (const ch of fmt) fmtChars++;
+console.log(fmtChars);
+console.log(fmt[0], fmt[1], JSON.stringify(fmt[4]));
+
+// --- empty and single-char concatenations ---------------------------------
+const empty = "" + "";
+console.log(empty.length, String(empty[0]), Array.from(empty).length);
+const one = "" + "x";
+console.log(one.length, one[0], Array.from(one).length);
+
+// --- non-ASCII, where UTF-16 indexing differs from bytes ------------------
+const uni = "é" + "ü";
+console.log(uni.length, uni[0], uni[1], Array.from(uni).length);
+
+// --- long (heap-backed) concatenation must still work ---------------------
+const long = "a".repeat(40) + "b".repeat(40);
+console.log(long.length, long[0], long[79], Array.from(long).length);
+
+// --- concatenation built in a loop ----------------------------------------
+let acc = "";
+for (const p of ["x", "y", "z"]) acc += p;
+console.log(acc.length, acc[0], acc[2], Array.from(acc).join(""));