Skip to content

vm.c: restore the GC arena in the allocating inline opcodes #7022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
matz merged 1 commit into from
Aug 9, 2026
Merged

vm.c: restore the GC arena in the allocating inline opcodes #7022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions src/vm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2077,9 +2077,11 @@ vm_op_getidx(mrb_state *mrb, uint32_t a, mrb_sym *midp)
else if (tt == MRB_TT_HASH) {
/* optimize only for Hash class; subclasses/singleton may override [] */
if (mrb_obj_ptr(va)->c != mrb->hash_class) goto getidx_fallback;
int ai = mrb_gc_arena_save(mrb);
va = mrb_hash_get(mrb, va, vb);
ci = mrb->c->ci;
regs[a] = va;
mrb_gc_arena_restore(mrb, ai);
return VM_NEXT;
}
else if (tt == MRB_TT_STRING) {
Expand All @@ -2089,9 +2091,13 @@ vm_op_getidx(mrb_state *mrb, uint32_t a, mrb_sym *midp)
case MRB_TT_INTEGER:
case MRB_TT_STRING:
case MRB_TT_RANGE:
va = mrb_str_aref(mrb, va, vb, mrb_undef_value());
regs[a] = va;
return VM_NEXT;
{
int ai = mrb_gc_arena_save(mrb);
va = mrb_str_aref(mrb, va, vb, mrb_undef_value());
regs[a] = va;
mrb_gc_arena_restore(mrb, ai);
return VM_NEXT;
}
default:
break;
}
Expand Down Expand Up @@ -2127,10 +2133,13 @@ vm_op_getidx0(mrb_state *mrb, uint32_t a, uint16_t b, mrb_sym *midp)
{
/* same as the Hash branch of vm_op_getidx(): mrb_hash_get() can run a
default proc and move the stack, so take the result first and store
it through the refreshed `regs` */
it through the refreshed `regs`. The arena is restored only after
that store, which is what roots the result. */
int ai = mrb_gc_arena_save(mrb);
mrb_value val = mrb_hash_get(mrb, recv, mrb_fixnum_value(0));
ci = mrb->c->ci;
regs[a] = val;
mrb_gc_arena_restore(mrb, ai);
}
return VM_NEXT;
}
Expand Down Expand Up @@ -2158,9 +2167,13 @@ vm_op_setidx(mrb_state *mrb, uint32_t a, mrb_sym *midp)
case MRB_TT_HASH:
/* optimize only for Hash class; subclasses/singleton may override []= */
if (mrb_obj_ptr(va)->c != mrb->hash_class) goto setidx_fallback;
mrb_hash_set(mrb, va, vb, vc);
ci = mrb->c->ci;
regs[a] = vc;
{
int ai = mrb_gc_arena_save(mrb);
mrb_hash_set(mrb, va, vb, vc);
ci = mrb->c->ci;
regs[a] = vc;
mrb_gc_arena_restore(mrb, ai);
}
return VM_NEXT;
default:
setidx_fallback:
Expand Down Expand Up @@ -3285,7 +3298,10 @@ mrb_vm_exec(mrb_state *mrb, const struct RProc *begin_proc, const mrb_code *iseq
break
#endif
#ifdef MRB_USE_BIGINT
#define OP_MATH_OVERFLOW_INT(op,x,y) regs[a] = mrb_bint_##op##_ii(mrb,x,y)
#define OP_MATH_OVERFLOW_INT(op,x,y) do { \
regs[a] = mrb_bint_##op##_ii(mrb,x,y); \
mrb_gc_arena_restore(mrb, ai); \
} while (0)
#else
#define OP_MATH_OVERFLOW_INT(op,x,y) goto L_INT_OVERFLOW
#endif
Expand Down
84 changes: 84 additions & 0 deletions test/t/gc.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,87 @@
GC.generational_mode = origin
end
end

# The inline `[]`, `[]=` and arithmetic opcodes answer from C without a method
# call, so the arena restore that every cfunc return performs never runs for
# them. What they allocate then stays arena-protected for the rest of the
# enclosing method. `GC.stat` is itself a cfunc, so it reads the count before
# its own restore and still sees what the loop pinned. The loop bodies below
# are built only from opcodes that do not restore, since a single send in the
# body would empty the arena and hide the retention.

assert('OP_GETIDX does not retain its result in the GC arena') do
s = "hello"
GC.start
base = GC.stat[:live]
i = 0
while i < 20000
s[1]
i += 1
end
assert_operator GC.stat[:live] - base, :<, 5000
end

assert('OP_GETIDX does not retain a Hash default in the GC arena') do
h = Hash.new { Object.new }
GC.start
base = GC.stat[:live]
i = 0
while i < 20000
h[1]
i += 1
end
assert_operator GC.stat[:live] - base, :<, 5000
end

assert('OP_GETIDX0 does not retain a Hash default in the GC arena') do
h = Hash.new { Object.new }
GC.start
base = GC.stat[:live]
i = 0
while i < 20000
h[0]
i += 1
end
assert_operator GC.stat[:live] - base, :<, 5000
end

assert('OP_SETIDX does not retain a duplicated Hash key in the GC arena') do
h = {}
k = "a"
GC.start
base = GC.stat[:live]
i = 0
while i < 20000
h[k] = 1
i += 1
end
assert_operator GC.stat[:live] - base, :<, 5000
end

assert('OP_ADD does not retain an overflowed Integer in the GC arena') do
# The overflow branch promotes to a big integer, so it only exists with
# mruby-bigint. The shift count is a variable because a constant shift is
# folded at compile time, and a folded result out of mrb_int range makes the
# build fail rather than raise.
begin
k = 62
x = 1 << k
x + x
rescue RangeError
skip "requires mruby-bigint"
end
# 1 << 30 overflows mrb_int on MRB_INT32 and 1 << 62 on MRB_INT64, so
# whichever width this build has, one of the two takes the overflow branch.
[30, 62].each do |shift|
x = 1 << shift
GC.start
base = GC.stat[:live]
i = 0
while i < 20000
x + x
i += 1
end
assert_operator GC.stat[:live] - base, :<, 5000
end
end
Loading