Merge "Remove spurious call graph edges to <clinit>()"
diff --git a/infra/config/global/cr-buildbucket.cfg b/infra/config/global/cr-buildbucket.cfg
index 0136562..863bbbe 100644
--- a/infra/config/global/cr-buildbucket.cfg
+++ b/infra/config/global/cr-buildbucket.cfg
@@ -86,6 +86,7 @@
builders {
name: "archive"
mixins: "linux"
+ execution_timeout_secs: 1800 # 1/2h
recipe {
properties: "archive:True"
}
@@ -93,6 +94,7 @@
builders {
name: "archive_release"
mixins: "linux"
+ execution_timeout_secs: 1800 # 1/2h
recipe {
# TODO(ricow): set archive flag when we flip over
# properties: "archive:True"
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index f26fb09..b0cef90 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.5.8-dev";
+ public static final String LABEL = "1.5.9-dev";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index 3ce4a75..0e58ec0 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -143,6 +143,7 @@
public final DexString compareToMethodName = createString("compareTo");
public final DexString compareToIgnoreCaseMethodName = createString("compareToIgnoreCase");
public final DexString cloneMethodName = createString("clone");
+ public final DexString substringName = createString("substring");
public final DexString valueOfMethodName = createString("valueOf");
public final DexString toStringMethodName = createString("toString");
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLense.java b/src/main/java/com/android/tools/r8/graph/GraphLense.java
index b964dd3..aa8523e 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLense.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLense.java
@@ -814,6 +814,12 @@
protected final BiMap<DexField, DexField> originalFieldSignatures;
protected final BiMap<DexMethod, DexMethod> originalMethodSignatures;
+ // Overrides this if the sub type needs to be a nested lense while it doesn't have any mappings
+ // at all, e.g., publicizer lense that changes invocation type only.
+ protected boolean isLegitimateToHaveEmptyMappings() {
+ return false;
+ }
+
public NestedGraphLense(
Map<DexType, DexType> typeMap,
Map<DexMethod, DexMethod> methodMap,
@@ -822,6 +828,8 @@
BiMap<DexMethod, DexMethod> originalMethodSignatures,
GraphLense previousLense,
DexItemFactory dexItemFactory) {
+ assert !typeMap.isEmpty() || !methodMap.isEmpty() || !fieldMap.isEmpty()
+ || isLegitimateToHaveEmptyMappings();
this.typeMap = typeMap.isEmpty() ? null : typeMap;
this.methodMap = methodMap;
this.fieldMap = fieldMap;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/string/StringOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/string/StringOptimizer.java
index 5dfbb78..5096294 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/string/StringOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/string/StringOptimizer.java
@@ -66,6 +66,8 @@
// int String#lastIndexOf(int)
// int String#compareTo(String)
// int String#compareToIgnoreCase(String)
+ // String String#substring(int)
+ // String String#substring(int, int)
public void computeTrivialOperationsOnConstString(IRCode code) {
if (!code.hasConstString) {
return;
@@ -77,7 +79,55 @@
continue;
}
InvokeVirtual invoke = instr.asInvokeVirtual();
+ if (!invoke.hasOutValue()) {
+ continue;
+ }
DexMethod invokedMethod = invoke.getInvokedMethod();
+ if (invokedMethod.name == appInfo.dexItemFactory.substringName) {
+ assert invoke.inValues().size() == 2 || invoke.inValues().size() == 3;
+ Value rcv = invoke.getReceiver().getAliasedValue();
+ if (rcv.definition == null
+ || !rcv.definition.isConstString()
+ || rcv.hasLocalInfo()) {
+ continue;
+ }
+ Value beginIndex = invoke.inValues().get(1).getAliasedValue();
+ if (beginIndex.definition == null
+ || !beginIndex.definition.isConstNumber()
+ || beginIndex.hasLocalInfo()) {
+ continue;
+ }
+ int beginIndexValue = beginIndex.definition.asConstNumber().getIntValue();
+ Value endIndex = null;
+ if (invoke.inValues().size() == 3) {
+ endIndex = invoke.inValues().get(2).getAliasedValue();
+ if (endIndex.definition == null
+ || !endIndex.definition.isConstNumber()
+ || endIndex.hasLocalInfo()) {
+ continue;
+ }
+ }
+ String rcvString = rcv.definition.asConstString().getValue().toString();
+ int endIndexValue =
+ endIndex == null
+ ? rcvString.length()
+ : endIndex.definition.asConstNumber().getIntValue();
+ if (beginIndexValue < 0
+ || endIndexValue > rcvString.length()
+ || beginIndexValue > endIndexValue) {
+ // This will raise StringIndexOutOfBoundsException.
+ continue;
+ }
+ String sub = rcvString.substring(beginIndexValue, endIndexValue);
+ Value stringValue =
+ code.createValue(
+ TypeLatticeElement.stringClassType(appInfo, definitelyNotNull()),
+ invoke.getLocalInfo());
+ it.replaceCurrentInstruction(
+ new ConstString(stringValue, factory.createString(sub), throwingInfo));
+ continue;
+ }
+
Function<String, Integer> operatorWithNoArg = null;
BiFunction<String, String, Integer> operatorWithString = null;
BiFunction<String, Integer, Integer> operatorWithInt = null;
@@ -115,7 +165,7 @@
Value rcv = invoke.getReceiver().getAliasedValue();
if (rcv.definition == null
|| !rcv.definition.isConstString()
- || !rcv.isConstant()) {
+ || rcv.hasLocalInfo()) {
continue;
}
DexString rcvString = rcv.definition.asConstString().getValue();
@@ -130,7 +180,7 @@
Value arg = invoke.inValues().get(1).getAliasedValue();
if (arg.definition == null
|| !arg.definition.isConstString()
- || !arg.isConstant()) {
+ || arg.hasLocalInfo()) {
continue;
}
int v = operatorWithString.apply(
@@ -143,7 +193,7 @@
Value arg = invoke.inValues().get(1).getAliasedValue();
if (arg.definition == null
|| !arg.definition.isConstNumber()
- || !arg.isConstant()) {
+ || arg.hasLocalInfo()) {
continue;
}
int v = operatorWithInt.apply(
@@ -203,7 +253,7 @@
Value in = invoke.getReceiver().getAliasedValue();
if (in.definition == null
|| !in.definition.isConstClass()
- || !in.isConstant()) {
+ || in.hasLocalInfo()) {
continue;
}
diff --git a/src/main/java/com/android/tools/r8/optimize/PublicizerLense.java b/src/main/java/com/android/tools/r8/optimize/PublicizerLense.java
index c685d84..29ecf33 100644
--- a/src/main/java/com/android/tools/r8/optimize/PublicizerLense.java
+++ b/src/main/java/com/android/tools/r8/optimize/PublicizerLense.java
@@ -20,8 +20,6 @@
private final Set<DexMethod> publicizedMethods;
private PublicizerLense(AppView appView, Set<DexMethod> publicizedMethods) {
- // This lense does not map any DexItem's at all.
- // It will just tweak invoke type for publicized methods from invoke-direct to invoke-virtual.
super(
ImmutableMap.of(),
ImmutableMap.of(),
@@ -35,6 +33,13 @@
}
@Override
+ protected boolean isLegitimateToHaveEmptyMappings() {
+ // This lense does not map any DexItem's at all.
+ // It will just tweak invoke type for publicized methods from invoke-direct to invoke-virtual.
+ return true;
+ }
+
+ @Override
public GraphLenseLookupResult lookupMethod(DexMethod method, DexMethod context, Type type) {
GraphLenseLookupResult previous = previousLense.lookupMethod(method, context, type);
method = previous.getMethod();
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/StringContentCheckTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/StringContentCheckTest.java
index 7044366..0578b85 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/StringContentCheckTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/StringContentCheckTest.java
@@ -56,6 +56,12 @@
System.out.println(s1.lastIndexOf("ix"));
System.out.println(s1.compareTo("prefix-CONST-suffix") == 0);
System.out.println(s1.compareToIgnoreCase("PREFIX-const-SUFFIX") == 0);
+ // "prefix" exists
+ System.out.println(s1.substring(0, 6));
+ // "suffix" exists
+ System.out.println(s1.substring(13));
+ // "-suffix" doesn't.
+ System.out.println(s1.substring(12));
}
{
@@ -74,6 +80,7 @@
System.out.println(s2.lastIndexOf("ix"));
System.out.println(s2.compareTo("prefix-CONST-suffix") == 0);
System.out.println(s2.compareToIgnoreCase("pre-con-suf") == 0);
+ System.out.println(s2.substring(13));
}
{
@@ -85,11 +92,21 @@
// expected
}
}
+
+ {
+ try {
+ System.out.println("qwerty".substring(8));
+ fail("Should raise StringIndexOutOfBoundsException");
+ } catch (StringIndexOutOfBoundsException e) {
+ // expected
+ }
+ }
}
}
@RunWith(Parameterized.class)
public class StringContentCheckTest extends TestBase {
+ private static final String STRING_DESCRIPTOR = "Ljava/lang/String;";
private final Backend backend;
private static final List<Class<?>> CLASSES = ImmutableList.of(
NeverInline.class,
@@ -122,6 +139,12 @@
"true",
// s1, compareToIgnoreCase(String)
"true",
+ // s1, substring(int)
+ "prefix",
+ // s1, substring(int, int)
+ "suffix",
+ // s1, substring(int, int)
+ "-suffix",
// s2, contains(String)
"false",
// s2, startsWith(String)
@@ -148,6 +171,8 @@
"false",
// s2, compareToIgnoreCase(String)
"false",
+ // s2, substring(int, int)
+ "SUFFIX",
// argCouldBeNull
"false"
);
@@ -169,10 +194,10 @@
}
private static boolean isStringContentChecker(DexMethod method) {
- return method.getHolder().toDescriptorString().equals("Ljava/lang/String;")
- && method.getArity() == 1
+ return method.getHolder().toDescriptorString().equals(STRING_DESCRIPTOR)
&& (method.proto.returnType.isBooleanType()
- || method.proto.returnType.isIntType())
+ || method.proto.returnType.isIntType()
+ || method.proto.returnType.toDescriptorString().equals(STRING_DESCRIPTOR))
&& (method.name.toString().equals("contains")
|| method.name.toString().equals("startsWith")
|| method.name.toString().equals("endsWith")
@@ -182,7 +207,8 @@
|| method.name.toString().equals("indexOf")
|| method.name.toString().equals("lastIndexOf")
|| method.name.toString().equals("compareTo")
- || method.name.toString().equals("compareToIgnoreCase"));
+ || method.name.toString().equals("compareToIgnoreCase")
+ || method.name.toString().equals("substring"));
}
private long countStringContentChecker(MethodSubject method) {
@@ -218,14 +244,14 @@
.addProgramClasses(CLASSES)
.run(MAIN)
.assertSuccessWithOutput(JAVA_OUTPUT);
- test(result, 26);
+ test(result, 31);
result = testForD8()
.release()
.addProgramClasses(CLASSES)
.run(MAIN)
.assertSuccessWithOutput(JAVA_OUTPUT);
- test(result, 14);
+ test(result, 16);
}
@Test
@@ -237,6 +263,6 @@
.addKeepMainRule(MAIN)
.run(MAIN)
.assertSuccessWithOutput(JAVA_OUTPUT);
- test(result, 14);
+ test(result, 16);
}
}
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/RetraceTestBase.java b/src/test/java/com/android/tools/r8/naming/retrace/RetraceTestBase.java
index f7b95f9..0ea1d7c 100644
--- a/src/test/java/com/android/tools/r8/naming/retrace/RetraceTestBase.java
+++ b/src/test/java/com/android/tools/r8/naming/retrace/RetraceTestBase.java
@@ -48,6 +48,7 @@
testForR8(backend)
.setMode(mode)
.enableProguardTestOptions()
+ .enableMergeAnnotations()
.enableInliningAnnotations()
.addProgramClasses(getClasses())
.addKeepMainRule(getMainClass())
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/VerticalClassMergingRetraceTest.java b/src/test/java/com/android/tools/r8/naming/retrace/VerticalClassMergingRetraceTest.java
new file mode 100644
index 0000000..6ef62be
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/retrace/VerticalClassMergingRetraceTest.java
@@ -0,0 +1,122 @@
+// 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.naming.retrace;
+
+import static com.android.tools.r8.naming.retrace.StackTrace.isSameExceptForFileName;
+import static com.android.tools.r8.naming.retrace.StackTrace.isSameExceptForFileNameAndLineNumber;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.naming.retrace.StackTrace.StackTraceLine;
+import com.google.common.collect.ImmutableList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class VerticalClassMergingRetraceTest extends RetraceTestBase {
+ private Set<StackTraceLine> haveSeenLines = new HashSet<>();
+
+ @Parameters(name = "Backend: {0}, mode: {1}")
+ public static Collection<Object[]> data() {
+ return buildParameters(Backend.values(), CompilationMode.values());
+ }
+
+ public VerticalClassMergingRetraceTest(Backend backend, CompilationMode mode) {
+ super(backend, mode);
+ }
+
+ @Override
+ public Collection<Class<?>> getClasses() {
+ return ImmutableList.of(getMainClass(), ResourceWrapper.class, TintResources.class);
+ }
+
+ @Override
+ public Class<?> getMainClass() {
+ return MainApp.class;
+ }
+
+ private int expectedActualStackTraceHeight() {
+ // In RELEASE mode, a synthetic bridge will be added by vertical class merger.
+ return mode == CompilationMode.RELEASE ? 3 : 2;
+ }
+
+ private boolean filterSynthesizedMethodWhenLineNumberAvailable(
+ StackTraceLine retracedStackTraceLine) {
+ return retracedStackTraceLine.lineNumber > 0;
+ }
+
+ private boolean filterSynthesizedMethod(StackTraceLine retracedStackTraceLine) {
+ return haveSeenLines.add(retracedStackTraceLine)
+ && (retracedStackTraceLine.className.contains("ResourceWrapper")
+ || retracedStackTraceLine.className.contains("MainApp"));
+ }
+
+ @Test
+ public void testSourceFileAndLineNumberTable() throws Exception {
+ runTest(
+ ImmutableList.of("-keepattributes SourceFile,LineNumberTable"),
+ (StackTrace actualStackTrace, StackTrace retracedStackTrace) -> {
+ // Even when SourceFile is present retrace replaces the file name in the stack trace.
+ StackTrace reprocessedStackTrace =
+ mode == CompilationMode.DEBUG ? retracedStackTrace
+ : retracedStackTrace.filter(this::filterSynthesizedMethodWhenLineNumberAvailable);
+ assertThat(reprocessedStackTrace, isSameExceptForFileName(expectedStackTrace));
+ assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
+ });
+ }
+
+ @Test
+ public void testLineNumberTableOnly() throws Exception {
+ runTest(
+ ImmutableList.of("-keepattributes LineNumberTable"),
+ (StackTrace actualStackTrace, StackTrace retracedStackTrace) -> {
+ StackTrace reprocessedStackTrace =
+ mode == CompilationMode.DEBUG ? retracedStackTrace
+ : retracedStackTrace.filter(this::filterSynthesizedMethodWhenLineNumberAvailable);
+ assertThat(reprocessedStackTrace, isSameExceptForFileName(expectedStackTrace));
+ assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
+ });
+ }
+
+ @Test
+ public void testNoLineNumberTable() throws Exception {
+ haveSeenLines.clear();
+ runTest(
+ ImmutableList.of(),
+ (StackTrace actualStackTrace, StackTrace retracedStackTrace) -> {
+ StackTrace reprocessedStackTrace =
+ mode == CompilationMode.DEBUG ? retracedStackTrace
+ : retracedStackTrace.filter(this::filterSynthesizedMethod);
+ assertThat(reprocessedStackTrace,
+ isSameExceptForFileNameAndLineNumber(expectedStackTrace));
+ assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
+ });
+ }
+}
+
+class ResourceWrapper {
+ // Will be merged down, and represented as:
+ // java.lang.String ...ResourceWrapper.foo() -> a
+ @NeverInline
+ String foo() {
+ throw null;
+ }
+}
+
+class TintResources extends ResourceWrapper {
+}
+
+class MainApp {
+ public static void main(String[] args) {
+ TintResources t = new TintResources();
+ System.out.println(t.foo());
+ }
+}
diff --git a/tools/archive.py b/tools/archive.py
index 833b4b0..f86530a 100755
--- a/tools/archive.py
+++ b/tools/archive.py
@@ -94,9 +94,8 @@
if not utils.is_bot() and not options.dry_run:
raise Exception('You are not a bot, don\'t archive builds')
- if utils.is_old_bot():
- print("Archiving is disabled on old bots, new bots are archiving, see " +
- "https://ci.chromium.org/p/r8")
+ if utils.is_new_bot():
+ print("Archiving is disabled on new bots.")
return
# Create maven release which uses a build that exclude dependencies.
diff --git a/tools/run_on_as_app.py b/tools/run_on_as_app.py
index 0a4f740..998c9ef 100755
--- a/tools/run_on_as_app.py
+++ b/tools/run_on_as_app.py
@@ -783,7 +783,7 @@
return
with utils.TempDir() as temp_dir:
- if not options.no_build or options.golem:
+ if not (options.no_build or options.golem):
gradle.RunGradle(['r8', 'r8lib'])
if options.version:
diff --git a/tools/utils.py b/tools/utils.py
index 2b0caaf..f2df0ed 100644
--- a/tools/utils.py
+++ b/tools/utils.py
@@ -481,8 +481,8 @@
android_optional_jar for android_optional_jar in android_optional_jars
if os.path.isfile(android_optional_jar)]
-def is_old_bot():
- return 'BUILDBOT_BUILDERNAME' in os.environ
+def is_new_bot():
+ return 'SWARMING_BOT_ID' in os.environ
def is_bot():
return 'USER' in os.environ and os.environ['USER'] == 'chrome-bot'
