From: "eightbitraptor (Matthew Valentine-House)" Date: 2022-05-25T12:19:07+00:00 Subject: [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: #@: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 0011 pop 0012 putobject 2 0014 putobject 3 0016 opt_plus [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: #@: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 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: #@: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 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: