summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--compile.c232
-rw-r--r--test/coverage/test_coverage.rb12
2 files changed, 141 insertions, 103 deletions
diff --git a/compile.c b/compile.c
index 0b808342c0..e261a38d7d 100644
--- a/compile.c
+++ b/compile.c
@@ -2861,109 +2861,135 @@ iseq_peephole_optimize(rb_iseq_t *iseq, LINK_ELEMENT *list, const int do_tailcal
* if L2
*/
INSN *nobj = (INSN *)get_destination_insn(iobj);
- INSN *pobj = (INSN *)iobj->link.prev;
- int prev_dup = 0;
- if (pobj) {
- if (!IS_INSN(&pobj->link))
- pobj = 0;
- else if (IS_INSN_ID(pobj, dup))
- prev_dup = 1;
- }
-
- for (;;) {
- if (IS_INSN_ID(nobj, jump)) {
- replace_destination(iobj, nobj);
- }
- else if (prev_dup && IS_INSN_ID(nobj, dup) &&
- !!(nobj = (INSN *)nobj->link.next) &&
- /* basic blocks, with no labels in the middle */
- nobj->insn_id == iobj->insn_id) {
- /*
- * dup
- * if L1
- * ...
- * L1:
- * dup
- * if L2
- * =>
- * dup
- * if L2
- * ...
- * L1:
- * dup
- * if L2
- */
- replace_destination(iobj, nobj);
- }
- else if (pobj) {
- /*
- * putnil
- * if L1
- * =>
- * # nothing
- *
- * putobject true
- * if L1
- * =>
- * jump L1
- *
- * putstring ".."
- * if L1
- * =>
- * jump L1
- *
- * putstring ".."
- * dup
- * if L1
- * =>
- * putstring ".."
- * jump L1
- *
- */
- int cond;
- if (prev_dup && IS_INSN(pobj->link.prev)) {
- pobj = (INSN *)pobj->link.prev;
- }
- if (IS_INSN_ID(pobj, putobject)) {
- cond = (IS_INSN_ID(iobj, branchif) ?
- OPERAND_AT(pobj, 0) != Qfalse :
- IS_INSN_ID(iobj, branchunless) ?
- OPERAND_AT(pobj, 0) == Qfalse :
- FALSE);
- }
- else if (IS_INSN_ID(pobj, putstring) ||
- IS_INSN_ID(pobj, duparray) ||
- IS_INSN_ID(pobj, newarray)) {
- cond = IS_INSN_ID(iobj, branchif);
- }
- else if (IS_INSN_ID(pobj, putnil)) {
- cond = !IS_INSN_ID(iobj, branchif);
- }
- else break;
- if (prev_dup || !IS_INSN_ID(pobj, newarray)) {
- ELEM_REMOVE(iobj->link.prev);
- }
- else if (!iseq_pop_newarray(iseq, pobj)) {
- pobj = new_insn_core(iseq, pobj->insn_info.line_no, BIN(pop), 0, NULL);
- ELEM_INSERT_PREV(&iobj->link, &pobj->link);
- }
- if (cond) {
- if (prev_dup) {
- pobj = new_insn_core(iseq, pobj->insn_info.line_no, BIN(putnil), 0, NULL);
- ELEM_INSERT_NEXT(&iobj->link, &pobj->link);
- }
- iobj->insn_id = BIN(jump);
- goto again;
- }
- else {
- unref_destination(iobj, 0);
- ELEM_REMOVE(&iobj->link);
- }
- break;
- }
- else break;
- nobj = (INSN *)get_destination_insn(nobj);
- }
+
+ /* This is super nasty hack!!!
+ *
+ * This jump-jump optimization may ignore event flags of the jump
+ * instruction being skipped. Actually, Line 2 TracePoint event
+ * is never fired in the following code:
+ *
+ * 1: raise if 1 == 2
+ * 2: while true
+ * 3: break
+ * 4: end
+ *
+ * This is critical for coverage measurement. [Bug #15980]
+ *
+ * This is a stopgap measure: stop the jump-jump optimization if
+ * coverage measurement is enabled and if the skipped instruction
+ * has any event flag.
+ *
+ * Note that, still, TracePoint Line event does not occur on Line 2.
+ * This should be fixed in future.
+ */
+ int stop_optimization =
+ ISEQ_COVERAGE(iseq) && ISEQ_LINE_COVERAGE(iseq) &&
+ nobj->insn_info.events;
+ if (!stop_optimization) {
+ INSN *pobj = (INSN *)iobj->link.prev;
+ int prev_dup = 0;
+ if (pobj) {
+ if (!IS_INSN(&pobj->link))
+ pobj = 0;
+ else if (IS_INSN_ID(pobj, dup))
+ prev_dup = 1;
+ }
+
+ for (;;) {
+ if (IS_INSN_ID(nobj, jump)) {
+ replace_destination(iobj, nobj);
+ }
+ else if (prev_dup && IS_INSN_ID(nobj, dup) &&
+ !!(nobj = (INSN *)nobj->link.next) &&
+ /* basic blocks, with no labels in the middle */
+ nobj->insn_id == iobj->insn_id) {
+ /*
+ * dup
+ * if L1
+ * ...
+ * L1:
+ * dup
+ * if L2
+ * =>
+ * dup
+ * if L2
+ * ...
+ * L1:
+ * dup
+ * if L2
+ */
+ replace_destination(iobj, nobj);
+ }
+ else if (pobj) {
+ /*
+ * putnil
+ * if L1
+ * =>
+ * # nothing
+ *
+ * putobject true
+ * if L1
+ * =>
+ * jump L1
+ *
+ * putstring ".."
+ * if L1
+ * =>
+ * jump L1
+ *
+ * putstring ".."
+ * dup
+ * if L1
+ * =>
+ * putstring ".."
+ * jump L1
+ *
+ */
+ int cond;
+ if (prev_dup && IS_INSN(pobj->link.prev)) {
+ pobj = (INSN *)pobj->link.prev;
+ }
+ if (IS_INSN_ID(pobj, putobject)) {
+ cond = (IS_INSN_ID(iobj, branchif) ?
+ OPERAND_AT(pobj, 0) != Qfalse :
+ IS_INSN_ID(iobj, branchunless) ?
+ OPERAND_AT(pobj, 0) == Qfalse :
+ FALSE);
+ }
+ else if (IS_INSN_ID(pobj, putstring) ||
+ IS_INSN_ID(pobj, duparray) ||
+ IS_INSN_ID(pobj, newarray)) {
+ cond = IS_INSN_ID(iobj, branchif);
+ }
+ else if (IS_INSN_ID(pobj, putnil)) {
+ cond = !IS_INSN_ID(iobj, branchif);
+ }
+ else break;
+ if (prev_dup || !IS_INSN_ID(pobj, newarray)) {
+ ELEM_REMOVE(iobj->link.prev);
+ }
+ else if (!iseq_pop_newarray(iseq, pobj)) {
+ pobj = new_insn_core(iseq, pobj->insn_info.line_no, BIN(pop), 0, NULL);
+ ELEM_INSERT_PREV(&iobj->link, &pobj->link);
+ }
+ if (cond) {
+ if (prev_dup) {
+ pobj = new_insn_core(iseq, pobj->insn_info.line_no, BIN(putnil), 0, NULL);
+ ELEM_INSERT_NEXT(&iobj->link, &pobj->link);
+ }
+ iobj->insn_id = BIN(jump);
+ goto again;
+ }
+ else {
+ unref_destination(iobj, 0);
+ ELEM_REMOVE(&iobj->link);
+ }
+ break;
+ }
+ else break;
+ nobj = (INSN *)get_destination_insn(nobj);
+ }
+ }
}
if (IS_INSN_ID(iobj, pop)) {
diff --git a/test/coverage/test_coverage.rb b/test/coverage/test_coverage.rb
index f88e97e9d6..1de39bf569 100644
--- a/test/coverage/test_coverage.rb
+++ b/test/coverage/test_coverage.rb
@@ -728,4 +728,16 @@ class TestCoverage < Test::Unit::TestCase
}
}
end
+
+ def test_stop_wrong_peephole_optimization
+ result = {
+ :lines => [1, 1, 1, nil]
+ }
+ assert_coverage(<<~"end;", { lines: true }, result)
+ raise if 1 == 2
+ while true
+ break
+ end
+ end;
+ end
end
generated by cgit v1.2.3 (git 2.25.1) at 2024-04-27 16:26:33 +0000