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
41 changes: 39 additions & 2 deletions crates/perry-codegen/src/lower_call/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -781,8 +781,45 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) ->
// Layout constants are duplicated here from the runtime; if
// `GcHeader` or `ObjectHeader` ever change in
// `crates/perry-runtime/src/{gc,object}.rs`, update both sides.
let obj_handle = if let Some(keys_global_name) = ctx.class_keys_globals.get(class_name).cloned()
{
let obj_handle = if class.extends_expr.is_some() {
// Wall 45: dynamic-parent subclass (`class X extends _mod.default`).
// The parent's field layout is unknown at this compile time (the
// `extends` target is an unresolvable cross-module value, so the
// parent-chain walk above contributed 0 fields and `field_count` /
// `packed_keys` cover only X's OWN fields). Allocating with that
// own-only layout under-sizes and mis-lays-out the instance: the
// parent's constructor and inherited methods address the inherited
// fields at the PARENT's slot indices (parent fields first), which fall
// past X's own slots → OOB heap reads (captures read as garbage).
// Route to `js_object_alloc_class_dynamic_parent`, which resolves the
// runtime-registered parent edge + keys-array (both established at
// module init by `js_register_class_parent_dynamic` /
// `js_build_class_keys_array`, before any `new X()`) and allocates with
// the merged `[parent keys..] ++ [own keys..]` layout. Bypasses the
// inline bump-alloc fast path (which would bake the wrong layout).
let mut packed_keys = String::new();
for f in &class.fields {
if f.key_expr.is_some() {
continue;
}
packed_keys.push_str(&f.name);
packed_keys.push('\0');
}
let keys_idx = ctx.strings.intern(&packed_keys);
let keys_entry = ctx.strings.entry(keys_idx);
let keys_global = format!("@{}", keys_entry.bytes_global);
let keys_len_str = keys_entry.byte_len.to_string();
ctx.block().call(
I64,
"js_object_alloc_class_dynamic_parent",
&[
(I32, &cid_str),
(I32, &n_str),
(PTR, &keys_global),
(I32, &keys_len_str),
],
)
} else if let Some(keys_global_name) = ctx.class_keys_globals.get(class_name).cloned() {
// Compile-time layout constants.
const GC_HEADER_SIZE: u64 = 8;
const OBJECT_HEADER_SIZE: u64 = 24;
Expand Down
10 changes: 10 additions & 0 deletions crates/perry-codegen/src/runtime_decls/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,16 @@ pub fn declare_phase_b_strings(module: &mut LlModule) {
I64,
&[I32, I32, I32, PTR, I32],
);
// Wall 45: merged-layout allocator for dynamic-parent subclasses
// (`class X extends _mod.default`). Args: (class_id, own_field_count,
// own_packed_keys, own_packed_keys_len). Resolves the runtime-registered
// parent layout and allocates `[parent keys..] ++ [own keys..]` so
// inherited fields land at the parent's slot indices.
module.declare_function(
"js_object_alloc_class_dynamic_parent",
I64,
&[I32, I32, PTR, I32],
);
// Fast class allocator that takes a pre-built keys_array pointer
// directly, bypassing the per-call SHAPE_CACHE lookup. The codegen
// emits one `js_build_class_keys_array` call at module init per
Expand Down
133 changes: 133 additions & 0 deletions crates/perry-runtime/src/object/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,139 @@ pub extern "C" fn js_object_alloc_class_with_keys(
ptr
}

/// Allocate a subclass instance whose parent was resolved DYNAMICALLY at
/// runtime — the `class X extends _mod.default` interop-ESM shape (wall 38).
///
/// At X's compile time the parent's field layout is unknown (the `extends`
/// target is an unresolvable cross-module value, so X's `extends_name` is the
/// unresolved `"default"` and `class_field_global_index`'s parent walk bails),
/// so codegen can only size the instance for X's OWN fields. That
/// under-allocates and mis-lays-out the instance: the parent's constructor (run
/// on this `this` via `run_class_constructor_on_this_flat`) and the parent's
/// inherited methods both address the inherited `__perry_cap_*` / declared
/// fields at the PARENT's own slot indices (parent fields come first in the
/// layout), which lie past X's own-only slots → out-of-bounds reads/writes into
/// adjacent heap. That is wall 45 (`Derived extends _base.default` reads
/// `_c10`/`_c20` captures as garbage numbers/functions).
///
/// The parent edge (`js_register_class_parent_dynamic`) and the parent's
/// keys-array (`js_build_class_keys_array`) are both registered at module-init
/// time, before any `new X()`. So here — at construction time — resolve them and
/// allocate with the MERGED layout: `field_count = parent_field_count +
/// own_field_count` and `keys_array = [parent keys..] ++ [own keys..]` (parent
/// first, exactly the slot order the parent's compiled methods/ctor expect).
/// The parent's keys-array already encodes its WHOLE chain (it was built
/// parent-first at the parent's own compile time, where its ancestors were
/// known), so the immediate parent's registered keys are sufficient. Falls back
/// to the own-only layout (`js_object_alloc_class_with_keys`) when no dynamic
/// parent / parent keys are registered (e.g. the parent is a builtin or a
/// not-yet-initialized module).
#[no_mangle]
pub extern "C" fn js_object_alloc_class_dynamic_parent(
class_id: u32,
own_field_count: u32,
own_packed_keys: *const u8,
own_packed_keys_len: u32,
) -> *mut ObjectHeader {
let parent_cid = crate::object::get_parent_class_id(class_id).unwrap_or(0);
let parent_keys = if parent_cid != 0 {
registered_class_keys_array(parent_cid)
} else {
None
};
let Some((parent_arr, _parent_fc)) = parent_keys else {
// No dynamic parent layout available — own-only fallback keeps the
// prior baseline (correct for parentless / builtin-parent classes).
return js_object_alloc_class_with_keys(
class_id,
parent_cid,
own_field_count,
own_packed_keys,
own_packed_keys_len,
);
};
let parent_len = unsafe { (*parent_arr).length };

// Cache the merged keys-array per class. The shape id is namespaced away
// from the own-only shape (`+ 2_000_000`) so it can't collide with the
// `js_build_class_keys_array` / `js_object_alloc_class_with_keys` shapes.
let shape_id = class_id.wrapping_mul(10007).wrapping_add(2_000_000);
let cached = shape_cache_get(shape_id);
let (merged_arr, field_count) = if !cached.is_null() {
(cached, unsafe { (*cached).length })
} else {
let own_keys: Vec<&[u8]> = if own_packed_keys.is_null() || own_packed_keys_len == 0 {
Vec::new()
} else {
let bytes = unsafe {
std::slice::from_raw_parts(own_packed_keys, own_packed_keys_len as usize)
};
bytes.split(|&b| b == 0).filter(|s| !s.is_empty()).collect()
};
let merged_len = parent_len as usize + own_keys.len();
let arr = crate::array::js_array_alloc_with_length_longlived(merged_len as u32);
let dst = unsafe { (arr as *mut u8).add(8) as *mut f64 };
let src = unsafe { (parent_arr as *mut u8).add(8) as *const f64 };
unsafe {
for i in 0..parent_len as usize {
let bits = (*src.add(i)).to_bits();
// GC_STORE_AUDIT(INIT): initializing fresh longlived keys-array slot
// with a longlived parent key; layout recorded below.
*dst.add(i) = f64::from_bits(bits);
crate::array::note_array_slot_layout_only(arr, i, bits);
}
for (j, key_bytes) in own_keys.iter().enumerate() {
let str_ptr = crate::string::js_string_from_bytes_longlived(
key_bytes.as_ptr(),
key_bytes.len() as u32,
);
let nanboxed = f64::from_bits(
crate::value::STRING_TAG | (str_ptr as u64 & crate::value::POINTER_MASK),
);
let idx = parent_len as usize + j;
// GC_STORE_AUDIT(INIT): initializing fresh longlived keys-array slot
// with a freshly interned longlived key string; layout recorded below.
*dst.add(idx) = nanboxed;
crate::array::note_array_slot_layout_only(arr, idx, nanboxed.to_bits());
}
}
shape_cache_insert(shape_id, arr);
(arr, merged_len as u32)
};

let header_size = std::mem::size_of::<ObjectHeader>();
let alloc_field_count = std::cmp::max(field_count as usize, 8);
let fields_size = alloc_field_count * std::mem::size_of::<JSValue>();
let total_size = header_size + fields_size;
let ptr = arena_alloc_gc(total_size, 8, crate::gc::GC_TYPE_OBJECT) as *mut ObjectHeader;
unsafe {
(*ptr).object_type = crate::error::OBJECT_TYPE_REGULAR;
(*ptr).class_id = class_id;
(*ptr).parent_class_id = parent_cid;
(*ptr).field_count = field_count;
let fields_ptr = (ptr as *mut u8).add(header_size) as *mut JSValue;
for i in 0..alloc_field_count {
// GC_STORE_AUDIT(INIT): freshly allocated object field slot is initialized pointer-free.
ptr::write(fields_ptr.add(i), JSValue::undefined());
}
set_object_keys_array(ptr, merged_arr);
crate::gc::layout_init_pointer_free(ptr as *mut u8);
}
remember_class_keys_array(class_id, field_count, merged_arr);
Comment on lines +427 to +511

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use the registered field counts to size the merged instance.

The merged path throws away both parent_fc and own_field_count and instead derives field_count from keys_array.length. That only works while visible key count exactly matches logical slot count. If those ever diverge, this allocator under-sizes the object/header while codegen still indexes slots by parent_fc + own_field_count, recreating the layout mismatch this function is meant to fix.

Possible fix
-    let Some((parent_arr, _parent_fc)) = parent_keys else {
+    let Some((parent_arr, parent_fc)) = parent_keys else {
         // No dynamic parent layout available — own-only fallback keeps the
         // prior baseline (correct for parentless / builtin-parent classes).
         return js_object_alloc_class_with_keys(
             class_id,
             parent_cid,
@@
-    let (merged_arr, field_count) = if !cached.is_null() {
-        (cached, unsafe { (*cached).length })
+    let field_count = parent_fc.saturating_add(own_field_count);
+    let merged_arr = if !cached.is_null() {
+        cached
     } else {
@@
-        let merged_len = parent_len as usize + own_keys.len();
+        let merged_len = parent_len as usize + own_keys.len();
+        debug_assert_eq!(merged_len as u32, field_count);
         let arr = crate::array::js_array_alloc_with_length_longlived(merged_len as u32);
@@
-        (arr, merged_len as u32)
+        arr
     };
+    debug_assert_eq!(unsafe { (*merged_arr).length }, field_count);
🤖 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/object/alloc.rs` around lines 427 - 507, The merged
path in this function derives field_count from the merged_arr.length (via
merged_len), but it should instead compute field_count by adding the registered
parent field count (parent_fc) and own_field_count parameters. Currently, the
registered field counts (parent_fc extracted from registered_class_keys_array
and own_field_count passed as a parameter) are retrieved but never used in the
merged path, causing the allocated object to be sized based on visible key count
rather than logical slot count. Replace the line that sets field_count based on
merged_len with a calculation that adds parent_fc and own_field_count to ensure
the object allocation matches the slot indexing that codegen will perform.

ptr
}

/// Keepalive anchor — `js_object_alloc_class_dynamic_parent` is a
/// generated-code-only callee, so the auto-optimize whole-program build would
/// otherwise dead-strip it (see the FFI-symbol-link-break class).
#[used]
static KEEP_JS_OBJECT_ALLOC_CLASS_DYNAMIC_PARENT: extern "C" fn(
u32,
u32,
*const u8,
u32,
) -> *mut ObjectHeader = js_object_alloc_class_dynamic_parent;

/// Allocate an object with a shape-cached keys array.
/// First call per shape_id creates the keys array from packed_keys (null-separated key names);
/// subsequent calls reuse the cached pointer. This eliminates per-object key string allocation
Expand Down
143 changes: 141 additions & 2 deletions crates/perry-runtime/src/object/class_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4291,9 +4291,94 @@ pub(crate) unsafe fn call_vtable_method(
arg_or_undefined(call_args_ptr, call_args_len, 8),
)
}
// Arities above the explicit arms: the generated method/ctor signature is
// `double(double this, double×param_count)`. Rust can't form a
// param_count-arity fn pointer dynamically, so transmute to a generous
// fixed arity (64) and pass `param_count` real args plus `undefined`
// padding (`arg_or_undefined` yields undefined past `call_args_len`).
// Passing MORE args than the callee declares is safe on every target —
// the arg area is caller-allocated and caller-cleaned, and the callee
// reads only its declared params. This is the runtime-dispatch counterpart
// to the codegen direct call, and matters for ctors/methods that take many
// params — notably a class capturing dozens of module-level `require`s
// (`__perry_cap_*` params), the wall-45 `Derived extends _mod.default`
// shape, where the pre-fix 10-arg cap silently dropped captures 10+.
// (The prior `_` arm called every >9-arity function as if it had 10
// params.) `debug_assert` flags the rare class that would still exceed
// the bound so it surfaces in tests rather than as silent corruption.
_ => {
let f: extern "C" fn(f64, f64, f64, f64, f64, f64, f64, f64, f64, f64, f64) -> f64 =
std::mem::transmute(func_ptr);
debug_assert!(
param_count as usize <= 64,
"call_vtable_method: param_count {} exceeds fixed dispatch arity 64",
param_count
);
let f: extern "C" fn(
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
f64,
) -> f64 = std::mem::transmute(func_ptr);
f(
this_f64,
arg_or_undefined(call_args_ptr, call_args_len, 0),
Expand All @@ -4306,6 +4391,60 @@ pub(crate) unsafe fn call_vtable_method(
arg_or_undefined(call_args_ptr, call_args_len, 7),
arg_or_undefined(call_args_ptr, call_args_len, 8),
arg_or_undefined(call_args_ptr, call_args_len, 9),
arg_or_undefined(call_args_ptr, call_args_len, 10),
arg_or_undefined(call_args_ptr, call_args_len, 11),
arg_or_undefined(call_args_ptr, call_args_len, 12),
arg_or_undefined(call_args_ptr, call_args_len, 13),
arg_or_undefined(call_args_ptr, call_args_len, 14),
arg_or_undefined(call_args_ptr, call_args_len, 15),
arg_or_undefined(call_args_ptr, call_args_len, 16),
arg_or_undefined(call_args_ptr, call_args_len, 17),
arg_or_undefined(call_args_ptr, call_args_len, 18),
arg_or_undefined(call_args_ptr, call_args_len, 19),
arg_or_undefined(call_args_ptr, call_args_len, 20),
arg_or_undefined(call_args_ptr, call_args_len, 21),
arg_or_undefined(call_args_ptr, call_args_len, 22),
arg_or_undefined(call_args_ptr, call_args_len, 23),
arg_or_undefined(call_args_ptr, call_args_len, 24),
arg_or_undefined(call_args_ptr, call_args_len, 25),
arg_or_undefined(call_args_ptr, call_args_len, 26),
arg_or_undefined(call_args_ptr, call_args_len, 27),
arg_or_undefined(call_args_ptr, call_args_len, 28),
arg_or_undefined(call_args_ptr, call_args_len, 29),
arg_or_undefined(call_args_ptr, call_args_len, 30),
arg_or_undefined(call_args_ptr, call_args_len, 31),
arg_or_undefined(call_args_ptr, call_args_len, 32),
arg_or_undefined(call_args_ptr, call_args_len, 33),
arg_or_undefined(call_args_ptr, call_args_len, 34),
arg_or_undefined(call_args_ptr, call_args_len, 35),
arg_or_undefined(call_args_ptr, call_args_len, 36),
arg_or_undefined(call_args_ptr, call_args_len, 37),
arg_or_undefined(call_args_ptr, call_args_len, 38),
arg_or_undefined(call_args_ptr, call_args_len, 39),
arg_or_undefined(call_args_ptr, call_args_len, 40),
arg_or_undefined(call_args_ptr, call_args_len, 41),
arg_or_undefined(call_args_ptr, call_args_len, 42),
arg_or_undefined(call_args_ptr, call_args_len, 43),
arg_or_undefined(call_args_ptr, call_args_len, 44),
arg_or_undefined(call_args_ptr, call_args_len, 45),
arg_or_undefined(call_args_ptr, call_args_len, 46),
arg_or_undefined(call_args_ptr, call_args_len, 47),
arg_or_undefined(call_args_ptr, call_args_len, 48),
arg_or_undefined(call_args_ptr, call_args_len, 49),
arg_or_undefined(call_args_ptr, call_args_len, 50),
arg_or_undefined(call_args_ptr, call_args_len, 51),
arg_or_undefined(call_args_ptr, call_args_len, 52),
arg_or_undefined(call_args_ptr, call_args_len, 53),
arg_or_undefined(call_args_ptr, call_args_len, 54),
arg_or_undefined(call_args_ptr, call_args_len, 55),
arg_or_undefined(call_args_ptr, call_args_len, 56),
arg_or_undefined(call_args_ptr, call_args_len, 57),
arg_or_undefined(call_args_ptr, call_args_len, 58),
arg_or_undefined(call_args_ptr, call_args_len, 59),
arg_or_undefined(call_args_ptr, call_args_len, 60),
arg_or_undefined(call_args_ptr, call_args_len, 61),
arg_or_undefined(call_args_ptr, call_args_len, 62),
arg_or_undefined(call_args_ptr, call_args_len, 63),
)
}
}
Expand Down