Skip to content

Commit 8d837df

Browse files
proggeramlugRalph Küpper
andauthored
fix(gc): complete IMPLICIT_THIS receiver rooting (two-line save forms) (#8519)
* fix(gc): root displaced receiver in two-line IMPLICIT_THIS save/restore forms * chore: bump version to 0.5.1515 * chore: drop the version bump and PR-key the changelog fragment Standing instruction is no version bump for now; the maintainer bumps at merge time. Reverts CLAUDE.md, Cargo.toml and the 76 Cargo.lock version strings to main's 0.5.1514. Renames the fragment to match this PR number so in-flight PRs cannot collide on the filename. --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 parent 84adb7f commit 8d837df

4 files changed

Lines changed: 30 additions & 12 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
**GC: complete the `IMPLICIT_THIS` receiver-rooting conversion started in #8500.**
2+
3+
#8500 converted 22 receiver save/restore pairs that manipulated the `IMPLICIT_THIS` cell directly, rooting the displaced receiver so an evacuating collection inside the callee cannot leave the restore publishing a pre-move address. Four sites of the identical shape were missed because the save spans two lines (`let prev =` on one line, the `IMPLICIT_THIS.with(|c| c.replace(…))` on the next) and so did not match the pattern used to find the others.
4+
5+
No behavioural test moves — these four are latent rather than currently reachable by a failing case — but leaving four instances of a shape that is known to produce a stale `this` is the kind of residue that costs a future session a week. The conversion is now exhaustive: no unrooted direct-access save/restore pair remains in `perry-runtime`.
6+
7+
Does **not** address #8507 (`defineProperty` / `Reflect` own-key paths under forced evacuation), which is a distinct defect — verified by re-running that suite with these four converted, and again with all 135 `js_implicit_this_set(...)`-call pairs converted.

crates/perry-runtime/src/object/native_call_method.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1535,8 +1535,10 @@ pub unsafe extern "C-unwind" fn js_native_call_method(
15351535
let getter = (acc.get & crate::value::POINTER_MASK)
15361536
as *const crate::closure::ClosureHeader;
15371537
if !getter.is_null() {
1538-
let prev_getter_this =
1539-
IMPLICIT_THIS.with(|c| c.replace(object().to_bits()));
1538+
// #8495: root the displaced receiver across the call below.
1539+
let prev_getter_this_scope = crate::gc::RuntimeHandleScope::new();
1540+
let prev_getter_this_h = prev_getter_this_scope
1541+
.root_nanbox_u64(IMPLICIT_THIS.with(|c| c.replace(object().to_bits())));
15401542
let method_fn = crate::closure::js_closure_call0(getter);
15411543
let bound = crate::closure::clone_closure_rebind_this(
15421544
method_fn.to_bits(),
@@ -1553,7 +1555,7 @@ pub unsafe extern "C-unwind" fn js_native_call_method(
15531555
call_args.as_ptr(),
15541556
call_args.len(),
15551557
);
1556-
IMPLICIT_THIS.with(|c| c.set(prev_getter_this));
1558+
IMPLICIT_THIS.with(|c| c.set(prev_getter_this_h.get_nanbox_u64()));
15571559
return result;
15581560
}
15591561
}

crates/perry-runtime/src/object/native_call_method/handle_methods.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,8 +1080,11 @@ pub(super) unsafe fn dispatch_handle(
10801080
// Mirrors `resolve_proto_chain_field_with_receiver`
10811081
// (the winston `get transports()` fix).
10821082
let receiver_f64 = f64::from_bits(jsval.bits());
1083-
let prev_this =
1084-
IMPLICIT_THIS.with(|c| c.replace(receiver_f64.to_bits()));
1083+
// #8495: root the displaced receiver across the call below.
1084+
let prev_this_scope = crate::gc::RuntimeHandleScope::new();
1085+
let prev_this_h = prev_this_scope.root_nanbox_u64(
1086+
IMPLICIT_THIS.with(|c| c.replace(receiver_f64.to_bits())),
1087+
);
10851088
let prev_override =
10861089
super::super::field_get_set::accessor_receiver_override_begin(
10871090
receiver_f64,
@@ -1093,7 +1096,7 @@ pub(super) unsafe fn dispatch_handle(
10931096
super::super::field_get_set::accessor_receiver_override_end(
10941097
prev_override,
10951098
);
1096-
IMPLICIT_THIS.with(|c| c.set(prev_this));
1099+
IMPLICIT_THIS.with(|c| c.set(prev_this_h.get_nanbox_u64()));
10971100
if !field_val.is_undefined() && !field_val.is_null() {
10981101
resolved_method = Some(ResolvedMethod::ProtoClosure {
10991102
field_bits: field_val.bits(),

crates/perry-runtime/src/object/native_call_method/primitive_methods.rs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,17 @@ pub(super) unsafe fn dispatch_primitive(
9797
);
9898
let prop_handle = root_scope.root_nanbox_f64(f64::from_bits(bound));
9999
let args = refreshed_args();
100-
let prev_this =
101-
IMPLICIT_THIS.with(|c| c.replace(object_handle.get_nanbox_f64().to_bits()));
100+
// #8495: root the displaced receiver across the call below.
101+
let prev_this_scope = crate::gc::RuntimeHandleScope::new();
102+
let prev_this_h = prev_this_scope.root_nanbox_u64(
103+
IMPLICIT_THIS.with(|c| c.replace(object_handle.get_nanbox_f64().to_bits())),
104+
);
102105
let result = crate::closure::js_native_call_value(
103106
prop_handle.get_nanbox_f64(),
104107
args.as_ptr(),
105108
args.len(),
106109
);
107-
IMPLICIT_THIS.with(|c| c.set(prev_this));
110+
IMPLICIT_THIS.with(|c| c.set(prev_this_h.get_nanbox_u64()));
108111
return Some(result);
109112
}
110113
}
@@ -528,14 +531,17 @@ pub(super) unsafe fn dispatch_primitive(
528531
);
529532
let prop_handle = root_scope.root_nanbox_f64(f64::from_bits(bound));
530533
let args = refreshed_args();
531-
let prev_this =
532-
IMPLICIT_THIS.with(|c| c.replace(object_handle.get_nanbox_f64().to_bits()));
534+
// #8495: root the displaced receiver across the call below.
535+
let prev_this_scope = crate::gc::RuntimeHandleScope::new();
536+
let prev_this_h = prev_this_scope.root_nanbox_u64(
537+
IMPLICIT_THIS.with(|c| c.replace(object_handle.get_nanbox_f64().to_bits())),
538+
);
533539
let result = crate::closure::js_native_call_value(
534540
prop_handle.get_nanbox_f64(),
535541
args.as_ptr(),
536542
args.len(),
537543
);
538-
IMPLICIT_THIS.with(|c| c.set(prev_this));
544+
IMPLICIT_THIS.with(|c| c.set(prev_this_h.get_nanbox_u64()));
539545
return Some(result);
540546
}
541547
// Own key present but NOT callable (`re.exec = 5; re.exec()`).

0 commit comments

Comments
 (0)