From d494cf4ddababb80660381e963f910ccacc3f7bc Mon Sep 17 00:00:00 2001 From: "U.Nakamura" Date: Mon, 6 Nov 2023 20:14:41 +0900 Subject: [PATCH] merge revision(s) 4a7d6c2852aa734506be83c932168e8f974687b5: [Backport #18991] Fix false LocalJumpError when branch coverage is enabled `throw TAG_BREAK` instruction makes a jump only if the continuation of catch of TAG_BREAK exactly matches the instruction immediately following the "send" instruction that is currently being executed. Otherwise, it seems to determine break from proc-closure. Branch coverage may insert some recording instructions after "send" instruction, which broke the conditions for TAG_BREAK to work properly. This change forces to set the continuation of catch of TAG_BREAK immediately after "send" (or "invokesuper") instruction. [Bug #18991] --- compile.c | 25 ++++++++++++++++++++++++- test/coverage/test_coverage.rb | 14 ++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) --- compile.c | 25 ++++++++++++++++++++++++- test/coverage/test_coverage.rb | 14 ++++++++++++++ version.h | 6 +++--- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/compile.c b/compile.c index 2ebc6a7755..2641accc03 100644 --- a/compile.c +++ b/compile.c @@ -7216,7 +7216,30 @@ compile_iter(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node, in ISEQ_TYPE_BLOCK, line); CHECK(COMPILE(ret, "iter caller", node->nd_iter)); } - ADD_LABEL(ret, retry_end_l); + + { + // We need to put the label "retry_end_l" immediately after the last "send" instruction. + // This because vm_throw checks if the break cont is equal to the index of next insn of the "send". + // (Otherwise, it is considered "break from proc-closure". See "TAG_BREAK" handling in "vm_throw_start".) + // + // Normally, "send" instruction is at the last. + // However, qcall under branch coverage measurement adds some instructions after the "send". + // + // Note that "invokesuper" appears instead of "send". + INSN *iobj; + LINK_ELEMENT *last_elem = LAST_ELEMENT(ret); + iobj = IS_INSN(last_elem) ? (INSN*) last_elem : (INSN*) get_prev_insn((INSN*) last_elem); + while (INSN_OF(iobj) != BIN(send) && INSN_OF(iobj) != BIN(invokesuper)) { + iobj = (INSN*) get_prev_insn(iobj); + } + ELEM_INSERT_NEXT(&iobj->link, (LINK_ELEMENT*) retry_end_l); + + // LINK_ANCHOR has a pointer to the last element, but ELEM_INSERT_NEXT does not update it + // even if we add an insn to the last of LINK_ANCHOR. So this updates it manually. + if (&iobj->link == LAST_ELEMENT(ret)) { + ret->last = (LINK_ELEMENT*) retry_end_l; + } + } if (popped) { ADD_INSN(ret, line_node, pop); diff --git a/test/coverage/test_coverage.rb b/test/coverage/test_coverage.rb index 882368363a..789c2c6e90 100644 --- a/test/coverage/test_coverage.rb +++ b/test/coverage/test_coverage.rb @@ -920,4 +920,18 @@ class TestCoverage < Test::Unit::TestCase p :NG end; end + + def test_tag_break_with_branch_coverage + result = { + :branches => { + [:"&.", 0, 1, 0, 1, 6] => { + [:then, 1, 1, 0, 1, 6] => 1, + [:else, 2, 1, 0, 1, 6] => 0, + }, + }, + } + assert_coverage(<<~"end;", { branches: true }, result) + 1&.tap do break end + end; + end end diff --git a/version.h b/version.h index 1ddc8863a8..b6c504b132 100644 --- a/version.h +++ b/version.h @@ -11,11 +11,11 @@ # define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR #define RUBY_VERSION_TEENY 4 #define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR -#define RUBY_PATCHLEVEL 244 +#define RUBY_PATCHLEVEL 245 #define RUBY_RELEASE_YEAR 2023 -#define RUBY_RELEASE_MONTH 10 -#define RUBY_RELEASE_DAY 30 +#define RUBY_RELEASE_MONTH 11 +#define RUBY_RELEASE_DAY 6 #include "ruby/version.h"