Preserve line events across jump optimization. (rubyGH-18122) [Backport #22218]

Author: samuel-williams-shopify

compile.c

@@ -3652,30 +3652,22 @@ iseq_peephole_optimize(rb_iseq_t *iseq, LINK_ELEMENT *list, const int do_tailcal
          */
         INSN *nobj = (INSN *)get_destination_insn(iobj);
 
-        /* This is super nasty hack!!!
-         *
-         * This jump-jump optimization may ignore event flags of the jump
-         * instruction being skipped.  Actually, Line 2 TracePoint event
-         * is never fired in the following code:
+        /* This jump-jump optimization may ignore line events on the jump
+         * instruction being skipped.  For example, the Line 2 TracePoint
+         * event would otherwise never fire in the following code:
          *
          *   1: raise if 1 == 2
          *   2: while true
          *   3:   break
          *   4: end
          *
-         * This is critical for coverage measurement.  [Bug #15980]
-         *
-         * This is a stopgap measure: stop the jump-jump optimization if
-         * coverage measurement is enabled and if the skipped instruction
-         * has any event flag.
-         *
-         * Note that, still, TracePoint Line event does not occur on Line 2.
-         * This should be fixed in future.
+         * Do not skip a jump that carries a line event.  This applies even
+         * when coverage is disabled because TracePoint consumes the same
+         * event.  [Bug #15980]
          */
         int stop_optimization =
-            ISEQ_COVERAGE(iseq) && ISEQ_LINE_COVERAGE(iseq) &&
             nobj->link.type == ISEQ_ELEMENT_INSN &&
-            nobj->insn_info.events;
+            (nobj->insn_info.events & (RUBY_EVENT_LINE | RUBY_EVENT_COVERAGE_LINE));
         if (!stop_optimization) {
             INSN *pobj = (INSN *)iobj->link.prev;
             int prev_dup = 0;

test/ruby/test_settracefunc.rb

@@ -2830,6 +2830,62 @@ def test_while_in_while
     assert_equal [__LINE__ - 5, __LINE__ - 4, __LINE__ - 3], lines, 'Bug #17868'
   end
 
+  def test_line_event_after_guard_before_while
+    lines = []
+    while_line = body_line = nil
+
+    TracePoint.new(:line) {|tp|
+      next unless target_thread?
+      lines << tp.lineno
+    }.enable {
+      raise if 1 == 2
+      while_line = __LINE__ + 1
+      while true
+        body_line = __LINE__ + 1
+        break
+      end
+    }
+
+    assert_includes lines, while_line
+    assert_includes lines, body_line
+    assert_operator lines.index(while_line), :<, lines.index(body_line)
+  end
+
+  def test_line_event_after_guard_before_while_predicate
+    parent = Class.new do
+      def read
+        @values.shift
+      end
+    end
+
+    child = Class.new(parent) do
+      def initialize
+        @values = ["chunk", nil]
+      end
+    end
+
+    start_line = __LINE__ + 2
+    while_line = start_line + 2
+    child.class_eval <<~RUBY, __FILE__, start_line
+      def read
+        return if @finished
+        while chunk = super
+          chunk.upcase
+        end
+      end
+    RUBY
+
+    lines = []
+    TracePoint.new(:line) {|tp|
+      next unless target_thread?
+      lines << tp.lineno
+    }.enable {
+      child.new.read
+    }
+
+    assert_includes lines, while_line
+  end
+
   def test_allow_reentry
     event_lines = []
     _l1 = _l2 = _l3 = _l4 = nil