Skip to content

fix(runtime): #5842 — Function.prototype defineProperty accessor inheritance through closures #5863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
proggeramlug merged 3 commits into from
Jul 2, 2026
Merged

fix(runtime): #5842 — Function.prototype defineProperty accessor inheritance through closures #5863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
774cba5
fix(runtime): #5842 — Function.prototype defineProperty accessor inhe…
Jul 1, 2026
cda4c75
refactor(runtime): dedupe closure named-property write path (#5842 fo…
Jul 1, 2026
09e20b7
fix(runtime): block non-writable Function.prototype data properties o…
Jul 2, 2026
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
197 changes: 128 additions & 69 deletions crates/perry-runtime/src/closure/dynamic_props.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,82 +478,141 @@ pub fn closure_get_dynamic_prop(ptr: usize, prop: &str) -> f64 {
break;
}
// Every function's [[Prototype]] is %Function.prototype% — an expando
// installed there (`Function.prototype.property = 12`) must be readable
// through any closure (`fn.property`, `boundFn.property`,
// `Function.indicator`). Synthesized own slots (`prototype`/`name`/
// `length`/`caller`/`arguments`/`constructor`) never come from the
// expando walk; excluding `prototype` also breaks the recursion through
// `builtin_prototype_value` (which reads `Function.prototype` via this
// very function). A re-entrancy guard covers the rest of that resolution
// cycle.
if !matches!(
prop,
"prototype" | "name" | "length" | "caller" | "arguments" | "constructor"
) && !prop.as_bytes().first().is_some_and(|b| b.is_ascii_digit())
{
thread_local! {
static IN_FN_PROTO_FALLBACK: std::cell::Cell<bool> =
const { std::cell::Cell::new(false) };
}
let reentrant = IN_FN_PROTO_FALLBACK.with(|c| c.replace(true));
if !reentrant {
let proto_val = crate::object::builtin_prototype_value("Function");
IN_FN_PROTO_FALLBACK.with(|c| c.set(false));
let proto_jv = crate::value::JSValue::from_bits(proto_val.to_bits());
if proto_jv.is_pointer() {
let proto_ptr = (proto_jv.bits() & crate::value::POINTER_MASK) as usize;
// ONLY user expandos walk through (a `Function.prototype.x
// = …` write records no attrs). Methods installed at init
// (`apply`, `call`, `hasOwnProperty`, …) stay excluded:
// serving those generic thunks to closure reads hijacks the
// dedicated dispatch arms (`p.call(...)`'s undefined-read
// fallback to method-dispatch-by-name is what routes the
// proxy APPLY trap). `fn.apply`-style VALUE reads through a
// proxy are reified receiver-correctly by `js_proxy_get`.
let routed_method = false;
if proto_ptr != 0
&& proto_ptr != ptr
&& !is_closure_ptr(proto_ptr)
&& (routed_method
|| crate::object::get_property_attrs(proto_ptr, prop).is_none())
{
// A defineProperty accessor on Function.prototype
// (`{ get: () => 12 }`) is invoked with the reading
// closure as receiver.
if !routed_method {
if let Some(acc) = crate::object::get_accessor_descriptor(proto_ptr, prop) {
if acc.get != 0 {
let getter = (acc.get & crate::value::POINTER_MASK)
as *const crate::closure::ClosureHeader;
if !getter.is_null() {
let receiver = crate::value::js_nanbox_pointer(ptr as i64);
let prev = crate::object::js_implicit_this_set(receiver);
let result = crate::closure::js_closure_call0(getter);
crate::object::js_implicit_this_set(prev);
return result;
}
}
return f64::from_bits(crate::value::TAG_UNDEFINED);
}
}
unsafe {
let key_hdr =
crate::string::js_string_from_bytes(prop.as_ptr(), prop.len() as u32);
let v = crate::object::js_object_get_field_by_name(
proto_ptr as *const crate::object::ObjectHeader,
key_hdr as *const crate::StringHeader,
);
if !v.is_undefined() {
return f64::from_bits(v.bits());
}
}
// installed there (`Function.prototype.property = 12`), or a property
// installed via `Object.defineProperty(Function.prototype, k, {...})`,
// must be readable through any closure (`fn.property`, `boundFn.property`,
// `Function.indicator`).
if let Some(proto_ptr) = function_prototype_fallback_target(ptr, prop) {
// A defineProperty accessor on Function.prototype
// (`{ get: () => 12 }`) is invoked with the reading
// closure as receiver.
if let Some(acc) = crate::object::get_accessor_descriptor(proto_ptr, prop) {
if acc.get != 0 {
let getter =
(acc.get & crate::value::POINTER_MASK) as *const crate::closure::ClosureHeader;
if !getter.is_null() {
let receiver = crate::value::js_nanbox_pointer(ptr as i64);
let prev = crate::object::js_implicit_this_set(receiver);
let result = crate::closure::js_closure_call0(getter);
crate::object::js_implicit_this_set(prev);
return result;
}
}
return f64::from_bits(crate::value::TAG_UNDEFINED);
}
unsafe {
let key_hdr = crate::string::js_string_from_bytes(prop.as_ptr(), prop.len() as u32);
let v = crate::object::js_object_get_field_by_name(
proto_ptr as *const crate::object::ObjectHeader,
key_hdr as *const crate::StringHeader,
);
if !v.is_undefined() {
return f64::from_bits(v.bits());
}
}
}
f64::from_bits(crate::value::TAG_UNDEFINED)
}

/// Resolve the real, mutable `%Function.prototype%` object pointer for a
/// closure-receiver fallback (GET or SET), or `None` if `prop` doesn't
/// qualify — a synthesized own slot, a reified method name (`apply`, `call`,
/// `bind`, …: serving those generic thunks to closure reads/writes hijacks
/// the dedicated dispatch arms, e.g. `p.call(...)`'s undefined-read fallback
/// to method-dispatch-by-name routes the proxy APPLY trap — `fn.apply`-style
/// VALUE reads through a proxy are reified receiver-correctly by
/// `js_proxy_get` instead), an array-index-shaped key, or resolving would
/// recurse back into `Function.prototype` itself. Shared by
/// [`closure_get_dynamic_prop`]'s expando/defineProperty walk and the
/// closure SET path in `object::field_set_by_name`, so
/// `Object.defineProperty(Function.prototype, k, {get,set})` round-trips
/// through `boundFn.k = v` the same way it does through `boundFn.k`. A
/// re-entrancy guard covers the recursion through `builtin_prototype_value`
/// (which reads `Function.prototype` via `closure_get_dynamic_prop` itself).
pub(crate) fn function_prototype_fallback_target(ptr: usize, prop: &str) -> Option<usize> {
if matches!(
prop,
"prototype" | "name" | "length" | "caller" | "arguments" | "constructor"
// Universal Object.prototype method names: every receiver (closures
// included) resolves these through a dedicated native dispatch arm,
// not a literal field on the walked prototype object. Serving a
// generic-lookup result for one of these hijacks that dispatch —
// e.g. `m.propertyIsEnumerable` resolved a same-named-but-wrong
// value via this fallback, so `m.propertyIsEnumerable("length")`
// called the wrong thing (test262 S15.2.4.3_A8 / S15.2.4.4_A8 /
// S15.2.4.7_A8 regressions caught after the initial fix).
| "toString" | "valueOf" | "hasOwnProperty" | "isPrototypeOf"
| "propertyIsEnumerable" | "toLocaleString"
) || prop.as_bytes().first().is_some_and(|b| b.is_ascii_digit())
|| crate::object::reified_function_method_name(prop).is_some()
{
return None;
}
thread_local! {
static IN_FN_PROTO_FALLBACK: std::cell::Cell<bool> =
const { std::cell::Cell::new(false) };
}
let reentrant = IN_FN_PROTO_FALLBACK.with(|c| c.replace(true));
if reentrant {
return None;
}
let proto_val = crate::object::builtin_prototype_value("Function");
IN_FN_PROTO_FALLBACK.with(|c| c.set(false));
let proto_jv = crate::value::JSValue::from_bits(proto_val.to_bits());
if !proto_jv.is_pointer() {
return None;
}
let proto_ptr = (proto_jv.bits() & crate::value::POINTER_MASK) as usize;
if proto_ptr == 0 || proto_ptr == ptr || is_closure_ptr(proto_ptr) {
return None;
}
Some(proto_ptr)
}

/// SET-side analog of `closure_get_dynamic_prop`'s inherited-accessor read:
/// if `prop` resolves to a descriptor installed on the real
/// `%Function.prototype%` object (`Object.defineProperty(Function.prototype,
/// k, {...})`), apply spec `[[Set]]` semantics for it and report the write
/// handled — an ACCESSOR invokes its setter (if any) with `receiver` as
/// `this`; a non-writable DATA property blocks the write (matches the
/// silent-no-op convention this file already uses for a non-writable OWN
/// attrs record, just above this function's callers). Returns `false` when
/// there's no inherited descriptor at all, or it's a writable DATA property —
/// an ordinary `[[Set]]` on those creates a new OWN property on the receiver,
/// which the caller's existing own-dynamic-prop fallback already does
/// correctly. `ptr` is the closure being checked against (used only to
/// reject the Function.prototype self-reference); `receiver` is the spec
/// `[[Set]]` receiver — ordinarily the same object, but callers reached via
/// `Reflect.set(target, k, v, R)` pass a distinct `R`.
pub(crate) fn closure_set_via_function_prototype_descriptor(
ptr: usize,
prop: &str,
value: f64,
receiver: f64,
) -> bool {
let Some(proto_ptr) = function_prototype_fallback_target(ptr, prop) else {
return false;
};
if let Some(acc) = crate::object::get_accessor_descriptor(proto_ptr, prop) {
if acc.set == 0 {
// Getter-only: matches `al_set_length`'s getter-only `length` throw
// (array/generic.rs) — a strict-mode write to an accessor with no
// setter is a TypeError, not a silent no-op.
crate::collection_iter::throw_type_error(&format!(
"Cannot set property {prop} of #<Function> which has only a getter"
));
}
unsafe { crate::object::invoke_accessor_setter(acc.set, receiver, value) };
return true;
}
if let Some(attrs) = crate::object::get_property_attrs(proto_ptr, prop) {
if !attrs.writable() {
return true;
}
}
false
}

/// Set a dynamic property on a closure.
pub fn closure_set_dynamic_prop(ptr: usize, prop: &str, value: f64) {
if let Ok(mut props) = get_closure_props().lock() {
Expand Down
3 changes: 2 additions & 1 deletion crates/perry-runtime/src/closure/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ pub use unbox::js_closure_unbox_callee_checked;
pub(crate) use dynamic_props::test_clear_closure_side_tables;
pub(crate) use dynamic_props::{
clone_closure_rebind_this, closure_dynamic_props_owner_moved,
visit_closure_dynamic_prop_value_slots_mut, visit_closure_static_prototype_slot_mut,
closure_set_via_function_prototype_descriptor, visit_closure_dynamic_prop_value_slots_mut,
visit_closure_static_prototype_slot_mut,
};
pub use dynamic_props::{
closure_delete_own_dynamic_prop, closure_dynamic_props_snapshot, closure_get_dynamic_prop,
Expand Down
130 changes: 62 additions & 68 deletions crates/perry-runtime/src/object/field_set_by_name.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,66 @@ unsafe fn string_key_eq(key: *const crate::StringHeader, expected: &[u8]) -> boo
std::slice::from_raw_parts(data, len) == expected
}

/// Shared closure-receiver named-property WRITE path — used by both ways a
/// closure is recognized in `js_object_set_field_by_name` (a `GC_TYPE_CLOSURE`
/// GcHeader-typed pointer, and the raw `CLOSURE_MAGIC`-tagged fallback for a
/// pointer reached without a full GC header). #3143: honors a non-writable
/// registered descriptor (a built-in method's `.name`/`.length` are spec'd
/// `writable: false`); `Object.defineProperty(Function.prototype, k, {...})`
/// round-trips via `closure_set_via_function_prototype_descriptor` before
/// falling back to a plain own-property write.
unsafe fn closure_set_field_by_name(
obj: *mut ObjectHeader,
key: *const crate::StringHeader,
value: f64,
) {
if key.is_null() {
return;
}
let name_ptr = (key as *const u8).add(std::mem::size_of::<crate::StringHeader>());
let name_len = (*key).byte_len as usize;
let name_bytes = std::slice::from_raw_parts(name_ptr, name_len);
let Ok(name_str) = std::str::from_utf8(name_bytes) else {
return;
};
// ECMAScript "poison pill" — assigning `caller`/`arguments` on any
// strict-mode function (Perry compiles everything strict: declarations,
// expressions, bound and built-in closures, arrows) throws via the
// %ThrowTypeError% accessor's missing setter. A genuine own data prop of
// that name (defineProperty round-trip) still wins.
// Refs test262 13.2-*-s / StrictFunction_restricted-*.
if matches!(name_str, "caller" | "arguments")
&& !crate::closure::closure_has_own_dynamic_prop(obj as usize, name_str)
{
crate::fs::validate::throw_type_error_with_code(
"Restricted function property assignment",
"ERR_INVALID_ARG_TYPE",
);
}
if let Some(attrs) = super::get_property_attrs(obj as usize, name_str) {
if !attrs.writable() {
return;
}
} else if matches!(name_str, "name" | "length") {
return;
} else if !crate::closure::closure_has_own_dynamic_prop(obj as usize, name_str)
&& crate::closure::closure_set_via_function_prototype_descriptor(
obj as usize,
name_str,
value,
crate::value::js_nanbox_pointer(obj as i64),
)
{
// Handled by an inherited %Function.prototype% descriptor
// (`Object.defineProperty(Function.prototype, k, {...})`) — an
// accessor's setter ran (or threw for a getter-only accessor), or a
// non-writable data property blocked the write; no own property is
// created.
return;
}
crate::closure::closure_set_dynamic_prop(obj as usize, name_str, value);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

/// Set a field value by its string key name (dynamic property access)
/// This searches the keys array for a match and sets the corresponding value.
/// If the key doesn't exist, it adds it to the object.
Expand Down Expand Up @@ -581,36 +641,7 @@ pub extern "C" fn js_object_set_field_by_name(
}

if gc_type == crate::gc::GC_TYPE_CLOSURE {
if !key.is_null() {
let name_ptr = (key as *const u8).add(std::mem::size_of::<crate::StringHeader>());
let name_len = (*key).byte_len as usize;
let name_bytes = std::slice::from_raw_parts(name_ptr, name_len);
if let Ok(name_str) = std::str::from_utf8(name_bytes) {
// ECMAScript "poison pill" — assigning `caller`/`arguments`
// on any strict-mode function (Perry compiles everything
// strict: declarations, expressions, bound and built-in
// closures, arrows) throws via the %ThrowTypeError%
// accessor's missing setter. A genuine own data prop of
// that name (defineProperty round-trip) still wins.
// Refs test262 13.2-*-s / StrictFunction_restricted-*.
if matches!(name_str, "caller" | "arguments")
&& !crate::closure::closure_has_own_dynamic_prop(obj as usize, name_str)
{
crate::fs::validate::throw_type_error_with_code(
"Restricted function property assignment",
"ERR_INVALID_ARG_TYPE",
);
}
if let Some(attrs) = super::get_property_attrs(obj as usize, name_str) {
if !attrs.writable() {
return;
}
} else if matches!(name_str, "name" | "length") {
return;
}
crate::closure::closure_set_dynamic_prop(obj as usize, name_str, value);
}
}
closure_set_field_by_name(obj, key, value);
return;
}

Expand All @@ -620,44 +651,7 @@ pub extern "C" fn js_object_set_field_by_name(
let type_tag_at_12 =
*((obj as *const u8).add(crate::closure::CLOSURE_TYPE_TAG_OFFSET) as *const u32);
if type_tag_at_12 == crate::closure::CLOSURE_MAGIC {
if !key.is_null() {
let name_ptr = (key as *const u8).add(std::mem::size_of::<crate::StringHeader>());
let name_len = (*key).byte_len as usize;
let name_bytes = std::slice::from_raw_parts(name_ptr, name_len);
if let Ok(name_str) = std::str::from_utf8(name_bytes) {
// ECMAScript "poison pill" — assigning `caller`/`arguments`
// on any strict-mode function (Perry compiles everything
// strict: declarations, expressions, bound and built-in
// closures, arrows) throws via the %ThrowTypeError%
// accessor's missing setter. A genuine own data prop of
// that name (defineProperty round-trip) still wins.
// Refs test262 13.2-*-s / StrictFunction_restricted-*.
if matches!(name_str, "caller" | "arguments")
&& !crate::closure::closure_has_own_dynamic_prop(obj as usize, name_str)
{
crate::fs::validate::throw_type_error_with_code(
"Restricted function property assignment",
"ERR_INVALID_ARG_TYPE",
);
}
// #3143: honor a non-writable registered descriptor — a
// built-in method's `.name`/`.length` are spec'd
// `writable: false`, so a sloppy-mode write must be a silent
// no-op (this is what Test262's `verifyProperty` checks).
// Only fires when a descriptor was actually recorded for
// this closure+key (built-in proto methods, or a user
// `Object.defineProperty`); plain `fn.x = 1` finds none and
// proceeds. Closure writes are not the object hot path.
if let Some(attrs) = super::get_property_attrs(obj as usize, name_str) {
if !attrs.writable() {
return;
}
} else if matches!(name_str, "name" | "length") {
return;
}
crate::closure::closure_set_dynamic_prop(obj as usize, name_str, value);
}
}
closure_set_field_by_name(obj, key, value);
return;
}

Expand Down
Loading
Loading