More stable naming scheme for lambda classes in desugared android code

RELNOTES: More stable naming scheme for lambda classes in desugared android code

--
PiperOrigin-RevId: 148506830
MOS_MIGRATED_REVID=148506830

5 files changed, 71 insertions, 40 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.java b/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.java
index 2d973de959..9dc07d7b29 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.java
@@ -184,12 +184,9 @@ class Desugar {
               visitor = new Java7Compatibility(visitor, readerFactory);
             }
 
-            visitor = new LambdaDesugaring(
-                    visitor,
-                    loader,
-                    lambdas,
-                    interfaceLambdaMethodCollector,
-                    allowDefaultMethods);
+            visitor =
+                new LambdaDesugaring(
+                    visitor, loader, lambdas, interfaceLambdaMethodCollector, allowDefaultMethods);
 
             reader.accept(visitor, 0);
 
@@ -225,7 +222,7 @@ class Desugar {
             visitor = new Java7Compatibility(visitor, (ClassReaderFactory) null);
           }
 
-          LambdaClassFixer lambdaFixer =
+          visitor =
               new LambdaClassFixer(
                   visitor,
                   lambdaClass.getValue(),
@@ -235,9 +232,11 @@ class Desugar {
           // Send lambda classes through desugaring to make sure there's no invokedynamic
           // instructions in generated lambda classes (checkState below will fail)
           reader.accept(
-              new LambdaDesugaring(lambdaFixer, loader, lambdas, null, allowDefaultMethods), 0);
-          String name = rewriter.unprefix(lambdaFixer.getInternalName() + ".class");
-          writeStoredEntry(out, name, writer.toByteArray());
+              new LambdaDesugaring(visitor, loader, lambdas, null, allowDefaultMethods),
+              0);
+          String filename =
+              rewriter.unprefix(lambdaClass.getValue().desiredInternalName()) + ".class";
+          writeStoredEntry(out, filename, writer.toByteArray());
         }
       }
 
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java
index 97c0249c5b..12f735d896 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java
@@ -51,7 +51,7 @@ class LambdaClassFixer extends ClassVisitor {
   private final HashSet<String> implementedMethods = new HashSet<>();
   private final LinkedHashSet<String> methodsToMoveIn = new LinkedHashSet<>();
 
-  private String internalName;
+  private String originalInternalName;
   private ImmutableList<String> interfaces;
 
   private boolean hasState;
@@ -71,10 +71,6 @@ class LambdaClassFixer extends ClassVisitor {
     this.allowDefaultMethods = allowDefaultMethods;
   }
 
-  public String getInternalName() {
-    return internalName;
-  }
-
   @Override
   public void visit(
       int version,
@@ -84,15 +80,16 @@ class LambdaClassFixer extends ClassVisitor {
       String superName,
       String[] interfaces) {
     checkArgument(BitFlags.noneSet(access, Opcodes.ACC_INTERFACE), "Not a class: %s", name);
-    checkState(internalName == null, "Already visited %s, can't reuse for %s", internalName, name);
-    internalName = name;
+    checkState(this.originalInternalName == null, "not intended for reuse but reused for %s", name);
+    originalInternalName = name;
     hasState = false;
     hasFactory = false;
     desc = null;
     this.signature = null;
     exceptions = null;
     this.interfaces = ImmutableList.copyOf(interfaces);
-    super.visit(version, access, name, signature, superName, interfaces);
+    // Rename to desired name
+    super.visit(version, access, getInternalName(), signature, superName, interfaces);
   }
 
   @Override
@@ -143,14 +140,14 @@ class LambdaClassFixer extends ClassVisitor {
   @Override
   public void visitEnd() {
     checkState(!hasState || hasFactory,
-        "Expected factory method for capturing lambda %s", internalName);
+        "Expected factory method for capturing lambda %s", getInternalName());
     if (!hasFactory) {
       // Fake factory method if LambdaMetafactory didn't generate it
       checkState(signature == null,
-          "Didn't expect generic constructor signature %s %s", internalName, signature);
+          "Didn't expect generic constructor signature %s %s", getInternalName(), signature);
 
       // Since this is a stateless class we populate and use a static singleton field "$instance"
-      String singletonFieldDesc = Type.getObjectType(internalName).getDescriptor();
+      String singletonFieldDesc = Type.getObjectType(getInternalName()).getDescriptor();
       super.visitField(
           Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_FINAL,
           "$instance",
@@ -166,11 +163,12 @@ class LambdaClassFixer extends ClassVisitor {
               "()V",
               (String) null,
               new String[0]);
-      codeBuilder.visitTypeInsn(Opcodes.NEW, internalName);
+      codeBuilder.visitTypeInsn(Opcodes.NEW, getInternalName());
       codeBuilder.visitInsn(Opcodes.DUP);
-      codeBuilder.visitMethodInsn(Opcodes.INVOKESPECIAL, internalName, "<init>",
-          checkNotNull(desc, "didn't see a constructor for %s", internalName), /*itf*/ false);
-      codeBuilder.visitFieldInsn(Opcodes.PUTSTATIC, internalName, "$instance", singletonFieldDesc);
+      codeBuilder.visitMethodInsn(Opcodes.INVOKESPECIAL, getInternalName(), "<init>",
+          checkNotNull(desc, "didn't see a constructor for %s", getInternalName()), /*itf*/ false);
+      codeBuilder.visitFieldInsn(
+          Opcodes.PUTSTATIC, getInternalName(), "$instance", singletonFieldDesc);
       codeBuilder.visitInsn(Opcodes.RETURN);
       codeBuilder.visitMaxs(2, 0); // two values are pushed onto the stack
       codeBuilder.visitEnd();
@@ -182,7 +180,8 @@ class LambdaClassFixer extends ClassVisitor {
               lambdaInfo.factoryMethodDesc(),
               (String) null,
               exceptions);
-      codeBuilder.visitFieldInsn(Opcodes.GETSTATIC, internalName, "$instance", singletonFieldDesc);
+      codeBuilder.visitFieldInsn(
+          Opcodes.GETSTATIC, getInternalName(), "$instance", singletonFieldDesc);
       codeBuilder.visitInsn(Opcodes.ARETURN);
       codeBuilder.visitMaxs(1, 0); // one value on the stack
     }
@@ -195,6 +194,10 @@ class LambdaClassFixer extends ClassVisitor {
     super.visitEnd();
   }
 
+  private String getInternalName() {
+    return lambdaInfo.desiredInternalName();
+  }
+
   private void copyRewrittenLambdaMethods() {
     for (String rewritten : methodsToMoveIn) {
       String interfaceInternalName = rewritten.substring(0, rewritten.indexOf('#'));
@@ -233,17 +236,41 @@ class LambdaClassFixer extends ClassVisitor {
         // Rewrite invocations of lambda methods in interfaces to anticipate the lambda method being
         // moved into the lambda class (i.e., the class being visited here).
         checkArgument(opcode == Opcodes.INVOKESTATIC, "Cannot move instance method %s", method);
-        owner = internalName;
+        owner = getInternalName();
         itf = false; // owner was interface but is now a class
         methodsToMoveIn.add(method);
-      } else if (name.startsWith("lambda$")) {
-        // Reflect renaming of lambda$ instance methods to avoid accidental overrides
-        name = LambdaDesugaring.uniqueInPackage(owner, name);
+      } else {
+        if (originalInternalName.equals(owner)) {
+          // Reflect renaming of lambda classes
+          owner = getInternalName();
+        }
+        if (name.startsWith("lambda$")) {
+          // Reflect renaming of lambda$ instance methods to avoid accidental overrides
+          name = LambdaDesugaring.uniqueInPackage(owner, name);
+        }
       }
       super.visitMethodInsn(opcode, owner, name, desc, itf);
     }
 
     @Override
+    public void visitTypeInsn(int opcode, String type) {
+      if (originalInternalName.equals(type)) {
+        // Reflect renaming of lambda classes
+        type = getInternalName();
+      }
+      super.visitTypeInsn(opcode, type);
+    }
+
+    @Override
+    public void visitFieldInsn(int opcode, String owner, String name, String desc) {
+      if (originalInternalName.equals(owner)) {
+        // Reflect renaming of lambda classes
+        owner = getInternalName();
+      }
+      super.visitFieldInsn(opcode, owner, name, desc);
+    }
+
+    @Override
     public AnnotationVisitor visitAnnotation(String desc, boolean visible) {
       // Drop annotation that's part of the generated lambda class that's not available on Android.
       // Proguard complains about this otherwise.
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java
index 8a2ebea51b..ef8740770b 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaClassMaker.java
@@ -14,7 +14,6 @@
 package com.google.devtools.build.android.desugar;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkState;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterators;
@@ -41,7 +40,7 @@ class LambdaClassMaker {
     this.rootDirectory = rootDirectory;
   }
 
-  public String generateLambdaClass(String invokerInternalName, LambdaInfo lambdaInfo,
+  public void generateLambdaClass(String invokerInternalName, LambdaInfo lambdaInfo,
       MethodHandle bootstrapMethod, ArrayList<Object> bsmArgs) throws IOException {
     // Invoking the bootstrap method will dump the generated class
     try {
@@ -52,11 +51,7 @@ class LambdaClassMaker {
     }
 
     Path generatedClassFile = findOnlyUnprocessed(invokerInternalName + "$$Lambda$");
-    String lambdaClassName = generatedClassFile.toString();
-    checkState(lambdaClassName.endsWith(".class"), "Unexpected filename %s", lambdaClassName);
-    lambdaClassName = lambdaClassName.substring(0, lambdaClassName.length() - ".class".length());
     generatedClasses.put(generatedClassFile, lambdaInfo);
-    return lambdaClassName;
   }
 
   /**
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java
index 79be2a3270..a3cf3bf532 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java
@@ -55,6 +55,7 @@ class LambdaDesugaring extends ClassVisitor {
 
   private String internalName;
   private boolean isInterface;
+  private int lambdaCount;
 
   public LambdaDesugaring(
       ClassVisitor dest,
@@ -77,6 +78,7 @@ class LambdaDesugaring extends ClassVisitor {
       String signature,
       String superName,
       String[] interfaces) {
+    checkState(internalName == null, "not intended for reuse but reused for %s", name);
     internalName = name;
     isInterface = BitFlags.isSet(access, Opcodes.ACC_INTERFACE);
     super.visit(version, access, name, signature, superName, interfaces);
@@ -360,9 +362,13 @@ class LambdaDesugaring extends ClassVisitor {
         // and ultimately we don't care if the bootstrap method was even on the bootclasspath
         // when this class was compiled (although it must've been since javac is unhappy otherwise).
         MethodHandle bsmMethod = toMethodHandle(publicLookup(), bsm, /*target*/ false);
-        String lambdaClassName = lambdas.generateLambdaClass(
+        // Give generated classes to have more stable names (b/35643761).  Use BSM's naming scheme
+        // but with separate counter for each surrounding class.
+        String lambdaClassName = internalName + "$$Lambda$" + (lambdaCount++);
+        lambdas.generateLambdaClass(
             internalName,
-            LambdaInfo.create(desc, bridgeInfo.methodReference(), bridgeInfo.bridgeMethod()),
+            LambdaInfo.create(
+                lambdaClassName, desc, bridgeInfo.methodReference(), bridgeInfo.bridgeMethod()),
             bsmMethod,
             args);
         // Emit invokestatic that calls the factory method generated in the lambda class
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaInfo.java b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaInfo.java
index e3bb84f891..dad340ceef 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaInfo.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/LambdaInfo.java
@@ -19,11 +19,15 @@ import org.objectweb.asm.Handle;
 @AutoValue
 abstract class LambdaInfo {
   public static LambdaInfo create(
-      String factoryMethodDesc, Handle methodReference, Handle bridgeMethod) {
+      String desiredInternalName,
+      String factoryMethodDesc,
+      Handle methodReference,
+      Handle bridgeMethod) {
     return new AutoValue_LambdaInfo(
-        factoryMethodDesc, methodReference, bridgeMethod);
+        desiredInternalName, factoryMethodDesc, methodReference, bridgeMethod);
   }
 
+  public abstract String desiredInternalName();
   public abstract String factoryMethodDesc();
   public abstract Handle methodReference();
   public abstract Handle bridgeMethod();