Simplecov: Until - not covered, loop - covered

[andrew-aladev]

Hello. I've received unusual issue with simplecov v0.17.0 ruby 2.6.5.

See project coverage.

Please look at line:

read_more_from_buffer until @io.eof?

I've updated it to:

loop do
  break if @io.eof?
  read_more_from_buffer
end

And it was covered fine. There is some unusual issue here. I will investigate it deeper and report results.

[andrew-aladev]

Ruby (coverage) VM returns 0 for until line. I was trying to extract a sample from project and failed. I am not able to reproduce this issue in any extracted source. It looks like an unusual bug inside ruby (coverage) VM.

[PragTob]

Hm that's truly strange, normally one line conditional constructs are marked as covered as soon as the condition was checked

[andrew-aladev]

I've checked that:

static void
update_line_coverage(VALUE data, const rb_trace_arg_t *trace_arg)

This function was never called for until lines. So RUBY_EVENT_COVERAGE_LINE may not work for until lines. The bug is somewhere in ruby vm. I will investigate it and report results later.

[PragTob]

@andrew-aladev thanks for your investigation! Have you reported a bug with ruby-core? When I know it's being tracked somewhere else I'd like to close this here with a reference to it :)

[andrew-aladev]

Now I've created the following patch:

diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 1c54de6375..014753a328 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -4800,6 +4800,20 @@ vm_trace(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, const VALUE *p
             return;
         }
         else {
+            const rb_control_frame_t *cfp = rb_vm_get_ruby_level_next_cfp(ec, ec->cfp);
+            if (cfp) {
+            fprintf(
+              stderr,
+              "ololo %s %d-%d-%d\n",
+              RSTRING_PTR(rb_iseq_path(cfp->iseq)),
+              rb_vm_get_sourceline(cfp),
+              pc_events & RUBY_EVENT_COVERAGE_LINE,
+              (pc_events & RUBY_EVENT_COVERAGE_LINE) & enabled_flags
+            );
+            } else {
+              fprintf(stderr, "ololo none\n");
+            }
+
             rb_hook_list_t *global_hooks = rb_vm_global_hooks(ec);

             if (0) {

Than commands:

rvm reinstall ruby-2.6.5 --patch /tmp/fit.patch
bundle exec rake > /dev/stdout 2>&1 | grep "^ololo /home/puchuu/workspace/ruby-zstds/lib/zstds/stream/reader.rb" | cut -d " " -f 3 | sort -h | uniq -c

This thing gave me the line coverage from vm point of view:

...
1838477 53-65536-65536
1818892 56-65536-65536
   5379 61-65536-65536
   5379 63-65536-65536
...

56 and 61 lines works perfect, so vm is fine. Bug is somewhere inside vm_trace_hook. I will continue investigating it and will report ruby bug with results.

[andrew-aladev]

That's it, bug is here:

/* increment PC because source line is calculated with PC-1 */
reg_cfp->pc++;
...
reg_cfp->pc--;

This is not true for until line. I will investigate this PC logic and find why it is not true.

[andrew-aladev]

Created ruby bug.

[andrew-aladev]

For now we can use the following workaround:

Coverage.start :lines => true, :branches => true

Enabling branches magically fixes lines behaviour.

# Workaround for https://bugs.ruby-lang.org/issues/15980
require "coverage"

Coverage.module_eval do
  singleton_class.send :alias_method, :original_start, :start
  def self.start
    original_start :lines => true, :branches => true
  end

  singleton_class.send :alias_method, :original_result, :result
  def self.result
    original_result.transform_values { |coverage| coverage[:lines] }
  end
end

[PragTob]

Thanks for opening the ruby bug! :tada:

uh that is indeed very strange... I'll make a note to investigate that - I'm not sure I'd wanna go with this workaroud or first look into it a bit more.

Also wow, they seem to have it fixed already: https://bugs.ruby-lang.org/projects/ruby-trunk/repository/git/revisions/f9e5c74cd24025a5aa19e318e8fecabf207f1b7b

[andrew-aladev]

This bug is definitely fixed in 2.7 mainstream branch, but I am not sure that it will be backported to 2.6. So for now I will use magic workaround =).

Thank you.

[PragTob]

No, thank you for all your work.

I won't implement the workaround in simplecov as I'm afraid it's somehow gonna shoot us in the foot and if it's already fixed people can upgrade to get it :)

Closing out here, thank you for all the great investigation! :tada: