CF backend bugfix for duplicate frames
Certain code patterns, e.g. empty catch clauses, generate empty blocks which
require explicit stack frames.
Ensure that only the last frame in a sequence of consecutive frames is output
to asm.
Change-Id: Ic340528c7bdf2eed5c61625fb6343e9343f8892e
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
index d709f49..ff37507 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
@@ -196,10 +196,11 @@
labels = new HashMap<>(code.blocks.size());
emittedLabels = new HashSet<>(code.blocks.size());
instructions = new ArrayList<>();
- Iterator<BasicBlock> blockIterator = code.listIterator();
+ ListIterator<BasicBlock> blockIterator = code.listIterator();
BasicBlock block = blockIterator.next();
CfLabel tryCatchStart = null;
CatchHandlers<BasicBlock> tryCatchHandlers = CatchHandlers.EMPTY_BASIC_BLOCK;
+ BasicBlock pendingFrame = null;
do {
CatchHandlers<BasicBlock> handlers = block.getCatchHandlers();
if (!tryCatchHandlers.equals(handlers)) {
@@ -229,10 +230,26 @@
pendingLocalChanges = true;
}
buildCfInstructions(block, fallthrough, stack);
- if (nextBlock != null && (!fallthrough || nextBlock.getPredecessors().size() > 1)) {
- assert stack.isEmpty();
- emitLabel(getLabel(nextBlock));
- addFrame(nextBlock, Collections.emptyList());
+ if (nextBlock != null) {
+ if (!fallthrough || nextBlock.getPredecessors().size() > 1) {
+ assert stack.isEmpty();
+ pendingFrame = nextBlock;
+ emitLabel(getLabel(nextBlock));
+ }
+ if (pendingFrame != null) {
+ BasicBlock nextNextBlock = null;
+ if (blockIterator.hasNext()) {
+ nextNextBlock = blockIterator.next();
+ blockIterator.previous();
+ }
+ boolean advancesPC = hasMaterializingInstructions(nextBlock, nextNextBlock);
+ // If nextBlock has no materializing instructions, then nextNextBlock must be non-null
+ // (or we would fall off the edge of the method).
+ assert advancesPC || nextNextBlock != null;
+ if (advancesPC) {
+ addFrame(pendingFrame, Collections.emptyList());
+ }
+ }
}
block = nextBlock;
} while (block != null);
@@ -251,6 +268,25 @@
localVariablesTable);
}
+ private static boolean isNopInstruction(Instruction instruction, BasicBlock nextBlock) {
+ // From DexBuilder
+ return instruction.isArgument()
+ || instruction.isDebugLocalsChange()
+ || (instruction.isGoto() && instruction.asGoto().getTarget() == nextBlock);
+ }
+
+ private boolean hasMaterializingInstructions(BasicBlock block, BasicBlock nextBlock) {
+ if (block == null) {
+ return false;
+ }
+ for (Instruction instruction : block.getInstructions()) {
+ if (!isNopInstruction(instruction, nextBlock)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
private void buildCfInstructions(BasicBlock block, boolean fallthrough, Stack stack) {
InstructionIterator it = block.iterator();
while (it.hasNext()) {
diff --git a/src/test/java/com/android/tools/r8/cf/NestedExceptionTest.java b/src/test/java/com/android/tools/r8/cf/NestedExceptionTest.java
new file mode 100644
index 0000000..f7823a9
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/cf/NestedExceptionTest.java
@@ -0,0 +1,19 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.cf;
+
+public class NestedExceptionTest {
+
+ public static void main(String[] args) {
+ try {
+ new Integer(1);
+ } catch (Exception t) {
+ try {
+ new Integer(1);
+ } catch (Exception tInner) {
+ // This block must be empty
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/cf/NestedExceptionTestRunner.java b/src/test/java/com/android/tools/r8/cf/NestedExceptionTestRunner.java
new file mode 100644
index 0000000..4c7effd
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/cf/NestedExceptionTestRunner.java
@@ -0,0 +1,36 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.cf;
+
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.OutputMode;
+import com.android.tools.r8.R8;
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.AndroidAppConsumers;
+import java.nio.file.Path;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class NestedExceptionTestRunner {
+ static final Class CLASS = NestedExceptionTest.class;
+ @Rule public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
+
+ @Test
+ public void testNestedException() throws Exception {
+ AndroidAppConsumers a = new AndroidAppConsumers();
+ R8.run(
+ R8Command.builder()
+ .setMode(CompilationMode.DEBUG)
+ .addClassProgramData(ToolHelper.getClassAsBytes(CLASS), Origin.unknown())
+ .addLibraryFiles(ToolHelper.getAndroidJar(ToolHelper.getMinApiLevelForDexVm()))
+ .setProgramConsumer(a.wrapClassFileConsumer(null))
+ .build());
+ Path out = temp.newFolder().toPath();
+ a.build().writeToDirectory(out, OutputMode.ClassFile);
+ ToolHelper.runJava(out, CLASS.getCanonicalName());
+ }
+}
