fix for legacy jacoco instrumentation in interfaces behind flag

RELNOTES: n/a

PiperOrigin-RevId: 167619442

2 files changed, 67 insertions, 3 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 63ca5e7a6b..31c362e67a 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
@@ -229,6 +229,19 @@ class Desugar {
       help = "Enables rewriting to desugar java.* classes."
     )
     public boolean coreLibrary;
+
+    /** Set to work around b/62623509 with JaCoCo versions prior to 0.7.9. */
+    // TODO(kmb): Remove when Android Studio doesn't need it anymore (see b/37116789)
+    @Option(
+      name = "legacy_jacoco_fix",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      help = "Consider setting this flag if you're using JaCoCo versions prior to 0.7.9 to work "
+          + "around issues with coverage instrumentation in default and static interface methods. "
+          + "This flag may be removed when no longer needed."
+    )
+    public boolean legacyJacocoFix;
   }
 
   private final DesugarOptions options;
@@ -511,7 +524,8 @@ class Desugar {
       if (options.desugarInterfaceMethodBodiesIfNeeded) {
         visitor =
             new DefaultMethodClassFixer(visitor, classpathReader, bootclasspathReader, loader);
-        visitor = new InterfaceDesugaring(visitor, bootclasspathReader, store);
+        visitor =
+            new InterfaceDesugaring(visitor, bootclasspathReader, store, options.legacyJacocoFix);
       }
     }
     visitor =
@@ -561,7 +575,8 @@ class Desugar {
         if (options.desugarInterfaceMethodBodiesIfNeeded) {
           visitor =
               new DefaultMethodClassFixer(visitor, classpathReader, bootclasspathReader, loader);
-          visitor = new InterfaceDesugaring(visitor, bootclasspathReader, store);
+          visitor =
+              new InterfaceDesugaring(visitor, bootclasspathReader, store, options.legacyJacocoFix);
         }
       }
       // LambdaDesugaring is relatively expensive, so check first whether we need it.  Additionally,
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java b/src/tools/android/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java
index 8c0ea7eb52..6bac9edb26 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java
@@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkState;
 import javax.annotation.Nullable;
 import org.objectweb.asm.AnnotationVisitor;
 import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.FieldVisitor;
 import org.objectweb.asm.MethodVisitor;
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.Type;
@@ -46,6 +47,7 @@ class InterfaceDesugaring extends ClassVisitor {
 
   private final ClassReaderFactory bootclasspath;
   private final GeneratedClassStore store;
+  private final boolean legacyJaCoCo;
 
   private String internalName;
   private int bytecodeVersion;
@@ -54,10 +56,12 @@ class InterfaceDesugaring extends ClassVisitor {
   @Nullable private FieldInfo interfaceFieldToAccessInCompanionMethodToTriggerInterfaceClinit;
 
   public InterfaceDesugaring(
-      ClassVisitor dest, ClassReaderFactory bootclasspath, GeneratedClassStore store) {
+      ClassVisitor dest, ClassReaderFactory bootclasspath, GeneratedClassStore store,
+      boolean legacyJaCoCo) {
     super(Opcodes.ASM5, dest);
     this.bootclasspath = bootclasspath;
     this.store = store;
+    this.legacyJaCoCo = legacyJaCoCo;
   }
 
   @Override
@@ -120,6 +124,24 @@ class InterfaceDesugaring extends ClassVisitor {
   }
 
   @Override
+  public FieldVisitor visitField(
+      int access, String name, String desc, String signature, Object value) {
+    if (legacyJaCoCo
+        && isInterface()
+        && BitFlags.isSet(access, Opcodes.ACC_FINAL)
+        && "$jacocoData".equals(name)) {
+      // Move $jacocoData field to companion class and remove final modifier. We'll rewrite field
+      // accesses accordingly. Code generated by older JaCoCo versions tried to assign to this
+      // final field in methods, and interface fields have to be private, so we move the field
+      // to a class, which ends up looking pretty similar to what JaCoCo generates for classes.
+      access &= ~Opcodes.ACC_FINAL;
+      return companion().visitField(access, name, desc, signature, value);
+    } else {
+      return super.visitField(access, name, desc, signature, value);
+    }
+  }
+
+  @Override
   public MethodVisitor visitMethod(
       int access, String name, String desc, String signature, String[] exceptions) {
     MethodVisitor result;
@@ -127,6 +149,9 @@ class InterfaceDesugaring extends ClassVisitor {
       result =
           new InterfaceFieldWriteCollector(
               super.visitMethod(access, name, desc, signature, exceptions));
+      if (result != null && legacyJaCoCo) {
+        result = new MoveJacocoFieldAccess(result);
+      }
     } else if (isInterface()
         && BitFlags.noneSet(access, Opcodes.ACC_ABSTRACT | Opcodes.ACC_BRIDGE)) {
       checkArgument(BitFlags.noneSet(access, Opcodes.ACC_NATIVE), "Forbidden per JLS ch 9.4");
@@ -176,6 +201,9 @@ class InterfaceDesugaring extends ClassVisitor {
 
         result = abstractDest != null ? new MultiplexAnnotations(codeDest, abstractDest) : codeDest;
       }
+      if (result != null && legacyJaCoCo) {
+        result = new MoveJacocoFieldAccess(result);
+      }
     } else {
       result = super.visitMethod(access, name, desc, signature, exceptions);
     }
@@ -343,6 +371,27 @@ class InterfaceDesugaring extends ClassVisitor {
   }
 
   /**
+   * Method visitor intended for interface method bodies that rewrites jacoco field accesses to
+   * expect the field in the companion class, to work around problematic bytecode emitted by older
+   * JaCoCo versions (b/62623509).
+   */
+  private static class MoveJacocoFieldAccess extends MethodVisitor {
+
+    public MoveJacocoFieldAccess(MethodVisitor mv) {
+      super(Opcodes.ASM5, mv);
+    }
+
+    @Override
+    public void visitFieldInsn(int opcode, String owner, String name, String desc) {
+      if ("$jacocoData".equals(name)) {
+        checkState(!owner.endsWith(COMPANION_SUFFIX), "Expected interface: %s", owner);
+        owner = getCompanionClassName(owner);
+      }
+      super.visitFieldInsn(opcode, owner, name, desc);
+    }
+  }
+
+  /**
    * Method visitor that behaves like a passthrough but additionally duplicates all annotations into
    * a second given {@link MethodVisitor}.
    */