Skip to content

Commit 0f10e48

Browse files
committed
YJIT: Fix super() from a block method not rooted in a "def" method
Previously, we unconditionally passed to the callee `GET_LEP(calling_frame)[VM_ENV_DATA_INDEX_SPECVAL]`, which in case the block containing super() is in e.g. a `class`, did not resolve to a block handler at all. Properly locate the block handler using logic used in Kernel#block_given? and defined?(yield). Rename BlockHandler::LEPSpecVal to FindFromCurrentFrame to reflect this change, as the block handler is not always in LEP. [Bug #22116]
1 parent 3a1832b commit 0f10e48

2 files changed

Lines changed: 66 additions & 6 deletions

File tree

test/ruby/test_yjit.rb

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,6 +1041,55 @@ class Target < Base; end
10411041
RUBY
10421042
end
10431043

1044+
def test_bmethod_super_block_forwarding
1045+
# super() in a block method forwards the block handler of environment
1046+
# it is in. In this test, the block is in a class, which never has
1047+
# a block handler, so Parent#foo gets no block.
1048+
assert_compiles(<<~RUBY, result: nil)
1049+
class Parent
1050+
def foo = defined?(yield)
1051+
end
1052+
1053+
class BmethodSuper < Parent
1054+
define_method(:foo) { super() }
1055+
end
1056+
1057+
BmethodSuper.new.foo {}
1058+
RUBY
1059+
1060+
# For contrast, the super() here forwards the block passed to add_then_override
1061+
# because block with super() is rooted in add_then_override, a "def" method.
1062+
assert_compiles(<<~RUBY, result: [1, 2])
1063+
def add_then_override(object)
1064+
object.define_singleton_method(:then) { super() }
1065+
end
1066+
1067+
add_then_override(one = Object.new) { 1 }
1068+
add_then_override(two = Object.new) { 2 }
1069+
[one.then {}, two.then {}]
1070+
RUBY
1071+
end
1072+
1073+
def test_bug_22116
1074+
# Regression test from report which used to crash during environment escape
1075+
# to pass block to Hash.new
1076+
assert_compiles(<<~RUBY, call_threshold: 1)
1077+
class C
1078+
def foo
1079+
Hash.new {|h, k| h[k] = [] }
1080+
end
1081+
end
1082+
1083+
class D < C
1084+
define_method(:foo) do
1085+
super()
1086+
end
1087+
end
1088+
1089+
D.new.foo
1090+
RUBY
1091+
end
1092+
10441093
# Tests calling a variadic cfunc with many args
10451094
def test_build_large_struct
10461095
assert_compiles(<<~RUBY, insns: %i[opt_send_without_block], call_threshold: 2)

yjit/src/codegen.rs

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6805,8 +6805,8 @@ enum SpecVal {
68056805
pub enum BlockHandler {
68066806
// send, invokesuper: blockiseq operand
68076807
BlockISeq(IseqPtr),
6808-
// invokesuper: GET_BLOCK_HANDLER() (GET_LEP()[VM_ENV_DATA_INDEX_SPECVAL])
6809-
LEPSpecVal,
6808+
// For invokesuper; equivalent to GET_BLOCK_HANDLER()
6809+
FindFromCurrentFrame,
68106810
// part of the allocate-free block forwarding scheme
68116811
BlockParamProxy,
68126812
// To avoid holding the block arg (e.g. proc and symbol) across C calls,
@@ -6870,9 +6870,20 @@ fn gen_push_frame(
68706870
let cfp_self = asm.lea(Opnd::mem(64, CFP, RUBY_OFFSET_CFP_SELF));
68716871
asm.or(cfp_self, Opnd::Imm(1))
68726872
}
6873-
BlockHandler::LEPSpecVal => {
6874-
let lep_opnd = gen_get_lep(jit, asm);
6875-
asm.load(Opnd::mem(64, lep_opnd, SIZEOF_VALUE_I32 * VM_ENV_DATA_INDEX_SPECVAL))
6873+
BlockHandler::FindFromCurrentFrame => {
6874+
// Find the calling frame's block handler. Similar shape to implementation for
6875+
// defined?(yield). See also: vm_caller_setup_arg_block() with is_super, which
6876+
// ends up in VM_CF_BLOCK_HANDLER().
6877+
//
6878+
// When the calling ISEQ is rooted in a ISEQ_TYPE_METHOD, the block handler is
6879+
// at local_ep[VM_ENV_DATA_INDEX_SPECVAL]. Otherwise, the frame has no block
6880+
// handler.
6881+
if ISEQ_TYPE_METHOD == unsafe { rb_get_iseq_body_type(rb_get_iseq_body_local_iseq(jit.iseq)) } {
6882+
let lep_opnd = gen_get_lep(jit, asm);
6883+
asm.load(Opnd::mem(64, lep_opnd, SIZEOF_VALUE_I32 * VM_ENV_DATA_INDEX_SPECVAL))
6884+
} else {
6885+
VM_BLOCK_HANDLER_NONE.into()
6886+
}
68766887
}
68776888
BlockHandler::BlockParamProxy => {
68786889
let ep_opnd = gen_get_lep(jit, asm);
@@ -9880,7 +9891,7 @@ fn gen_invokesuper_specialized(
98809891
let block = if let Some(iseq) = jit.get_arg(1).as_optional_ptr() {
98819892
BlockHandler::BlockISeq(iseq)
98829893
} else {
9883-
BlockHandler::LEPSpecVal
9894+
BlockHandler::FindFromCurrentFrame
98849895
};
98859896

98869897
// Fallback to dynamic dispatch if this callsite is megamorphic

0 commit comments

Comments
 (0)