Merge "Make Proguard -dontobfuscate option turn on debug mode in R8"
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 026c2b5..d234702 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -155,6 +155,9 @@
return isVirtualMethod() && !accessFlags.isAbstract();
}
+ public boolean isPublicMethod() {
+ return accessFlags.isPublic();
+ }
public boolean isPrivateMethod() {
return accessFlags.isPrivate();
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
index 73c5118..76a80d4 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
@@ -66,7 +66,7 @@
// Public for testing.
public static final String COMPANION_CLASS_NAME_SUFFIX = "$-CC";
public static final String DEFAULT_METHOD_PREFIX = "$default$";
- private static final String PRIVATE_METHOD_PREFIX = "$private$";
+ public static final String PRIVATE_METHOD_PREFIX = "$private$";
private final IRConverter converter;
private final InternalOptions options;
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
index 1e8d57a..f4a76e6 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
@@ -91,7 +91,7 @@
remainingMethods.clear();
// Process static and private methods, move them into companion class as well,
- // make private instance methods static.
+ // make private instance methods public static.
for (DexEncodedMethod direct : iface.directMethods()) {
MethodAccessFlags originalFlags = direct.accessFlags;
MethodAccessFlags newFlags = originalFlags.copy();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index ae57f48..e610d20 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -762,6 +762,11 @@
return;
}
+ DexClass clazz = appInfo.definitionFor(method.method.holder);
+ if (clazz == null) {
+ return;
+ }
+
boolean receiverUsedAsReturnValue = false;
boolean seenSuperInitCall = false;
for (Instruction insn : receiver.uniqueUsers()) {
@@ -773,7 +778,7 @@
if (insn.isInstanceGet() ||
(insn.isInstancePut() && insn.asInstancePut().object() == receiver)) {
DexField field = insn.asFieldInstruction().getField();
- if (field.clazz == method.method.holder) {
+ if (field.clazz == clazz.type && clazz.lookupInstanceField(field) != null) {
// Since class inliner currently only supports classes directly extending
// java.lang.Object, we don't need to worry about fields defined in superclasses.
continue;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
index 7bdff33..d7a57df 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
@@ -169,14 +169,16 @@
Map<InvokeMethodWithReceiver, InliningInfo> methodCalls = new IdentityHashMap<>();
+ DexClass definition = appInfo.definitionFor(clazz);
+
for (Instruction user : receiver.uniqueUsers()) {
// Field read/write.
if (user.isInstanceGet() ||
(user.isInstancePut() && user.asInstancePut().value() != receiver)) {
- if (user.asFieldInstruction().getField().clazz == newInstanceInsn.clazz) {
- // Eligible field read or write. Note: as long as we consider only classes eligible
- // if they directly extend java.lang.Object we don't need to check if the field
- // really exists in the class.
+ DexField field = user.asFieldInstruction().getField();
+ if (field.clazz == newInstanceInsn.clazz && definition.lookupInstanceField(field) != null) {
+ // Since class inliner currently only supports classes directly extending
+ // java.lang.Object, we don't need to worry about fields defined in superclasses.
continue;
}
return null; // Not eligible.
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 2b51720..dae98ad 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1651,7 +1651,7 @@
rewriteKeysWhileMergingValues(previous.staticFieldWrites, lense::lookupField);
this.fieldsRead = rewriteItems(previous.fieldsRead, lense::lookupField);
this.fieldsWritten = rewriteItems(previous.fieldsWritten, lense::lookupField);
- this.pinnedItems = rewriteMixedItems(previous.pinnedItems, lense);
+ this.pinnedItems = rewriteMixedItemsConservatively(previous.pinnedItems, lense);
this.virtualInvokes = rewriteMethodsConservatively(previous.virtualInvokes, lense);
this.interfaceInvokes = rewriteMethodsConservatively(previous.interfaceInvokes, lense);
this.superInvokes = rewriteMethodsConservatively(previous.superInvokes, lense);
@@ -1665,7 +1665,8 @@
this.assumedValues = previous.assumedValues;
assert assertNotModifiedByLense(previous.alwaysInline, lense);
this.alwaysInline = previous.alwaysInline;
- this.identifierNameStrings = rewriteMixedItems(previous.identifierNameStrings, lense);
+ this.identifierNameStrings =
+ rewriteMixedItemsConservatively(previous.identifierNameStrings, lense);
// Switchmap classes should never be affected by renaming.
assert assertNotModifiedByLense(
previous.switchMaps.keySet().stream().map(this::definitionFor).filter(Objects::nonNull)
@@ -1846,7 +1847,7 @@
return Collections.unmodifiableMap(result);
}
- private static ImmutableSet<DexItem> rewriteMixedItems(
+ private static ImmutableSet<DexItem> rewriteMixedItemsConservatively(
Set<DexItem> original, GraphLense lense) {
ImmutableSet.Builder<DexItem> builder = ImmutableSet.builder();
for (DexItem item : original) {
@@ -1854,7 +1855,12 @@
if (item instanceof DexType) {
builder.add(lense.lookupType((DexType) item));
} else if (item instanceof DexMethod) {
- builder.add(lense.lookupMethod((DexMethod) item));
+ DexMethod method = (DexMethod) item;
+ if (lense.isContextFreeForMethod(method)) {
+ builder.add(lense.lookupMethod(method));
+ } else {
+ builder.addAll(lense.lookupMethodInAllContexts(method));
+ }
} else if (item instanceof DexField) {
builder.add(lense.lookupField((DexField) item));
} else {
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index 4b57209..c737055 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexItem;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexProto;
@@ -87,28 +88,20 @@
// Returns a set of types that must not be merged into other types.
private Set<DexType> getPinnedTypes(Iterable<DexProgramClass> classes) {
Set<DexType> pinnedTypes = new HashSet<>();
+
+ // For all pinned fields, also pin the type of the field (because changing the type of the field
+ // implicitly changes the signature of the field). Similarly, for all pinned methods, also pin
+ // the return type and the parameter types of the method.
+ extractPinnedItems(appInfo.pinnedItems, pinnedTypes);
+
+ // TODO(christofferqa): Remove the invariant that the graph lense should not modify any
+ // methods from the sets alwaysInline and noSideEffects (see use of assertNotModifiedBy-
+ // Lense).
+ extractPinnedItems(appInfo.alwaysInline, pinnedTypes);
+ extractPinnedItems(appInfo.noSideEffects.keySet(), pinnedTypes);
+
for (DexProgramClass clazz : classes) {
for (DexEncodedMethod method : clazz.methods()) {
- // TODO(christofferqa): Remove the invariant that the graph lense should not modify any
- // methods from the sets alwaysInline and noSideEffects (see use of assertNotModifiedBy-
- // Lense).
- if (appInfo.alwaysInline.contains(method) || appInfo.noSideEffects.containsKey(method)) {
- DexClass other = appInfo.definitionFor(method.method.proto.returnType);
- if (other != null && other.isProgramClass()) {
- // If we were to merge [other] into its sub class, then we would implicitly change the
- // signature of this method, and therefore break the invariant.
- pinnedTypes.add(other.type);
- }
- for (DexType parameterType : method.method.proto.parameters.values) {
- other = appInfo.definitionFor(parameterType);
- if (other != null && other.isProgramClass()) {
- // If we were to merge [other] into its sub class, then we would implicitly change the
- // signature of this method, and therefore break the invariant.
- pinnedTypes.add(other.type);
- }
- }
- }
-
// Avoid merging two types if this could remove a NoSuchMethodError, as illustrated by the
// following example. (Alternatively, it would be possible to merge A and B and rewrite the
// "invoke-super A.m" instruction into "invoke-super Object.m" to preserve the error. This
@@ -143,6 +136,38 @@
return pinnedTypes;
}
+ private void extractPinnedItems(Iterable<DexItem> items, Set<DexType> pinnedTypes) {
+ for (DexItem item : items) {
+ // Note: Nothing to do for the case where item is a DexType, since we check for this case
+ // using appInfo.isPinned.
+ if (item instanceof DexField) {
+ // Pin the type of the field.
+ DexField field = (DexField) item;
+ DexClass clazz = appInfo.definitionFor(field.type);
+ if (clazz != null && clazz.isProgramClass()) {
+ pinnedTypes.add(clazz.type);
+ }
+ } else if (item instanceof DexMethod) {
+ // Pin the return type and the parameter types of the method.
+ DexMethod method = (DexMethod) item;
+ DexClass clazz = appInfo.definitionFor(method.proto.returnType);
+ if (clazz != null && clazz.isProgramClass()) {
+ // If we were to merge [other] into its sub class, then we would implicitly change the
+ // signature of this method, and therefore break the invariant.
+ pinnedTypes.add(clazz.type);
+ }
+ for (DexType parameterType : method.proto.parameters.values) {
+ clazz = appInfo.definitionFor(parameterType);
+ if (clazz != null && clazz.isProgramClass()) {
+ // If we were to merge [other] into its sub class, then we would implicitly change the
+ // signature of this method, and therefore break the invariant.
+ pinnedTypes.add(clazz.type);
+ }
+ }
+ }
+ }
+ }
+
private boolean isMergeCandidate(DexProgramClass clazz, Set<DexType> pinnedTypes) {
if (appInfo.instantiatedTypes.contains(clazz.type)
|| appInfo.isPinned(clazz.type)
@@ -432,6 +457,10 @@
directMethods.add(renameConstructor(directMethod));
} else {
directMethods.add(directMethod);
+
+ if (!directMethod.isStaticMethod()) {
+ blockRedirectionOfSuperCalls(directMethod.method);
+ }
}
}
@@ -467,6 +496,7 @@
// Record that invoke-super instructions in the target class should be redirected to the
// newly created direct method.
redirectSuperCallsInTarget(virtualMethod.method, resultingDirectMethod.method);
+ blockRedirectionOfSuperCalls(resultingDirectMethod.method);
if (shadowedBy == null) {
// In addition to the newly added direct method, create a virtual method such that we do
@@ -490,6 +520,7 @@
(existing, method) -> {
DexEncodedMethod renamedMethod = renameMethod(method, target.type, true);
deferredRenamings.map(method.method, renamedMethod.method);
+ blockRedirectionOfSuperCalls(renamedMethod.method);
return renamedMethod;
});
Collection<DexEncodedMethod> mergedVirtualMethods =
@@ -578,6 +609,26 @@
}
}
+ private void blockRedirectionOfSuperCalls(DexMethod method) {
+ // We are merging a class B into C. The methods from B are being moved into C, and then we
+ // subsequently rewrite the invoke-super instructions in C that hit a method in B, such that
+ // they use an invoke-direct instruction instead. In this process, we need to avoid rewriting
+ // the invoke-super instructions that originally was in the superclass B.
+ //
+ // Example:
+ // class A {
+ // public void m() {}
+ // }
+ // class B extends A {
+ // public void m() { super.m(); } <- invoke must not be rewritten to invoke-direct
+ // (this would lead to an infinite loop)
+ // }
+ // class C extends B {
+ // public void m() { super.m(); } <- invoke needs to be rewritten to invoke-direct
+ // }
+ deferredRenamings.markMethodAsMerged(method);
+ }
+
private boolean resolutionSucceeds(DexMethod targetMethod) {
DexClass enclosingClass = appInfo.definitionFor(targetMethod.holder);
return enclosingClass != null
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java
index ea57d1d..942a857 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java
@@ -45,16 +45,19 @@
private final Map<DexField, DexField> fieldMap;
private final Map<DexMethod, DexMethod> methodMap;
+ private final Set<DexMethod> mergedMethods;
private final Map<DexType, Map<DexMethod, DexMethod>> contextualVirtualToDirectMethodMaps;
public VerticalClassMergerGraphLense(
Map<DexField, DexField> fieldMap,
Map<DexMethod, DexMethod> methodMap,
+ Set<DexMethod> mergedMethods,
Map<DexType, Map<DexMethod, DexMethod>> contextualVirtualToDirectMethodMaps,
GraphLense previousLense) {
this.previousLense = previousLense;
this.fieldMap = fieldMap;
this.methodMap = methodMap;
+ this.mergedMethods = mergedMethods;
this.contextualVirtualToDirectMethodMaps = contextualVirtualToDirectMethodMaps;
}
@@ -67,7 +70,7 @@
public DexMethod lookupMethod(DexMethod method, DexEncodedMethod context, Type type) {
assert isContextFreeForMethod(method) || (context != null && type != null);
DexMethod previous = previousLense.lookupMethod(method, context, type);
- if (type == Type.SUPER) {
+ if (type == Type.SUPER && !mergedMethods.contains(context.method)) {
Map<DexMethod, DexMethod> virtualToDirectMethodMap =
contextualVirtualToDirectMethodMaps.get(context.method.holder);
if (virtualToDirectMethodMap != null) {
@@ -127,6 +130,7 @@
private final ImmutableMap.Builder<DexField, DexField> fieldMapBuilder = ImmutableMap.builder();
private final ImmutableMap.Builder<DexMethod, DexMethod> methodMapBuilder =
ImmutableMap.builder();
+ private final ImmutableSet.Builder<DexMethod> mergedMethodsBuilder = ImmutableSet.builder();
private final Map<DexType, Map<DexMethod, DexMethod>> contextualVirtualToDirectMethodMaps =
new HashMap<>();
@@ -141,7 +145,15 @@
return previousLense;
}
return new VerticalClassMergerGraphLense(
- fieldMap, methodMap, contextualVirtualToDirectMethodMaps, previousLense);
+ fieldMap,
+ methodMap,
+ mergedMethodsBuilder.build(),
+ contextualVirtualToDirectMethodMaps,
+ previousLense);
+ }
+
+ public void markMethodAsMerged(DexMethod method) {
+ mergedMethodsBuilder.add(method);
}
public void map(DexField from, DexField to) {
@@ -161,6 +173,7 @@
public void merge(VerticalClassMergerGraphLense.Builder builder) {
fieldMapBuilder.putAll(builder.fieldMapBuilder.build());
methodMapBuilder.putAll(builder.methodMapBuilder.build());
+ mergedMethodsBuilder.addAll(builder.mergedMethodsBuilder.build());
for (DexType context : builder.contextualVirtualToDirectMethodMaps.keySet()) {
Map<DexMethod, DexMethod> current = contextualVirtualToDirectMethodMaps.get(context);
Map<DexMethod, DexMethod> other = builder.contextualVirtualToDirectMethodMaps.get(context);
diff --git a/src/test/examples/classmerging/RewritePinnedMethodTest.java b/src/test/examples/classmerging/RewritePinnedMethodTest.java
new file mode 100644
index 0000000..3d25805
--- /dev/null
+++ b/src/test/examples/classmerging/RewritePinnedMethodTest.java
@@ -0,0 +1,41 @@
+// 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 classmerging;
+
+public class RewritePinnedMethodTest {
+
+ public static void main(String[] args) {
+ C obj = new C();
+ obj.m();
+ }
+
+ // A.m is pinned by a keep rule, to prevent it from being merged into B.
+ public static class A {
+ public void m() {
+ System.out.println("In A.m");
+ }
+ }
+
+ // B is merged into C.
+ public static class B extends A {
+ @Override
+ public void m() {
+ System.out.println("In B.m");
+ super.m();
+ }
+ }
+
+ public static class C extends B {
+ @Override
+ public void m() {
+ System.out.println("In C.m");
+ // This invocation is changed from invoke-super to invoke-direct. It would be valid for this
+ // instruction to be on the form "invoke-super A.m". Therefore, the graph lense contains a
+ // mapping for "A.m" from (context: "C.m", type: SUPER) to C.m$B, where the method C.m$B is
+ // the direct method that gets created for B.m during vertical class merging.
+ super.m();
+ }
+ }
+}
diff --git a/src/test/examples/classmerging/keep-rules.txt b/src/test/examples/classmerging/keep-rules.txt
index 01e35f3..1a20ce3 100644
--- a/src/test/examples/classmerging/keep-rules.txt
+++ b/src/test/examples/classmerging/keep-rules.txt
@@ -13,6 +13,9 @@
-keep public class classmerging.ExceptionTest {
public static void main(...);
}
+-keep public class classmerging.RewritePinnedMethodTest {
+ public static void main(...);
+}
-keep public class classmerging.SimpleInterfaceAccessTest {
public static void main(...);
}
diff --git a/src/test/java/com/android/tools/r8/RunExamplesJava9Test.java b/src/test/java/com/android/tools/r8/RunExamplesJava9Test.java
index 9560c1f..60ebf39 100644
--- a/src/test/java/com/android/tools/r8/RunExamplesJava9Test.java
+++ b/src/test/java/com/android/tools/r8/RunExamplesJava9Test.java
@@ -4,28 +4,28 @@
package com.android.tools.r8;
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
import static com.android.tools.r8.utils.FileUtils.JAR_EXTENSION;
import static com.android.tools.r8.utils.FileUtils.ZIP_EXTENSION;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import com.android.tools.r8.ToolHelper.DexVm;
-import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.ir.desugar.InterfaceMethodRewriter;
import com.android.tools.r8.utils.AndroidApiLevel;
-import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
import com.android.tools.r8.utils.DexInspector.FoundClassSubject;
import com.android.tools.r8.utils.DexInspector.FoundMethodSubject;
import com.android.tools.r8.utils.DexInspector.InstructionSubject;
+import com.android.tools.r8.utils.DexInspector.MethodSubject;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.OffOrAuto;
import com.android.tools.r8.utils.TestDescriptionWatcher;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.common.io.ByteStreams;
import java.io.IOException;
-import java.io.InputStream;
-import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
@@ -33,20 +33,17 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
-import java.util.concurrent.ExecutionException;
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.function.UnaryOperator;
import java.util.stream.Collectors;
-import java.util.zip.ZipException;
-import java.util.zip.ZipFile;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.rules.TemporaryFolder;
public abstract class RunExamplesJava9Test
- <B extends BaseCommand.Builder<? extends BaseCommand, B>> {
+ <B extends BaseCommand.Builder<? extends BaseCommand, B>> {
static final String EXAMPLE_DIR = ToolHelper.EXAMPLES_JAVA9_BUILD_DIR;
abstract class TestRunner<C extends TestRunner<C>> {
@@ -99,10 +96,6 @@
return self();
}
- C withMainDexClass(String... classes) {
- return withBuilderTransformation(builder -> builder.addMainDexClasses(classes));
- }
-
C withInterfaceMethodDesugaring(OffOrAuto behavior) {
return withOptionConsumer(o -> o.interfaceMethodDesugaring = behavior);
}
@@ -111,15 +104,19 @@
return withOptionConsumer(o -> o.tryWithResourcesDesugaring = behavior);
}
+ void combinedOptionConsumer(InternalOptions options) {
+ for (Consumer<InternalOptions> consumer : optionConsumers) {
+ consumer.accept(options);
+ }
+ }
+
C withBuilderTransformation(UnaryOperator<B> builderTransformation) {
builderTransformations.add(builderTransformation);
return self();
}
- void combinedOptionConsumer(InternalOptions options) {
- for (Consumer<InternalOptions> consumer : optionConsumers) {
- consumer.accept(options);
- }
+ C withMainDexClass(String... classes) {
+ return withBuilderTransformation(builder -> builder.addMainDexClasses(classes));
}
Path build() throws Throwable {
@@ -179,19 +176,19 @@
ImmutableMap.Builder<DexVm.Version, List<String>> builder = ImmutableMap.builder();
builder
.put(DexVm.Version.V4_0_4, ImmutableList.of(
- "native-private-interface-methods",// Dex version not supported
+ "native-private-interface-methods", // Dex version not supported
"varhandle"
))
.put(DexVm.Version.V4_4_4, ImmutableList.of(
- "native-private-interface-methods",// Dex version not supported
+ "native-private-interface-methods", // Dex version not supported
"varhandle"
))
.put(DexVm.Version.V5_1_1, ImmutableList.of(
- "native-private-interface-methods",// Dex version not supported
+ "native-private-interface-methods", // Dex version not supported
"varhandle"
))
- .put(DexVm.Version.V6_0_1, ImmutableList.of("native-private-interface-methods",
- // Dex version not supported
+ .put(DexVm.Version.V6_0_1, ImmutableList.of(
+ "native-private-interface-methods", // Dex version not supported
"varhandle"
))
.put(DexVm.Version.V7_0_0, ImmutableList.of(
@@ -208,12 +205,14 @@
// Defines methods failing on JVM, specifies the output to be used for comparison.
private static Map<String, String> expectedJvmResult =
ImmutableMap.of(
- "native-private-interface-methods", "0: s>i>a\n"
+ "native-private-interface-methods",
+ "0: s>i>a\n"
+ "1: d>i>s>i>a\n"
+ "2: l>i>s>i>a\n"
+ "3: x>s\n"
+ "4: c>d>i>s>i>a\n",
- "desugared-private-interface-methods", "0: s>i>a\n"
+ "desugared-private-interface-methods",
+ "0: s>i>a\n"
+ "1: d>i>s>i>a\n"
+ "2: l>i>s>i>a\n"
+ "3: x>s\n"
@@ -245,21 +244,34 @@
@Test
public void nativePrivateInterfaceMethods() throws Throwable {
- test("native-private-interface-methods", "privateinterfacemethods", "PrivateInterfaceMethods")
+ test("native-private-interface-methods",
+ "privateinterfacemethods", "PrivateInterfaceMethods")
.withMinApiLevel(AndroidApiLevel.N.getLevel())
.run();
}
@Test
public void desugaredPrivateInterfaceMethods() throws Throwable {
- test("desugared-private-interface-methods", "privateinterfacemethods",
- "PrivateInterfaceMethods")
+ final String iName = "privateinterfacemethods.I";
+ test("desugared-private-interface-methods",
+ "privateinterfacemethods", "PrivateInterfaceMethods")
.withMinApiLevel(AndroidApiLevel.M.getLevel())
+ .withDexCheck(dexInspector -> {
+ ClassSubject companion = dexInspector.clazz(
+ iName + InterfaceMethodRewriter.COMPANION_CLASS_NAME_SUFFIX);
+ assertThat(companion, isPresent());
+ MethodSubject iFoo = companion.method(
+ "java.lang.String",
+ InterfaceMethodRewriter.PRIVATE_METHOD_PREFIX + "iFoo",
+ ImmutableList.of(iName, "boolean"));
+ assertThat(iFoo, isPresent());
+ assertTrue(iFoo.getMethod().isPublicMethod());
+ })
.run();
}
@Test
- public void varHAndle() throws Throwable {
+ public void varHandle() throws Throwable {
test("varhandle", "varhandle", "VarHandleTests")
.withMinApiLevel(AndroidApiLevel.P.getLevel())
.run();
@@ -285,7 +297,7 @@
thrown.expect(Throwable.class);
}
String output = ToolHelper.runArtNoVerificationErrors(
- Arrays.stream(dexes).map(path -> path.toString()).collect(Collectors.toList()),
+ Arrays.stream(dexes).map(Path::toString).collect(Collectors.toList()),
qualifiedMainClass,
null);
String jvmResult = null;
@@ -308,17 +320,4 @@
}
}
- protected DexInspector getMainDexInspector(Path zip)
- throws ZipException, IOException, ExecutionException {
- try (ZipFile zipFile = new ZipFile(zip.toFile(), StandardCharsets.UTF_8)) {
- try (InputStream in =
- zipFile.getInputStream(zipFile.getEntry(ToolHelper.DEFAULT_DEX_FILENAME))) {
- return new DexInspector(
- AndroidApp.builder()
- .addDexProgramData(ByteStreams.toByteArray(in), Origin.unknown())
- .build());
- }
- }
- }
-
-}
+}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
index 7016163..1e71a82 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
@@ -289,6 +289,32 @@
"-keep public class classmerging.pkg.SimpleInterfaceImplRetriever"));
}
+ // TODO(christofferqa): This test checks that the invoke-super instruction in B is not rewritten
+ // into an invoke-direct instruction after it gets merged into class C. We should add a test that
+ // checks that this works with and without inlining of the method B.m().
+ @Test
+ public void testRewritePinnedMethod() throws Exception {
+ String main = "classmerging.RewritePinnedMethodTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("RewritePinnedMethodTest.class"),
+ CF_DIR.resolve("RewritePinnedMethodTest$A.class"),
+ CF_DIR.resolve("RewritePinnedMethodTest$B.class"),
+ CF_DIR.resolve("RewritePinnedMethodTest$C.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.RewritePinnedMethodTest",
+ "classmerging.RewritePinnedMethodTest$A",
+ "classmerging.RewritePinnedMethodTest$C");
+ runTest(
+ main,
+ programFiles,
+ preservedClassNames,
+ getProguardConfig(
+ EXAMPLE_KEEP, "-keep class classmerging.RewritePinnedMethodTest$A { *; }"));
+ }
+
@Test
public void testTemplateMethodPattern() throws Exception {
String main = "classmerging.TemplateMethodTest";
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
index 5df1cee..38768cd 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
@@ -4,9 +4,11 @@
package com.android.tools.r8.ir.optimize.classinliner;
+import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import com.android.tools.r8.OutputMode;
@@ -32,10 +34,13 @@
import com.android.tools.r8.ir.optimize.classinliner.trivial.Iface2Impl;
import com.android.tools.r8.ir.optimize.classinliner.trivial.ReferencedFields;
import com.android.tools.r8.ir.optimize.classinliner.trivial.TrivialTestClass;
+import com.android.tools.r8.jasmin.JasminBuilder;
+import com.android.tools.r8.jasmin.JasminBuilder.ClassBuilder;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DexInspector;
import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.Sets;
import java.nio.file.Path;
import java.util.Collections;
@@ -178,6 +183,39 @@
assertFalse(inspector.clazz(BuildersTestClass.Pos.class).isPresent());
}
+ @Test
+ public void testErroneousInput() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+
+ ClassBuilder testClass = builder.addClass("A");
+ testClass.addStaticFinalField("f", "I", "123");
+ testClass.addDefaultConstructor();
+
+ ClassBuilder mainClass = builder.addClass("Main");
+ mainClass.addMainMethod(
+ ".limit stack 3",
+ ".limit locals 1",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " new A",
+ " dup",
+ " invokespecial A/<init>()V",
+ " getfield A/f I",
+ " invokevirtual java/io/PrintStream/print(I)V",
+ " return");
+
+ AndroidApp compiled =
+ compileWithR8(builder.build(), getProguardConfig(mainClass.name), this::configure);
+
+ // Check that the code fails with an IncompatibleClassChangeError with Java.
+ ProcessResult javaResult =
+ runOnJavaRaw(mainClass.name, builder.buildClasses().toArray(new byte[2][]));
+ assertThat(javaResult.stderr, containsString("IncompatibleClassChangeError"));
+
+ // Check that the code fails with an IncompatibleClassChangeError with ART.
+ ProcessResult artResult = runOnArtRaw(compiled, mainClass.name);
+ assertThat(artResult.stderr, containsString("IncompatibleClassChangeError"));
+ }
+
private Set<String> collectNewInstanceTypes(
ClassSubject clazz, String methodName, String... params) {
assertNotNull(clazz);
@@ -189,15 +227,8 @@
}
private AndroidApp runR8(AndroidApp app, Class mainClass) throws Exception {
- String config = keepMainProguardConfiguration(mainClass) + "\n"
- + "-dontobfuscate\n"
- + "-allowaccessmodification";
-
- AndroidApp compiled = compileWithR8(app, config,
- o -> {
- o.enableClassInlining = true;
- o.classInliningInstructionLimit = 10000;
- });
+ AndroidApp compiled =
+ compileWithR8(app, getProguardConfig(mainClass.getCanonicalName()), this::configure);
// Materialize file for execution.
Path generatedDexFile = temp.getRoot().toPath().resolve("classes.jar");
@@ -220,4 +251,16 @@
return compiled;
}
+
+ private String getProguardConfig(String main) {
+ return keepMainProguardConfiguration(main)
+ + "\n"
+ + "-dontobfuscate\n"
+ + "-allowaccessmodification";
+ }
+
+ private void configure(InternalOptions options) {
+ options.enableClassInlining = true;
+ options.classInliningInstructionLimit = 10000;
+ }
}
