Strip invalid locals information.
Some yet-unknown version of javac produces invalid locals information by
associating the information with the wrong local indexes. This CL replaces
several asserts with checks that will restart the current compilation without
locals information if the locals information is inconsistent.
R=ager
Bug: 63412730
Change-Id: I296c7dee232f95bd943ff4bf38a0e69f4c190f95
diff --git a/src/main/java/com/android/tools/r8/errors/InvalidDebugInfoException.java b/src/main/java/com/android/tools/r8/errors/InvalidDebugInfoException.java
new file mode 100644
index 0000000..f3f99e2
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/errors/InvalidDebugInfoException.java
@@ -0,0 +1,10 @@
+// Copyright (c) 2017, 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.errors;
+
+public class InvalidDebugInfoException extends InternalCompilerError {
+ public InvalidDebugInfoException(String message) {
+ super(message);
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/graph/JarCode.java b/src/main/java/com/android/tools/r8/graph/JarCode.java
index bbf3d57..6b9c06d 100644
--- a/src/main/java/com/android/tools/r8/graph/JarCode.java
+++ b/src/main/java/com/android/tools/r8/graph/JarCode.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph;
+import com.android.tools.r8.errors.InvalidDebugInfoException;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.ValueNumberGenerator;
import com.android.tools.r8.ir.conversion.IRBuilder;
@@ -80,17 +81,34 @@
@Override
public IRCode buildIR(DexEncodedMethod encodedMethod, InternalOptions options) {
- return internalBuild(encodedMethod, null, options);
+ triggerDelayedParsingIfNeccessary();
+ return options.debug
+ ? internalBuildWithLocals(encodedMethod, null, options)
+ : internalBuild(encodedMethod, null, options);
}
public IRCode buildIR(
DexEncodedMethod encodedMethod, ValueNumberGenerator generator, InternalOptions options) {
- return internalBuild(encodedMethod, generator, options);
+ assert generator != null;
+ triggerDelayedParsingIfNeccessary();
+ return options.debug
+ ? internalBuildWithLocals(encodedMethod, generator, options)
+ : internalBuild(encodedMethod, generator, options);
+ }
+
+ private IRCode internalBuildWithLocals(
+ DexEncodedMethod encodedMethod, ValueNumberGenerator generator, InternalOptions options) {
+ try {
+ return internalBuild(encodedMethod, generator, options);
+ } catch (InvalidDebugInfoException e) {
+ options.warningInvalidDebugInfo(encodedMethod, e);
+ node.localVariables.clear();
+ return internalBuild(encodedMethod, generator, options);
+ }
}
private IRCode internalBuild(
DexEncodedMethod encodedMethod, ValueNumberGenerator generator, InternalOptions options) {
- triggerDelayedParsingIfNeccessary();
if (!options.debug) {
node.localVariables.clear();
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index 809e3ac..c25f476 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -7,6 +7,7 @@
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.InternalCompilerError;
+import com.android.tools.r8.errors.InvalidDebugInfoException;
import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.graph.DexCallSite;
import com.android.tools.r8.graph.DexEncodedMethod;
@@ -1433,7 +1434,11 @@
DebugLocalInfo local = getCurrentLocal(register);
Value value = readRegister(register, currentBlock, EdgeType.NON_EDGE, type, local);
// Check that any information about a current-local is consistent with the read.
- assert local == null || value.getLocalInfo() == local || value.isUninitializedLocal();
+ if (local != null && value.getLocalInfo() != local && !value.isUninitializedLocal()) {
+ throw new InvalidDebugInfoException(
+ "Attempt to read local " + local
+ + " but no local information was associated with the value being read.");
+ }
// Check that any local information on the value is actually visible.
// If this assert triggers, the probable cause is that we end up reading an SSA value
// after it should have been ended on a fallthrough from a conditional jump or a trivial-phi
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/JarState.java b/src/main/java/com/android/tools/r8/ir/conversion/JarState.java
index ee5252d..5009d5e 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/JarState.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/JarState.java
@@ -3,7 +3,9 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.conversion;
+import com.android.tools.r8.errors.InvalidDebugInfoException;
import com.android.tools.r8.graph.DebugLocalInfo;
+import com.android.tools.r8.utils.DescriptorUtils;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Multimap;
@@ -336,7 +338,10 @@
public int writeLocal(int index, Type type) {
assert nonNullType(type);
Local local = getLocal(index, type);
- assert local == null || local.info == null || local.slot.isCompatibleWith(type);
+ if (local != null && local.info != null && !local.slot.isCompatibleWith(type)) {
+ throw new InvalidDebugInfoException(
+ "Attempt to write value of type " + prettyType(type) + " to local " + local.info);
+ }
// We cannot assume consistency for writes because we do not have complete information about the
// scopes of locals. We assume the program to be verified and overwrite if the types mismatch.
if (local == null || (local.info == null && !typeEquals(local.slot.type, type))) {
@@ -353,6 +358,10 @@
public Slot readLocal(int index, Type type) {
Local local = getLocal(index, type);
assert local != null;
+ if (local.info != null && !local.slot.isCompatibleWith(type)) {
+ throw new InvalidDebugInfoException(
+ "Attempt to read value of type " + prettyType(type) + " from local " + local.info);
+ }
assert local.slot.isCompatibleWith(type);
return local.slot;
}
@@ -395,7 +404,12 @@
public Slot pop(Type type) {
Slot slot = pop();
- assert slot.isCompatibleWith(type);
+ boolean compatible = slot.isCompatibleWith(type);
+ if (!compatible && !localVariables.isEmpty()) {
+ throw new InvalidDebugInfoException("Expected to read stack value of type " + prettyType(type)
+ + " but found value of type " + prettyType(slot.type));
+ }
+ assert compatible;
return slot;
}
@@ -579,4 +593,17 @@
builder.append(" }");
return builder.toString();
}
+
+ private String prettyType(Type type) {
+ if (type == BYTE_OR_BOOL_TYPE) {
+ return "<byte|bool>";
+ }
+ if (type == ARRAY_TYPE) {
+ return type.getElementType().getInternalName();
+ }
+ if (type == REFERENCE_TYPE || type == OBJECT_TYPE || type == NULL_TYPE) {
+ return type.getInternalName();
+ }
+ return DescriptorUtils.descriptorToJavaType(type.getDescriptor());
+ }
}
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 541b622..ac64e92 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -6,6 +6,7 @@
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.dex.Marker;
import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.errors.InvalidDebugInfoException;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.shaking.ProguardConfigurationRule;
@@ -104,6 +105,12 @@
public boolean warningMissingEnclosingMember = false;
+ public int warningInvalidDebugInfoCount = 0;
+
+ public void warningInvalidDebugInfo(DexEncodedMethod method, InvalidDebugInfoException e) {
+ warningInvalidDebugInfoCount++;
+ }
+
public boolean printWarnings() {
boolean printed = false;
boolean printOutdatedToolchain = false;
@@ -111,6 +118,13 @@
System.out.println("Warning: " + warningInvalidParameterAnnotations);
printed = true;
}
+ if (warningInvalidDebugInfoCount > 0) {
+ System.out.println("Warning: stripped invalid locals information from "
+ + warningInvalidDebugInfoCount
+ + (warningInvalidDebugInfoCount == 1 ? " method." : " methods."));
+ printed = true;
+ printOutdatedToolchain = true;
+ }
if (warningMissingEnclosingMember) {
System.out.println(
"Warning: InnerClass annotations are missing corresponding EnclosingMember annotations."
diff --git a/src/main/java/com/android/tools/r8/utils/StringUtils.java b/src/main/java/com/android/tools/r8/utils/StringUtils.java
index 268228f..8404703 100644
--- a/src/main/java/com/android/tools/r8/utils/StringUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/StringUtils.java
@@ -12,6 +12,8 @@
public class StringUtils {
+ public final static String LINE_SEPARATOR = System.getProperty("line.separator");
+
private final static char[] IDENTIFIER_LETTERS
= "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_".toCharArray();
private final static int NUMBER_OF_LETTERS = IDENTIFIER_LETTERS.length;
@@ -107,7 +109,7 @@
return join(collection, separator, BraceType.NONE);
}
- public static String join(String separator, String... strings) {
+ public static <T> String join(String separator, T... strings) {
return join(Arrays.asList(strings), separator, BraceType.NONE);
}
@@ -122,6 +124,18 @@
return builder.toString();
}
+ public static <T> String lines(T... lines) {
+ StringBuilder builder = new StringBuilder();
+ for (T line : lines) {
+ builder.append(line).append(LINE_SEPARATOR);
+ }
+ return builder.toString();
+ }
+
+ public static <T> String joinLines(T... lines) {
+ return join(LINE_SEPARATOR, lines);
+ }
+
public static String zeroPrefix(int i, int width) {
return zeroPrefixString(Integer.toString(i), width);
}
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index 562518f..fd4cdccb 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -21,6 +21,7 @@
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.ListUtils;
+import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.Timing;
import com.google.common.collect.ImmutableList;
@@ -61,7 +62,7 @@
public static final String EXAMPLES_ANDROID_O_BUILD_DIR = BUILD_DIR + "test/examplesAndroidO/";
public static final String SMALI_BUILD_DIR = BUILD_DIR + "test/smali/";
- public static final String LINE_SEPARATOR = System.getProperty("line.separator");
+ public static final String LINE_SEPARATOR = StringUtils.LINE_SEPARATOR;
private static final String ANDROID_JAR_PATTERN = "third_party/android_jar/lib-v%d/android.jar";
private static final int DEFAULT_MIN_SDK = Constants.ANDROID_I_API;
diff --git a/src/test/java/com/android/tools/r8/jasmin/InvalidDebugInfoTests.java b/src/test/java/com/android/tools/r8/jasmin/InvalidDebugInfoTests.java
index 155438f..81d299e 100644
--- a/src/test/java/com/android/tools/r8/jasmin/InvalidDebugInfoTests.java
+++ b/src/test/java/com/android/tools/r8/jasmin/InvalidDebugInfoTests.java
@@ -6,6 +6,7 @@
import static org.junit.Assert.assertEquals;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.ImmutableList;
import org.junit.Test;
@@ -82,7 +83,7 @@
JasminBuilder builder = new JasminBuilder();
JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
- clazz.addStaticMethod("foo", ImmutableList.of("I","I"), "V",
+ clazz.addStaticMethod("foo", ImmutableList.of("I", "I"), "V",
".limit stack 2",
".limit locals 3",
".var 0 is x I from LabelInit to LabelExit",
@@ -123,4 +124,165 @@
assertEquals(expected, artResult);
}
+ @Test
+ public void invalidInfoBug63412730_onWrite() throws Throwable {
+ JasminBuilder builder = new JasminBuilder();
+ JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
+ clazz.addStaticMethod("bar", ImmutableList.of(), "V",
+ ".limit stack 3",
+ ".limit locals 2",
+ ".var 1 is i I from LI to End",
+ ".var 0 is f F from LF to End",
+ "Init:",
+ " ldc 42",
+ " istore 0",
+ "LI:",
+ " ldc 7.5",
+ " fstore 1",
+ "LF:",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " dup",
+ " iload 0",
+ " invokevirtual java/io/PrintStream/println(I)V",
+ " fload 1",
+ " invokevirtual java/io/PrintStream/println(F)V",
+ " return",
+ "End:");
+
+ clazz.addMainMethod(
+ ".limit stack 1",
+ ".limit locals 1",
+ " invokestatic Test/bar()V",
+ " return");
+
+ String expected = StringUtils.lines("42", "7.5");
+ String javaResult = runOnJava(builder, clazz.name);
+ assertEquals(expected, javaResult);
+ String artResult = runOnArtD8(builder, clazz.name);
+ assertEquals(expected, artResult);
+ }
+
+ @Test
+ public void invalidInfoBug63412730_onRead() throws Throwable {
+ JasminBuilder builder = new JasminBuilder();
+ JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
+ clazz.addStaticMethod("bar", ImmutableList.of(), "V",
+ ".limit stack 3",
+ ".limit locals 2",
+ ".var 1 is i I from Locals to End",
+ ".var 0 is f F from Locals to End",
+ "Init:",
+ " ldc 42",
+ " istore 0",
+ " ldc 7.5",
+ " fstore 1",
+ "Locals:",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " dup",
+ " iload 0",
+ " invokevirtual java/io/PrintStream/println(I)V",
+ " fload 1",
+ " invokevirtual java/io/PrintStream/println(F)V",
+ " return",
+ "End:");
+
+ clazz.addMainMethod(
+ ".limit stack 1",
+ ".limit locals 1",
+ " invokestatic Test/bar()V",
+ " return");
+
+ String expected = StringUtils.lines("42", "7.5");
+ String javaResult = runOnJava(builder, clazz.name);
+ assertEquals(expected, javaResult);
+ String artResult = runOnArtD8(builder, clazz.name);
+ assertEquals(expected, artResult);
+ }
+
+ @Test
+ public void invalidInfoBug63412730_onMove() throws Throwable {
+ JasminBuilder builder = new JasminBuilder();
+ JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
+ clazz.addStaticMethod("bar", ImmutableList.of(), "V",
+ ".limit stack 3",
+ ".limit locals 2",
+ ".var 1 is i I from LI to End",
+ ".var 0 is j F from LJ to End",
+ "Init:",
+ " ldc 42",
+ " istore 0",
+ "LI:",
+ " ldc 0",
+ " ldc 0",
+ " ifeq LZ",
+ " ldc 75",
+ " istore 1",
+ "LJ:",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " iload 1",
+ " invokevirtual java/io/PrintStream/println(I)V",
+ " return",
+ "LZ:",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " iload 0",
+ " invokevirtual java/io/PrintStream/println(I)V",
+ " return",
+ "End:");
+
+ clazz.addMainMethod(
+ ".limit stack 1",
+ ".limit locals 1",
+ " invokestatic Test/bar()V",
+ " return");
+
+ String expected = StringUtils.lines("42");
+ String javaResult = runOnJava(builder, clazz.name);
+ assertEquals(expected, javaResult);
+ String artResult = runOnArtD8(builder, clazz.name);
+ assertEquals(expected, artResult);
+ }
+
+ @Test
+ public void invalidInfoBug63412730_onPop() throws Throwable {
+ JasminBuilder builder = new JasminBuilder();
+ JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
+ clazz.addStaticMethod("bar", ImmutableList.of(), "V",
+ ".limit stack 3",
+ ".limit locals 2",
+ ".var 1 is a [Ljava/lang/Object; from Locals to End",
+ ".var 0 is o LObject; from Locals to End",
+ "Init:",
+ " ldc 1",
+ " anewarray java/lang/Object",
+ " astore 0",
+ " new java/lang/Integer",
+ " dup",
+ " ldc 42",
+ " invokespecial java/lang/Integer/<init>(I)V",
+ " astore 1",
+ "Locals:",
+ " aload 0",
+ " ldc 0",
+ " aload 1",
+ " aastore",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " aload 0",
+ " ldc 0",
+ " aaload",
+ " invokevirtual java/io/PrintStream/println(Ljava/lang/Object;)V",
+ " return",
+ "End:");
+
+ clazz.addMainMethod(
+ ".limit stack 1",
+ ".limit locals 1",
+ " invokestatic Test/bar()V",
+ " return");
+
+ String expected = StringUtils.lines("42");
+ String javaResult = runOnJava(builder, clazz.name);
+ assertEquals(expected, javaResult);
+ String artResult = runOnArtD8(builder, clazz.name);
+ assertEquals(expected, artResult);
+ }
}
