From 56031cb270834dea0d6e013641c582a3a56c33ff Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Tue, 10 Mar 2015 17:13:01 +0300 Subject: [PATCH] Control-Flow Analysis: Fix bug in finally-block repetition in the presence of non-local returns #EA-65982 Fixed --- .../kotlin/cfg/pseudocode/PseudocodeImpl.java | 14 +- ...AndNonlocalReturnsWithFinally.instructions | 195 ++++++++++++++++++ .../localAndNonlocalReturnsWithFinally.kt | 14 ++ .../localAndNonlocalReturnsWithFinally.values | 56 +++++ .../kotlin/cfg/ControlFlowTestGenerated.java | 6 + .../kotlin/cfg/PseudoValueTestGenerated.java | 6 + 6 files changed, 288 insertions(+), 3 deletions(-) create mode 100644 compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.instructions create mode 100644 compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.kt create mode 100644 compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.values diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/pseudocode/PseudocodeImpl.java b/compiler/frontend/src/org/jetbrains/kotlin/cfg/pseudocode/PseudocodeImpl.java index 91460434854..d9d30cb1ad7 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/pseudocode/PseudocodeImpl.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/cfg/pseudocode/PseudocodeImpl.java @@ -86,6 +86,10 @@ public class PseudocodeImpl implements Pseudocode { public PseudocodeLabel copy(int newLabelIndex) { return new PseudocodeLabel("L" + newLabelIndex, "copy of " + name + ", " + comment); } + + public PseudocodeImpl getPseudocode() { + return PseudocodeImpl.this; + } } private final List mutableInstructionList = new ArrayList(); @@ -455,6 +459,8 @@ public class PseudocodeImpl implements Pseudocode { } public int repeatPart(@NotNull Label startLabel, @NotNull Label finishLabel, int labelCount) { + PseudocodeImpl originalPseudocode = ((PseudocodeLabel) startLabel).getPseudocode(); + Integer startIndex = ((PseudocodeLabel) startLabel).getTargetInstructionIndex(); assert startIndex != null; Integer finishIndex = ((PseudocodeLabel) finishLabel).getTargetInstructionIndex(); @@ -462,7 +468,7 @@ public class PseudocodeImpl implements Pseudocode { Map originalToCopy = Maps.newLinkedHashMap(); Multimap originalLabelsForInstruction = HashMultimap.create(); - for (PseudocodeLabel label : labels) { + for (PseudocodeLabel label : originalPseudocode.labels) { Integer index = label.getTargetInstructionIndex(); if (index == null) continue; //label is not bounded yet if (label == startLabel || label == finishLabel) continue; @@ -476,11 +482,13 @@ public class PseudocodeImpl implements Pseudocode { labels.add((PseudocodeLabel) label); } for (int index = startIndex; index < finishIndex; index++) { - Instruction originalInstruction = mutableInstructionList.get(index); + Instruction originalInstruction = originalPseudocode.mutableInstructionList.get(index); repeatLabelsBindingForInstruction(originalInstruction, originalToCopy, originalLabelsForInstruction); addInstruction(copyInstruction(originalInstruction, originalToCopy)); } - repeatLabelsBindingForInstruction(mutableInstructionList.get(finishIndex), originalToCopy, originalLabelsForInstruction); + repeatLabelsBindingForInstruction(originalPseudocode.mutableInstructionList.get(finishIndex), + originalToCopy, + originalLabelsForInstruction); return labelCount; } diff --git a/compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.instructions b/compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.instructions new file mode 100644 index 00000000000..b7155a80260 --- /dev/null +++ b/compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.instructions @@ -0,0 +1,195 @@ +== let == +fun T.let(f: (T) -> U): U = f(this) +--------------------- +L0: + 1 + v(f: (T) -> U) + magic[FAKE_INITIALIZER](f: (T) -> U) -> + w(f|) + r(f) -> + r(this) -> + mark(f(this)) + call(f(this), invoke|, ) -> + ret(*|) L1 +L1: + NEXT:[] +error: + PREV:[] +sink: + PREV:[, ] +===================== +== bar == +fun bar(): Int = 1 +--------------------- +L0: + 1 + r(1) -> + ret(*|) L1 +L1: + NEXT:[] +error: + PREV:[] +sink: + PREV:[, ] +===================== +== foo == +fun foo(n: Int): Int { + try { + if (n < 0) return 0 + n.let { return it } + } + finally { + for (i in 1..2) { } + return bar() + } +} +--------------------- +L0: + 1 + v(n: Int) + magic[FAKE_INITIALIZER](n: Int) -> + w(n|) + 2 mark({ try { if (n < 0) return 0 n.let { return it } } finally { for (i in 1..2) { } return bar() } }) + mark(try { if (n < 0) return 0 n.let { return it } } finally { for (i in 1..2) { } return bar() }) + jmp?(L2) NEXT:[mark({ for (i in 1..2) { } return bar() }), mark({ if (n < 0) return 0 n.let { return it } })] + 3 mark({ if (n < 0) return 0 n.let { return it } }) + mark(if (n < 0) return 0) + r(n) -> PREV:[mark(if (n < 0) return 0), jmp(L15)] + r(0) -> + mark(n < 0) + call(n < 0, compareTo|, ) -> + jf(L3|) NEXT:[read (Unit), r(0) -> ] + r(0) -> +L4 [start finally]: + 4 mark({ for (i in 1..2) { } return bar() }) + 5 r(1) -> PREV:[mark({ for (i in 1..2) { } return bar() }), jmp?(L16)] + r(2) -> + mark(1..2) + call(1..2, rangeTo|, ) -> + v(i) +L5 [loop entry point]: +L9 [condition entry point]: + jmp?(L6) NEXT:[read (Unit), magic[LOOP_RANGE_ITERATION](1..2|) -> ] PREV:[v(i), jmp(L5)] + magic[LOOP_RANGE_ITERATION](1..2|) -> + w(i|) + mark(for (i in 1..2) { }) +L7 [body entry point]: + 6 mark({ }) + read (Unit) + 5 jmp(L5) NEXT:[jmp?(L6)] +L6 [loop exit point]: +L8 [body exit point]: + read (Unit) PREV:[jmp?(L6)] + 4 mark(bar()) + call(bar(), bar) -> + ret(*|) L1 NEXT:[] +L10 [finish finally]: +- 3 ret(*|) L1 NEXT:[] PREV:[] +- jmp(L11) NEXT:[merge(if (n < 0) return 0|!) -> ] PREV:[] +L3 [else branch]: + read (Unit) PREV:[jf(L3|)] +L11 ['if' expression result]: + merge(if (n < 0) return 0|!) -> + mark(n.let { return it }) + r(n) -> + mark({ return it }) + jmp?(L12) NEXT:[r({ return it }) -> , d({ return it })] + d({ return it }) NEXT:[] +L12 [after local declaration]: + r({ return it }) -> PREV:[jmp?(L12)] + mark(let { return it }) + call(let { return it }, let|, ) -> + 2 jmp(L20) NEXT:[mark({ for (i in 1..2) { } return bar() })] +L2 [onExceptionToFinallyBlock]: + 4 mark({ for (i in 1..2) { } return bar() }) PREV:[jmp?(L2)] + 5 r(1) -> + r(2) -> + mark(1..2) + call(1..2, rangeTo|, ) -> + v(i) +L21 [copy of L5, loop entry point]: +L25 [copy of L9, condition entry point]: + jmp?(L22) NEXT:[read (Unit), magic[LOOP_RANGE_ITERATION](1..2|) -> ] PREV:[v(i), jmp(L21)] + magic[LOOP_RANGE_ITERATION](1..2|) -> + w(i|) + mark(for (i in 1..2) { }) +L23 [copy of L7, body entry point]: + 6 mark({ }) + read (Unit) + 5 jmp(L21) NEXT:[jmp?(L22)] +L22 [copy of L6, loop exit point]: +L24 [copy of L8, body exit point]: + read (Unit) PREV:[jmp?(L22)] + 4 mark(bar()) + call(bar(), bar) -> + ret(*|) L1 NEXT:[] +- 2 jmp(error) NEXT:[] PREV:[] +L20 [skipFinallyToErrorBlock]: + 4 mark({ for (i in 1..2) { } return bar() }) PREV:[jmp(L20)] + 5 r(1) -> + r(2) -> + mark(1..2) + call(1..2, rangeTo|, ) -> + v(i) +L26 [copy of L5, loop entry point]: +L30 [copy of L9, condition entry point]: + jmp?(L27) NEXT:[read (Unit), magic[LOOP_RANGE_ITERATION](1..2|) -> ] PREV:[v(i), jmp(L26)] + magic[LOOP_RANGE_ITERATION](1..2|) -> + w(i|) + mark(for (i in 1..2) { }) +L28 [copy of L7, body entry point]: + 6 mark({ }) + read (Unit) + 5 jmp(L26) NEXT:[jmp?(L27)] +L27 [copy of L6, loop exit point]: +L29 [copy of L8, body exit point]: + read (Unit) PREV:[jmp?(L27)] + 4 mark(bar()) + call(bar(), bar) -> + ret(*|) L1 NEXT:[] +- 2 merge(try { if (n < 0) return 0 n.let { return it } } finally { for (i in 1..2) { } return bar() }|) -> PREV:[] +L1: + 1 NEXT:[] PREV:[ret(*|) L1, ret(*|) L1, ret(*|) L1] +error: + PREV:[] +sink: + PREV:[, , d({ return it })] +===================== +== anonymous_0 == +{ return it } +--------------------- +L13: + 4 + 5 mark(return it) + r(it) -> + 4 mark({ for (i in 1..2) { } return bar() }) + 5 r(1) -> + r(2) -> + mark(1..2) + call(1..2, rangeTo|, ) -> + v(i) +L15 [copy of L5, loop entry point]: +L19 [copy of L9, condition entry point]: + jmp?(L16) NEXT:[r(1) -> , magic[LOOP_RANGE_ITERATION](1..2|) -> ] + magic[LOOP_RANGE_ITERATION](1..2|) -> + w(i|) + mark(for (i in 1..2) { }) +L17 [copy of L7, body entry point]: + 6 mark({ }) + read (Unit) + 5 jmp(L15) NEXT:[r(n) -> ] +L16 [copy of L6, loop exit point]: +L18 [copy of L8, body exit point]: +- read (Unit) PREV:[] +- 4 mark(bar()) PREV:[] +- call(bar(), bar) -> PREV:[] +- ret(*|) L1 NEXT:[] PREV:[] +- 5 ret(*|) L1 NEXT:[] PREV:[] +- 4 ret(*|!) L14 PREV:[] +L14: + NEXT:[] PREV:[] +error: + PREV:[] +sink: + PREV:[, ] +===================== diff --git a/compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.kt b/compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.kt new file mode 100644 index 00000000000..cc9c3177624 --- /dev/null +++ b/compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.kt @@ -0,0 +1,14 @@ +fun T.let(f: (T) -> U): U = f(this) + +fun bar(): Int = 1 + +fun foo(n: Int): Int { + try { + if (n < 0) return 0 + n.let { return it } + } + finally { + for (i in 1..2) { } + return bar() + } +} \ No newline at end of file diff --git a/compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.values b/compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.values new file mode 100644 index 00000000000..8419322f996 --- /dev/null +++ b/compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.values @@ -0,0 +1,56 @@ +== let == +fun T.let(f: (T) -> U): U = f(this) +--------------------- + : {<: (T) -> U} NEW: magic[FAKE_INITIALIZER](f: (T) -> U) -> +f : {<: (T) -> U} NEW: r(f) -> +this : {<: T} COPY +this : {<: T} NEW: r(this) -> +f(this) : {<: U} NEW: call(f(this), invoke|, ) -> +===================== +== bar == +fun bar(): Int = 1 +--------------------- +1 : Int NEW: r(1) -> +===================== +== foo == +fun foo(n: Int): Int { + try { + if (n < 0) return 0 + n.let { return it } + } + finally { + for (i in 1..2) { } + return bar() + } +} +--------------------- + : Int NEW: magic[FAKE_INITIALIZER](n: Int) -> + : Int NEW: magic[LOOP_RANGE_ITERATION](1..2|) -> +n : {<: Comparable} NEW: r(n) -> +0 : Int NEW: r(0) -> +n < 0 : Boolean NEW: call(n < 0, compareTo|, ) -> +0 : Int NEW: r(0) -> +return 0 !: * +if (n < 0) return 0 : * NEW: merge(if (n < 0) return 0|!) -> +n : Int NEW: r(n) -> +{ return it } : {<: (Int) -> ???} NEW: r({ return it }) -> +let { return it } : * NEW: call(let { return it }, let|, ) -> +n.let { return it } : * COPY +{ if (n < 0) return 0 n.let { return it } } : * COPY +1 : Int NEW: r(1) -> +2 : Int NEW: r(2) -> +1..2 : {<: Iterable} NEW: call(1..2, rangeTo|, ) -> +bar() : Int NEW: call(bar(), bar) -> +return bar() !: * +{ for (i in 1..2) { } return bar() } !: * COPY +try { if (n < 0) return 0 n.let { return it } } finally { for (i in 1..2) { } return bar() } : * NEW: merge(try { if (n < 0) return 0 n.let { return it } } finally { for (i in 1..2) { } return bar() }|) -> +{ try { if (n < 0) return 0 n.let { return it } } finally { for (i in 1..2) { } return bar() } } : * COPY +===================== +== anonymous_0 == +{ return it } +--------------------- + : Int NEW: magic[LOOP_RANGE_ITERATION](1..2|) -> +it : Int NEW: r(it) -> +return it !: * +return it !: * COPY +===================== diff --git a/compiler/tests/org/jetbrains/kotlin/cfg/ControlFlowTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/cfg/ControlFlowTestGenerated.java index 8107c01dce2..f7d682edb22 100644 --- a/compiler/tests/org/jetbrains/kotlin/cfg/ControlFlowTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/cfg/ControlFlowTestGenerated.java @@ -204,6 +204,12 @@ public class ControlFlowTestGenerated extends AbstractControlFlowTest { doTest(fileName); } + @TestMetadata("localAndNonlocalReturnsWithFinally.kt") + public void testLocalAndNonlocalReturnsWithFinally() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.kt"); + doTest(fileName); + } + @TestMetadata("OnlyWhileInFunctionBody.kt") public void testOnlyWhileInFunctionBody() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/cfg/controlStructures/OnlyWhileInFunctionBody.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java index 889e304aa41..fda6ae9a420 100644 --- a/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java @@ -210,6 +210,12 @@ public class PseudoValueTestGenerated extends AbstractPseudoValueTest { doTest(fileName); } + @TestMetadata("localAndNonlocalReturnsWithFinally.kt") + public void testLocalAndNonlocalReturnsWithFinally() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/cfg/controlStructures/localAndNonlocalReturnsWithFinally.kt"); + doTest(fileName); + } + @TestMetadata("OnlyWhileInFunctionBody.kt") public void testOnlyWhileInFunctionBody() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/cfg/controlStructures/OnlyWhileInFunctionBody.kt");