From 8f68c64193831ea2d56057a64a0545c81a79204d Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Mon, 18 May 2020 17:24:21 +0300 Subject: [PATCH] Fix bug in repeated field read elimination. There's a case that was missing in this optimization. We install invalidation points on a block's dominance frontiers when the block contains som invalidation instructions. However, if a block is an entry to exception handler, state is always invalidated. This should be done since exception handler may recover and proceed with some code that follows try/catch block. Without this change code after try/catch inherits state of `try` block, which is invalid, since `catch` is another source from where we can get there. We can't rely on regular instruction analysis in `catch` blocks, since we get into `catch` from an unpredictable point. --- .../RepeatedFieldReadElimination.java | 85 +++++++++++++++---- .../RepeatedFieldReadEliminationTest.java | 10 +++ .../readAfterException.expected.txt | 13 +++ .../readAfterException.original.txt | 13 +++ 4 files changed, 105 insertions(+), 16 deletions(-) create mode 100644 core/src/test/resources/model/optimization/repeated-field-read-elimination/readAfterException.expected.txt create mode 100644 core/src/test/resources/model/optimization/repeated-field-read-elimination/readAfterException.original.txt diff --git a/core/src/main/java/org/teavm/model/optimization/RepeatedFieldReadElimination.java b/core/src/main/java/org/teavm/model/optimization/RepeatedFieldReadElimination.java index e86c8bb5f..f84e2e9e4 100644 --- a/core/src/main/java/org/teavm/model/optimization/RepeatedFieldReadElimination.java +++ b/core/src/main/java/org/teavm/model/optimization/RepeatedFieldReadElimination.java @@ -28,7 +28,9 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Deque; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import org.teavm.common.DominatorTree; import org.teavm.common.Graph; @@ -40,6 +42,7 @@ import org.teavm.model.FieldReader; import org.teavm.model.FieldReference; import org.teavm.model.Instruction; import org.teavm.model.Program; +import org.teavm.model.TryCatchBlock; import org.teavm.model.analysis.AliasAnalysis; import org.teavm.model.instructions.AbstractInstructionVisitor; import org.teavm.model.instructions.AssignInstruction; @@ -316,6 +319,12 @@ public class RepeatedFieldReadElimination implements MethodOptimization { int[][] domFrontiers = GraphUtils.findDominanceFrontiers(cfg, dom); everythingInvalid = new boolean[program.basicBlockCount()]; invalidFields.addAll(Collections.nCopies(program.basicBlockCount(), null)); + boolean[] exceptionHandlers = new boolean[program.basicBlockCount()]; + for (BasicBlock block : program.getBasicBlocks()) { + for (TryCatchBlock tryCatchBlock : block.getTryCatchBlocks()) { + exceptionHandlers[tryCatchBlock.getHandler().getIndex()] = true; + } + } IntArrayDeque worklist = new IntArrayDeque(); InstructionAnalyzer instructionAnalyzer = new InstructionAnalyzer(); @@ -325,26 +334,43 @@ public class RepeatedFieldReadElimination implements MethodOptimization { continue; } - for (Instruction instruction : block) { - instructionAnalyzer.reset(); - instruction.acceptVisitor(instructionAnalyzer); + Set fieldsToInvalidate = new LinkedHashSet<>(); + boolean allInvalid = false; + if (exceptionHandlers[block.getIndex()]) { + allInvalid = true; + } else { + for (Instruction instruction : block) { + instructionAnalyzer.reset(); + instruction.acceptVisitor(instructionAnalyzer); - if (instructionAnalyzer.invalidatesAll) { - worklist.addLast(frontiers); - while (!worklist.isEmpty()) { - int target = worklist.removeFirst(); - if (!everythingInvalid[target]) { - everythingInvalid[target] = true; - invalidFields.set(target, null); - worklist.addLast(domFrontiers[target]); - } + if (instructionAnalyzer.invalidatesAll) { + allInvalid = true; + fieldsToInvalidate.clear(); + break; + } else if (instructionAnalyzer.invalidatedField != null + && !isVolatile(instructionAnalyzer.invalidatedField)) { + fieldsToInvalidate.add(new FieldAndInstance(instructionAnalyzer.invalidatedField, + instructionAnalyzer.instance)); } - } else if (instructionAnalyzer.invalidatedField != null - && !isVolatile(instructionAnalyzer.invalidatedField)) { + } + } + + if (allInvalid) { + worklist.addLast(frontiers); + while (!worklist.isEmpty()) { + int target = worklist.removeFirst(); + if (!everythingInvalid[target]) { + everythingInvalid[target] = true; + invalidFields.set(target, null); + worklist.addLast(domFrontiers[target]); + } + } + } else { + for (FieldAndInstance fieldAndInstance : fieldsToInvalidate) { worklist.addLast(frontiers); - int instance = instructionAnalyzer.instance; - FieldReference field = instructionAnalyzer.invalidatedField; + int instance = fieldAndInstance.instance; + FieldReference field = fieldAndInstance.field; while (!worklist.isEmpty()) { int target = worklist.removeFirst(); @@ -401,4 +427,31 @@ public class RepeatedFieldReadElimination implements MethodOptimization { invalidatesAll = true; } } + + static class FieldAndInstance { + final FieldReference field; + final int instance; + + FieldAndInstance(FieldReference field, int instance) { + this.field = field; + this.instance = instance; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof FieldAndInstance)) { + return false; + } + FieldAndInstance that = (FieldAndInstance) o; + return instance == that.instance && field.equals(that.field); + } + + @Override + public int hashCode() { + return Objects.hash(field, instance); + } + } } diff --git a/core/src/test/java/org/teavm/model/optimization/test/RepeatedFieldReadEliminationTest.java b/core/src/test/java/org/teavm/model/optimization/test/RepeatedFieldReadEliminationTest.java index da5c62bda..09eaff1f0 100644 --- a/core/src/test/java/org/teavm/model/optimization/test/RepeatedFieldReadEliminationTest.java +++ b/core/src/test/java/org/teavm/model/optimization/test/RepeatedFieldReadEliminationTest.java @@ -86,6 +86,11 @@ public class RepeatedFieldReadEliminationTest { doTest(); } + @Test + public void readAfterException() { + doTest(); + } + private void doTest() { String originalPath = PREFIX + name.getMethodName() + ".original.txt"; String expectedPath = PREFIX + name.getMethodName() + ".expected.txt"; @@ -127,6 +132,11 @@ public class RepeatedFieldReadEliminationTest { getFoo.getModifiers().add(ElementModifier.NATIVE); foo.addMethod(getFoo); + MethodHolder getIntValue = new MethodHolder("getIntValue", ValueType.INTEGER); + getIntValue.getModifiers().add(ElementModifier.STATIC); + getIntValue.getModifiers().add(ElementModifier.NATIVE); + foo.addMethod(getIntValue); + classSource.putClassHolder(foo); MethodOptimizationContext context = new MethodOptimizationContext() { diff --git a/core/src/test/resources/model/optimization/repeated-field-read-elimination/readAfterException.expected.txt b/core/src/test/resources/model/optimization/repeated-field-read-elimination/readAfterException.expected.txt new file mode 100644 index 000000000..3c25aff8d --- /dev/null +++ b/core/src/test/resources/model/optimization/repeated-field-read-elimination/readAfterException.expected.txt @@ -0,0 +1,13 @@ +var @this as this + +$start + @o := invokeStatic `Foo.getFoo()LFoo;` + @v := invokeStatic `Foo.getIntValue()I` + field Foo.intField @o := @v as I + goto $exit + catch java.lang.RuntimeException goto $handler +$handler + goto $exit +$exit + @x := field Foo.intField @o as I + return \ No newline at end of file diff --git a/core/src/test/resources/model/optimization/repeated-field-read-elimination/readAfterException.original.txt b/core/src/test/resources/model/optimization/repeated-field-read-elimination/readAfterException.original.txt new file mode 100644 index 000000000..3c25aff8d --- /dev/null +++ b/core/src/test/resources/model/optimization/repeated-field-read-elimination/readAfterException.original.txt @@ -0,0 +1,13 @@ +var @this as this + +$start + @o := invokeStatic `Foo.getFoo()LFoo;` + @v := invokeStatic `Foo.getIntValue()I` + field Foo.intField @o := @v as I + goto $exit + catch java.lang.RuntimeException goto $handler +$handler + goto $exit +$exit + @x := field Foo.intField @o as I + return \ No newline at end of file