diff options
author | mame <mame@b2dd03c8-39d4-4d8f-98ff-823fe69b080e> | 2017-12-18 02:44:36 +0000 |
---|---|---|
committer | mame <mame@b2dd03c8-39d4-4d8f-98ff-823fe69b080e> | 2017-12-18 02:44:36 +0000 |
commit | 2e24a66b883b7f14d9dd0aeffa749d68dd5d6939 (patch) | |
tree | acc092cd4b620f77c355b13f06f6f510604e79b9 | |
parent | fb6db414a199a6d2e43952413823198b69b6d1e3 (diff) | |
download | ruby-2e24a66b883b7f14d9dd0aeffa749d68dd5d6939.tar.gz |
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.c | 18 | ||||
-rw-r--r-- | iseq.c | 12 | ||||
-rw-r--r-- | test/coverage/test_coverage.rb | 33 |
3 files changed, 52 insertions, 11 deletions
@@ -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:{ @@ -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 3fbd8159be..281d525f0c 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 => { |