From c0e1b3ef2d5db33992f2916a0001d1851ab4885d Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Wed, 12 Jul 2017 13:15:38 +0300 Subject: [PATCH] CFG: fix label binding for last instruction of finally block So #KT-18698 Fixed --- .../kotlin/cfg/pseudocode/PseudocodeImpl.kt | 2 +- .../incorrectIndex.instructions | 151 ++++++++++++++++++ .../cfg/controlStructures/incorrectIndex.kt | 22 +++ .../controlStructures/incorrectIndex.values | 68 ++++++++ .../kotlin/cfg/ControlFlowTestGenerated.java | 6 + .../kotlin/cfg/PseudoValueTestGenerated.java | 6 + 6 files changed, 254 insertions(+), 1 deletion(-) create mode 100644 compiler/testData/cfg/controlStructures/incorrectIndex.instructions create mode 100644 compiler/testData/cfg/controlStructures/incorrectIndex.kt create mode 100644 compiler/testData/cfg/controlStructures/incorrectIndex.values diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/pseudocode/PseudocodeImpl.kt b/compiler/frontend/src/org/jetbrains/kotlin/cfg/pseudocode/PseudocodeImpl.kt index 3b7b5297aeb..f3b53f2fde1 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/pseudocode/PseudocodeImpl.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/cfg/pseudocode/PseudocodeImpl.kt @@ -395,7 +395,7 @@ class PseudocodeImpl(override val correspondingElement: KtElement) : Pseudocode internalSinkInstruction = copy } } - if (finishIndex < mutableInstructionList.size) { + if (finishIndex < originalPseudocode.mutableInstructionList.size) { repeatLabelsBindingForInstruction(originalPseudocode.mutableInstructionList[finishIndex], originalToCopy, originalLabelsForInstruction) diff --git a/compiler/testData/cfg/controlStructures/incorrectIndex.instructions b/compiler/testData/cfg/controlStructures/incorrectIndex.instructions new file mode 100644 index 00000000000..6dc30cc30c5 --- /dev/null +++ b/compiler/testData/cfg/controlStructures/incorrectIndex.instructions @@ -0,0 +1,151 @@ +== s == +val s = mutableListOf() +--------------------- +L0: + 1 + v(val s = mutableListOf()) + mark(mutableListOf()) + magic[UNRESOLVED_CALL](mutableListOf()|!) -> + w(s|) +L1: + NEXT:[] +error: + PREV:[] +sink: + PREV:[, ] +===================== +== test == +fun test(name: String?, flag: Boolean): Boolean { + try { + name?.let { + if (flag) { + s.add(it) + } + else { + s.remove(it) + } + + return true + } + + return false + } finally { + name?.hashCode() + } +} +--------------------- +L0: + 1 + v(name: String?) + magic[FAKE_INITIALIZER](name: String?) -> + w(name|) + v(flag: Boolean) + magic[FAKE_INITIALIZER](flag: Boolean) -> + w(flag|) + 2 mark({ try { name?.let { if (flag) { s.add(it) } else { s.remove(it) } return true } return false } finally { name?.hashCode() } }) + mark(try { name?.let { if (flag) { s.add(it) } else { s.remove(it) } return true } return false } finally { name?.hashCode() }) + jmp?(L2) NEXT:[mark({ name?.hashCode() }), mark({ name?.let { if (flag) { s.add(it) } else { s.remove(it) } return true } return false })] + 3 mark({ name?.let { if (flag) { s.add(it) } else { s.remove(it) } return true } return false }) + mark(name?.let { if (flag) { s.add(it) } else { s.remove(it) } return true }) + jf(L3) NEXT:[jmp?(L2), r(name) -> ] + r(name) -> + mark({ if (flag) { s.add(it) } else { s.remove(it) } return true }) + jmp?(L4) NEXT:[r({ if (flag) { s.add(it) } else { s.remove(it) } return true }) -> , d({ if (flag) { s.add(it) } else { s.remove(it) } return true })] + d({ if (flag) { s.add(it) } else { s.remove(it) } return true }) NEXT:[] +L4 [after local declaration]: + r({ if (flag) { s.add(it) } else { s.remove(it) } return true }) -> PREV:[jmp?(L4)] + mark(let { if (flag) { s.add(it) } else { s.remove(it) } return true }) + call(let { if (flag) { s.add(it) } else { s.remove(it) } return true }, let|, ) +L3 [result of call]: + jmp?(L2) NEXT:[mark({ name?.hashCode() }), r(false) -> ] PREV:[jf(L3), call(let { if (flag) { s.add(it) } else { s.remove(it) } return true }, let|, )] + r(false) -> + 6 mark({ name?.hashCode() }) + mark(name?.hashCode()) + jf(L12) NEXT:[ret(*|) L1, r(name) -> ] + r(name) -> + mark(hashCode()) + call(hashCode(), hashCode|) -> +L12 [copy of L10, result of call]: + 3 ret(*|) L1 NEXT:[] PREV:[jf(L12), call(hashCode(), hashCode|) -> ] +- 2 jmp?(L2) NEXT:[mark({ name?.hashCode() }), jmp(L13)] PREV:[] +- jmp(L13) NEXT:[mark({ name?.hashCode() })] PREV:[] +L2 [onExceptionToFinallyBlock]: + 6 mark({ name?.hashCode() }) PREV:[jmp?(L2), jmp?(L2)] + mark(name?.hashCode()) + jf(L14) NEXT:[jmp(error), r(name) -> ] + r(name) -> + mark(hashCode()) + call(hashCode(), hashCode|) -> +L14 [copy of L10, result of call]: + 2 jmp(error) NEXT:[] PREV:[jf(L14), call(hashCode(), hashCode|) -> ] +L13 [skipFinallyToErrorBlock]: +- 6 mark({ name?.hashCode() }) PREV:[] +- mark(name?.hashCode()) PREV:[] +- jf(L15) NEXT:[merge(try { name?.let { if (flag) { s.add(it) } else { s.remove(it) } return true } return false } finally { name?.hashCode() }|!) -> , r(name) -> ] PREV:[] +- r(name) -> PREV:[] +- mark(hashCode()) PREV:[] +- call(hashCode(), hashCode|) -> PREV:[] +L15 [copy of L10, result of call]: +- 2 merge(try { name?.let { if (flag) { s.add(it) } else { s.remove(it) } return true } return false } finally { name?.hashCode() }|!) -> PREV:[] +L1: + 1 NEXT:[] PREV:[ret(*|) L1, ret(*|) L1] +error: + PREV:[jmp(error)] +sink: + PREV:[, , d({ if (flag) { s.add(it) } else { s.remove(it) } return true })] +===================== +== anonymous_0 == +{ + if (flag) { + s.add(it) + } + else { + s.remove(it) + } + + return true + } +--------------------- +L5: + 4 + 5 mark(if (flag) { s.add(it) } else { s.remove(it) } return true) + mark(if (flag) { s.add(it) } else { s.remove(it) }) + r(flag) -> + jf(L7|) NEXT:[mark({ s.remove(it) }), mark({ s.add(it) })] + 6 mark({ s.add(it) }) + mark(s.add(it)) + r(s) -> + r(it) -> + magic[VALUE_CONSUMER](it|) -> + mark(add(it)) + call(add(it), |) -> + 5 jmp(L8) NEXT:[merge(if (flag) { s.add(it) } else { s.remove(it) }|, ) -> ] +L7 [else branch]: + 6 mark({ s.remove(it) }) PREV:[jf(L7|)] + mark(s.remove(it)) + r(s) -> + r(it) -> + magic[VALUE_CONSUMER](it|) -> + mark(remove(it)) + call(remove(it), |) -> +L8 ['if' expression result]: + 5 merge(if (flag) { s.add(it) } else { s.remove(it) }|, ) -> PREV:[jmp(L8), call(remove(it), |) -> ] + r(true) -> +L9 [start finally]: + 6 mark({ name?.hashCode() }) + mark(name?.hashCode()) + jf(L10) NEXT:[ret(*|) L1, r(name) -> ] + r(name) -> + mark(hashCode()) + call(hashCode(), hashCode|) -> +L10 [result of call]: +L11 [finish finally]: + 5 ret(*|) L1 NEXT:[] PREV:[jf(L10), call(hashCode(), hashCode|) -> ] +- 4 ret(*|!) L6 PREV:[] +L6: + NEXT:[] PREV:[] +error: + PREV:[] +sink: + PREV:[, ] +===================== diff --git a/compiler/testData/cfg/controlStructures/incorrectIndex.kt b/compiler/testData/cfg/controlStructures/incorrectIndex.kt new file mode 100644 index 00000000000..6502ae81a5b --- /dev/null +++ b/compiler/testData/cfg/controlStructures/incorrectIndex.kt @@ -0,0 +1,22 @@ +// See KT-18698 + +val s = mutableListOf() + +fun test(name: String?, flag: Boolean): Boolean { + try { + name?.let { + if (flag) { + s.add(it) + } + else { + s.remove(it) + } + + return true + } + + return false + } finally { + name?.hashCode() + } +} \ No newline at end of file diff --git a/compiler/testData/cfg/controlStructures/incorrectIndex.values b/compiler/testData/cfg/controlStructures/incorrectIndex.values new file mode 100644 index 00000000000..2f7eb655633 --- /dev/null +++ b/compiler/testData/cfg/controlStructures/incorrectIndex.values @@ -0,0 +1,68 @@ +== s == +val s = mutableListOf() +--------------------- +mutableListOf !: * +mutableListOf() : {<: [ERROR : Type for mutableListOf()]} NEW: magic[UNRESOLVED_CALL](mutableListOf()|!) -> +===================== +== test == +fun test(name: String?, flag: Boolean): Boolean { + try { + name?.let { + if (flag) { + s.add(it) + } + else { + s.remove(it) + } + + return true + } + + return false + } finally { + name?.hashCode() + } +} +--------------------- + : {<: String?} NEW: magic[FAKE_INITIALIZER](name: String?) -> + : Boolean NEW: magic[FAKE_INITIALIZER](flag: Boolean) -> +name : String NEW: r(name) -> +{ if (flag) { s.add(it) } else { s.remove(it) } return true } : {<: (String) -> Nothing} NEW: r({ if (flag) { s.add(it) } else { s.remove(it) } return true }) -> +let { if (flag) { s.add(it) } else { s.remove(it) } return true } !: * +name?.let { if (flag) { s.add(it) } else { s.remove(it) } return true } !: * COPY +false : Boolean NEW: r(false) -> +return false !: * +{ name?.let { if (flag) { s.add(it) } else { s.remove(it) } return true } return false } !: * COPY +try { name?.let { if (flag) { s.add(it) } else { s.remove(it) } return true } return false } finally { name?.hashCode() } : * NEW: merge(try { name?.let { if (flag) { s.add(it) } else { s.remove(it) } return true } return false } finally { name?.hashCode() }|!) -> +{ try { name?.let { if (flag) { s.add(it) } else { s.remove(it) } return true } return false } finally { name?.hashCode() } } : * COPY +===================== +== anonymous_0 == +{ + if (flag) { + s.add(it) + } + else { + s.remove(it) + } + + return true + } +--------------------- + : * NEW: magic[VALUE_CONSUMER](it|) -> + : * NEW: magic[VALUE_CONSUMER](it|) -> +flag : Boolean NEW: r(flag) -> +s : * NEW: r(s) -> +it : * NEW: r(it) -> +add(it) : * NEW: call(add(it), |) -> +s.add(it) : * COPY +{ s.add(it) } : * COPY +s : * NEW: r(s) -> +it : * NEW: r(it) -> +remove(it) : * NEW: call(remove(it), |) -> +s.remove(it) : * COPY +{ s.remove(it) } : * COPY +if (flag) { s.add(it) } else { s.remove(it) } : * NEW: merge(if (flag) { s.add(it) } else { s.remove(it) }|, ) -> +true : Boolean NEW: r(true) -> +return true !: * +if (flag) { s.add(it) } else { s.remove(it) } return true !: * COPY +===================== diff --git a/compiler/tests/org/jetbrains/kotlin/cfg/ControlFlowTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/cfg/ControlFlowTestGenerated.java index 45563641586..6edf498e592 100644 --- a/compiler/tests/org/jetbrains/kotlin/cfg/ControlFlowTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/cfg/ControlFlowTestGenerated.java @@ -233,6 +233,12 @@ public class ControlFlowTestGenerated extends AbstractControlFlowTest { doTest(fileName); } + @TestMetadata("incorrectIndex.kt") + public void testIncorrectIndex() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/cfg/controlStructures/incorrectIndex.kt"); + doTest(fileName); + } + @TestMetadata("InfiniteLoops.kt") public void testInfiniteLoops() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/cfg/controlStructures/InfiniteLoops.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java index 950f1333f66..30c02f1d0a8 100644 --- a/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java @@ -235,6 +235,12 @@ public class PseudoValueTestGenerated extends AbstractPseudoValueTest { doTest(fileName); } + @TestMetadata("incorrectIndex.kt") + public void testIncorrectIndex() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/cfg/controlStructures/incorrectIndex.kt"); + doTest(fileName); + } + @TestMetadata("InfiniteLoops.kt") public void testInfiniteLoops() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/cfg/controlStructures/InfiniteLoops.kt");