Skip to content

Commit 14bbe9a

Browse files
committed
ZJIT: Track super method entries directly to avoid GC issues
Because the method entry isn't typed as a `VALUE`, we set up barriers on its `VALUE` fields. But, that was insufficient as the method entry itself could be collected in certain cases, resulting in dangling objects. Now we track the method entry as a `VALUE` and can more naturally mark it and its children.
1 parent 166d4d8 commit 14bbe9a

File tree

1 file changed

+18
-13
lines changed

1 file changed

+18
-13
lines changed

zjit/src/profile.rs

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -157,16 +157,17 @@ fn profile_block_handler(profiler: &mut Profiler, profile: &mut IseqProfile) {
157157

158158
fn profile_invokesuper(profiler: &mut Profiler, profile: &mut IseqProfile) {
159159
let cme = unsafe { rb_vm_frame_method_entry(profiler.cfp) };
160+
let cme_value = VALUE(cme as usize); // CME is a T_IMEMO, which is a VALUE
160161

161162
match profile.super_cme.get(&profiler.insn_idx) {
162163
None => {
163164
// If `None`, then this is our first time looking at `super` for this instruction.
164-
profile.super_cme.insert(profiler.insn_idx, Some(cme));
165+
profile.super_cme.insert(profiler.insn_idx, Some(cme_value));
165166
},
166167
Some(Some(existing_cme)) => {
167168
// Check if the stored method entry is the same as the current one. If it isn't, then
168169
// mark the call site as polymorphic.
169-
if *existing_cme != cme {
170+
if *existing_cme != cme_value {
170171
profile.super_cme.insert(profiler.insn_idx, None);
171172
}
172173
}
@@ -176,8 +177,7 @@ fn profile_invokesuper(profiler: &mut Profiler, profile: &mut IseqProfile) {
176177
}
177178
}
178179

179-
unsafe { rb_gc_writebarrier(profiler.iseq.into(), (*cme).defined_class) };
180-
unsafe { rb_gc_writebarrier(profiler.iseq.into(), (*cme).owner) };
180+
unsafe { rb_gc_writebarrier(profiler.iseq.into(), cme_value) };
181181

182182
let cd: *const rb_call_data = profiler.insn_opnd(0).as_ptr();
183183
let argc = unsafe { vm_ci_argc((*cd).ci) };
@@ -358,8 +358,8 @@ pub struct IseqProfile {
358358
/// Number of profiled executions for each YARV instruction, indexed by the instruction index
359359
num_profiles: Vec<NumProfiles>,
360360

361-
/// Method entries for `super` calls
362-
super_cme: HashMap<usize, Option<*const rb_callable_method_entry_t>>
361+
/// Method entries for `super` calls (stored as VALUE to be GC-safe)
362+
super_cme: HashMap<usize, Option<VALUE>>
363363
}
364364

365365
impl IseqProfile {
@@ -377,7 +377,8 @@ impl IseqProfile {
377377
}
378378

379379
pub fn get_super_method_entry(&self, insn_idx: usize) -> Option<*const rb_callable_method_entry_t> {
380-
self.super_cme.get(&insn_idx).copied().flatten()
380+
self.super_cme.get(&insn_idx)
381+
.and_then(|opt| opt.map(|v| v.0 as *const rb_callable_method_entry_t))
381382
}
382383

383384
/// Run a given callback with every object in IseqProfile
@@ -391,12 +392,9 @@ impl IseqProfile {
391392
}
392393
}
393394

394-
for method_entry in self.super_cme.values() {
395-
if let Some(cme) = method_entry {
396-
unsafe {
397-
callback((**cme).defined_class);
398-
callback((**cme).owner);
399-
}
395+
for cme_value in self.super_cme.values() {
396+
if let Some(cme) = cme_value {
397+
callback(*cme);
400398
}
401399
}
402400
}
@@ -411,6 +409,13 @@ impl IseqProfile {
411409
}
412410
}
413411
}
412+
413+
// Update CME references if they move during compaction
414+
for cme_value in self.super_cme.values_mut() {
415+
if let Some(cme) = cme_value {
416+
callback(cme);
417+
}
418+
}
414419
}
415420
}
416421

0 commit comments

Comments
 (0)