From 111cc792104dc867e659501fbd9f3ba25fe80eb7 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Mon, 7 Sep 2026 14:14:24 +0900 Subject: [PATCH] Look up the nearest user box for :caller_user_box The box of the immediately previous frame can be the root box, e.g. when a proc made in the root box calls Marshal.load, and the classes of the caller's data are invisible there. Walk the caller frames up to the nearest user box instead, and rename the attribute after what it finds. Co-Authored-By: Claude Opus 5 --- compile.c | 4 ++-- marshal.rb | 2 +- test/ruby/test_box.rb | 12 +++++++++++- tool/mk_builtin_loader.rb | 2 +- vm.c | 33 +++++++++++++++++++++++++++++---- vm_core.h | 4 ++-- yjit/src/cruby_bindings.inc.rs | 2 +- zjit/src/cruby_bindings.inc.rs | 2 +- 8 files changed, 48 insertions(+), 13 deletions(-) diff --git a/compile.c b/compile.c index 4aa3946493d..be83950c55f 100644 --- a/compile.c +++ b/compile.c @@ -9394,8 +9394,8 @@ compile_builtin_attr_symbol(rb_iseq_t *iseq, VALUE symbol) else if (rb_streql_lit(string, "without_interrupts")) { ISEQ_BODY(iseq)->builtin_attrs |= BUILTIN_ATTR_WITHOUT_INTERRUPTS; } - else if (rb_streql_lit(string, "caller_box")) { - ISEQ_BODY(iseq)->builtin_attrs |= BUILTIN_ATTR_CALLER_BOX; + else if (rb_streql_lit(string, "caller_user_box")) { + ISEQ_BODY(iseq)->builtin_attrs |= BUILTIN_ATTR_CALLER_USER_BOX; } else { return COMPILE_NG; diff --git a/marshal.rb b/marshal.rb index 8f46b996149..0b8a9305a1a 100644 --- a/marshal.rb +++ b/marshal.rb @@ -31,7 +31,7 @@ module Marshal # # => [1039360, 1039380, 1039360, 1039380] -- only 2 different objects, object_ids repeating # def self.load(source, proc = nil, freeze: false) - Primitive.attr! :caller_box + Primitive.attr! :caller_user_box Primitive.marshal_load(source, proc, freeze) end diff --git a/test/ruby/test_box.rb b/test/ruby/test_box.rb index 33ff1cf9acf..ed9896bff12 100644 --- a/test/ruby/test_box.rb +++ b/test/ruby/test_box.rb @@ -456,7 +456,7 @@ class TestBox < Test::Unit::TestCase end; end - def test_marshal_resolves_classes_in_the_caller_box + def test_marshal_resolves_classes_in_the_caller_user_box setup_box obj = @box.eval("class BoxMarshalBar; end; Marshal.load(Marshal.dump(BoxMarshalBar.new))") @@ -468,6 +468,16 @@ class TestBox < Test::Unit::TestCase Marshal.load(dump) end end + + def test_marshal_skips_root_box_frames_in_the_caller_stack + assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) + begin; + class BoxMarshalBaz; end + # the proc runs in the root box, where BoxMarshalBaz is invisible + loader = Ruby::Box.root.eval("->(dump) { Marshal.load(dump) }") + assert_instance_of BoxMarshalBaz, loader.call(Marshal.dump(BoxMarshalBaz.new)) + end; + end end class TestBoxDescendantsMain diff --git a/tool/mk_builtin_loader.rb b/tool/mk_builtin_loader.rb index 6b343bc860a..87335015dd9 100644 --- a/tool/mk_builtin_loader.rb +++ b/tool/mk_builtin_loader.rb @@ -7,7 +7,7 @@ require_relative 'ruby_vm/helpers/c_escape' SUBLIBS = {} REQUIRED = {} -BUILTIN_ATTRS = %w[leaf inline_block use_block c_trace without_interrupts caller_box] +BUILTIN_ATTRS = %w[leaf inline_block use_block c_trace without_interrupts caller_user_box] module CompileWarning @@warnings = 0 diff --git a/vm.c b/vm.c index 9f3a610157f..426ffcd8a3c 100644 --- a/vm.c +++ b/vm.c @@ -3317,6 +3317,31 @@ rb_vm_frame_flag_set_box_require(const rb_execution_context_t *ec) VM_ENV_FLAGS_SET(ec->cfp->ep, VM_FRAME_FLAG_BOX_REQUIRE); } +static const rb_box_t *current_box_on_cfp(const rb_execution_context_t *ec, const rb_control_frame_t *cfp); + +/** + * Returns the nearest user box in the caller frames, or NULL if there is none. + * + * Builtin methods written in Ruby are defined in the master box, so their own + * frame tells nothing about the caller. Those marked with + * `Primitive.attr! :caller_user_box` need the box owning the caller code, and the + * frames in between may belong to the master or the root box, e.g. when another + * builtin method or a proc made in the root box calls them. + */ +static const rb_box_t * +caller_user_box_on_cfp(const rb_execution_context_t *ec, const rb_control_frame_t *cfp) +{ + const rb_control_frame_t * const eocfp = RUBY_VM_END_CONTROL_FRAME(ec); + + while (RUBY_VM_VALID_CONTROL_FRAME_P(cfp, eocfp)) { + const rb_box_t *box = current_box_on_cfp(ec, cfp); + if (BOX_USER_P(box)) + return box; + cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp); + } + return NULL; +} + static const rb_box_t * current_box_on_cfp(const rb_execution_context_t *ec, const rb_control_frame_t *cfp) { @@ -3331,12 +3356,12 @@ current_box_on_cfp(const rb_execution_context_t *ec, const rb_control_frame_t *c VM_BOX_ASSERT(cme, "cme should be valid"); VM_BOX_ASSERT(cme->def, "cme->def shold be valid"); if (cme->def->type == VM_METHOD_TYPE_ISEQ && - (ISEQ_BODY(cme->def->body.iseq.iseqptr)->builtin_attrs & BUILTIN_ATTR_CALLER_BOX)) { - // Builtin methods with `Primitive.attr! :caller_box` operate on the caller box, - // just like CFUNC frames. See the comment in VM_EP_RUBY_LEP(). + (ISEQ_BODY(cme->def->body.iseq.iseqptr)->builtin_attrs & BUILTIN_ATTR_CALLER_USER_BOX)) { const rb_control_frame_t *owner_cfp = rb_vm_search_cf_from_ep(ec, cfp, lep); if (owner_cfp) { - return current_box_on_cfp(ec, RUBY_VM_PREVIOUS_CONTROL_FRAME(owner_cfp)); + box = caller_user_box_on_cfp(ec, RUBY_VM_PREVIOUS_CONTROL_FRAME(owner_cfp)); + if (box) + return box; } } return cme->def->box; diff --git a/vm_core.h b/vm_core.h index 840c85f4cb5..4754feae6f6 100644 --- a/vm_core.h +++ b/vm_core.h @@ -407,8 +407,8 @@ enum rb_builtin_attr { BUILTIN_ATTR_C_TRACE = 0x08, // The iseq uses noint branch/jump opcodes that skip interrupt checking. BUILTIN_ATTR_WITHOUT_INTERRUPTS = 0x10, - // The iseq operates on its caller's box, like C functions do. - BUILTIN_ATTR_CALLER_BOX = 0x20, + // The iseq operates on the nearest user box in its caller frames. + BUILTIN_ATTR_CALLER_USER_BOX = 0x20, }; typedef VALUE (*rb_jit_func_t)(struct rb_execution_context_struct *, struct rb_control_frame_struct *); diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index d51708b2d1a..be4653d1966 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -487,7 +487,7 @@ pub const BUILTIN_ATTR_SINGLE_NOARG_LEAF: rb_builtin_attr = 2; pub const BUILTIN_ATTR_INLINE_BLOCK: rb_builtin_attr = 4; pub const BUILTIN_ATTR_C_TRACE: rb_builtin_attr = 8; pub const BUILTIN_ATTR_WITHOUT_INTERRUPTS: rb_builtin_attr = 16; -pub const BUILTIN_ATTR_CALLER_BOX: rb_builtin_attr = 32; +pub const BUILTIN_ATTR_CALLER_USER_BOX: rb_builtin_attr = 32; pub type rb_builtin_attr = u32; #[repr(C)] #[derive(Debug, Copy, Clone)] diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index c400f3e2fcf..6343cf69e10 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -642,7 +642,7 @@ pub const BUILTIN_ATTR_SINGLE_NOARG_LEAF: rb_builtin_attr = 2; pub const BUILTIN_ATTR_INLINE_BLOCK: rb_builtin_attr = 4; pub const BUILTIN_ATTR_C_TRACE: rb_builtin_attr = 8; pub const BUILTIN_ATTR_WITHOUT_INTERRUPTS: rb_builtin_attr = 16; -pub const BUILTIN_ATTR_CALLER_BOX: rb_builtin_attr = 32; +pub const BUILTIN_ATTR_CALLER_USER_BOX: rb_builtin_attr = 32; pub type rb_builtin_attr = u32; pub type rb_jit_func_t = ::std::option::Option< unsafe extern "C" fn(