diff --git a/changelog.d/9818-primitive-string-property-reads.md b/changelog.d/9818-primitive-string-property-reads.md new file mode 100644 index 0000000000..9e2a86d644 --- /dev/null +++ b/changelog.d/9818-primitive-string-property-reads.md @@ -0,0 +1,10 @@ +Fix primitive string property reads with computed keys. Named properties now +consult `String.prototype` and preserve the original method value, so reflective +read-then-call code and method identity checks work. Inherited accessors receive +the primitive string as `this`; object and symbol keys follow `ToPropertyKey`. +Character indices and `length` keep precedence over prototype properties, and +boxed strings retain their own-property lookup before their custom prototype. + +Cover typed and untyped receivers, short strings, borrowed methods, inherited +accessors, symbol keys, key coercion, and prototype mutation in runtime unit +tests and a Node parity fixture. Direct method-call lowering is unchanged. diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs index 242f726e75..55910e1fd6 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs @@ -928,25 +928,16 @@ pub extern "C" fn js_object_get_field_by_name( return JSValue::undefined(); } } - // A primitive string receiver inherits `.constructor` from String.prototype: - // `"x".constructor === String` (test262 language/types/string/S8.4_A9/A12). - // The common string members (`.length`, indices, methods) are served by the - // codegen fast paths and never reach this generic slow path, so only the - // inherited `constructor` read needs routing here; resolve it to the same - // global `String` value bare-`String` yields so identity holds. - { - let bits = obj as u64; - if !key.is_null() && crate::value::JSValue::from_bits(bits).is_any_string() { - unsafe { - let key_ptr = (key as *const u8).add(std::mem::size_of::()); - let key_len = (*key).byte_len as usize; - if std::slice::from_raw_parts(key_ptr, key_len) == b"constructor" { - let ctor = - super::super::js_get_global_this_builtin_value(b"String".as_ptr(), 6); - return JSValue::from_bits(ctor.to_bits()); - } - } - } + // A named read on a primitive string uses the same own/prototype lookup + // as a computed read, including SSO receivers and function identity. + if crate::value::JSValue::from_bits(obj as u64).is_any_string() { + return JSValue::from_bits( + crate::string::js_string_index_get_boxed( + f64::from_bits(obj as u64), + crate::value::js_nanbox_string(key as i64), + ) + .to_bits(), + ); } // Native module registry handles can arrive here either as raw small // integers or as POINTER_TAG-boxed small integers. Route them before any @@ -1639,34 +1630,6 @@ pub extern "C" fn js_object_get_field_by_name( } } } - // SSO property access (v0.5.213 Step 1 gate). The codegen inline - // `.length` path routes SHORT_STRING_TAG receivers here because - // it doesn't yet know about the SSO tag. Handle `.length` by - // reading the length byte directly from the NaN-box payload. - // Other property accesses on an SSO string (e.g. `.charAt` via - // `[0]`, `.slice`) aren't yet routed here — handled by the - // string method dispatch in a future migration step; today they - // fall through to "undefined" which matches the behavior for - // string-valued property access on untyped locals in general. - { - let obj_bits = obj as u64; - if (obj_bits & crate::value::TAG_MASK) == crate::value::SHORT_STRING_TAG { - if !key.is_null() { - unsafe { - let key_ptr = - (key as *const u8).add(std::mem::size_of::()); - let key_len = (*key).byte_len as usize; - let key_bytes = std::slice::from_raw_parts(key_ptr, key_len); - if key_bytes == b"length" { - let len = (obj_bits & crate::value::SHORT_STRING_LEN_MASK) - >> crate::value::SHORT_STRING_LEN_SHIFT; - return JSValue::number(len as f64); - } - } - } - return JSValue::undefined(); - } - } // #1670: Web Streams handles are returned as `id as f64` (a normal // float, NOT NaN-boxed) just above the pointer-tagged small-handle band, so // an inline `res.body.locked` reaches this generic field-get with `obj` diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs index 10ea9cdae4..dfa7a37746 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs @@ -1002,14 +1002,6 @@ pub(crate) fn get_field_by_name_object_tail( let s = obj as *const crate::StringHeader; return JSValue::number((*s).utf16_len as f64); } - // A primitive string inherits `.constructor` from String.prototype: - // `"x".constructor === String` (test262 language/types/string/ - // S8.4_A9/A12). Resolve to the same global `String` value bare- - // `String` yields so identity holds — mirrors the Array branch above. - if key_bytes == b"constructor" { - let v = js_get_global_this_builtin_value(b"String".as_ptr(), 6); - return JSValue::from_bits(v.to_bits()); - } if let Some((kind, asym_type)) = crate::buffer::asymmetric_key_meta(obj as usize) { if key_bytes == b"type" { let label = if kind == 1 { @@ -1069,7 +1061,13 @@ pub(crate) fn get_field_by_name_object_tail( } } } - return JSValue::undefined(); + return JSValue::from_bits( + crate::string::js_string_index_get_boxed( + crate::value::js_nanbox_string(obj as i64), + crate::value::js_nanbox_string(key as i64), + ) + .to_bits(), + ); } // Maps/Sets: `.size`, expando keys, and prototype member values — // see `map_set_receiver.rs` (extracted for the file-size gate). diff --git a/crates/perry-runtime/src/object/polymorphic_index.rs b/crates/perry-runtime/src/object/polymorphic_index.rs index accf224d09..0468240fe0 100644 --- a/crates/perry-runtime/src/object/polymorphic_index.rs +++ b/crates/perry-runtime/src/object/polymorphic_index.rs @@ -273,7 +273,10 @@ pub extern "C" fn js_object_get_index_polymorphic(obj_handle: i64, idx: f64) -> }; if gc_type == crate::gc::GC_TYPE_STRING { - return crate::string::js_string_index_get(raw as *const crate::StringHeader, idx); + return crate::string::js_string_index_get_boxed( + crate::value::js_nanbox_string(raw as i64), + idx, + ); } if let Some(index) = numeric_key_u32_index(idx) { diff --git a/crates/perry-runtime/src/string/char_ops.rs b/crates/perry-runtime/src/string/char_ops.rs index 9bdf5cbba0..6a2b791ccf 100644 --- a/crates/perry-runtime/src/string/char_ops.rs +++ b/crates/perry-runtime/src/string/char_ops.rs @@ -3,6 +3,9 @@ use super::*; +#[cfg(test)] +mod computed_property_tests; + /// JS index coercion for the String character-access methods (#2787). /// Applies `ToIntegerOrInfinity`: a non-numeric argument is first run through /// the full `ToNumber` (`js_number_coerce`) so an object index with a custom @@ -139,19 +142,66 @@ 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 scope = crate::gc::RuntimeHandleScope::new(); + let key = scope.root_nanbox_f64(key); let hdr = crate::string::js_string_materialize_to_heap(value); if hdr.is_null() { return UNDEFINED; } - return js_string_index_get(hdr, key); + let own = js_string_index_get(hdr, key.get_nanbox_f64()); + return if own.to_bits() != crate::value::TAG_UNDEFINED { + own + } else { + string_property_get_miss(value, key.get_nanbox_f64()) + }; } // 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( + let own = js_string_index_get( (value.to_bits() & crate::value::POINTER_MASK) as *const StringHeader, key, - ) + ); + if !jsval.is_string() || own.to_bits() != crate::value::TAG_UNDEFINED { + return own; + } + string_property_get_miss(value, key) +} + +/// Complete a primitive string Get after its own character lookup misses. +/// Keep the raw index helper own-only: boxed strings also use it while walking +/// their own properties, before consulting their potentially custom prototype. +fn string_property_get_miss(value: f64, key: f64) -> f64 { + let scope = crate::gc::RuntimeHandleScope::new(); + let receiver = scope.root_nanbox_f64(value); + let key = scope.root_nanbox_f64(key); + let key = + scope.root_nanbox_f64(unsafe { crate::object::js_to_property_key(key.get_nanbox_f64()) }); + // Object / bigint keys can coerce to an own index or to `length`. + if let Some(name) = crate::builtins::jsvalue_string_content(key.get_nanbox_f64()) { + let value = receiver.get_nanbox_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; + } + let string = (bits & crate::value::POINTER_MASK) as *const StringHeader; + return unsafe { (*string).utf16_len as f64 }; + } + if canonical_string_index(&name).is_some() { + let string = crate::value::js_get_string_pointer_unified(value) as *const StringHeader; + let own = js_string_index_get(string, key.get_nanbox_f64()); + if own.to_bits() != crate::value::TAG_UNDEFINED { + return own; + } + } + } + // Reflect.get preserves the original function value (no binding wrapper), + // and gives inherited accessors the primitive as their receiver. Both + // operands stay rooted across lazy prototype creation and user code. + let prototype = crate::object::builtin_prototype_value("String"); + crate::proxy::js_reflect_get(prototype, key.get_nanbox_f64(), receiver.get_nanbox_f64()) } /// `s[key]` indexed read with ECMAScript CanonicalNumericIndexString semantics diff --git a/crates/perry-runtime/src/string/char_ops/computed_property_tests.rs b/crates/perry-runtime/src/string/char_ops/computed_property_tests.rs new file mode 100644 index 0000000000..6743921dbf --- /dev/null +++ b/crates/perry-runtime/src/string/char_ops/computed_property_tests.rs @@ -0,0 +1,70 @@ +use super::*; +use crate::value::{js_dyn_index_get, js_nanbox_string, JSValue}; + +fn string(value: &str) -> f64 { + js_nanbox_string(js_string_from_str(value) as i64) +} + +#[test] +fn computed_string_method_is_the_prototype_function() { + let scope = crate::gc::RuntimeHandleScope::new(); + let proto = scope.root_nanbox_f64(crate::object::builtin_prototype_value("String")); + for receiver in [ + string("abcdef"), + f64::from_bits(JSValue::try_short_string(b"abc").unwrap().bits()), + ] { + let receiver = scope.root_nanbox_f64(receiver); + for name in ["charAt", "trim", "toUpperCase", "toString", "constructor"] { + let key = scope.root_nanbox_f64(string(name)); + let expected = scope.root_nanbox_f64(crate::proxy::js_reflect_get( + proto.get_nanbox_f64(), + key.get_nanbox_f64(), + proto.get_nanbox_f64(), + )); + assert_ne!( + expected.get_nanbox_f64().to_bits(), + crate::value::TAG_UNDEFINED, + "prototype has {name}" + ); + for get in [js_string_index_get_boxed, js_dyn_index_get] { + let actual = get(receiver.get_nanbox_f64(), key.get_nanbox_f64()); + assert_eq!( + actual.to_bits(), + expected.get_nanbox_f64().to_bits(), + "{name} must preserve method identity" + ); + } + } + } +} + +#[test] +fn computed_string_own_properties_keep_index_semantics() { + let scope = crate::gc::RuntimeHandleScope::new(); + let receiver = scope.root_nanbox_f64(string("abcdef")); + for get in [js_string_index_get_boxed, js_dyn_index_get] { + assert_eq!(get(receiver.get_nanbox_f64(), string("length")), 6.0); + for key in [1.0, string("1")] { + let value = get(receiver.get_nanbox_f64(), key); + assert_eq!( + crate::builtins::jsvalue_string_content(value).as_deref(), + Some("b") + ); + } + for key in [ + -1.0, + 1.5, + 6.0, + f64::NAN, + f64::INFINITY, + string("01"), + string("1.0"), + string("missing"), + ] { + assert_eq!( + get(receiver.get_nanbox_f64(), key).to_bits(), + crate::value::TAG_UNDEFINED + ); + } + } +} diff --git a/crates/perry-runtime/src/value/dyn_index.rs b/crates/perry-runtime/src/value/dyn_index.rs index 9d94ab7329..440ef47bda 100644 --- a/crates/perry-runtime/src/value/dyn_index.rs +++ b/crates/perry-runtime/src/value/dyn_index.rs @@ -168,6 +168,9 @@ pub extern "C" fn js_dyn_index_get(value: f64, index: f64) -> f64 { return js_dyn_index_get(boxed, index.get_nanbox_f64()); } let jsval = JSValue::from_bits(bits); + if jsval.is_any_string() { + return crate::string::js_string_index_get_boxed(value, index); + } // #5525: a Symbol *index* (`obj[Symbol.iterator]`) must resolve through the // symbol side-table, never the integer-index / stringify paths below (which // would coerce the symbol's NaN-boxed bits to a garbage i32). The codegen @@ -191,20 +194,6 @@ pub extern "C" fn js_dyn_index_get(value: f64, index: f64) -> f64 { } } } - if jsval.is_string() || jsval.is_short_string() { - // Spec: string INDEXING `s[i]` returns `undefined` for a non-canonical - // or out-of-bounds index — unlike `s.charAt(i)`, which returns "". - // Route through the canonical-index helper (`js_string_index_get`, - // #3987) so an OOB read here is `undefined`. Calling `js_string_char_at` - // directly (charAt semantics) returned "" for OOB, which every - // generator/async LOCAL string read hit: the CPS box pass erases the - // local's static type, so `line[i]` reaches this dyn path instead of the - // `is_string_expr` static path — the `yaml` lexer's `parseDocument` - // `switch (line[n])` then never observed `undefined` at line-ends and - // its `*lex` state machine spun forever (#6067). - let s_ptr = js_get_string_pointer_unified(value) as *const crate::StringHeader; - return crate::string::js_string_index_get(s_ptr, index); - } // Class-ref value (INT32-tagged, top16 == 0x7FFE): `C[key]` where `C` is a // runtime class-ref value (e.g. a function parameter). Member-expression // access (`C.key`) already routes through `js_object_get_field_by_name_f64`, diff --git a/scripts/string_payload_access_baseline.txt b/scripts/string_payload_access_baseline.txt index cc8c57e8b0..16be08324c 100644 --- a/scripts/string_payload_access_baseline.txt +++ b/scripts/string_payload_access_baseline.txt @@ -12,7 +12,7 @@ inline-offset | perry-ext-nodemailer | 1 inline-offset | perry-ext-pg | 2 inline-offset | perry-ext-zlib | 3 inline-offset | perry-ffi | 3 -inline-offset | perry-runtime | 353 +inline-offset | perry-runtime | 351 inline-offset | perry-stdlib | 40 inline-offset | perry-updater | 5 reader-helper | perry-ext-ethers | 1 diff --git a/test-files/test_gap_9815_primitive_computed_properties.ts b/test-files/test_gap_9815_primitive_computed_properties.ts new file mode 100644 index 0000000000..28258f802b --- /dev/null +++ b/test-files/test_gap_9815_primitive_computed_properties.ts @@ -0,0 +1,67 @@ +// Dynamic value reads must return the original prototype method (#9815). +function typed(s: string, key: any): any { return s[key]; } +function unknown(s: any, key: any): any { return s[key]; } +function named(s: any, key: string): any { return s[key]; } + +for (const s of ["abcde", " abcdef ", "a" + "bc"]) { + for (const key of ["charAt", "trim", "toUpperCase", "toString", "constructor"]) { + const expected = String.prototype[key]; + console.log(key, typeof typed(s, key), typed(s, key) === expected, + unknown(s, key) === expected, named(s, key) === expected); + } + const charAt = typed(s, "charAt"); + const trim = unknown(s, "trim"); + const upper = named(s, "toUpperCase"); + console.log("borrowed", charAt.call(s, 1), trim.apply(s, []), upper.bind(s)()); + console.log("length", typed(s, "length"), unknown(s, "length"), named(s, "length")); + for (const key of [0, -0, 1, "1", -1, 1.5, 99, NaN, Infinity, "01", "1.0", "missing"]) { + console.log("index", String(key), typed(s, key), unknown(s, key)); + } +} + +const proto: any = String.prototype; +const sym = Symbol("computed"); +proto[sym] = 73; +proto.custom9815 = function () { "use strict"; return this; }; +proto["01"] = 81; +proto["99"] = 99; +proto["1"] = "shadowed"; +Object.defineProperty(proto, "get9815", { + configurable: true, + get: function () { "use strict"; return typeof this + ":" + this; } +}); +Object.defineProperty(Object.prototype, "inherited9815", { + configurable: true, + get: function () { "use strict"; return typeof this + ":" + this; } +}); + +for (const key of ["custom9815", "get9815", "inherited9815", "01", "99", "1", sym]) { + const value = typed("abc", key); + console.log("custom", typeof value, value === unknown("abc", key)); + if (typeof value === "function") console.log("receiver", value.call("xyz")); + else console.log("value", value); +} +let coercions = 0; +const indexKey = { toString() { coercions++; return "1"; } }; +const nameKey = { [Symbol.toPrimitive](hint) { coercions++; return "get9815"; } }; +const symbolKey = { [Symbol.toPrimitive](hint) { coercions++; return sym; } }; +console.log("coercion", typed("abc", indexKey), unknown("abc", nameKey), typed("abc", symbolKey), coercions); +console.log("bigint index", typed("abc", 1n)); +console.log("symbol identity", typed("abc", Symbol.iterator) === String.prototype[Symbol.iterator]); +const boxed: any = new String("abc"); +Object.setPrototypeOf(boxed, { custom9815: 42, "1": "shadowed" }); +console.log("boxed", unknown(boxed, "custom9815"), unknown(boxed, "1"), unknown(boxed, "length")); +const savedConstructor = proto.constructor; +proto.constructor = 91; +console.log("constructor override", typed("abcdef", "constructor"), named("abcdef", "constructor")); +proto.constructor = savedConstructor; +const callKey = "charAt"; +console.log("direct call", "abc"[callKey](1), "abc"["charAt"](1)); +delete proto[sym]; +delete proto.custom9815; +delete proto["01"]; +delete proto["99"]; +delete proto["1"]; +delete proto.get9815; +delete Object.prototype.inherited9815; +console.log("deleted", typed("abc", "custom9815"), typed("abc", "99"));