Skip to content
Merged
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
12 changes: 10 additions & 2 deletions crates/perry-runtime/src/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -900,8 +900,16 @@ fn own_set_descriptor(target: f64, key: f64) -> Option<OwnSetDescriptor> {

if unsafe { crate::symbol::js_is_symbol(key) } != 0 {
let value = unsafe { crate::symbol::js_object_get_symbol_property(target, key) };
return (value.to_bits() != TAG_UNDEFINED)
.then_some(OwnSetDescriptor::Data { writable: true });
if value.to_bits() == TAG_UNDEFINED {
return None;
}
// An existing symbol-keyed own data property is non-writable when the
// receiver is frozen or its per-symbol attrs say so — so a strict
// `obj[sym] = v` is rejected (throws) rather than silently no-op'd
// (test262 Object/freeze/frozen-object-contains-symbol-properties-strict).
// Mirrors the string-keyed / `set_symbol_property` guards.
let writable = !crate::symbol::symbol_property_is_non_writable(target, key);
return Some(OwnSetDescriptor::Data { writable });
}
Comment on lines 901 to 913

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n "fn js_object_get_symbol_property" -A 20 crates/perry-runtime/src/symbol
rg -n "fn object_symbol_data_property_exists" -A 10 crates/perry-runtime/src/symbol

Repository: PerryTS/perry

Length of output: 3248


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '240,340p' crates/perry-runtime/src/symbol/get.rs
printf '\n==== proxy.rs ====\n'
sed -n '880,940p' crates/perry-runtime/src/proxy.rs
printf '\n==== properties.rs ====\n'
sed -n '340,390p' crates/perry-runtime/src/symbol/properties.rs

Repository: PerryTS/perry

Length of output: 10316


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "own_set_descriptor\(" crates/perry-runtime/src/proxy.rs crates/perry-runtime/src -g '!target'
printf '\n==== symbol property writes ====\n'
rg -n "store_object_symbol_property_root|object_symbol_data_property_exists|symbol_property_is_non_writable|js_object_get_symbol_property" crates/perry-runtime/src/symbol crates/perry-runtime/src -g '!target'
printf '\n==== assignment path around own_set_descriptor usage ====\n'
sed -n '940,1080p' crates/perry-runtime/src/proxy.rs

Repository: PerryTS/perry

Length of output: 17423


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '==== js_object_get_symbol_property internals ====\\n'
sed -n '1,220p' crates/perry-runtime/src/symbol/get.rs
printf '\\n==== own_set_descriptor call sites ====\\n'
sed -n '1110,1165p' crates/perry-runtime/src/proxy.rs
printf '\\n==== later own_set_descriptor call site ====\\n'
sed -n '1290,1338p' crates/perry-runtime/src/proxy.rs
printf '\\n==== symbol data property tests / docs ====\\n'
rg -n "undefined|TAG_UNDEFINED|object_symbol_data_property_exists|defineProperty\\(obj, sym" crates/perry-runtime/src/symbol crates/perry-runtime/src/object -g '!target'

Repository: PerryTS/perry

Length of output: 50370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "own_symbol_property|pub use .*object_symbol_data_property_exists|pub use .*own_symbol_property|object_symbol_data_property_exists" crates/perry-runtime/src/symbol.rs crates/perry-runtime/src/symbol crates/perry-runtime/src/proxy.rs

Repository: PerryTS/perry

Length of output: 1895


Use an existence check for symbol own-properties here
js_object_get_symbol_property() can return TAG_UNDEFINED for a property that exists with value undefined, so this branch still conflates “missing” with “present but undefined”. That lets an extensible receiver fall through to the create/new-property path instead of treating the existing symbol data property as read-only. Use the same presence check as the symbol setter path (object_symbol_data_property_exists / own_symbol_property) instead of testing the returned value bits.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/proxy.rs` around lines 901 - 913, The
symbol-own-property branch in proxy.rs is using js_object_get_symbol_property()
and TAG_UNDEFINED to detect presence, which incorrectly treats an existing
symbol property whose value is undefined as missing. Update this logic in the
same area as OwnSetDescriptor::Data to use the symbol existence check already
used by the setter path, such as object_symbol_data_property_exists or
own_symbol_property, and then derive writability from that result instead of
inspecting the returned value bits.


let obj_ptr = extract_pointer(target.to_bits()) as usize;
Expand Down
2 changes: 1 addition & 1 deletion crates/perry-runtime/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub(crate) use properties::{
get_symbol_property_attrs, inspect_custom_symbol_ptr, js_object_define_symbol_accessor,
js_object_delete_symbol_property, js_object_has_own_symbol_property,
reflect_symbol_getter_closure_bits, set_symbol_property_attrs, symbol_accessor_descriptor_bits,
symbol_property_is_enumerable, symbol_property_root_bits,
symbol_property_is_enumerable, symbol_property_is_non_writable, symbol_property_root_bits,
};
pub use properties::{
class_static_symbol_lookup, js_class_register_static_symbol, js_object_has_own_symbol,
Expand Down
27 changes: 27 additions & 0 deletions crates/perry-runtime/src/symbol/properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,33 @@ fn object_symbol_data_property_exists(obj_key: usize, sym_key: usize) -> bool {
})
}

/// True when an existing symbol-keyed own data property is non-writable — the
/// receiver was frozen (`Object.freeze`), or the per-symbol attrs recorded via
/// `Object.defineProperty(obj, sym, {writable:false})` say so. Mirrors the
/// frozen / non-writable rejection in `set_symbol_property`, but as a query so
/// the ordinary-`[[Set]]` walk (`own_set_descriptor`) can report the slot as
/// read-only and let a strict write throw. `obj_f64` / `sym_f64` are the same
/// NaN-boxed values passed to the symbol setters.
pub(crate) fn symbol_property_is_non_writable(obj_f64: f64, sym_f64: f64) -> bool {
let obj_key = unsafe { obj_key_from_f64(obj_f64) };
let sym_key = unsafe { sym_key_from_f64(sym_f64) };
if obj_key == 0 || sym_key == 0 {
return false;
}
// Only heap receivers carry the GC integrity flag word.
if (obj_f64.to_bits() >> 48) == 0x7FFD
&& obj_key >= 0x10000
&& crate::object::is_valid_obj_ptr(obj_key as *const u8)
{
let gc = (obj_key - crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
let flags = unsafe { (*gc)._reserved };
if flags & crate::gc::OBJ_FLAG_FROZEN != 0 {
return true;
}
}
get_symbol_property_attrs(obj_key, sym_key).is_some_and(|attrs| !attrs.writable())
}

/// `obj[sym] = value` where `sym` is a Symbol. Stores into the side table.
/// Returns the value (NaN-boxed) for chained assignment semantics.
#[no_mangle]
Expand Down
Loading