vm.c: honor a [] redefinition installed on a core class - #7198

OP_GETIDX, OP_GETIDX0 and OP_SETIDX answer [] and []= from C for an Array, Hash or String receiver, guarded only by the receiver's class pointer being the core class. A redefinition installed on the core class itself passes that guard and is silently ignored; only a subclass or singleton receiver, which fails the pointer test for an unrelated reason, ever reached one.

$ ruby -e 'class String; def [](i); "hooked"; end; end; p "ab"[0]'
"hooked"
$ mruby -e 'class String; def [](i); "hooked"; end; end; p "ab"[0]'
"a"

The bypass is not self-consistent

self[idx] compiles to OP_SSEND :[] rather than to an index opcode:

$ cat ssend.rb
def bar(i); self[i]; end
$ mrbc -v -o /dev/null ssend.rb
    1 007 SSEND		R3	:[]	n=1

so the mrblib core methods that index self, Array#each among them with its yield self[idx], go through the method table and honor the very redefinition the opcode ignores. Redefining a core [] half-works today: iterators change, direct indexing does not.

$ mruby -e 'class Array; def [](i); :x; end; end
            r = ""; [7,8].each {|y| r << y.to_s}
            puts r; puts [7,8][1].to_s'
xx
8

What this changes

Each (class, operator) pair gets a slot in mrb->idx_class[]. The slot holds the core class while the name still resolves to the builtin the opcode reimplements, and NULL once it does not. The opcodes compare the receiver's class against that slot instead of against the core class, so a disarmed slot fails the comparison and the operator is sent like any other method. NULL is safe as the disabled value because no live object has a NULL class pointer.

-    if (mrb_unlikely(ary->c != mrb->array_class)) goto getidx_fallback;
+    if (mrb_unlikely(ary->c != mrb->idx_class[MRB_IDX_OP_ARY_AREF])) goto getidx_fallback;

Validity is the resolved mrb_method_t itself, not a "was assigned" flag: a slot is armed while the operator resolves, from the core class, to exactly the method recorded at startup. That covers def, alias_method, undef_method, remove_method, visibility changes and prepend without enumerating them, re-arms when an override is aliased back away, and stays armed when a module without [] is included. The recheck hangs off the four places that already invalidate the method cache, mrb_define_method_raw(), include_module_at(), mrb_mod_visibility() and mrb_remove_method(), and returns immediately unless the changed name is [] or []=.

mrb_idx_op_init() runs at the end of mrb_open_core(), after bootstrapping, so a core [] that mrblib itself replaces is recorded as replaced rather than as the builtin. A slot whose operator was already not a C function at that point is never armed.

Cost on the fast path

The guard is the same single compare it always was. In the generated code only the displacement widens, because the slots sit past the method cache (build_config/default.rb, objdump -d src/vm.o, a String branch of the index opcodes):

 cmp    $0x12,%eax
-jne    5c7c <mrb_vm_exec+0x4bbc>
-mov    0x50(%rbp),%rax
+jne    5c7f <mrb_vm_exec+0x4bbf>
+mov    0x5fe8(%rbp),%rax
 cmp    %rax,(%rsi)
-jne    5c7c <mrb_vm_exec+0x4bbc>
+jne    5c7f <mrb_vm_exec+0x4bbf>

All eight guard sites, three in vm_op_getidx(), three in vm_op_getidx0() and two in vm_op_setidx(), change the same way, for +16 bytes in vm.o.

Under callgrind the executed instruction count per iteration is unchanged for every one of them. Measured in a build without mruby-regexp, so that all eight are live; each figure is Ir(200,000 iterations) - Ir(100,000 iterations), divided by 100,000, which cancels startup:

Size

build_config/default.rb, .text from size:

sizeof(struct mrb_state) goes from 24,536 to 24,656, once per state: 5 class pointers plus 5 mrb_method_t. The slots sit at the end of the struct, so no existing field moves.

One consequence is not free

mruby-regexp replaces String#[] with a Ruby method at gem initialization, so in a build that includes it the String read fast path is now correctly off and str[i] costs a send. The comment in mrbgems/mruby-regexp/mrblib/string_regexp.rb claiming the opcode keeps bypassing the override is corrected in the same commit.

N = 2_000_000
def bench(name)
  best = nil
  25.times do
    t0 = Time.now
    yield
    t = Time.now - t0
    best = t if best.nil? || t < best
  end
  puts sprintf("%-10s %8.2f ns/iter", name, best * 1e9 / N)
end

str = "hello"
ary = [1, 2, 3, 4]
hsh = {1 => :a, 2 => :b}

bench("str[i]")  { i = 0; while i < N; str[1]; i += 1; end }
bench("str[0]")  { i = 0; while i < N; str[0]; i += 1; end }
bench("ary[i]")  { i = 0; while i < N; ary[1]; i += 1; end }
bench("ary[0]")  { i = 0; while i < N; ary[0]; i += 1; end }
bench("hash[k]") { i = 0; while i < N; hsh[1]; i += 1; end }

build_config/default.rb, which carries mruby-regexp, best of 25:

Only the String read moves. The Array and Hash rows sit at the measurement floor on this box: run to run they swing by more than the difference between the two columns, in either direction, which is why the ratios there are noise and not a result. The same build under callgrind separates the two cleanly:

That slowdown is the price of the regexp forms of String#[] being reachable at all. A build without mruby-regexp keeps all eight fast paths, and str[1] there measures 38.25 ns on master and 39.02 ns with this PR, at the identical instruction count shown above.

Tests

test/t/string.rb had a named test pinning the bypass as intended. It now pins the redefinition being honored, and Array and Hash gain the equivalent: two named tests in test/t/array.rb, one in test/t/hash.rb. Between them they cover the read, the write, a subclass receiver, the receiver being left untouched by a redefined []=, and re-arming by aliasing the original implementation back.

The four GC arena assertions in test/t/gc.rb measure whether a C branch of OP_GETIDX or OP_GETIDX0 leaves its result in the arena, and they measure it only while the loop body reaches the opcode. A send does not: its cfunc epilogue drains the arena, so the assertion would pass whatever the branch under test does. The first commit puts the C implementation back around the two String loops through a with_builtin_string_aref helper, which is a no-op where no gem replaced the operator. Without it those loop bodies would stop reaching the opcode in any build that carries mruby-regexp.

bintest is green in both builds that enable it: 106 of 106 under build_config/default.rb, 117 of 117 under build_config/ci/gcc-clang.rb. Both commits are green on their own under build_config/default.rb.

Environment

# build_config/default.rb, host
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DMRB_USE_COMPLEX -DMRB_USE_BIGINT -DMRB_USE_DEBUG_HOOK

# build_config/ci/gcc-clang.rb, full-debug
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -g3 -O0 -DMRB_GC_STRESS -DMRB_USE_DEBUG_HOOK -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER

# build_config/ci/gcc-clang.rb, bintest
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_GC_FIXED_ARENA -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER -DMRB_USE_DEBUG_HOOK

# build_config/ci/gcc-clang.rb, cxx_abi (gcc -x c++; g++ links only)
gcc -g -O3 -Wall -Wundef -Wwrite-strings -x c++ -std=gnu++03 -DMRB_GC_FIXED_ARENA -DMRB_USE_CXX_EXCEPTION -DMRB_USE_CXX_ABI -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER

# build_config/ci/gcc-clang.rb, byte-string
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER

# build_config/ci/gcc-clang.rb, ascii-case
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_USE_ASCII_CASE -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER

# full-core + enable_debug, one per boxing
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -g3 -O0 -DMRB_NO_BOXING -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -g3 -O0 -DMRB_WORD_BOXING -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -g3 -O0 -DMRB_NAN_BOXING -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER

# full-core + enable_debug, MRB_NO_METHOD_CACHE
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -g3 -O0 -DMRB_NO_METHOD_CACHE -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER

# the mruby-regexp-free timings and instruction counts
# (the second -O3 is the config repeating the toolchain default)
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -O3 -DHAVE_MRUBY_IO_GEM

Summary by CodeRabbit

• Bug Fixes
  • Corrected indexed reads and writes for Array, Hash, and String when their indexing methods are overridden, removed, or redefined.
  • Ensured subclasses and objects with custom indexing behavior use the correct method dispatch instead of built-in shortcuts.
  • Preserved normal built-in indexing after temporary method overrides are restored.
• Tests
  • Added regression coverage for customized Array, Hash, and String indexing, including subclass behavior and garbage-collection scenarios.