https://bugs.ruby-lang.org/https://bugs.ruby-lang.org/favicon.ico?17113305112019-12-03T21:52:10ZRuby Issue Tracking SystemRuby master - Bug #15980: Coverage shows while/until after raise if/unless as uncovered linehttps://bugs.ruby-lang.org/issues/15980?journal_id=829442019-12-03T21:52:10Zjeremyevans0 (Jeremy Evans)merch-redmine@jeremyevans.net
<ul><li><strong>Has duplicate</strong> <i><a class="issue tracker-1 status-5 priority-4 priority-default closed" href="/issues/16397">Bug #16397</a>: Line coverage is broken for until and while after guard clause</i> added</li></ul> Ruby master - Bug #15980: Coverage shows while/until after raise if/unless as uncovered linehttps://bugs.ruby-lang.org/issues/15980?journal_id=829452019-12-03T22:05:23Zpuchuu (Andrew Aladjev)aladjev.andrew@gmail.com
<ul></ul><p>Hello. This thing affects both <code>return</code>, <code>break</code> and <code>raise</code> guard clauses. I want to insist on covering this issue with tests, <code>test_line_coverage_for_multiple_lines</code> is too weak. I will try to prepare tests.</p>
<p>Unfortunately this bug was not fixed by <a href="https://github.com/ruby/ruby/commit/100bf2757468439106775a7d95a791a8c10b874a" class="external">100bf2757468439106775a7d95a791a8c10b874a</a>. I am able to reproduce it now using <code>ruby-head</code> with the following code:</p>
<p>test.rb:</p>
<pre><code>require "coverage"
Coverage.start
require_relative "cov"
pp Coverage.result
</code></pre>
<p>cov.rb:</p>
<pre><code>a = 2
raise StandardError if a.zero?
a -= 1 until a.zero?
puts a
</code></pre>
<p>It returns <code>[1, nil, 1, nil, 0, nil, 1]</code>, but the right answer is <code>[1, nil, 1, nil, 1, nil, 1]</code>.</p>
<p><code>ruby -v</code> equals <code>ruby 2.7.0dev (2019-12-03T21:40:54Z trunk 8852fa8760) [x86_64-linux]</code></p> Ruby master - Bug #15980: Coverage shows while/until after raise if/unless as uncovered linehttps://bugs.ruby-lang.org/issues/15980?journal_id=829462019-12-03T22:32:11Zmame (Yusuke Endoh)mame@ruby-lang.org
<ul></ul><p>Oops, I overlooked this issue. Sorry.</p>
<p>This is a tough problem. The compiler produces the following code:</p>
<pre><code>0006 branchunless 13
...
0013 jump 17 (Line 2)
...
0017 putnil (Line 3)
</code></pre>
<p>The peephole optimizer changes it to:</p>
<pre><code>0006 branchunless 17
...
0013 jump 17 (Line 2)
...
0017 putnil (Line 3)
</code></pre>
<p>Now "jump 17" instruction is unreachable, so Line 2 event is never fired. But coverage measurement doesn't know it, so it initializes zero entry for Line 2 in coverage.</p>
<p>I think of some options to fix:</p>
<ol>
<li>Stop the peephole optimization (in some cases); it reduces performance even when coverage is not used, so I don't want to do this.</li>
<li>Remove unreachable instructions; it requires revamp of the compiler.</li>
<li>Analyze unreachable instructions and suppress coverage entry; it is relatively easy, but still requires unreachable code analysis.</li>
</ol>
<p><a class="user active user-mention" href="https://bugs.ruby-lang.org/users/17">@ko1 (Koichi Sasada)</a> Do you have any idea?</p> Ruby master - Bug #15980: Coverage shows while/until after raise if/unless as uncovered linehttps://bugs.ruby-lang.org/issues/15980?journal_id=829482019-12-03T22:44:48Zjeremyevans0 (Jeremy Evans)merch-redmine@jeremyevans.net
<ul></ul><p>mame (Yusuke Endoh) wrote:</p>
<blockquote>
<p>I think of some options to fix:</p>
<ol>
<li>Stop the peephole optimization (in some cases); it reduces performance even when coverage is not used, so I don't want to do this.</li>
<li>Remove unreachable instructions; it requires revamp of the compiler.</li>
<li>Analyze unreachable instructions and suppress coverage entry; it is relatively easy, but still requires unreachable code analysis.</li>
</ol>
</blockquote>
<p>My idea for a hacky workaround: As the correct result is shown when branch coverage is enabled, always run with branch coverage enabled, even if the branch results will not be used.</p> Ruby master - Bug #15980: Coverage shows while/until after raise if/unless as uncovered linehttps://bugs.ruby-lang.org/issues/15980?journal_id=829532019-12-04T00:53:29Zmame (Yusuke Endoh)mame@ruby-lang.org
<ul></ul><p>jeremyevans0 (Jeremy Evans) wrote:</p>
<blockquote>
<p>My idea for a hacky workaround: As the correct result is shown when branch coverage is enabled, always run with branch coverage enabled, even if the branch results will not be used.</p>
</blockquote>
<p>This issue is not only coverage but also TracePoint line events. It does not fix TracePoint.</p>
<p>I was trying to implement Approach 3, but I found it does not fix the issue. The instruction <code>jump 17</code> that I said in my previous comment is theoretically reachable if <code>1==2</code> returns true and <code>raise</code> is redefined as no-op.</p>
<p>Here is a patch by Approach 1:</p>
<pre><code class="diff syntaxhl" data-language="diff"><span class="gh">diff --git a/compile.c b/compile.c
index 0b808342c0..150515139c 100644
</span><span class="gd">--- a/compile.c
</span><span class="gi">+++ b/compile.c
</span><span class="p">@@ -2861,6 +2861,7 @@</span> iseq_peephole_optimize(rb_iseq_t *iseq, LINK_ELEMENT *list, const int do_tailcal
* if L2
*/
INSN *nobj = (INSN *)get_destination_insn(iobj);
<span class="gi">+ if (nobj->insn_info.events == 0) {
</span> INSN *pobj = (INSN *)iobj->link.prev;
int prev_dup = 0;
if (pobj) {
<span class="p">@@ -2965,6 +2966,7 @@</span> iseq_peephole_optimize(rb_iseq_t *iseq, LINK_ELEMENT *list, const int do_tailcal
nobj = (INSN *)get_destination_insn(nobj);
}
}
<span class="gi">+ }
</span> if (IS_INSN_ID(iobj, pop)) {
/*
</code></pre>
<p>It is so simple; it just suppresses the jump-jump peephole optimization when the jump instruction being skipped has an event flag. But I'm unsure how much it reduces performance.</p> Ruby master - Bug #15980: Coverage shows while/until after raise if/unless as uncovered linehttps://bugs.ruby-lang.org/issues/15980?journal_id=829562019-12-04T01:19:47Zmame (Yusuke Endoh)mame@ruby-lang.org
<ul></ul><p>I talked with <a class="user active user-mention" href="https://bugs.ruby-lang.org/users/17">@ko1 (Koichi Sasada)</a>, and decide to introduce a stopgap measurement: stop the optimization in question only when coverage measurement is enabled. This is because this issue is critical in coverage measurement, but minor in TracePoint.</p>
<p>The current compiler has many minor flaws, so we need to revamp it anyway in near future.</p>
<p>I will commit a patch soon.</p> Ruby master - Bug #15980: Coverage shows while/until after raise if/unless as uncovered linehttps://bugs.ruby-lang.org/issues/15980?journal_id=829582019-12-04T01:41:16Zmame (Yusuke Endoh)mame@ruby-lang.org
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li></ul><p>Applied in changeset <a class="changeset" title="compile.c: stop wrong peephole optimization when covearge is enabled jump-jump optimization igno..." href="https://bugs.ruby-lang.org/projects/ruby-master/repository/git/revisions/f9e5c74cd24025a5aa19e318e8fecabf207f1b7b">git|f9e5c74cd24025a5aa19e318e8fecabf207f1b7b</a>.</p>
<hr>
<p>compile.c: stop wrong peephole optimization when covearge is enabled</p>
<p>jump-jump optimization ignores the event flags of the jump instruction<br>
being skipped, which leads to overlook of line events.</p>
<p>This changeset stops the wrong optimization when coverage measurement is<br>
neabled and when the jump instruction has any event flag.</p>
<p>Note that this issue is not only for coverage but also for TracePoint,<br>
and this change does not fix TracePoint.<br>
However, fixing it fundamentally is tough (which requires revamp of<br>
the compiler). This issue is critical in terms of coverage measurement,<br>
but minor for TracePoint (ko1 said), so we here choose a stopgap<br>
measurement.</p>
<p>[Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Coverage shows while/until after raise if/unless as uncovered line (Closed)" href="https://bugs.ruby-lang.org/issues/15980">#15980</a>] [Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Line coverage is broken for until and while after guard clause (Closed)" href="https://bugs.ruby-lang.org/issues/16397">#16397</a>]</p>
<p>Note for backporters: this changeset can be viewed by <code>git diff -w</code>.</p> Ruby master - Bug #15980: Coverage shows while/until after raise if/unless as uncovered linehttps://bugs.ruby-lang.org/issues/15980?journal_id=829632019-12-04T09:26:15Zpuchuu (Andrew Aladjev)aladjev.andrew@gmail.com
<ul></ul><p>There is a workaround for everyone wants to use lines coverage only with 2.6.5 version (recommended by Jeremy Evans):</p>
<pre><code class="ruby syntaxhl" data-language="ruby"><span class="c1"># Workaround for https://bugs.ruby-lang.org/issues/15980</span>
<span class="nb">require</span> <span class="s2">"coverage"</span>
<span class="no">Coverage</span><span class="p">.</span><span class="nf">module_eval</span> <span class="k">do</span>
<span class="n">singleton_class</span><span class="p">.</span><span class="nf">send</span> <span class="ss">:alias_method</span><span class="p">,</span> <span class="ss">:original_start</span><span class="p">,</span> <span class="ss">:start</span>
<span class="k">def</span> <span class="nc">self</span><span class="o">.</span><span class="nf">start</span>
<span class="n">original_start</span> <span class="ss">:lines</span> <span class="o">=></span> <span class="kp">true</span><span class="p">,</span> <span class="ss">:branches</span> <span class="o">=></span> <span class="kp">true</span>
<span class="k">end</span>
<span class="n">singleton_class</span><span class="p">.</span><span class="nf">send</span> <span class="ss">:alias_method</span><span class="p">,</span> <span class="ss">:original_result</span><span class="p">,</span> <span class="ss">:result</span>
<span class="k">def</span> <span class="nc">self</span><span class="o">.</span><span class="nf">result</span>
<span class="n">original_result</span><span class="p">.</span><span class="nf">transform_values</span> <span class="p">{</span> <span class="o">|</span><span class="n">coverage</span><span class="o">|</span> <span class="n">coverage</span><span class="p">[</span><span class="ss">:lines</span><span class="p">]</span> <span class="p">}</span>
<span class="k">end</span>
<span class="k">end</span>
</code></pre>
<p>It works for simplecov.</p> Ruby master - Bug #15980: Coverage shows while/until after raise if/unless as uncovered linehttps://bugs.ruby-lang.org/issues/15980?journal_id=829682019-12-04T16:09:35Zpuchuu (Andrew Aladjev)aladjev.andrew@gmail.com
<ul></ul><p>Yusuke Endoh, I've tested using the following code:</p>
<pre><code class="ruby syntaxhl" data-language="ruby"><span class="kp">loop</span> <span class="k">do</span>
<span class="k">break</span> <span class="k">if</span> <span class="nb">rand</span> <span class="o"><</span> <span class="mi">0</span>
<span class="k">break</span> <span class="k">while</span> <span class="kp">true</span>
<span class="k">next</span> <span class="k">if</span> <span class="nb">rand</span> <span class="o"><</span> <span class="mi">0</span>
<span class="k">break</span> <span class="k">until</span> <span class="kp">false</span>
<span class="k">return</span> <span class="k">if</span> <span class="nb">rand</span> <span class="o"><</span> <span class="mi">0</span>
<span class="k">break</span> <span class="k">while</span> <span class="kp">true</span>
<span class="k">raise</span> <span class="k">if</span> <span class="nb">rand</span> <span class="o"><</span> <span class="mi">0</span>
<span class="k">break</span> <span class="k">until</span> <span class="kp">false</span>
<span class="nb">exit</span> <span class="k">if</span> <span class="nb">rand</span> <span class="o"><</span> <span class="mi">0</span>
<span class="k">break</span> <span class="k">while</span> <span class="kp">true</span>
<span class="k">break</span>
<span class="k">end</span>
</code></pre>
<p>Result is <code>[1, 1, 1, nil, 1, 1, nil, 1, 1, nil, 1, 1, nil, 1, 1, nil, 1, nil]</code>, works fine, thank you.</p> Ruby master - Bug #15980: Coverage shows while/until after raise if/unless as uncovered linehttps://bugs.ruby-lang.org/issues/15980?journal_id=906522021-02-28T14:16:41Zusa (Usaku NAKAMURA)usa@garbagecollect.jp
<ul><li><strong>Backport</strong> changed from <i>2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: REQUIRED</i> to <i>2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: DONE</i></li></ul><p>ruby_2_6 r67898 merged revision(s) f9e5c74c.</p>