Version 2.0.56 Cherry-pick: Desugared library: allow invalid library superclasses CL: https://r8-review.googlesource.com/c/r8/+/49905 Bug: 151969439 Change-Id: I7d7d4a076c58616fd565bf25302d9e4b28a108da
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java index 9bc4e43..eefcb5d 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 = "2.0.55"; + public static final String LABEL = "2.0.56"; private Version() { }
diff --git a/src/main/java/com/android/tools/r8/errors/InvalidLibrarySuperclassDiagnostic.java b/src/main/java/com/android/tools/r8/errors/InvalidLibrarySuperclassDiagnostic.java new file mode 100644 index 0000000..2392f0a --- /dev/null +++ b/src/main/java/com/android/tools/r8/errors/InvalidLibrarySuperclassDiagnostic.java
@@ -0,0 +1,71 @@ +// Copyright (c) 2020, 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; + +import com.android.tools.r8.Keep; +import com.android.tools.r8.origin.Origin; +import com.android.tools.r8.position.Position; +import com.android.tools.r8.references.ClassReference; +import com.android.tools.r8.references.MethodReference; +import com.android.tools.r8.utils.StringUtils; +import java.util.List; + +/** + * Diagnostic for super types of library classes which are not library classes but required for + * desugaring. + */ +@Keep +public class InvalidLibrarySuperclassDiagnostic implements DesugarDiagnostic { + + private final Origin origin; + private final List<MethodReference> methods; + private final ClassReference libraryType; + private final ClassReference invalidSuperType; + private final String message; + + public InvalidLibrarySuperclassDiagnostic( + Origin origin, + ClassReference libraryType, + ClassReference invalidSuperType, + String message, + List<MethodReference> methods) { + assert origin != null; + assert libraryType != null; + assert invalidSuperType != null; + assert message != null; + this.origin = origin; + this.libraryType = libraryType; + this.invalidSuperType = invalidSuperType; + this.message = message; + this.methods = methods; + } + + @Override + public Origin getOrigin() { + return origin; + } + + @Override + public Position getPosition() { + return Position.UNKNOWN; + } + + @Override + public String getDiagnosticMessage() { + StringBuilder builder = + new StringBuilder() + .append("Superclass `") + .append(invalidSuperType.getTypeName()) + .append("` of library class `") + .append(libraryType.getTypeName()) + .append("` is ") + .append(message) + .append( + ". A superclass of a library class should be a library class. This is required for" + + " the desugaring of "); + StringUtils.append(builder, methods, ", ", StringUtils.BraceType.NONE); + return builder.toString(); + } +}
diff --git a/src/main/java/com/android/tools/r8/graph/DexMethod.java b/src/main/java/com/android/tools/r8/graph/DexMethod.java index a1e98f7..de97baf 100644 --- a/src/main/java/com/android/tools/r8/graph/DexMethod.java +++ b/src/main/java/com/android/tools/r8/graph/DexMethod.java
@@ -8,6 +8,11 @@ import com.android.tools.r8.naming.NamingLens; import com.google.common.collect.Maps; import java.util.Map; +import com.android.tools.r8.references.MethodReference; +import com.android.tools.r8.references.Reference; +import com.android.tools.r8.references.TypeReference; +import java.util.ArrayList; +import java.util.List; public class DexMethod extends Descriptor<DexEncodedMethod, DexMethod> implements PresortedComparable<DexMethod> { @@ -35,6 +40,22 @@ return "Method " + holder + "." + name + " " + proto.toString(); } + public MethodReference asMethodReference(AppView<?> appView) { + List<TypeReference> parameters = new ArrayList<>(); + for (DexType value : proto.parameters.values) { + parameters.add(Reference.typeFromDescriptor(value.toDescriptorString())); + } + TypeReference returnType = + proto.returnType == appView.dexItemFactory().voidType + ? null + : Reference.typeFromDescriptor(proto.returnType.toDescriptorString()); + return Reference.method( + Reference.classFromDescriptor(holder.toDescriptorString()), + name.toString(), + parameters, + returnType); + } + public int getArity() { return proto.parameters.size(); }
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java index 1dce2ed..0ec2004 100644 --- a/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java +++ b/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java
@@ -289,11 +289,42 @@ if (current.type == typeToInherit) { return true; } - current = appView.definitionFor(current.superType).asLibraryClass(); + DexClass dexClass = appView.definitionFor(current.superType); + if (dexClass == null || dexClass.isClasspathClass()) { + reportInvalidLibrarySupertype(current, rewritableMethods.getEmulatedDispatchMethods()); + return false; + } else if (dexClass.isProgramClass()) { + // If dexClass is a program class, then it is already correctly desugared. + return false; + } + current = dexClass.asLibraryClass(); } return false; } + private void reportInvalidLibrarySupertype( + DexLibraryClass libraryClass, Set<DexMethod> retarget) { + DexClass dexClass = appView.definitionFor(libraryClass.superType); + String message; + if (dexClass == null) { + message = "missing"; + } else if (dexClass.isClasspathClass()) { + message = "a classpath class"; + } else { + message = "INVALID"; + assert false; + } + appView + .options() + .warningInvalidLibrarySuperclassForDesugar( + dexClass == null ? libraryClass.getOrigin() : dexClass.getOrigin(), + libraryClass.type, + libraryClass.superType, + message, + retarget, + appView); + } + private List<DexEncodedMethod> addInterfacesAndForwardingMethods( DexProgramClass clazz, List<DexMethod> dexMethods) { // BackportedMethodRewriter emulate dispatch: insertion of a marker interface & forwarding
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 2de4c37..21ff710 100644 --- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java +++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -21,6 +21,7 @@ import com.android.tools.r8.errors.IncompleteNestNestDesugarDiagnosic; import com.android.tools.r8.errors.InterfaceDesugarMissingTypeDiagnostic; import com.android.tools.r8.errors.InvalidDebugInfoException; +import com.android.tools.r8.errors.InvalidLibrarySuperclassDiagnostic; import com.android.tools.r8.errors.MissingNestHostNestDesugarDiagnostic; import com.android.tools.r8.experimental.graphinfo.GraphConsumer; import com.android.tools.r8.features.FeatureSplitConfiguration; @@ -61,6 +62,7 @@ import java.util.TreeSet; import java.util.function.BiPredicate; import java.util.function.Consumer; +import java.util.stream.Collectors; import org.objectweb.asm.Opcodes; public class InternalOptions { @@ -650,6 +652,8 @@ /** A set of dexitems we have reported missing to dedupe warnings. */ private final Set<DexItem> reportedMissingForDesugaring = Sets.newConcurrentHashSet(); + private final Set<DexItem> invalidLibraryClasses = Sets.newConcurrentHashSet(); + public void errorMissingClassMissingNestHost(DexClass compiledClass) { throw reporter.fatalError(messageErrorMissingNestHost(compiledClass)); } @@ -793,6 +797,26 @@ } } + public void warningInvalidLibrarySuperclassForDesugar( + Origin origin, + DexType libraryType, + DexType invalidSuperType, + String message, + Set<DexMethod> retarget, + AppView<?> appView) { + if (invalidLibraryClasses.add(invalidSuperType)) { + reporter.warning( + new InvalidLibrarySuperclassDiagnostic( + origin, + Reference.classFromDescriptor(libraryType.toDescriptorString()), + Reference.classFromDescriptor(invalidSuperType.toDescriptorString()), + message, + retarget.stream() + .map(method -> method.asMethodReference(appView)) + .collect(Collectors.toList()))); + } + } + public void warningMissingEnclosingMember(DexType clazz, Origin origin, int version) { TypeVersionPair pair = new TypeVersionPair(version, clazz); synchronized (missingEnclosingMembers) {
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/InvalidLibraryTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/InvalidLibraryTest.java new file mode 100644 index 0000000..c5c7f22 --- /dev/null +++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/InvalidLibraryTest.java
@@ -0,0 +1,176 @@ +// Copyright (c) 2020, 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.desugar.desugaredlibrary; + +import static org.hamcrest.core.StringContains.containsString; + +import com.android.tools.r8.TestDiagnosticMessages; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.errors.InvalidLibrarySuperclassDiagnostic; +import com.android.tools.r8.utils.AndroidApiLevel; +import com.android.tools.r8.utils.BooleanUtils; +import com.android.tools.r8.utils.StringUtils; +import java.nio.file.Path; +import java.time.Instant; +import java.util.Date; +import java.util.List; +import org.junit.Assume; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class InvalidLibraryTest extends DesugaredLibraryTestBase { + + private static Path customLib; + private static Path superclassAsClasspath; + private static final String EXPECTED_RESULT = + StringUtils.lines("1970-01-02T10:17:36.789Z", "1970-01-12T10:20:54.321123456Z"); + private static final String INVALID_RESULT = + StringUtils.lines("1970-01-02T10:17:36.789Z", "1970-01-12T10:20:54.321Z"); + + private final TestParameters parameters; + private final boolean shrinkDesugaredLibrary; + + @Parameterized.Parameters(name = "{1}, shrinkDesugaredLibrary: {0}") + public static List<Object[]> data() { + return buildParameters( + BooleanUtils.values(), getTestParameters().withDexRuntimes().withAllApiLevels().build()); + } + + public InvalidLibraryTest(boolean shrinkDesugaredLibrary, TestParameters parameters) { + this.shrinkDesugaredLibrary = shrinkDesugaredLibrary; + this.parameters = parameters; + } + + @BeforeClass + public static void compileCustomLib() throws Exception { + customLib = + testForD8(getStaticTemp()) + .addProgramClasses(CustomLibraryClass.class) + .setMinApi(AndroidApiLevel.B) + .compile() + .writeToZip(); + superclassAsClasspath = + testForD8(getStaticTemp()) + .addProgramClasses(SuperLibraryClass.class) + .setMinApi(AndroidApiLevel.B) + .compile() + .writeToZip(); + } + + @Test + public void testProgramSupertype() throws Exception { + KeepRuleConsumer keepRuleConsumer = createKeepRuleConsumer(parameters); + testForD8() + .setMinApi(parameters.getApiLevel()) + .addProgramClasses( + Executor.class, SuperLibraryClass.class, LocalClass.class, LocalClassOverride.class) + .addLibraryClasses(CustomLibraryClass.class) + .enableCoreLibraryDesugaring(parameters.getApiLevel(), keepRuleConsumer) + .compile() + .addDesugaredCoreLibraryRunClassPath( + this::buildDesugaredLibrary, + parameters.getApiLevel(), + keepRuleConsumer.get(), + shrinkDesugaredLibrary) + .addRunClasspathFiles(customLib) + .run(parameters.getRuntime(), Executor.class) + .assertSuccessWithOutput(EXPECTED_RESULT); + } + + @Test + public void testClasspathSupertype() throws Exception { + Assume.assumeTrue(requiresAnyCoreLibDesugaring(parameters)); + KeepRuleConsumer keepRuleConsumer = createKeepRuleConsumer(parameters); + testForD8() + .setMinApi(parameters.getApiLevel()) + .addProgramClasses(Executor.class, LocalClass.class, LocalClassOverride.class) + .addClasspathClasses(SuperLibraryClass.class) + .addLibraryClasses(CustomLibraryClass.class) + .enableCoreLibraryDesugaring(parameters.getApiLevel(), keepRuleConsumer) + .compile() + .addDesugaredCoreLibraryRunClassPath( + this::buildDesugaredLibrary, + parameters.getApiLevel(), + keepRuleConsumer.get(), + shrinkDesugaredLibrary) + .addRunClasspathFiles(customLib, superclassAsClasspath) + .run(parameters.getRuntime(), Executor.class) + // The code requires desugaring to be run correctly, but with the classpath superclass, + // desugaring is incorrectly performed. The code therefore falls-backs to the default + // implementation in Date, which happens to be correct in one case, but incorrect + // in the other case (Warning was raised). + .assertSuccessWithOutput(INVALID_RESULT); + } + + @Test + public void testNullSupertype() throws Exception { + Assume.assumeTrue(requiresAnyCoreLibDesugaring(parameters)); + KeepRuleConsumer keepRuleConsumer = createKeepRuleConsumer(parameters); + testForD8() + .setMinApi(parameters.getApiLevel()) + .addProgramClasses(Executor.class, LocalClass.class, LocalClassOverride.class) + .addLibraryClasses(CustomLibraryClass.class) + .enableCoreLibraryDesugaring(parameters.getApiLevel(), keepRuleConsumer) + .compile() + .addDesugaredCoreLibraryRunClassPath( + this::buildDesugaredLibrary, + parameters.getApiLevel(), + keepRuleConsumer.get(), + shrinkDesugaredLibrary) + .addRunClasspathFiles(customLib, superclassAsClasspath) + .run(parameters.getRuntime(), Executor.class) + // The code requires desugaring to be run correctly, but with the missing supertype, + // desugaring could not be performed and the code cannot simply run (Warning was raised). + .assertFailureWithErrorThatMatches(containsString("NoSuchMethodError")); + } + + private void assertWarningInvalidLibrary(TestDiagnosticMessages testDiagnosticMessages) { + assert testDiagnosticMessages.getWarnings().stream() + .anyMatch(diagnostic -> diagnostic instanceof InvalidLibrarySuperclassDiagnostic); + } + + static class Executor { + public static void main(String[] args) { + System.out.println(new LocalClass(123456789).toInstant()); + System.out.println(getOverrideAsLocalClass().toInstant()); + } + + public static LocalClass getOverrideAsLocalClass() { + return new LocalClassOverride(987654321); + } + } + + static class SuperLibraryClass extends Date { + public SuperLibraryClass(int nanos) { + super(nanos); + } + } + + static class CustomLibraryClass extends SuperLibraryClass { + public CustomLibraryClass(int nanos) { + super(nanos); + } + } + + static class LocalClass extends CustomLibraryClass { + public LocalClass(int nanos) { + super(nanos); + } + } + + static class LocalClassOverride extends LocalClass { + public LocalClassOverride(int nanos) { + super(nanos); + } + + @Override + public Instant toInstant() { + return super.toInstant().plusNanos(123456); + } + } +}