Reflect core library moves in super calls, even in default method stubs. Always generate default method stubs for emulated methods.

RELNOTES: None.

PiperOrigin-RevId: 189423933

4 files changed, 145 insertions, 21 deletions

diff --git a/src/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java b/src/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java
index 42f1f78c97..9b43207d60 100644
--- a/src/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java
+++ b/src/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java
@@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.android.desugar.io.CoreLibraryRewriter;
 import java.util.Collection;
 import java.util.Comparator;
+import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.concurrent.ConcurrentMap;
 import org.junit.Test;
@@ -165,6 +166,58 @@ public class CoreLibrarySupportTest {
   }
 
   @Test
+  public void testGetCoreInterfaceRewritingTarget_emulatedImplementationMoved() throws Exception {
+    CoreLibrarySupport support =
+        new CoreLibrarySupport(
+            new CoreLibraryRewriter(""),
+            Thread.currentThread().getContextClassLoader(),
+            ImmutableList.of("java/util/Moved"),
+            ImmutableList.of("java/util/Map"),
+            ImmutableList.of("java/util/LinkedHashMap#forEach->java/util/Moved"),
+            ImmutableList.of());
+    assertThat(
+            support.getCoreInterfaceRewritingTarget(
+                Opcodes.INVOKEINTERFACE,
+                "java/util/Map",
+                "forEach",
+                "(Ljava/util/function/BiConsumer;)V",
+                true))
+        .isEqualTo(Map.class);
+    assertThat(
+            support.getCoreInterfaceRewritingTarget(
+                Opcodes.INVOKESPECIAL,
+                "java/util/Map",
+                "forEach",
+                "(Ljava/util/function/BiConsumer;)V",
+                true))
+        .isEqualTo(Map.class);
+    assertThat(
+            support.getCoreInterfaceRewritingTarget(
+                Opcodes.INVOKEVIRTUAL,
+                "java/util/LinkedHashMap",
+                "forEach",
+                "(Ljava/util/function/BiConsumer;)V",
+                false))
+        .isEqualTo(Map.class);
+    assertThat(
+            support.getCoreInterfaceRewritingTarget(
+                Opcodes.INVOKESPECIAL,
+                "java/util/LinkedHashMap",
+                "forEach",
+                "(Ljava/util/function/BiConsumer;)V",
+                false))
+        .isEqualTo(LinkedHashMap.class);
+    assertThat(
+            support.getCoreInterfaceRewritingTarget(
+                Opcodes.INVOKESPECIAL,
+                "java/util/HashMap",
+                "forEach",
+                "(Ljava/util/function/BiConsumer;)V",
+                false))
+        .isEqualTo(Map.class);
+  }
+
+  @Test
   public void testGetCoreInterfaceRewritingTarget_abstractMethod() throws Exception {
     CoreLibrarySupport support =
         new CoreLibrarySupport(
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibraryInvocationRewriter.java b/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibraryInvocationRewriter.java
index 77db915f4b..381a3443c7 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibraryInvocationRewriter.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibraryInvocationRewriter.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.android.desugar;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 
 import org.objectweb.asm.ClassVisitor;
@@ -65,9 +66,13 @@ public class CoreLibraryInvocationRewriter extends ClassVisitor {
         }
 
         if (opcode == Opcodes.INVOKESTATIC || opcode == Opcodes.INVOKESPECIAL) {
-          checkArgument(itf, "Expected interface to rewrite %s.%s : %s", owner, name, desc);
-          owner = InterfaceDesugaring.getCompanionClassName(coreInterfaceName);
+          checkArgument(itf || opcode == Opcodes.INVOKESPECIAL,
+              "Expected interface to rewrite %s.%s : %s", owner, name, desc);
+          owner = coreInterface.isInterface()
+              ? InterfaceDesugaring.getCompanionClassName(coreInterfaceName)
+              : checkNotNull(support.getMoveTarget(coreInterfaceName, name));
         } else {
+          checkState(coreInterface.isInterface());
           owner = coreInterfaceName + "$$Dispatch";
         }
 
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java b/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java
index fd10e5e9f1..f247074852 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java
@@ -32,6 +32,7 @@ import java.lang.reflect.Method;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Objects;
@@ -89,7 +90,8 @@ class CoreLibrarySupport {
     this.emulatedInterfaces = classBuilder.build();
 
     // We can call isRenamed and rename below b/c we initialized the necessary fields above
-    ImmutableMap.Builder<String, String> movesBuilder = ImmutableMap.builder();
+    // Use LinkedHashMap to tolerate identical duplicates
+    LinkedHashMap<String, String> movesBuilder = new LinkedHashMap<>();
     Splitter splitter = Splitter.on("->").trimResults().omitEmptyStrings();
     for (String move : memberMoves) {
       List<String> pair = splitter.splitToList(move);
@@ -103,9 +105,12 @@ class CoreLibrarySupport {
       checkArgument(!this.excludeFromEmulation.contains(pair.get(0)),
           "Retargeted invocation %s shouldn't overlap with excluded", move);
 
-      movesBuilder.put(pair.get(0), renameCoreLibrary(pair.get(1)));
+      String value = renameCoreLibrary(pair.get(1));
+      String existing = movesBuilder.put(pair.get(0), value);
+      checkArgument(existing == null || existing.equals(value),
+          "Two move destinations %s and %s configured for %s", existing, value, pair.get(0));
     }
-    this.memberMoves = movesBuilder.build();
+    this.memberMoves = ImmutableMap.copyOf(movesBuilder);
   }
 
   public boolean isRenamedCoreLibrary(String internalName) {
@@ -165,9 +170,13 @@ class CoreLibrarySupport {
    * core interface, this methods returns that interface.  This is a helper method for
    * {@link CoreLibraryInvocationRewriter}.
    *
-   * <p>Always returns an interface (or {@code null}), even if {@code owner} is a class. Can only
-   * return non-{@code null} if {@code owner} is a core library type.
+   * <p>This method can only return non-{@code null} if {@code owner} is a core library type.
+   * It usually returns an emulated interface, unless the given invocation is a super-call to a
+   * core class's implementation of an emulated method that's being moved (other implementations
+   * of emulated methods in core classes are ignored). In that case the class is returned and the
+   * caller can use {@link #getMoveTarget} to find out where to redirect the invokespecial to.
    */
+  // TODO(kmb): Rethink this API and consider combining it with getMoveTarget().
   @Nullable
   public Class<?> getCoreInterfaceRewritingTarget(
       int opcode, String owner, String name, String desc, boolean itf) {
@@ -175,15 +184,20 @@ class CoreLibrarySupport {
       // Regular desugaring handles generated classes, no emulation is needed
       return null;
     }
-    if (!itf && (opcode == Opcodes.INVOKESTATIC || opcode == Opcodes.INVOKESPECIAL)) {
-      // Ignore staticly dispatched invocations on classes--they never need rewriting
+    if (!itf && opcode == Opcodes.INVOKESTATIC) {
+      // Ignore static invocations on classes--they never need rewriting (unless moved but that's
+      // handled separately).
       return null;
     }
+    if ("<init>".equals(name)) {
+      return null; // Constructors aren't rewritten
+    }
+
     Class<?> clazz;
     if (isRenamedCoreLibrary(owner)) {
       // For renamed invocation targets we just need to do what InterfaceDesugaring does, that is,
       // only worry about invokestatic and invokespecial interface invocations; nothing to do for
-      // invokevirtual and invokeinterface.  InterfaceDesugaring ignores bootclasspath interfaces,
+      // classes and invokeinterface.  InterfaceDesugaring ignores bootclasspath interfaces,
       // so we have to do its work here for renamed interfaces.
       if (itf
           && (opcode == Opcodes.INVOKESTATIC || opcode == Opcodes.INVOKESPECIAL)) {
@@ -213,6 +227,19 @@ class CoreLibrarySupport {
       if (isExcluded(callee)) {
         return null;
       }
+
+      if (!itf && opcode == Opcodes.INVOKESPECIAL) {
+        // See if the invoked implementation is moved; note we ignore all other overrides in classes
+        Class<?> impl = clazz; // we know clazz is not an interface because !itf
+        while (impl != null) {
+          String implName = impl.getName().replace('.', '/');
+          if (getMoveTarget(implName, name) != null) {
+            return impl;
+          }
+          impl = impl.getSuperclass();
+        }
+      }
+
       Class<?> result = callee.getDeclaringClass();
       if (isRenamedCoreLibrary(result.getName().replace('.', '/'))
           || emulatedInterfaces.stream().anyMatch(emulated -> emulated.isAssignableFrom(result))) {
@@ -232,7 +259,7 @@ class CoreLibrarySupport {
       checkState(!roots.hasNext(), "Ambiguous emulation substitute: %s", callee);
       return substitute;
     } else {
-      checkArgument(opcode != Opcodes.INVOKESPECIAL,
+      checkArgument(!itf || opcode != Opcodes.INVOKESPECIAL,
           "Couldn't resolve interface super call %s.super.%s : %s", owner, name, desc);
     }
     return null;
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java b/src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java
index 960cfeb106..f0fc6d1d7e 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java
@@ -233,11 +233,13 @@ public class DefaultMethodClassFixer extends ClassVisitor {
         // superclass is also rewritten and already implements this interface, so we _must_ skip it.
         continue;
       }
-      stubMissingDefaultAndBridgeMethods(interfaceToVisit.getName().replace('.', '/'));
+      stubMissingDefaultAndBridgeMethods(
+          interfaceToVisit.getName().replace('.', '/'), mayNeedStubsForSuperclass);
     }
   }
 
-  private void stubMissingDefaultAndBridgeMethods(String implemented) {
+  private void stubMissingDefaultAndBridgeMethods(
+      String implemented, boolean mayNeedStubsForSuperclass) {
     ClassReader bytecode;
     boolean isBootclasspath;
     if (bootclasspath.isKnown(implemented)) {
@@ -258,7 +260,9 @@ public class DefaultMethodClassFixer extends ClassVisitor {
               "Couldn't find interface %s implemented by %s", implemented, internalName);
       isBootclasspath = false;
     }
-    bytecode.accept(new DefaultMethodStubber(isBootclasspath), ClassReader.SKIP_DEBUG);
+    bytecode.accept(
+        new DefaultMethodStubber(isBootclasspath, mayNeedStubsForSuperclass),
+        ClassReader.SKIP_DEBUG);
   }
 
   private Class<?> loadFromInternal(String internalName) {
@@ -281,7 +285,8 @@ public class DefaultMethodClassFixer extends ClassVisitor {
   }
 
   private void recordInheritedMethods() {
-    InstanceMethodRecorder recorder = new InstanceMethodRecorder();
+    InstanceMethodRecorder recorder =
+        new InstanceMethodRecorder(mayNeedInterfaceStubsForEmulatedSuperclass());
     String internalName = superName;
     while (internalName != null) {
       ClassReader bytecode = bootclasspath.readIfKnown(internalName);
@@ -429,11 +434,15 @@ public class DefaultMethodClassFixer extends ClassVisitor {
   private class DefaultMethodStubber extends ClassVisitor {
 
     private final boolean isBootclasspathInterface;
+    private final boolean mayNeedStubsForSuperclass;
+
     private String stubbedInterfaceName;
 
-    public DefaultMethodStubber(boolean isBootclasspathInterface) {
+    public DefaultMethodStubber(
+        boolean isBootclasspathInterface, boolean mayNeedStubsForSuperclass) {
       super(Opcodes.ASM6);
       this.isBootclasspathInterface = isBootclasspathInterface;
+      this.mayNeedStubsForSuperclass = mayNeedStubsForSuperclass;
     }
 
     @Override
@@ -472,6 +481,21 @@ public class DefaultMethodClassFixer extends ClassVisitor {
         MethodVisitor stubMethod =
             DefaultMethodClassFixer.this.visitMethod(access, name, desc, (String) null, exceptions);
 
+        String receiverName = stubbedInterfaceName;
+        String owner = InterfaceDesugaring.getCompanionClassName(stubbedInterfaceName);
+        if (mayNeedStubsForSuperclass) {
+          // Reflect what CoreLibraryInvocationRewriter would do if it encountered a super-call to a
+          // moved implementation of an emulated method.  Equivalent to emitting the invokespecial
+          // super call here and relying on CoreLibraryInvocationRewriter for the rest
+          Class<?> emulatedImplementation =
+              coreLibrarySupport.getCoreInterfaceRewritingTarget(
+                  Opcodes.INVOKESPECIAL, superName, name, desc, /*itf=*/ false);
+          if (emulatedImplementation != null && !emulatedImplementation.isInterface()) {
+            receiverName = emulatedImplementation.getName().replace('.', '/');
+            owner = checkNotNull(coreLibrarySupport.getMoveTarget(receiverName, name));
+          }
+        }
+
         int slot = 0;
         stubMethod.visitVarInsn(Opcodes.ALOAD, slot++); // load the receiver
         Type neededType = Type.getMethodType(desc);
@@ -481,10 +505,10 @@ public class DefaultMethodClassFixer extends ClassVisitor {
         }
         stubMethod.visitMethodInsn(
             Opcodes.INVOKESTATIC,
-            InterfaceDesugaring.getCompanionClassName(stubbedInterfaceName),
+            owner,
             name,
-            InterfaceDesugaring.companionDefaultMethodDescriptor(stubbedInterfaceName, desc),
-            /*itf*/ false);
+            InterfaceDesugaring.companionDefaultMethodDescriptor(receiverName, desc),
+            /*itf=*/ false);
         stubMethod.visitInsn(neededType.getReturnType().getOpcode(Opcodes.IRETURN));
 
         stubMethod.visitMaxs(0, 0); // rely on class writer to compute these
@@ -563,8 +587,13 @@ public class DefaultMethodClassFixer extends ClassVisitor {
 
   private class InstanceMethodRecorder extends ClassVisitor {
 
-    public InstanceMethodRecorder() {
+    private final boolean ignoreEmulatedMethods;
+
+    private String className;
+
+    public InstanceMethodRecorder(boolean ignoreEmulatedMethods) {
       super(Opcodes.ASM6);
+      this.ignoreEmulatedMethods = ignoreEmulatedMethods;
     }
 
     @Override
@@ -576,13 +605,23 @@ public class DefaultMethodClassFixer extends ClassVisitor {
         String superName,
         String[] interfaces) {
       checkArgument(BitFlags.noneSet(access, Opcodes.ACC_INTERFACE));
+      className = name;  // updated every time we start visiting another superclass
       super.visit(version, access, name, signature, superName, interfaces);
     }
 
     @Override
     public MethodVisitor visitMethod(
         int access, String name, String desc, String signature, String[] exceptions) {
-      // TODO(kmb): what if this method only exists on some devices, e.g., ArrayList.spliterator?
+      if (ignoreEmulatedMethods
+          && BitFlags.noneSet(access, Opcodes.ACC_STATIC) // short-circuit
+          && coreLibrarySupport.getCoreInterfaceRewritingTarget(
+                  Opcodes.INVOKEVIRTUAL, className, name, desc, /*itf=*/ false)
+              != null) {
+        // *don't* record emulated core library method implementations in immediate subclasses of
+        // emulated core library clasess so that they can be stubbed (since the inherited
+        // implementation may be missing at runtime).
+        return null;
+      }
       recordIfInstanceMethod(access, name, desc);
       return null;
     }