mame	2017-12-18 11:44:36 +0900 (Mon, 18 Dec 2017)

  New Revision: 61313

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=61313

  Log:
    iseq.c (finish_iseq_build): fix coverage leakage [Bug #14191]
    
    Before this change, coverage.so had failed to measure some multiple-line
    code fragments.  This is because removing trace instructions (#14104)
    changed TracePoint's lineno (new lineno), and coverage counter array was
    based on old lineno.
    This change initializes coverage counter array based on new lineno.

  Modified files:
    trunk/compile.c
    trunk/iseq.c
    trunk/test/coverage/test_coverage.rb
Index: iseq.c
===================================================================
--- iseq.c	(revision 61312)
+++ iseq.c	(revision 61313)
@@ -350,9 +350,21 @@ finish_iseq_build(rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/iseq.c#L350
 {
     struct iseq_compile_data *data = ISEQ_COMPILE_DATA(iseq);
     VALUE err = data->err_info;
+    unsigned int i;
     ISEQ_COMPILE_DATA_CLEAR(iseq);
     compile_data_free(data);
 
+    if (ISEQ_COVERAGE(iseq) && ISEQ_LINE_COVERAGE(iseq)) {
+	for (i = 0; i < iseq->body->insns_info_size; i++) {
+	    if (iseq->body->insns_info[i].events & RUBY_EVENT_LINE) {
+		int line_no = iseq->body->insns_info[i].line_no - 1;
+		if (0 <= line_no && line_no < RARRAY_LEN(ISEQ_LINE_COVERAGE(iseq))) {
+		    RARRAY_ASET(ISEQ_LINE_COVERAGE(iseq), line_no, INT2FIX(0));
+		}
+	    }
+	}
+    }
+
     if (RTEST(err)) {
 	VALUE path = pathobj_path(iseq->body->location.pathobj);
 	if (err == Qtrue) err = rb_exc_new_cstr(rb_eSyntaxError, "compile error");
Index: test/coverage/test_coverage.rb
===================================================================
--- test/coverage/test_coverage.rb	(revision 61312)
+++ test/coverage/test_coverage.rb	(revision 61313)
@@ -201,6 +201,39 @@ class TestCoverage < Test::Unit::TestCas https://github.com/ruby/ruby/blob/trunk/test/coverage/test_coverage.rb#L201
     }
   end
 
+  def test_line_coverage_for_multiple_lines
+    result = {
+      :lines => [nil, 1, nil, nil, nil, 1, nil, nil, nil, 1, nil, 1, nil, nil, nil, nil, 1, 1, nil, 1, nil, nil, nil, nil, 1]
+    }
+    assert_coverage(<<~"end;", { lines: true }, result) # Bug #14191
+      FOO = [
+        { foo: 'bar' },
+        { bar: 'baz' }
+      ]
+
+      'some string'.split
+                   .map(&:length)
+
+      some =
+        'value'
+
+      Struct.new(
+        :foo,
+        :bar
+      ).new
+
+      class Test
+        def foo(bar)
+          {
+            foo: bar
+          }
+        end
+      end
+
+      Test.new.foo(Object.new)
+    end;
+  end
+
   def test_branch_coverage_for_if_statement
     result = {
       :branches => {
Index: compile.c
===================================================================
--- compile.c	(revision 61312)
+++ compile.c	(revision 61313)
@@ -251,14 +251,6 @@ struct iseq_compile_data_ensure_node_sta https://github.com/ruby/ruby/blob/trunk/compile.c#L251
 #define ADD_TRACE(seq, event) \
   ADD_ELEM((seq), (LINK_ELEMENT *)new_trace_body(iseq, (event)))
 
-#define SETUP_LINE_COVERAGE(seq, line) \
-  do { \
-      if (ISEQ_COVERAGE(iseq) && \
-	  ISEQ_LINE_COVERAGE(iseq) && \
-	  (line) > 0) { \
-	  RARRAY_ASET(ISEQ_LINE_COVERAGE(iseq), (line) - 1, INT2FIX(0)); \
-      } \
-  } while (0)
 #define DECL_BRANCH_BASE(branches, first_line, first_column, last_line, last_column, type) \
   do { \
       if (ISEQ_COVERAGE(iseq) && \
@@ -5422,7 +5414,6 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK https://github.com/ruby/ruby/blob/trunk/compile.c#L5414
     else {
 	if (node->flags & NODE_FL_NEWLINE) {
 	    ISEQ_COMPILE_DATA(iseq)->last_line = line;
-	    SETUP_LINE_COVERAGE(ret, line);
 	    ADD_TRACE(ret, RUBY_EVENT_LINE);
 	}
     }
@@ -7040,7 +7031,6 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK https://github.com/ruby/ruby/blob/trunk/compile.c#L7031
       }
       case NODE_PRELUDE:{
 	const rb_compile_option_t *orig_opt = ISEQ_COMPILE_DATA(iseq)->option;
-	VALUE orig_cov = ISEQ_COVERAGE(iseq);
 	rb_compile_option_t new_opt = *orig_opt;
 	if (node->nd_compile_option) {
 	    rb_iseq_make_compile_option(&new_opt, node->nd_compile_option);
@@ -7050,7 +7040,13 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK https://github.com/ruby/ruby/blob/trunk/compile.c#L7040
 	CHECK(COMPILE_POPPED(ret, "prelude", node->nd_head));
 	CHECK(COMPILE_(ret, "body", node->nd_body, popped));
 	ISEQ_COMPILE_DATA(iseq)->option = orig_opt;
-	ISEQ_COVERAGE_SET(iseq, orig_cov);
+	/* Do NOT restore ISEQ_COVERAGE!
+	 * If ISEQ_COVERAGE is not false, finish_iseq_build function in iseq.c
+	 * will initialize the counter array of line coverage.
+	 * We keep ISEQ_COVERAGE as nil to disable this initialization.
+	 * This is not harmful assuming that NODE_PRELUDE pragma does not occur
+	 * in NODE tree except the root.
+	 */
 	break;
       }
       case NODE_LAMBDA:{

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/