Skip to content

Commit 38d31dc

Browse files
authored
ZJIT: Untag block handler (ruby#15085)
Storing the tagged block handler in profiles is not GC-safe (nice catch, Kokubun). Store the untagged block handler instead. Fix bug in ruby#15051
1 parent c6c92bd commit 38d31dc

File tree

6 files changed

+28
-23
lines changed

6 files changed

+28
-23
lines changed

vm_insnhelper.c

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5497,12 +5497,6 @@ vm_invoke_proc_block(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp,
54975497
return vm_invoke_block(ec, reg_cfp, calling, ci, is_lambda, block_handler);
54985498
}
54995499

5500-
enum rb_block_handler_type
5501-
rb_vm_block_handler_type(VALUE block_handler)
5502-
{
5503-
return vm_block_handler_type(block_handler);
5504-
}
5505-
55065500
static inline VALUE
55075501
vm_invoke_block(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp,
55085502
struct rb_calling_info *calling, const struct rb_callinfo *ci,
@@ -6065,10 +6059,30 @@ vm_define_method(const rb_execution_context_t *ec, VALUE obj, ID id, VALUE iseqv
60656059
}
60666060
}
60676061

6062+
// Return the untagged block handler:
6063+
// * If it's an ISEQ or an IFUNC, fetch it from its rb_captured_block
6064+
// * If it's a PROC or SYMBOL, return it as is
6065+
static VALUE
6066+
rb_vm_untag_block_handler(VALUE block_handler)
6067+
{
6068+
switch (vm_block_handler_type(block_handler)) {
6069+
case block_handler_type_iseq:
6070+
case block_handler_type_ifunc: {
6071+
struct rb_captured_block *captured = VM_TAGGED_PTR_REF(block_handler, 0x03);
6072+
return captured->code.val;
6073+
}
6074+
case block_handler_type_proc:
6075+
case block_handler_type_symbol:
6076+
return block_handler;
6077+
default:
6078+
rb_bug("rb_vm_untag_block_handler: unreachable");
6079+
}
6080+
}
6081+
60686082
VALUE
6069-
rb_vm_get_block_handler(rb_control_frame_t *reg_cfp)
6083+
rb_vm_get_untagged_block_handler(rb_control_frame_t *reg_cfp)
60706084
{
6071-
return VM_CF_BLOCK_HANDLER(reg_cfp);
6085+
return rb_vm_untag_block_handler(VM_CF_BLOCK_HANDLER(reg_cfp));
60726086
}
60736087

60746088
static VALUE

zjit.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,8 +302,7 @@ rb_zjit_class_has_default_allocator(VALUE klass)
302302
}
303303

304304

305-
VALUE rb_vm_get_block_handler(rb_control_frame_t *reg_cfp);
306-
enum rb_block_handler_type rb_vm_block_handler_type(VALUE block_handler);
305+
VALUE rb_vm_get_untagged_block_handler(rb_control_frame_t *reg_cfp);
307306

308307
// Primitives used by zjit.rb. Don't put other functions below, which wouldn't use them.
309308
VALUE rb_zjit_assert_compiles(rb_execution_context_t *ec, VALUE self);

zjit/bindgen/src/main.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -399,8 +399,7 @@ fn main() {
399399
.allowlist_function("rb_yarv_str_eql_internal")
400400
.allowlist_function("rb_str_neq_internal")
401401
.allowlist_function("rb_yarv_ary_entry_internal")
402-
.allowlist_function("rb_vm_get_block_handler")
403-
.allowlist_function("rb_vm_block_handler_type")
402+
.allowlist_function("rb_vm_get_untagged_block_handler")
404403
.allowlist_function("rb_FL_TEST")
405404
.allowlist_function("rb_FL_TEST_RAW")
406405
.allowlist_function("rb_RB_TYPE_P")

zjit/src/cruby_bindings.inc.rs

Lines changed: 1 addition & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

zjit/src/hir.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4421,10 +4421,9 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
44214421
let summary = TypeDistributionSummary::new(&self_type_distribution);
44224422
if summary.is_monomorphic() {
44234423
let obj = summary.bucket(0).class();
4424-
let bh_type = unsafe { rb_vm_block_handler_type(obj) };
4425-
if bh_type == block_handler_type_iseq {
4424+
if unsafe { rb_IMEMO_TYPE_P(obj, imemo_iseq) == 1 } {
44264425
fun.push_insn(block, Insn::IncrCounter(Counter::invokeblock_handler_monomorphic_iseq));
4427-
} else if bh_type == block_handler_type_ifunc {
4426+
} else if unsafe { rb_IMEMO_TYPE_P(obj, imemo_ifunc) == 1 } {
44284427
fun.push_insn(block, Insn::IncrCounter(Counter::invokeblock_handler_monomorphic_ifunc));
44294428
} else {
44304429
fun.push_insn(block, Insn::IncrCounter(Counter::invokeblock_handler_monomorphic_other));

zjit/src/profile.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ impl Profiler {
4545
}
4646

4747
fn peek_at_block_handler(&self) -> VALUE {
48-
unsafe { rb_vm_get_block_handler(self.cfp) }
48+
unsafe { rb_vm_get_untagged_block_handler(self.cfp) }
4949
}
5050
}
5151

0 commit comments

Comments
 (0)