Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog.d/10228-sso-utf16-length.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix `.length` on short inline strings containing non-ASCII text. Runtime
property reads, suffix cursors, typed and generic generated reads, and
element-shape loop clones now count UTF-16 code units instead of UTF-8 bytes.
Non-ASCII strings remain eligible for inline storage; the generated ASCII
path uses the stored byte count, and the Unicode path stays call-free.
18 changes: 4 additions & 14 deletions crates/perry-codegen/src/expr/element_shape_reads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,9 @@
//! count in `StringHeader::utf16_len`, the leading `u32` — the identical load
//! the inline `.length` fast path in `property_get/generic_dispatch.rs` emits.
//! An SSO immediate (`SHORT_STRING_TAG`, up to five bytes packed into the
//! NaN-box) keeps its BYTE length in bits 40..=47, and this reads it the same
//! way the runtime's own SSO `.length` arms do
//! (`string/char_ops.rs::string_property_get_miss`,
//! `property_get/generic_dispatch.rs`). That convention is byte length, not
//! code-unit length, so a non-ASCII SSO string reports its UTF-8 size — a
//! PRE-EXISTING Perry-wide answer, reproduced here deliberately: the clone must
//! agree with the path it is a clone of, and diverging from it would be a
//! miscompile even where the shared answer is itself wrong.
//! NaN-box) keeps its byte length in bits 40..=47. The shared SSO length
//! lowering returns that count for ASCII and counts UTF-16 units inline for
//! non-ASCII payloads, matching heap strings without adding a call (#10191).
//!
//! **The ternary.** JS truthiness of an arbitrary value is a runtime question
//! (`""`, `0`, `NaN`, `null`, every object). The clone does not guess it: only
Expand All @@ -58,8 +53,6 @@ use crate::types::{DOUBLE, I1, I32, I64};
const STRING_TAG_TOP16: &str = crate::nanbox::STRING_TAG_TOP16_I64;
/// `SHORT_STRING_TAG >> 48` — an SSO immediate.
const SHORT_STRING_TAG_TOP16: &str = crate::nanbox::SHORT_STRING_TAG_TOP16_I64;
/// `SHORT_STRING_LEN_SHIFT` — the length byte sits at bits 40..=47.
const SHORT_STRING_LEN_SHIFT: &str = "40";

/// `TAG_TRUE` (`0x7FFC_0000_0000_0004`) as a decimal i64 literal.
const TAG_TRUE_I64: &str = "9222246136947933188";
Expand Down Expand Up @@ -212,9 +205,7 @@ pub(crate) fn lower_cloned_string_length(
ctx.block().br(&done_label);

ctx.current_block = sso_idx;
let sso_shifted = ctx.block().lshr(I64, &bits, SHORT_STRING_LEN_SHIFT);
let sso_len_byte = ctx.block().and(I64, &sso_shifted, "255");
let sso_len = ctx.block().uitofp(I64, &sso_len_byte, DOUBLE);
let sso_len = super::string_length::lower_sso_length(ctx, &bits);
let sso_end = ctx.block().label.clone();
ctx.block().br(&done_label);

Expand Down Expand Up @@ -311,7 +302,6 @@ mod tests {
fn string_tag_literals_match_the_runtime() {
assert_eq!(STRING_TAG_TOP16, (0x7FFFu64).to_string());
assert_eq!(SHORT_STRING_TAG_TOP16, (0x7FF9u64).to_string());
assert_eq!(SHORT_STRING_LEN_SHIFT, "40");
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,13 +306,9 @@ pub(crate) fn lower_generic_property_get(
let is_sso = ctx.block().icmp_eq(I64, &obj_tag, "32761"); // 0x7FF9
ctx.block().cond_br(&is_sso, &sso_label, &nonptr_label);

// `.length` of an SSO string is the length byte in bits 40..47 of the
// NaN-box itself — the same extract `js_object_get_field_by_name_f64`
// performs, minus the call and the key decode.
// SSO stores a byte count; JavaScript observes UTF-16 code units.
ctx.current_block = sso_idx;
let len_shifted = ctx.block().lshr(I64, &obj_bits, "40");
let len_byte = ctx.block().and(I64, &len_shifted, "255");
let sso_val = ctx.block().uitofp(I64, &len_byte, DOUBLE);
let sso_val = super::super::string_length::lower_sso_length(ctx, &obj_bits);
let sso_end_label = ctx.block().label.clone();
ctx.block().br(&merge_label);
ctx.current_block = nonptr_idx;
Expand Down
4 changes: 4 additions & 0 deletions crates/perry-codegen/src/expr/property_get/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1006,6 +1006,10 @@ fn generic_length_read_serves_a_string_inline() {
.map(|i| i + 1)
.unwrap_or(sso_body.len());
let sso_body = &sso_body[..sso_end];
assert!(
ir.contains("\nsso.utf16") && ir.contains("\nsso.length.done"),
"non-ASCII inline strings must have a UTF-16 counting arm:\n{ir}"
);
assert!(
sso_body.contains("lshr i64") && sso_body.contains(", 40"),
"the SSO arm must extract the inline length byte, not call the \
Expand Down
71 changes: 66 additions & 5 deletions crates/perry-codegen/src/expr/string_length.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ use perry_hir::Expr;

use crate::nanbox::POINTER_MASK_I64;
use crate::type_analysis::{is_array_expr, is_string_expr, string_value_is_runtime_guaranteed};
use crate::types::{DOUBLE, I32, I64};
use crate::types::{DOUBLE, I1, I32, I64};

use super::{lower_expr, static_string_lowering_enabled, FnCtx};

/// Lower string `.length` as SSO-byte extraction or a heap-header load.
/// Lower string `.length` as an inline UTF-16 count or a heap-header load.
///
/// A declared type is only a dispatch candidate, so its miss retains ordinary
/// property semantics. A constructive proof (including the guarded string
Expand Down Expand Up @@ -44,9 +44,7 @@ pub(crate) fn try_lower(ctx: &mut FnCtx<'_>, object: &Expr) -> Result<Option<Str
ctx.block().cond_br(&is_sso, &sso_label, &non_sso_label);

ctx.current_block = sso_idx;
let len_shifted = ctx.block().lshr(I64, &bits, "40");
let len_byte = ctx.block().and(I64, &len_shifted, "255");
let sso_len = ctx.block().uitofp(I64, &len_byte, DOUBLE);
let sso_len = lower_sso_length(ctx, &bits);
let sso_pred = ctx.block().label.clone();
ctx.block().br(&merge_label);

Expand Down Expand Up @@ -98,3 +96,66 @@ pub(crate) fn try_lower(ctx: &mut FnCtx<'_>, object: &Expr) -> Result<Option<Str
&[(&sso_len, &sso_pred), (&heap_len, &heap_pred)],
)))
}

/// Read an already tag-checked SSO receiver's UTF-16 length without calls or
/// heap access. ASCII keeps the stored byte count; the other arm unrolls the
/// runtime's bounded WTF-8 counter over the five inline payload bytes. Tracking
/// the next sequence boundary also preserves its malformed-byte convention.
/// Call-free lowering is required by the element-shape loop clone (#10191).
pub(super) fn lower_sso_length(ctx: &mut FnCtx<'_>, bits: &str) -> String {
let shifted = ctx.block().lshr(I64, bits, "40");
let byte_len = ctx.block().and(I64, &shifted, "255");
let ascii_len = ctx.block().uitofp(I64, &byte_len, DOUBLE);
// High bit of each of the five payload bytes; excludes the length/tag.
let high_bits = ctx.block().and(I64, bits, &0x80_80_80_80_80u64.to_string());
let ascii = ctx.block().icmp_eq(I64, &high_bits, "0");
let ascii_end = ctx.block().label.clone();
let unicode_idx = ctx.new_block("sso.utf16");
let done_idx = ctx.new_block("sso.length.done");
let unicode_label = ctx.block_label(unicode_idx);
let done_label = ctx.block_label(done_idx);
ctx.block().cond_br(&ascii, &done_label, &unicode_label);

ctx.current_block = unicode_idx;
let mut next = "0".to_string();
let mut count = "0".to_string();
for i in 0..5 {
let index = i.to_string();
let shifted = ctx.block().lshr(I64, bits, &(i * 8).to_string());
let byte = ctx.block().and(I64, &shifted, "255");
let in_payload = ctx.block().icmp_ult(I64, &index, &byte_len);
let at_boundary = ctx.block().icmp_ule(I64, &next, &index);
let active = ctx.block().and(I1, &in_payload, &at_boundary);
let ascii_byte = ctx.block().icmp_ult(I64, &byte, "128");
let multibyte = ctx.block().icmp_uge(I64, &byte, "192");
let has_unit = ctx.block().or(I1, &ascii_byte, &multibyte);
let astral = ctx.block().icmp_uge(I64, &byte, "240");
let first_unit = ctx.block().zext(I1, &has_unit, I64);
let second_unit = ctx.block().zext(I1, &astral, I64);
let units = ctx.block().add(I64, &first_unit, &second_unit);
let contribution = ctx.block().select(I1, &active, I64, &units, "0");
count = ctx.block().add(I64, &count, &contribution);

if i < 4 {
let three_or_four =
ctx.block()
.select(I1, &astral, I64, &(i + 4).to_string(), &(i + 3).to_string());
let two_byte = ctx.block().icmp_ult(I64, &byte, "224");
let multi_next =
ctx.block()
.select(I1, &two_byte, I64, &(i + 2).to_string(), &three_or_four);
let step_next =
ctx.block()
.select(I1, &multibyte, I64, &multi_next, &(i + 1).to_string());
next = ctx.block().select(I1, &active, I64, &step_next, &next);
}
}
let unicode_len = ctx.block().uitofp(I64, &count, DOUBLE);
let unicode_end = ctx.block().label.clone();
ctx.block().br(&done_label);
ctx.current_block = done_idx;
ctx.block().phi(
DOUBLE,
&[(&ascii_len, &ascii_end), (&unicode_len, &unicode_end)],
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,10 @@ fn a_recurrence_over_a_second_variable_declines() {
#[test]
fn the_fields_body_gets_a_shape_keyed_clone() {
let ir = emit(&access_module(fields_body()));
assert!(
fast_clone_slice(&ir).contains("sso.utf16"),
"the entered, call-free clone must count non-ASCII SSO code units"
);
assert_shape_keyed_clone(&ir, "three reads of one element");
let fast = fast_clone_slice(&ir);
assert!(
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-codegen/src/stmt/element_shape_loop_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,8 @@ fn fast_clone_slice(ir: &str) -> String {
|| trimmed.starts_with("element_shape.load")
|| trimmed.starts_with("element_shape.number")
|| trimmed.starts_with("element_shape.strlen")
|| trimmed.starts_with("sso.utf16")
|| trimmed.starts_with("sso.length.done")
|| trimmed.starts_with("element_shape.bool");
// Only a repeated CLONE label means the second copy: unrelated
// functions share ordinary labels (`entry:`), and breaking on one
Expand Down
7 changes: 3 additions & 4 deletions crates/perry-runtime/src/string/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,9 @@ pub extern "C" fn js_string_materialize_to_heap(value: f64) -> *mut StringHeader
pub extern "C" fn js_string_new_sso(data: *const u8, len: u32) -> f64 {
unsafe {
let ulen = len as usize;
// SSO stores its length tag as the JS `.length`, which is only valid
// when byte length == UTF-16 length — i.e. pure ASCII. Non-ASCII (incl.
// WTF-8 lone surrogates) must take the heap path so `compute_utf16_len`
// records the correct code-unit count (#4793).
// This constructor keeps its ASCII-only fast path. Other producers
// (notably JSON.parse) also create non-ASCII SSO values; their length
// readers count UTF-16 units separately from the stored byte count.
if ulen <= crate::value::SHORT_STRING_MAX_LEN && (ulen == 0 || !data.is_null()) {
let bytes = std::slice::from_raw_parts(data, ulen);
if bytes.iter().all(|&b| b < 0x80) {
Expand Down
3 changes: 1 addition & 2 deletions crates/perry-runtime/src/string/char_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,7 @@ fn string_property_get_miss(value: f64, key: f64) -> f64 {
if name == "length" {
let bits = value.to_bits();
if crate::value::JSValue::from_bits(bits).is_short_string() {
return ((bits & crate::value::SHORT_STRING_LEN_MASK)
>> crate::value::SHORT_STRING_LEN_SHIFT) as f64;
return crate::value::JSValue::from_bits(bits).short_string_utf16_len() as f64;
}
let string = (bits & crate::value::POINTER_MASK) as *const StringHeader;
return unsafe { (*string).utf16_len as f64 };
Expand Down
9 changes: 3 additions & 6 deletions crates/perry-runtime/src/string/concat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ pub(crate) fn canonicalize_surrogate_pairs(ptr: *mut StringHeader) -> *mut Strin

/// True when the `len` bytes at `data` are all ASCII (`< 0x80`), or the slice
/// is empty/null. Used to decide whether a concat result may be stored inline
/// as an SSO value (whose length tag doubles as the JS `.length`).
/// through the concat helpers' ASCII SSO fast path.
#[inline]
fn bytes_all_ascii(data: *const u8, len: u32) -> bool {
if data.is_null() || len == 0 {
Expand Down Expand Up @@ -489,11 +489,8 @@ pub(crate) fn test_clear_concat_memo() {
fn concat_byte_parts(l: (*const u8, u32), r: (*const u8, u32)) -> f64 {
let total_blen = l.1 + r.1;

// SSO encodes its length tag as the JS `.length`, so it is only sound for
// ASCII operands (byte length == UTF-16 length). A non-ASCII operand —
// multi-byte UTF-8 or a WTF-8 lone surrogate — must take the heap path so
// the result's `utf16_len` is computed, not assumed equal to `byte_len`
// (#4793: `("é"+"x").length` was 3, `("\uD800"+"x").length` was 4).
// Keep the existing ASCII-only concat fast path. Non-ASCII results use
// the heap path, which also handles WTF-8 surrogate-pair boundaries.
let both_ascii = bytes_all_ascii(l.0, l.1) && bytes_all_ascii(r.0, r.1);

// SSO fast path — assemble the result inline when it fits (≤ 5
Expand Down
3 changes: 1 addition & 2 deletions crates/perry-runtime/src/string/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1088,8 +1088,7 @@ pub(crate) fn string_data(s: *const StringHeader) -> *const u8 {

/// The SSO immediate bits a heap string's CONTENT would encode as, or `None`
/// when it doesn't fit the inline form (> `SHORT_STRING_MAX_LEN` bytes, or
/// any non-ASCII byte — SSO's length tag doubles as the JS `.length`, so a
/// multi-byte sequence must not take this form).
/// any non-ASCII byte — this cache-folding helper is deliberately ASCII-only).
///
/// This is the representation-folding half of SSO: the same short string can
/// reach a cache as an immediate or as a heap pointer depending on what the
Expand Down
2 changes: 1 addition & 1 deletion crates/perry-runtime/src/string/suffix_cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub struct SuffixCursor {
fn length(source: f64) -> u32 {
let value = crate::value::JSValue::from_bits(source.to_bits());
if value.is_short_string() {
value.short_string_len() as u32
value.short_string_utf16_len()
} else {
unsafe { (*value.as_string_ptr()).utf16_len }
}
Expand Down
13 changes: 3 additions & 10 deletions crates/perry-runtime/src/value/dynamic_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,9 @@ pub extern "C" fn js_value_length_f64(value: f64) -> f64 {
let bits = value.to_bits();
let top16 = bits >> 48;

// SHORT_STRING_TAG (SSO) — length is the byte count stored in
// bits 40..=47. Fast path, no heap access. For multibyte UTF-8
// content the byte length and UTF-16 code-unit count differ,
// but SSO strings are ≤5 bytes and the vast majority are ASCII
// where they match. Non-ASCII SSO values go through a slower
// full-parse path — tolerated because the distinction doesn't
// come up in practice for 5-byte strings.
// SHORT_STRING_TAG stores a byte count; JS length counts UTF-16 units.
if top16 == 0x7FF9 {
return ((bits & SHORT_STRING_LEN_MASK) >> SHORT_STRING_LEN_SHIFT) as f64;
return JSValue::from_bits(bits).short_string_utf16_len() as f64;
}

// STRING_TAG — length is code-unit count from js_string_length.
Expand Down Expand Up @@ -299,8 +293,7 @@ fn value_length_property_with_cache(value: f64, cache_slot: *mut LengthPicCacheS
}

if jsval.is_short_string() {
let string = crate::string::js_string_materialize_to_heap(value);
return crate::string::js_string_length(string) as f64;
return jsval.short_string_utf16_len() as f64;
}

if let Some(length) = crate::array::array_subclass_fast_length_with_ic(value, cache_slot) {
Expand Down
11 changes: 10 additions & 1 deletion crates/perry-runtime/src/value/jsvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,13 +294,22 @@ impl JSValue {
len
}

/// Return the length of an SSO string (0..=5).
/// Return the byte length of an SSO string (0..=5).
#[inline]
pub fn short_string_len(&self) -> usize {
debug_assert!(self.is_short_string());
((self.bits & SHORT_STRING_LEN_MASK) >> SHORT_STRING_LEN_SHIFT) as usize
}

/// JavaScript-visible UTF-16 length, without materializing a heap string.
/// Keep the byte-length accessor for storage, hashing, and byte copying.
#[inline]
pub fn short_string_utf16_len(&self) -> u32 {
let mut bytes = [0; SHORT_STRING_MAX_LEN];
let len = self.short_string_to_buf(&mut bytes);
crate::string::compute_utf16_len_wtf8(&bytes[..len])
}

/// Get string pointer (panics if not a string)
#[inline]
pub fn as_string_ptr(&self) -> *const crate::string::StringHeader {
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-runtime/src/value/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ pub(crate) mod to_string;
pub(crate) mod to_string_class_ref;
mod truthy;

#[cfg(test)]
mod sso_length_tests;
#[cfg(test)]
mod tests;

Expand Down
81 changes: 81 additions & 0 deletions crates/perry-runtime/src/value/sso_length_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
//! #10191: inline string storage must not change JavaScript's UTF-16 length.

use super::*;

#[test]
fn non_ascii_sso_length_matches_heap_strings_through_every_runtime_path() {
let cases: &[(&[u8], u32)] = &[
(b"", 0),
(b"abcde", 5),
(b"a\0b", 3),
("é".as_bytes(), 1),
("éé".as_bytes(), 2),
("éabc".as_bytes(), 4),
("abcé".as_bytes(), 4),
("中".as_bytes(), 1),
("é中".as_bytes(), 2),
("😀".as_bytes(), 2),
("a😀".as_bytes(), 3),
("😀a".as_bytes(), 3),
("e\u{301}".as_bytes(), 2),
(&[0xED, 0xA0, 0x80], 1),
(&[b'a', 0xED, 0xBF, 0xBF, b'b'], 3),
];
for &(bytes, expected) in cases {
let value = JSValue::try_short_string(bytes).expect("fixture must stay inline");
assert!(value.is_short_string());
assert_eq!(
value.short_string_len(),
bytes.len(),
"storage still counts bytes"
);
let boxed = f64::from_bits(value.bits());
assert_eq!(
js_value_length_f64(boxed),
expected as f64,
"bytes={bytes:?}"
);
assert_eq!(js_value_length_property_f64(boxed), expected as f64);

let key = crate::string::js_string_from_bytes(b"length".as_ptr(), 6);
let key = f64::from_bits(JSValue::string_ptr(key).bits());
assert_eq!(
crate::string::js_string_index_get_boxed(boxed, key),
expected as f64
);

let mut cursor = crate::string::suffix_cursor::SuffixCursor::default();
unsafe {
assert_eq!(
crate::string::suffix_cursor::js_string_suffix_length(boxed, &cursor),
expected as f64
);
crate::string::suffix_cursor::js_string_suffix_advance(boxed, &mut cursor, 1);
assert_eq!(
crate::string::suffix_cursor::js_string_suffix_length(boxed, &cursor),
expected.saturating_sub(1) as f64
);
}
}
}

#[test]
fn malformed_sso_length_keeps_the_heap_counter_convention() {
for bytes in [
&[0x80][..],
&[0xF0][..],
&[0xE2, 0x82][..],
&[0xC3, b'a', b'b'][..],
&[b'a', 0xF0, b'b', b'c', b'd'][..],
&[0xED, 0xA0, 0x80, 0x80, b'x'][..],
] {
let heap = crate::string::js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32);
let expected = crate::string::js_string_length(heap);
let sso = JSValue::try_short_string(bytes).unwrap();
assert_eq!(
js_value_length_f64(f64::from_bits(sso.bits())),
expected as f64,
"bytes={bytes:?}"
);
}
}
Loading
Loading