Introduce test in desugared lib resolving all invokes
- Ensure all desugared library invokes resolve or are
in an allowed white list
- Simplify and Fix ApiFlip conversions
- Fix desugared library flag
- Clean emulated interface to avoid dead code, removing
non resolving invokes and reducing L8 output code size.
Bug: b/244294200
Change-Id: I2295fe2b3ce1610c0069b7da29310eb5b7e69fa6
diff --git a/buildSrc/src/main/java/desugaredlibrary/CustomConversionAsmRewriteDescription.java b/buildSrc/src/main/java/desugaredlibrary/CustomConversionAsmRewriteDescription.java
index 52d878b..089a293 100644
--- a/buildSrc/src/main/java/desugaredlibrary/CustomConversionAsmRewriteDescription.java
+++ b/buildSrc/src/main/java/desugaredlibrary/CustomConversionAsmRewriteDescription.java
@@ -12,7 +12,6 @@
public class CustomConversionAsmRewriteDescription {
static final String WRAP_CONVERT = "wrap_convert";
- static final String INVERTED_WRAP_CONVERT = "inverted_wrap_convert";
static final String CONVERT = "convert";
private static final Set<String> ENUM_WRAP_CONVERT_OWNER =
diff --git a/buildSrc/src/main/java/desugaredlibrary/CustomConversionAsmRewriter.java b/buildSrc/src/main/java/desugaredlibrary/CustomConversionAsmRewriter.java
index c20de96..0848c09 100644
--- a/buildSrc/src/main/java/desugaredlibrary/CustomConversionAsmRewriter.java
+++ b/buildSrc/src/main/java/desugaredlibrary/CustomConversionAsmRewriter.java
@@ -6,7 +6,6 @@
import static desugaredlibrary.AsmRewriter.ASM_VERSION;
import static desugaredlibrary.CustomConversionAsmRewriteDescription.CONVERT;
-import static desugaredlibrary.CustomConversionAsmRewriteDescription.INVERTED_WRAP_CONVERT;
import static desugaredlibrary.CustomConversionAsmRewriteDescription.WRAP_CONVERT;
import static desugaredlibrary.CustomConversionAsmRewriter.CustomConversionVersion.LEGACY;
import static org.objectweb.asm.Opcodes.INVOKESTATIC;
@@ -133,51 +132,46 @@
@Override
public void visitMethodInsn(
int opcode, String owner, String name, String descriptor, boolean isInterface) {
- if (opcode == INVOKESTATIC) {
- if (name.equals(WRAP_CONVERT)) {
- convertInvoke(
- opcode,
- owner,
- descriptor,
- isInterface,
- javaWrapConvertOwnerMap,
- j$WrapConvertOwnerMap);
- return;
- }
- if (name.equals(INVERTED_WRAP_CONVERT)) {
- convertInvoke(
- opcode,
- owner,
- descriptor,
- isInterface,
- j$WrapConvertOwnerMap,
- javaWrapConvertOwnerMap);
- return;
- }
+ if (opcode == INVOKESTATIC && name.equals(WRAP_CONVERT)) {
+ convertInvoke(opcode, owner, descriptor, isInterface);
+ return;
}
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
}
- private void convertInvoke(
- int opcode,
- String owner,
- String descriptor,
- boolean isInterface,
- Map<String, String> javaMap,
- Map<String, String> j$Map) {
- if (!javaMap.containsKey(owner)
- || !j$Map.containsKey(owner)
- || !(owner.startsWith("java") || owner.startsWith("j$"))) {
- throw new RuntimeException("Cannot transform wrap_convert method for " + owner);
+ private String extractFirstArg(String descriptor) {
+ assert descriptor.startsWith("(L");
+ int end = descriptor.indexOf(';');
+ assert end > 2;
+ return descriptor.substring(2, end);
+ }
+
+ private void convertInvoke(int opcode, String owner, String descriptor, boolean isInterface) {
+ String firstArg = extractFirstArg(descriptor);
+ assert sameBaseName(firstArg, owner);
+ if (!javaWrapConvertOwnerMap.containsKey(owner)
+ || !j$WrapConvertOwnerMap.containsKey(owner)
+ || !(firstArg.startsWith("java") || firstArg.startsWith("j$"))) {
+ throw new RuntimeException(
+ "Cannot transform wrap_convert method for " + firstArg + " (owner: " + owner + ")");
}
- if (owner.startsWith("java")) {
- String newOwner = j$Map.get(owner);
+ if (firstArg.startsWith("java")) {
+ String newOwner = javaWrapConvertOwnerMap.get(owner);
super.visitMethodInsn(opcode, newOwner, CONVERT, descriptor, isInterface);
return;
}
- assert owner.startsWith("j$");
- String newOwner = javaMap.get(owner);
+ assert firstArg.startsWith("j$");
+ String newOwner = j$WrapConvertOwnerMap.get(owner);
super.visitMethodInsn(opcode, newOwner, CONVERT, descriptor, isInterface);
}
+
+ private boolean sameBaseName(String firstArg, String owner) {
+ assert owner.startsWith("j$");
+ if (firstArg.equals(owner)) {
+ return true;
+ }
+ String javaName = owner.replace("j$", "java");
+ return firstArg.equals(javaName);
+ }
}
}
diff --git a/src/library_desugar/java/j$/nio/file/spi/FileSystemProvider.java b/src/library_desugar/java/j$/nio/file/spi/FileSystemProvider.java
index 8db22b3..510d621 100644
--- a/src/library_desugar/java/j$/nio/file/spi/FileSystemProvider.java
+++ b/src/library_desugar/java/j$/nio/file/spi/FileSystemProvider.java
@@ -5,7 +5,7 @@
package j$.nio.file.spi;
public class FileSystemProvider {
- public static java.nio.file.spi.FileSystemProvider inverted_wrap_convert(
+ public static java.nio.file.spi.FileSystemProvider wrap_convert(
j$.nio.file.spi.FileSystemProvider provider) {
// Rewritten in ASM to the wrapper method.
return null;
diff --git a/src/library_desugar/java/j$/util/stream/Stream.java b/src/library_desugar/java/j$/util/stream/Stream.java
index dc5861d..7e06e4a 100644
--- a/src/library_desugar/java/j$/util/stream/Stream.java
+++ b/src/library_desugar/java/j$/util/stream/Stream.java
@@ -6,14 +6,6 @@
public class Stream<T> {
- public static java.util.stream.Stream<?> inverted_wrap_convert(j$.util.stream.Stream<?> stream) {
- return null;
- }
-
- public static j$.util.stream.Stream<?> inverted_wrap_convert(java.util.stream.Stream<?> stream) {
- return null;
- }
-
public static java.util.stream.Stream<?> wrap_convert(j$.util.stream.Stream<?> stream) {
return null;
}
diff --git a/src/library_desugar/java/java/adapter/HybridFileSystemProvider.java b/src/library_desugar/java/java/adapter/HybridFileSystemProvider.java
index 5be05a8..e34e571 100644
--- a/src/library_desugar/java/java/adapter/HybridFileSystemProvider.java
+++ b/src/library_desugar/java/java/adapter/HybridFileSystemProvider.java
@@ -29,7 +29,7 @@
Class.forName("java.nio.file.FileSystems");
j$.nio.file.FileSystem fileSystem = FileSystems.getDefault();
j$.nio.file.spi.FileSystemProvider provider = fileSystem.provider();
- return j$.nio.file.spi.FileSystemProvider.inverted_wrap_convert(provider);
+ return j$.nio.file.spi.FileSystemProvider.wrap_convert(provider);
} catch (ClassNotFoundException ignored) {
// We reach this path is API < 26.
}
diff --git a/src/library_desugar/java/java/util/stream/FlatMapApiFlips.java b/src/library_desugar/java/java/util/stream/FlatMapApiFlips.java
index b9e1b9e..ac81093 100644
--- a/src/library_desugar/java/java/util/stream/FlatMapApiFlips.java
+++ b/src/library_desugar/java/java/util/stream/FlatMapApiFlips.java
@@ -37,6 +37,7 @@
this.function = function;
}
+ @SuppressWarnings("unchecked")
private R flipStream(R maybeStream) {
if (maybeStream == null) {
return null;
@@ -89,6 +90,7 @@
this.function = function;
}
+ @SuppressWarnings("unchecked")
private R flipStream(R maybeStream) {
if (maybeStream == null) {
return null;
@@ -115,6 +117,7 @@
this.function = function;
}
+ @SuppressWarnings("unchecked")
private R flipStream(R maybeStream) {
if (maybeStream == null) {
return null;
@@ -143,6 +146,7 @@
this.function = function;
}
+ @SuppressWarnings("unchecked")
private R flipStream(R maybeStream) {
if (maybeStream == null) {
return null;
diff --git a/src/library_desugar/java/java/util/stream/StackWalkerApiFlips.java b/src/library_desugar/java/java/util/stream/StackWalkerApiFlips.java
index 300c5ad..0951704 100644
--- a/src/library_desugar/java/java/util/stream/StackWalkerApiFlips.java
+++ b/src/library_desugar/java/java/util/stream/StackWalkerApiFlips.java
@@ -22,17 +22,16 @@
this.function = function;
}
+ @SuppressWarnings("unchecked")
private T flipStream(T maybeStream) {
if (maybeStream == null) {
return null;
}
if (maybeStream instanceof java.util.stream.Stream<?>) {
- return (T)
- j$.util.stream.Stream.inverted_wrap_convert((java.util.stream.Stream<?>) maybeStream);
+ return (T) j$.util.stream.Stream.wrap_convert((java.util.stream.Stream<?>) maybeStream);
}
if (maybeStream instanceof j$.util.stream.Stream<?>) {
- return (T)
- j$.util.stream.Stream.inverted_wrap_convert((j$.util.stream.Stream<?>) maybeStream);
+ return (T) j$.util.stream.Stream.wrap_convert((j$.util.stream.Stream<?>) maybeStream);
}
throw exception("java.util.stream.Stream", maybeStream.getClass());
}
diff --git a/src/library_desugar/jdk11/desugar_jdk_libs_nio.json b/src/library_desugar/jdk11/desugar_jdk_libs_nio.json
index 5a1d513..9230e9b 100644
--- a/src/library_desugar/jdk11/desugar_jdk_libs_nio.json
+++ b/src/library_desugar/jdk11/desugar_jdk_libs_nio.json
@@ -390,7 +390,6 @@
"rewrite_prefix": {
"desugar.": "j$.desugar.",
"libcore.": "j$.libcore.",
- "java.lang.Desugar": "j$.lang.Desugar",
"sun.security.action.": "j$.sun.security.action."
}
},
@@ -450,6 +449,9 @@
},
"java.nio.file.Path": {
"j$.nio.file.Path": "java.nio.file.Path"
+ },
+ "java.nio.file.WatchEvent": {
+ "j$.nio.file.WatchEvent": "java.nio.file.WatchEvent"
}
},
"retarget_method": {
@@ -510,6 +512,9 @@
},
{
"api_level_below_or_equal": 18,
+ "rewrite_prefix": {
+ "java.lang.DesugarCharacter": "j$.lang.DesugarCharacter"
+ },
"retarget_static_field": {
"sun.nio.cs.US_ASCII sun.nio.cs.US_ASCII#INSTANCE": "java.nio.charset.Charset java.nio.charset.StandardCharsets#US_ASCII",
"sun.nio.cs.ISO_8859_1 sun.nio.cs.ISO_8859_1#INSTANCE": "java.nio.charset.Charset java.nio.charset.StandardCharsets#ISO_8859_1",
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/itf/EmulatedInterfaceApplicationRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/itf/EmulatedInterfaceApplicationRewriter.java
index c764377..fd607ee 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/itf/EmulatedInterfaceApplicationRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/itf/EmulatedInterfaceApplicationRewriter.java
@@ -13,6 +13,7 @@
import com.android.tools.r8.graph.GenericSignature;
import com.android.tools.r8.graph.GenericSignature.ClassSignature;
import com.android.tools.r8.graph.MethodCollection.MethodCollectionFactory;
+import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.utils.IterableUtils;
import java.util.ArrayList;
import java.util.Collections;
@@ -59,9 +60,8 @@
}
DexType newType = emulatedInterfaces.get(emulatedInterface.type);
assert newType != null;
- DexEncodedMethod[] newVirtualMethods =
- renameHolder(emulatedInterface.virtualMethods(), newType);
- DexEncodedMethod[] newDirectMethods = renameHolder(emulatedInterface.directMethods(), newType);
+ DexEncodedMethod[] newVirtualMethods = computeNewVirtualMethods(emulatedInterface, newType);
+ DexEncodedMethod[] newDirectMethods = DexEncodedMethod.EMPTY_ARRAY;
assert emulatedInterface.getSuperType() == appView.dexItemFactory().objectType;
assert !emulatedInterface.hasFields();
assert emulatedInterface.getNestHost() == null;
@@ -116,11 +116,24 @@
return newInterfaces;
}
- private DexEncodedMethod[] renameHolder(Iterable<DexEncodedMethod> methods, DexType newName) {
- List<DexEncodedMethod> methodArray = IterableUtils.toNewArrayList(methods);
+ private DexEncodedMethod[] computeNewVirtualMethods(
+ DexProgramClass emulatedInterface, DexType newName) {
+ Iterable<ProgramMethod> emulatedMethods =
+ emulatedInterface.virtualProgramMethods(
+ m ->
+ appView
+ .options()
+ .machineDesugaredLibrarySpecification
+ .getEmulatedInterfaceEmulatedDispatchMethodDescriptor(m.getReference())
+ != null);
+ List<ProgramMethod> methodArray = IterableUtils.toNewArrayList(emulatedMethods);
DexEncodedMethod[] newMethods = new DexEncodedMethod[methodArray.size()];
for (int i = 0; i < newMethods.length; i++) {
- newMethods[i] = methodArray.get(i).toRenamedHolderMethod(newName, appView.dexItemFactory());
+ newMethods[i] =
+ methodArray
+ .get(i)
+ .getDefinition()
+ .toRenamedHolderMethod(newName, appView.dexItemFactory());
}
return newMethods;
}
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryContentTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryContentTest.java
index 2838f2c..c3243c0 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryContentTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryContentTest.java
@@ -15,20 +15,39 @@
import static org.hamcrest.core.StringContains.containsString;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.desugar.desugaredlibrary.test.CompilationSpecification;
import com.android.tools.r8.desugar.desugaredlibrary.test.LibraryDesugaringSpecification;
+import com.android.tools.r8.dex.ApplicationReader;
+import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
+import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DirectMappedDexApplication;
+import com.android.tools.r8.graph.MethodResolutionResult;
+import com.android.tools.r8.synthesis.SyntheticItems.GlobalSyntheticsStrategy;
import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.Pair;
+import com.android.tools.r8.utils.Timing;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.FoundClassSubject;
import com.android.tools.r8.utils.codeinspector.FoundMethodSubject;
import com.android.tools.r8.utils.codeinspector.InstructionSubject;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
import java.util.List;
+import java.util.Map;
+import java.util.Set;
import org.hamcrest.CoreMatchers;
import org.junit.Assume;
import org.junit.Test;
@@ -39,6 +58,30 @@
@RunWith(Parameterized.class)
public class DesugaredLibraryContentTest extends DesugaredLibraryTestBase {
+ // The class sun.misc.Unsafe is runnable on Android despite not being in android.jar.
+ private static final Set<String> ALLOWED_MISSING_HOLDER = ImmutableSet.of("sun.misc.Unsafe");
+ private static final Set<String> ALLOWED_MISSING_METHOD =
+ ImmutableSet.of(
+ // The takeWhile/dropWhile methods are present in the wrappers but not yet present on
+ // android.jar
+ // leading to NoSuchMethod errors, yet, we keep them for subsequent android versions.
+ "java.util.stream.IntStream"
+ + " java.util.stream.IntStream.dropWhile(java.util.function.IntPredicate)",
+ "java.util.stream.Stream java.util.stream.Stream.takeWhile(java.util.function.Predicate)",
+ "java.util.stream.LongStream"
+ + " java.util.stream.LongStream.dropWhile(java.util.function.LongPredicate)",
+ "java.util.stream.DoubleStream"
+ + " java.util.stream.DoubleStream.takeWhile(java.util.function.DoublePredicate)",
+ "java.util.stream.IntStream"
+ + " java.util.stream.IntStream.takeWhile(java.util.function.IntPredicate)",
+ "java.util.stream.Stream java.util.stream.Stream.dropWhile(java.util.function.Predicate)",
+ "java.util.stream.LongStream"
+ + " java.util.stream.LongStream.takeWhile(java.util.function.LongPredicate)",
+ "java.util.stream.DoubleStream"
+ + " java.util.stream.DoubleStream.dropWhile(java.util.function.DoublePredicate)",
+ // FileStore.getBlockSize() was added in 33 which confuses the required library (30).
+ "long java.nio.file.FileStore.getBlockSize()");
+
private final TestParameters parameters;
private final CompilationSpecification compilationSpecification;
private final LibraryDesugaringSpecification libraryDesugaringSpecification;
@@ -67,7 +110,63 @@
.apply(libraryDesugaringSpecification::configureL8TestBuilder)
.compile()
.assertNoMessages()
- .inspect(this::assertCorrect);
+ .inspect(this::assertCorrect)
+ .inspect(this::assertAllInvokeResolve);
+ }
+
+ private void assertAllInvokeResolve(CodeInspector inspector) throws IOException {
+ AndroidApp build =
+ AndroidApp.builder()
+ .addLibraryFiles(libraryDesugaringSpecification.getLibraryFiles())
+ .build();
+ DirectMappedDexApplication libHolder =
+ new ApplicationReader(build, inspector.getApplication().options, Timing.empty())
+ .read()
+ .toDirect();
+ DirectMappedDexApplication finalApp =
+ inspector
+ .getApplication()
+ .toDirect()
+ .builder()
+ .replaceLibraryClasses(libHolder.libraryClasses())
+ .build();
+ AppInfoWithClassHierarchy appInfo =
+ AppView.createForD8(
+ AppInfo.createInitialAppInfo(
+ finalApp, GlobalSyntheticsStrategy.forNonSynthesizing()))
+ .appInfoForDesugaring();
+ Map<DexMethod, Object> failures = new IdentityHashMap<>();
+ for (FoundClassSubject clazz : inspector.allClasses()) {
+ for (FoundMethodSubject method : clazz.allMethods()) {
+ if (method.hasCode()) {
+ for (InstructionSubject instruction : method.instructions(InstructionSubject::isInvoke)) {
+ assertInvokeResolution(instruction, appInfo, method, failures);
+ }
+ }
+ }
+ }
+ for (DexMethod dexMethod : new HashSet<>(failures.keySet())) {
+ if (ALLOWED_MISSING_HOLDER.contains(dexMethod.getHolderType().toString())) {
+ failures.remove(dexMethod);
+ } else if (ALLOWED_MISSING_METHOD.contains(dexMethod.toString())) {
+ failures.remove(dexMethod);
+ }
+ }
+ assertTrue(failures.isEmpty());
+ }
+
+ private void assertInvokeResolution(
+ InstructionSubject instruction,
+ AppInfoWithClassHierarchy appInfo,
+ FoundMethodSubject context,
+ Map<DexMethod, Object> failures) {
+ DexMethod method = instruction.getMethod();
+ assert method != null;
+ MethodResolutionResult methodResolutionResult =
+ appInfo.unsafeResolveMethodDueToDexFormatLegacy(method);
+ if (methodResolutionResult.isFailedResolution()) {
+ failures.put(method, new Pair(context, methodResolutionResult));
+ }
}
@Test
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
index aeb419e..b569a57 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
@@ -178,6 +178,10 @@
}
}
+ public DexApplication getApplication() {
+ return application;
+ }
+
public DexItemFactory getFactory() {
return dexItemFactory;
}
