Fix various invalid local information issues in the new CF frontend.
The new CF frontend deviates from the old frontend by *not* type segmenting the
local variable information. The will lead the new frontend to (correctly)
identify more inputs as having type inconsistent local variable
information. With this CL D8 will remove the local variable table from such
methods. If this turns out to be problematic, we can consider reintroducing the
type segmentation or selectively removing only the local variable entries
causing the type inconsistency.
In addition, this CL fixes an issue where the new frontend failed to create
uninitialized values on method entry.
Bug: 136544304
Change-Id: I89ee6d486807c26ebb97a8c0a2dac7b1d1d20174
diff --git a/src/main/java/com/android/tools/r8/ir/code/Phi.java b/src/main/java/com/android/tools/r8/ir/code/Phi.java
index 863da0a..99383d1 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Phi.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Phi.java
@@ -28,7 +28,6 @@
public enum RegisterReadType {
NORMAL,
DEBUG,
- NORMAL_AND_DEBUG
}
private final BasicBlock block;
@@ -90,38 +89,14 @@
ValueTypeConstraint constraint =
TypeConstraintResolver.constraintForType(operand.getTypeLattice());
if (constrainedType(constraint) == null) {
- // If the phi has been requested from instructions and from local info, throw out locals
- // and retry compilation.
- if (readType == RegisterReadType.NORMAL_AND_DEBUG) {
- throw new InvalidDebugInfoException(
- "Type information in locals-table is inconsistent with instructions."
- + " Value of type "
- + operand.getTypeLattice()
- + " cannot be used as local of type "
- + typeLattice
- + ".");
- }
- // Otherwise only local info requested the phi and we replace with an uninitialized value.
- assert readType == RegisterReadType.DEBUG;
- BasicBlock block = getBlock();
- InstructionListIterator it = block.listIterator();
- assert readConstraint.isPrecise();
- TypeLatticeElement type =
- readConstraint.isObject()
- ? TypeLatticeElement.NULL
- : readConstraint.toPrimitiveTypeLattice();
- Value value = new Value(builder.getValueNumberGenerator().next(), type, null);
- Position position = block.getPosition();
- Instruction definition = new DebugLocalUninitialized(value);
- definition.setBlock(block);
- definition.setPosition(position);
- it.add(definition);
- // Update current definition and all users.
- block.updateCurrentDefinition(register, value, EdgeType.NON_EDGE);
- replaceUsers(value);
- // Remove the phi from the block.
- block.removePhi(this);
- return;
+ // If the phi has been requested from local info, throw out locals and retry compilation.
+ throw new InvalidDebugInfoException(
+ "Type information in locals-table is inconsistent."
+ + " Value of type "
+ + operand.getTypeLattice()
+ + " cannot be used as local of type "
+ + typeLattice
+ + ".");
}
}
}
@@ -155,9 +130,7 @@
@Override
public void markNonDebugLocalRead() {
- if (readType == RegisterReadType.DEBUG) {
- readType = RegisterReadType.NORMAL_AND_DEBUG;
- }
+ readType = RegisterReadType.NORMAL;
}
private void throwUndefinedValueError() {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
index fc359dd..674bf79 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
@@ -384,6 +384,25 @@
state.buildPrelude(canonicalPositions.getPreamblePosition());
setLocalVariableLists();
DexSourceCode.buildArgumentsWithUnusedArgumentStubs(builder, 0, method, state::write);
+ // Add debug information for all locals at the initial label.
+ Int2ObjectMap<DebugLocalInfo> locals = getLocalVariables(0).locals;
+ if (!locals.isEmpty()) {
+ int firstLocalIndex = 0;
+ if (!method.isStatic()) {
+ firstLocalIndex++;
+ }
+ for (DexType value : method.method.proto.parameters.values) {
+ firstLocalIndex++;
+ if (value.isLongType() || value.isDoubleType()) {
+ firstLocalIndex++;
+ }
+ }
+ for (Entry<DebugLocalInfo> entry : locals.int2ObjectEntrySet()) {
+ if (firstLocalIndex <= entry.getIntKey()) {
+ builder.addDebugLocalStart(entry.getIntKey(), entry.getValue());
+ }
+ }
+ }
if (needsGeneratedMethodSynchronization) {
buildMethodEnterSynchronization(builder);
}
diff --git a/src/test/java/com/android/tools/r8/TestDiagnosticMessagesImpl.java b/src/test/java/com/android/tools/r8/TestDiagnosticMessagesImpl.java
index aea4b4a..16800fc 100644
--- a/src/test/java/com/android/tools/r8/TestDiagnosticMessagesImpl.java
+++ b/src/test/java/com/android/tools/r8/TestDiagnosticMessagesImpl.java
@@ -19,6 +19,24 @@
private final List<Diagnostic> errors = new ArrayList<>();
@Override
+ public String toString() {
+ StringBuilder builder = new StringBuilder();
+ builder.append("Infos: ").append('\n');
+ for (Diagnostic info : infos) {
+ builder.append(" - ").append(info.getDiagnosticMessage()).append('\n');
+ }
+ builder.append("Warnings: ").append('\n');
+ for (Diagnostic warning : warnings) {
+ builder.append(" - ").append(warning.getDiagnosticMessage()).append('\n');
+ }
+ builder.append("Errors: ").append('\n');
+ for (Diagnostic error : errors) {
+ builder.append(" - ").append(error.getDiagnosticMessage()).append('\n');
+ }
+ return builder.toString();
+ }
+
+ @Override
public void info(Diagnostic info) {
infos.add(info);
}
diff --git a/src/test/java/com/android/tools/r8/debug/UninitializedInitialLocalsTest.java b/src/test/java/com/android/tools/r8/debug/UninitializedInitialLocalsTest.java
new file mode 100644
index 0000000..15112b4
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/debug/UninitializedInitialLocalsTest.java
@@ -0,0 +1,87 @@
+// Copyright (c) 2019, 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.debug;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+
+@RunWith(Parameterized.class)
+public class UninitializedInitialLocalsTest extends TestBase implements Opcodes {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().withAllApiLevels().build();
+ }
+
+ public UninitializedInitialLocalsTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ if (parameters.isCfRuntime()) {
+ testForJvm()
+ .addProgramClassFileData(dump())
+ .run(parameters.getRuntime(), "Test")
+ .assertSuccessWithOutputLines("42");
+ } else {
+ testForD8()
+ .addProgramClassFileData(dump())
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), "Test")
+ .assertSuccessWithOutputLines("42");
+ }
+ }
+
+ public static byte[] dump() {
+ ClassWriter cw = new ClassWriter(0);
+ cw.visit(V1_6, ACC_PUBLIC | ACC_SUPER, "Test", null, "java/lang/Object", null);
+ cw.visitSource("Test.java", null);
+ Label label0 = new Label();
+ Label label1 = new Label();
+ Label label2 = new Label();
+ MethodVisitor mv =
+ cw.visitMethod(ACC_PUBLIC | ACC_STATIC, "main", "([Ljava/lang/String;)V", null, null);
+ mv.visitCode();
+ mv.visitLabel(label0);
+ mv.visitLdcInsn(42);
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitInsn(ARRAYLENGTH);
+ mv.visitJumpInsn(IFEQ, label1);
+ mv.visitInsn(DUP);
+ mv.visitVarInsn(ISTORE, 1);
+ mv.visitLabel(label1); // Here index 1 will be a phi of an undefined and real value.
+ printInt(mv);
+ println(mv);
+ mv.visitInsn(RETURN);
+ mv.visitLabel(label2);
+ mv.visitMaxs(10, 20);
+ mv.visitLocalVariable("args", "[Ljava/lang/String;", null, label0, label2, 0);
+ mv.visitLocalVariable("invalid", "I", null, label0, label2, 1);
+ mv.visitEnd();
+ cw.visitEnd();
+ return cw.toByteArray();
+ }
+
+ private static void printInt(MethodVisitor mv) {
+ mv.visitFieldInsn(GETSTATIC, "java/lang/System", "out", "Ljava/io/PrintStream;");
+ mv.visitInsn(SWAP);
+ mv.visitMethodInsn(INVOKEVIRTUAL, "java/io/PrintStream", "print", "(I)V", false);
+ }
+
+ private static void println(MethodVisitor mv) {
+ mv.visitFieldInsn(GETSTATIC, "java/lang/System", "out", "Ljava/io/PrintStream;");
+ mv.visitMethodInsn(INVOKEVIRTUAL, "java/io/PrintStream", "println", "()V", false);
+ }
+}
