fix(object): an SSO string receiver is indexable, not unindexable (#8117)

changelog.d/8151-sso-string-receiver-index.md

@@ -0,0 +1,35 @@
+### Fixed
+
+- **Indexing a short concatenated string returned `undefined`** (#8117, reopening
+  #6887 at a different funnel). `const s = "ab" + "c"; s[0]` read `undefined`
+  instead of `"a"`, while `s.length`, `typeof`, printing, `charAt`, `for-of`,
+  spread, `Array.from` and `split` were all correct. Every ASCII concatenation
+  of five bytes or fewer is an inline `SHORT_STRING_TAG` (SSO) value, so
+  `(a + b)[0]`, an index-accumulation loop, and `parts.join("") + "\n"` were all
+  affected; the byte-identical heap string — a non-ASCII or >5-byte
+  concatenation — was fine.
+
+  `js_object_get_index_polymorphic` opened by asking *"does this receiver's low
+  48 bits hold a heap pointer?"* and returned `undefined` for every tag that
+  does not, conflating *not a pointer* with *not indexable*. It is the recurring
+  family — a receiver-specific arm claiming the operation before the
+  receiver-kind question is asked; this dispatcher asks the *key*'s kind
+  carefully, including a dedicated SSO arm for an SSO key, and never asked the
+  receiver's. #6888 had fixed codegen's proven-`string` fast path, but a
+  receiver whose string type is not *proven* (an inferred `const`, an annotated
+  `const s: string`, or a `string` parameter) reaches the generic dispatcher
+  instead, which passes the raw NaN-boxed bits to this helper. The symptom
+  changed from #6887's segfault to a silent wrong value because
+  `is_valid_string_ptr` now rejects the bogus pointer rather than dereferencing
+  it.
+
+  Fixed by adding the `SHORT_STRING_TAG` arm, delegating to
+  `js_string_index_get_boxed` so both string representations share one copy of
+  the CanonicalNumericIndexString semantics. Fixing at the shared funnel covers
+  every call site that funnels an unboxed receiver here. Sabotage-verified:
+  removing the arm fails `an_sso_string_receiver_reads_its_characters` with
+  `left: None  right: Some("a")` — the gap test's own wrong answer — while
+  controls for the heap-string arm, non-string primitives, and
+  out-of-range/fractional/`NaN` indices stay green.
+
+  Fixes `test-files/test_gap_sso_concat_string_index.ts`.

crates/perry-runtime/src/object/mod.rs

@@ -120,6 +120,8 @@ pub(crate) use object_ops::{ensure_key_in_keys_array, install_builtin_getter};
 mod object_ops_frozen;
 mod polymorphic_index;
 #[cfg(test)]
+mod polymorphic_index_sso_tests;
+#[cfg(test)]
 mod polymorphic_index_symbol_tests;
 mod primitive_proto_thunks;
 mod property_key;

crates/perry-runtime/src/object/polymorphic_index.rs

@@ -139,6 +139,28 @@ pub extern "C" fn js_object_get_index_polymorphic(obj_handle: i64, idx: f64) ->
         // *registered* class-id takes the class-ref arm.
         match (obj_handle as u64) >> 48 {
             0x7FFD | 0x7FFF => (obj_handle as u64) & 0x0000_FFFF_FFFF_FFFF,
+            // SHORT_STRING_TAG (0x7FF9): an SSO string IS a string receiver,
+            // it just carries its characters inline instead of a heap address
+            // (#6887/#8117). The question this match asks is "does the low 48
+            // hold a heap pointer?", and answering `undefined` for everything
+            // that does not conflates "not a pointer" with "not indexable" —
+            // so `("ab" + "c")[0]` read `undefined` while the byte-identical
+            // heap string read `"a"`. Codegen hands this helper the receiver's
+            // RAW NaN-boxed bits whenever the tag is not POINTER_TAG (see the
+            // `select` in `index_get.rs`'s generic dispatcher), so the value is
+            // intact here and the string question can still be asked.
+            //
+            // Delegate to the boxed string entry point, which materializes the
+            // SSO payload onto the heap and lands in exactly the
+            // `js_string_index_get` the 0x7FFF arm reaches below via
+            // `GC_TYPE_STRING` — so both string representations share one copy
+            // of the CanonicalNumericIndexString key semantics.
+            0x7FF9 => {
+                return crate::string::js_string_index_get_boxed(
+                    f64::from_bits(obj_handle as u64),
+                    idx,
+                );
+            }
             0x7FFE => {
                 let class_id = (obj_handle as u64 & 0xFFFF_FFFF) as u32;
                 if class_id != 0 && crate::object::class_registry::is_class_id_registered(class_id)

crates/perry-runtime/src/object/polymorphic_index_sso_tests.rs

@@ -0,0 +1,130 @@
+//! #8117/#6887: an SSO string receiver reaching the polymorphic index funnel.
+//!
+//! `js_object_get_index_polymorphic` opens by asking "does this receiver's low
+//! 48 bits hold a heap pointer?" and answering `undefined` when they do not.
+//! That conflates *not a pointer* with *not indexable*. An inline
+//! `SHORT_STRING_TAG` (SSO) string — every ASCII concatenation of five bytes or
+//! fewer, so `"ab" + "c"` — carries its characters in the payload rather than an
+//! address, so it fell into the reject arm and `s[0]` read `undefined` while the
+//! byte-identical heap string read `"a"`.
+//!
+//! # Why a direct call witnesses this bug
+//!
+//! The sibling file `polymorphic_index_symbol_tests.rs` opens with a warning
+//! that its obvious end-to-end shape passes without the fix, because a direct
+//! call reaches a different sub-arm than the compiled path. That hazard does
+//! not apply here, and the reason is worth stating rather than assuming:
+//! codegen's generic index dispatcher hands this helper the receiver's **raw
+//! NaN-boxed bits** whenever the tag is not `POINTER_TAG` — the emitted IR is
+//!
+//! ```text
+//!   %r7 = icmp eq i64 %tag, 32765            ; POINTER_TAG?
+//!   %r9 = select i1 %r7, i64 %masked, i64 %raw_bits
+//!   %rN = call double @js_object_get_index_polymorphic(i64 %r9, double 0.0)
+//! ```
+//!
+//! so for an SSO receiver the argument these tests construct by hand is
+//! bit-for-bit the argument the compiled program passes. Verified by sabotage:
+//! with the `0x7FF9` arm removed, `an_sso_string_receiver_reads_its_characters`
+//! fails `left: None  right: Some("a")`, which is the same wrong answer
+//! `test_gap_sso_concat_string_index.ts` reports against node.
+
+use crate::builtins::jsvalue_string_content;
+use crate::value::JSValue;
+
+/// Boxed bits exactly as codegen's dispatcher passes them for a non-pointer
+/// receiver: the whole NaN-boxed value reinterpreted as `i64`, unmasked.
+fn handle_of(value: JSValue) -> i64 {
+    value.bits() as i64
+}
+
+/// THE regression. An SSO receiver must yield its characters, not `undefined`.
+#[test]
+fn an_sso_string_receiver_reads_its_characters() {
+    let _serialized = crate::array::test_serialize();
+    let sso = JSValue::try_short_string(b"abc").expect("3 ASCII bytes fit SSO");
+    assert!(
+        sso.is_short_string(),
+        "precondition: the receiver under test must really be an inline SSO \
+         value — if concat ever stops producing one, this test is measuring \
+         the wrong thing and must be updated, not deleted"
+    );
+
+    for (idx, expected) in [(0.0, "a"), (1.0, "b"), (2.0, "c")] {
+        let got = jsvalue_string_content(crate::object::js_object_get_index_polymorphic(
+            handle_of(sso),
+            idx,
+        ));
+        assert_eq!(
+            got.as_deref(),
+            Some(expected),
+            "sso[{idx}] must read the inline character; pre-fix the tag match \
+             rejected SHORT_STRING_TAG as 'not a heap pointer' and returned \
+             undefined"
+        );
+    }
+}
+
+/// Control for the 0x7FFF arm: the fix must not disturb a heap string receiver,
+/// which reaches `js_string_index_get` via the `GC_TYPE_STRING` dispatch below
+/// the tag match. A concatenation longer than the SSO threshold is heap-backed,
+/// which is why the gap test's 80-character case never regressed.
+#[test]
+fn a_heap_string_receiver_still_reads_its_characters() {
+    let _serialized = crate::array::test_serialize();
+    let bytes = b"abcdefghij";
+    let hdr = crate::string::js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32);
+    let heap = JSValue::string_ptr(hdr);
+    assert!(
+        !heap.is_short_string() && heap.is_string(),
+        "precondition: this control must exercise the heap arm, not the new one"
+    );
+
+    let got = jsvalue_string_content(crate::object::js_object_get_index_polymorphic(
+        handle_of(heap),
+        0.0,
+    ));
+    assert_eq!(got.as_deref(), Some("a"));
+}
+
+/// Control against over-reach in the other direction: the new arm is keyed on
+/// `SHORT_STRING_TAG` alone. Every other non-pointer tag in the reject arm is
+/// a genuine primitive whose indexed read is `undefined` per JS, and widening
+/// the arm to "any non-pointer tag" would break that.
+#[test]
+fn a_non_string_primitive_receiver_still_reads_undefined() {
+    let _serialized = crate::array::test_serialize();
+    let undefined_bits = crate::value::TAG_UNDEFINED;
+
+    for handle in [
+        undefined_bits as i64,
+        crate::value::TAG_NULL as i64,
+        crate::value::TAG_TRUE as i64,
+    ] {
+        let got = crate::object::js_object_get_index_polymorphic(handle, 0.0);
+        assert_eq!(
+            got.to_bits(),
+            undefined_bits,
+            "indexing a non-string primitive is `undefined` in JS; the SSO arm \
+             must not have widened the accepted tag set"
+        );
+    }
+}
+
+/// An out-of-range index on an SSO receiver is `undefined`, same as on a heap
+/// string — the delegation must carry the CanonicalNumericIndexString
+/// semantics, not merely return *some* character.
+#[test]
+fn an_out_of_range_index_on_an_sso_receiver_reads_undefined() {
+    let _serialized = crate::array::test_serialize();
+    let sso = JSValue::try_short_string(b"abc").expect("3 ASCII bytes fit SSO");
+
+    for idx in [3.0, 99.0, -1.0, 1.5, f64::NAN] {
+        let got = crate::object::js_object_get_index_polymorphic(handle_of(sso), idx);
+        assert_eq!(
+            got.to_bits(),
+            crate::value::TAG_UNDEFINED,
+            "sso[{idx}] is not a canonical in-range array index"
+        );
+    }
+}

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

@@ -58,7 +58,7 @@ pub use char_ops::{
     js_string_at, js_string_char_at, js_string_char_code_at, js_string_code_point_at,
     js_string_end_index_to_i32, js_string_from_char_code, js_string_from_char_code_array,
     js_string_from_code_point, js_string_from_code_point_array, js_string_index_get,
-    js_string_index_to_i32, js_string_to_char_array,
+    js_string_index_get_boxed, js_string_index_to_i32, js_string_to_char_array,
 };
 pub use compare::{
     js_string_compare, js_string_ends_with, js_string_ends_with_at, js_string_equals,