[ruby-core:108695] [Ruby master Bug#18801] Dead YARV instructions produced when `branchif` is used

Issue #18801 has been updated by eightbitraptor (Matthew Valentine-House).



I investigated these instructions and whether or not they were dead code and
could be removed.

From the outside it looks like there are two things to note:

* they immediately follow an unconditional jump and so should theoretically
  never be reached, and
* when executed together they are effectively a no-op (push nil onto the stack,
  and then remove it)

However, this isn't the whole picture. The summary is that they currently are
necessary for the correct operation of the `next` keyword and cannot be removed.

A jump target (`next_catch_label`) is inserted in between the `putnil` and the
`pop`. I did not investigate thorougly enough to fully understand how `next`
works and what the significance of the `putnil` is, however all of my naive
attempts to remove these instructions caused either memory errors, or test
failures when working with loops.

It's possible to view the instructions generated, with the labels still in place
using `RubyVM::InstructionSequence` directly:

```
笶ッ ruby -e "RubyVM::InstructionSequence.compile('(while rand < 0.9; class C; next 2; end; end)').to_a.last.each { |e| puts e.inspect }"
1
:RUBY_EVENT_LINE
[:jump, :label_14]
[:putnil]
:label_3
[:pop]
[:jump, :label_14]
```

The instructions in question are generated by the `compile_loop` function in
`compile.c`, which is executed when a `NODE_UNTIL` or `NODE_WHILE` tree node is
compiled.

```
    ADD_LABEL(ret, adjust_label);
    ADD_INSN(ret, line_node, putnil);
    ADD_LABEL(ret, next_catch_label);
    ADD_INSN(ret, line_node, pop);
    ADD_INSNL(ret, line_node, jump, next_label);
```

If we comment out the `ADD_INSN` lines for `putnil` and `pop`, leaving in the
jump targets `next_catch_label` and `next_label`, then one of the flow control
tests in `bootstraptest/test_flow.rb` will fail. That test looks like this:

```
assert_equal %q{4}, %q{
  def m a, b
    a + b
  end
  m(1,
    (i=0; while i<2
       i+=1
       class C
         next 2
       end
     end; 3)
    )
}
```

Without these instructions this code outputs `5` instead of `4` causing an
assertion failure.

In addition, the following will cause a Segmentation Fault with miniruby, when
those instructions are not present.

```
./miniruby -e "while rand < 0.9; class C; next 2; end; end"
```

Interestingly, removing the `next` keyword from our test code snippet results in
different bytecode being generated by the `compile_loop` function: The
`next_catch_label` only appears when the `next` keyword is used, despite the
code that inserts it not being obviously conditional

With `next`:

```
笶ッ ruby -e "RubyVM::InstructionSequence.compile('(while rand < 0.9; class C; next 2; end; end)').to_a.last.each { |e| puts e.inspect }"
1
:RUBY_EVENT_LINE
[:jump, :label_14]
[:putnil]
:label_3
[:pop]
[:jump, :label_14]
```

Without `next`:

```
笶ッ ruby -e "RubyVM::InstructionSequence.compile('(while rand < 0.9; class C; end; end)').to_a.last.each { |e| puts e.inspect }"
1
:RUBY_EVENT_LINE
[:jump, :label_14]
[:putnil]
[:pop]
[:jump, :label_14]
```

The questions I have are:

* How does that test output 5?
* Even given the presence of the `next_catch_label`, the `putnil` instruction looks
  like it can never be reached. So why does removing it result in a Stack
  Underflow?
* How does the conditional insertion of the `next_catch_label` work?
* What is the significance of `adjust_label` (inserted before the `putnil`), and
  why is it never visible in the instruction sequence?

----------------------------------------
Bug #18801: Dead YARV instructions produced when `branchif` is used
https://bugs.ruby-lang.org/issues/18801#change-97740

* Author: wildmaples (Maple Ong)
* Status: Open
* Priority: Normal
* ruby -v: 3.1.0
* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
# Description

It seems there are unused YARV instructions produced when the snippet contains a `branchif` instruction.

In the following example, the instructions numbers 0002 to 0004 won't ever be executed: 

```
irb(main):003:0> puts RubyVM::InstructionSequence.compile("while 2+3; puts 'hi'; end").disasm

== disasm: #<ISeq:<compiled>@<compiled>:1 (1,0)-(1,25)> (catch: FALSE)
0000 jump                                   12                        (   1)[Li]
0002 putnil
0003 pop
0004 jump                                   12
0006 putself
0007 putstring                              "hi"
0009 opt_send_without_block                 <calldata!mid:puts, argc:1, FCALL|ARGS_SIMPLE>
0011 pop
0012 putobject                              2
0014 putobject                              3
0016 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>[CcCr]
0018 branchif                               6
0020 putnil
0021 leave
```

Similarly in this example, 0006-0008 won't be executed.  

```
irb(main):003:0> puts RubyVM::InstructionSequence.compile("x = 9; while x; puts 'hi'; end").disasm
== disasm: #<ISeq:<compiled>@<compiled>:1 (1,0)-(1,30)> (catch: FALSE)
local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1])
[ 1] x@0
0000 putobject                              9                         (   1)[Li]
0002 setlocal_WC_0                          x@0
0004 jump                                   16
0006 putnil
0007 pop
0008 jump                                   16
0010 putself
0011 putstring                              "hi"
0013 opt_send_without_block                 <calldata!mid:puts, argc:1, FCALL|ARGS_SIMPLE>
0015 pop
0016 getlocal_WC_0                          x@0
0018 branchif                               10
0020 putnil
0021 leave
```

Initially we thought those instructions (i.e. putnil, pop, jump) were used when the return value of the while-loop is needed.

```
irb(main):012:0> puts RubyVM::InstructionSequence.compile("x = while foo; 9; end").disasm
== disasm: #<ISeq:<compiled>@<compiled>:1 (1,0)-(1,21)> (catch: FALSE)
local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1])
[ 1] x@0
0000 jump                                   4                         (   1)[Li]
0002 putnil
0003 pop
0004 putself
0005 opt_send_without_block                 <calldata!mid:foo, argc:0, FCALL|VCALL|ARGS_SIMPLE>
0007 branchif                               4
0009 putnil
0010 dup
0011 setlocal_WC_0                          x@0
0013 leave
```

But it seems like some dead instructions (0002, 0003) in the example above still remains. 

Are those instructions meant to be used for something else or is it a "bug" that it sticks around? 
Perhaps it can be optimized away? 



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

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>