summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authormame <mame@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2017-12-18 02:44:36 +0000
committermame <mame@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2017-12-18 02:44:36 +0000
commit2e24a66b883b7f14d9dd0aeffa749d68dd5d6939 (patch)
treeacc092cd4b620f77c355b13f06f6f510604e79b9
parentfb6db414a199a6d2e43952413823198b69b6d1e3 (diff)
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. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@61313 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
-rw-r--r--compile.c18
-rw-r--r--iseq.c12
-rw-r--r--test/coverage/test_coverage.rb33
3 files changed, 52 insertions, 11 deletions
diff --git a/compile.c b/compile.c
index 2677c8a..37f024b 100644
--- a/compile.c
+++ b/compile.c
@@ -251,14 +251,6 @@ struct iseq_compile_data_ensure_node_stack {
#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_ANCHOR *const ret, const NODE *node, in
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_ANCHOR *const ret, const NODE *node, in
}
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_ANCHOR *const ret, const NODE *node, in
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:{
diff --git a/iseq.c b/iseq.c
index 186f862..f0239cc 100644
--- a/iseq.c
+++ b/iseq.c
@@ -350,9 +350,21 @@ finish_iseq_build(rb_iseq_t *iseq)
{
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");
diff --git a/test/coverage/test_coverage.rb b/test/coverage/test_coverage.rb
index 3fbd815..281d525 100644
--- a/test/coverage/test_coverage.rb
+++ b/test/coverage/test_coverage.rb
@@ -201,6 +201,39 @@ class TestCoverage < Test::Unit::TestCase
}
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 => {