Merge "Strip invalid locals information."
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 a718da2..976020c2 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;
@@ -1434,7 +1435,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 30472f9..70a2e53 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;
@@ -119,6 +120,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;
@@ -126,6 +133,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);
+ }
}
