[ruby-core:109824] [Ruby master Bug#18991] False LocalJumpError when branch coverage is enabled

["mame (Yusuke Endoh)" <noreply@ruby-lang.org>]

Issue #18991 has been updated by mame (Yusuke Endoh).


Good catch, thank you for your report. I could recreate the issue without coverage.

```
RubyVM::InstructionSequence.compile_option = false

eval("1&.tap { break }") #=> break from proc-closure (LocalJumpError)
```
Here is my analysis.
`throw TAG_BREAK` instruction makes a jump only if the continuation of catch of `TAG_BREAK`exactly matches the instruction immediately following the "send" instruction that is currently being executed. Otherwise, it seems to determine break from proc-closure.

https://github.com/ruby/ruby/blob/0d2422cf63ff330e372613894995e762d122e6b7/vm_insnhelper.c#L1484

This is very fragile. In the case in question, some instructions for recoding branch coverage were inserted immediately after the "send" instruction, which made the condition above unmatch. Also, when the compiler optimization is disabled, a "jump" instruction seems to be inserted after "send" for some reason, resulting in the same error.

Here is a proof-of-concept, an extremely dirty patch to force to move the continuation of catch of TAG_BREAK immediately after "send" (or "invokesuper"):

```diff
diff --git a/compile.c b/compile.c
index e906bd1e10..6497a8d64e 100644
--- a/compile.c
+++ b/compile.c
@@ -7353,7 +7353,30 @@ compile_iter(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node, in
                            ISEQ_TYPE_BLOCK, line);
         CHECK(COMPILE(ret, "iter caller", node->nd_iter));
     }
-    ADD_LABEL(ret, retry_end_l);
+
+    {
+        // We need to put the label "retry_end_l" immediately after the last "send" instruction.
+        // This because vm_throw checks if the break cont is equal to the index of next insn of the "send".
+        // (Otherwise, it is considered "break from proc-closure". See "TAG_BREAK" handling in "vm_throw_start".)
+        //
+        // Normally, "send" instruction is at the last.
+        // However, qcall under branch coverage measurement adds some instructions after the "send".
+        //
+        // Note that "invokesuper" appears instead of "send".
+        INSN *iobj;
+        LINK_ELEMENT *last_elem = LAST_ELEMENT(ret);
+        iobj = IS_INSN(last_elem) ? (INSN*) last_elem : (INSN*) get_prev_insn((INSN*) last_elem);
+        while (INSN_OF(iobj) != BIN(send) && INSN_OF(iobj) != BIN(invokesuper)) {
+            iobj = (INSN*) get_prev_insn(iobj);
+        }
+        ELEM_INSERT_NEXT(&iobj->link, (LINK_ELEMENT*) retry_end_l);
+
+        // LINK_ANCHOR has a pointer to the last element, but ELEM_INSERT_NEXT does not update it
+        // even if we add an insn to the last of LINK_ANCHOR. So this updates it manually.
+        if (&iobj->link == LAST_ELEMENT(ret)) {
+            ret->last = (LINK_ELEMENT*) retry_end_l;
+        }
+    }

     if (popped) {
         ADD_INSN(ret, line_node, pop);
```

I want to change the break handling itself, but I haven't come up with a good fix.

What do you think? @nobu @ko1

----------------------------------------
Bug #18991: False LocalJumpError when branch coverage is enabled
https://bugs.ruby-lang.org/issues/18991#change-99062

* Author: qnighy (Masaki Hara)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [arm64-darwin21]
* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
Enabling branch coverage leads to a false LocalJumpError where it should not be raised.

```ruby
# test.rb
require "coverage"
Coverage.start(branches: true)
# Coverage.start(lines: true)

load "./test2.rb"
```

```ruby
# test2.rb
1&.tap do break end
```

Output:

```
$ ruby test.rb
/Users/qnighy/workdir/branch-coverage-bug/test2.rb:1:in `block in <top (required)>': break from proc-closure (LocalJumpError)
	from <internal:kernel>:90:in `tap'
	from /Users/qnighy/workdir/branch-coverage-bug/test2.rb:1:in `<top (required)>'
	from test.rb:5:in `load'
	from test.rb:5:in `<main>'
```



-- 
https://bugs.ruby-lang.org/