OP_ADDILV and OP_SUBILV set the method call fallback up on the local variables, corrupting them #7044

x += 1 on a local variable corrupts the surrounding local variables whenever the
receiver is neither an Integer nor a Float. A while loop that increments a counter
next to such a variable never terminates.

Found while writing the arithmetic assertions for #7042: the loop body there was going
to include x += 1 to cover the third expansion of the OP_MATH family, and it broke
the i686 run. That line was dropped from the PR, which is otherwise unrelated to this.

Everything below was measured against 9a3d566, the current head.

The opcode

OP_ADDILV and OP_SUBILV are the fused form of MOVE temp, local,
ADDI temp, imm, MOVE local, temp
(mrbgems/mruby-compiler/src/codegen.c:1047-1049).
The operands are a = the local being assigned, b = working space reserved for a
method call, c = the immediate, as include/mruby/ops.h:88-89 records.

The compiler only fuses when the destination is a local:
if (... || data.a != src || data.a < s->nlocals) goto normal;. So a is a local
variable slot by construction, never a temporary.

The fallback arm sets the call up on the locals

src/vm.c:3415-3418:

    default:
      SET_INT_VALUE(mrb,regs[a+1], c);
      mid = MRB_OPSYM(op_name);
      goto L_SEND_SYM;

L_SEND_SYM also writes nil into regs[a+2] and pushes the callee frame at regs[a]
via cipush(mrb, a, ...). That is right in OP_MATH and OP_MATHI, where a is a
temporary above the locals, and wrong here: the argument lands on the next local, the
nil on the one after it, and the callee frame covers everything from a on.

$ ./build/host/bin/mruby -e 'obj = Class.new { def +(n); [:added, n]; end }.new
a = 10
b = 20
obj += 1
p [obj, a, b]'
[[:added, 1], 1, nil]

[[:added, 1], 10, 20] is right. It reproduces the same way at top level, inside a
method and inside a block.

A loop counter is a local too, so the same write makes the loop run forever:

$ ./build/host/bin/mruby -e 'x = Class.new { def +(n); self; end }.new
i = 0
while i < 3
  x += 1
  i += 1
end
puts "done"'
^C

No user-defined method is needed where mrb_int is 32 bits wide: a big integer takes
the fallback arm as well, so this hangs on an i686 build.

$ ./build/i686/bin/mruby -e 'k = 31; x = 1 << k; i = 0; while i < 3; x += 1; i += 1; end; p [x, i]'
^C

Where it comes from

The opcodes were added in 5475ea5 (2026-01) with mrb_funcall_argv() in the fallback
arm, which is correct. It was replaced in 6a6e2b4 (2026-03), "avoid re-entrant VM call
from C; use the same dispatch pattern as OP_MATH and OP_MATHI for consistency".
5475ea5 is in 4.0.0 and 6a6e2b4 is not, so this is a master-only regression.

The working space cannot repair it in place

b is exactly what the send needs, but a call set up there leaves its result in
regs[b] and the opcode has to leave it in regs[a]. The MOVE local, temp that used
to do that is what the fusion removed, and nothing in the send path runs after the
callee returns: OP_RETURN writes into ci->stack[0] of the popped frame and execution
resumes at the instruction after the fused one. There is no "call and store elsewhere"
facility in the VM to hang the copy on.

Three candidate fixes

1. Go back to calling from C. Restore mrb_funcall_argv(), taking the result into a
   temporary and refreshing ci first, since the call can grow the stack and the
   callinfo array and regs is ci->stack. One hunk, restores 4.0.0 behaviour.
   Against it: it reintroduces the re-entrant VM call that 6a6e2b48a set out to remove,
   which matz has objected to elsewhere.
2. Drop the fusion. Remove the OP_ADDILV/OP_SUBILV branch from gen_move() and
   go back to three instructions. Correct and no re-entrancy, but it gives up the
   optimisation entirely (5475ea573 counts 5 bytes per instance, 40 in stdlib) and
   leaves two opcodes generated by nothing.
3. Redefine the opcodes as R[b] = R[a] op c and keep a trailing MOVE. Two
   instructions instead of three, correct in both arms, no re-entrancy. It changes the
   meaning of an opcode that 4.0.0 already ships, so old bytecode would silently stop
   updating the local, and it needs a binary format version bump.

Option 1 is the one written out below, because it is the smallest change that restores
released behaviour, but the choice looks like yours to make rather than mine, which is
why this is an issue and not a pull request. Happy to send whichever one you prefer as a
PR, tests included.

The patch

diff --git a/src/vm.c b/src/vm.c
--- a/src/vm.c
+++ b/src/vm.c
@@ -3398,6 +3398,12 @@ RETRY_TRY_BLOCK:
     }                                                                       \
     break
 #endif
+/* `a` is a local variable slot, so the L_SEND_SYM path the other arithmetic
+   opcodes take cannot be used for the fallback here: it writes the argument
+   into regs[a+1] and places the callee frame over the locals from regs[a] on.
+   `b` is the working space the compiler reserved for the call, but a send set
+   up there leaves its result in regs[b] and nothing copies it back into the
+   local, so the call is made from C instead. */
 #define OP_MATHILV(op_name)                                                 \
   /* a=local, b=working space, c=immediate */                               \
   switch (mrb_type(regs[a])) {                                              \
@@ -3414,9 +3420,15 @@ RETRY_TRY_BLOCK:
       break;                                                                \
     OP_MATHILV_CASE_FLOAT(op_name);                                         \
     default:                                                                \
-      SET_INT_VALUE(mrb,regs[a+1], c);                                      \
-      mid = MRB_OPSYM(op_name);                                             \
-      goto L_SEND_SYM;                                                      \
+      {                                                                     \
+        mrb_value arg = mrb_int_value(mrb, c);                              \
+        mrb_value v;                                                        \
+        v = mrb_funcall_argv(mrb, regs[a], MRB_OPSYM(op_name), 1, &arg);    \
+        ci = mrb->c->ci;         /* the call may have moved the stack */    \
+        regs[a] = v;                                                        \
+        mrb_gc_arena_restore(mrb, ai);                                      \
+      }                                                                     \
+      break;                                                                \
   }                                                                         \
   NEXT

5475ea573's version stored straight through regs[a] = mrb_funcall_argv(...), which
is not safe: regs is ci->stack and the C standard does not say whether the lvalue is
computed before or after the call, so a call that moves the stack can make it write
through the old pointer. The result is taken first here and ci is refreshed, the same
shape the OP_GETIDX branches use.

The regression test

Goes in test/t/syntax.rb, next to the other operator-assignment assertions. It covers
both opcodes, checks the neighbouring locals rather than only the assigned one, and
checks that an exception from the method propagates, since the call mechanism is what
changes.

Deliberately no loop: a loop reproduces the bug by hanging, and an assertion that fails
is worth more in CI than one that times out.

assert('local variable operator-assignment with a non-numeric receiver') do
  # `x += 1` on a local variable compiles to OP_ADDILV, whose fast path handles
  # Integer and Float in place.  Anything else has to go through the method,
  # and that call must not be set up on the local variables: both the argument
  # register and the callee frame would start at the local being assigned.
  obj = Class.new { def +(n); [:added, n]; end }.new
  a = 10
  b = 20
  obj += 1
  assert_equal [:added, 1], obj
  assert_equal 10, a
  assert_equal 20, b

  obj2 = Class.new { def -(n); [:subtracted, n]; end }.new
  c = 30
  obj2 -= 2
  assert_equal [:subtracted, 2], obj2
  assert_equal 30, c

  # the operand is passed as an Integer, and an exception from the method
  # propagates rather than being swallowed
  obj3 = Class.new { def +(n); raise ArgumentError, n.to_s; end }.new
  assert_raise_with_message(ArgumentError, "7") { obj3 += 7 }
end

Verification

The i686 build is MRuby::Build with the compiler commands pointed at
i686-linux-gnu-gcc-13; the resulting binaries run natively on an x86-64 host.
MRuby::CrossBuild does not work for this, since mruby-io's HAL is not selected for a
cross target and the link fails.