Correct stack map frame computation. When visiting CHOP Frame (discarding n local local variables), the current implementation deletes n local slots, which is wrong when the local variable to delete is of type category 2.

This CL deletes local variables instead of local slots.

Another change is using the Label objects to identify uninitialized values, instead of the offsets of labels, because the offsets of labels might not be computed.

RELNOTES:n/a.
PiperOrigin-RevId: 178762969

2 files changed, 111 insertions, 54 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/BytecodeTypeInference.java b/src/tools/android/java/com/google/devtools/build/android/desugar/BytecodeTypeInference.java
index 777a4ab722..783069fee4 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/BytecodeTypeInference.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/BytecodeTypeInference.java
@@ -14,12 +14,14 @@
 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 com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.Optional;
+import javax.annotation.Nullable;
 import org.objectweb.asm.Handle;
 import org.objectweb.asm.Label;
 import org.objectweb.asm.MethodVisitor;
@@ -41,11 +43,6 @@ public final class BytecodeTypeInference extends MethodVisitor {
   private FrameInfo previousFrame;
   /** For debugging purpose. */
   private final String methodSignature;
-  /**
-   * Stores mapping from "uninitialized" value to concrete value. This is for the "new" instruction.
-   */
-  private final HashMap<InferredType, InferredType> uninitializedToConcreteTypeMap =
-      new HashMap<>();
 
   public BytecodeTypeInference(int access, String owner, String name, String methodDescriptor) {
     super(Opcodes.ASM6);
@@ -81,6 +78,10 @@ public final class BytecodeTypeInference extends MethodVisitor {
     return operandStack.toString();
   }
 
+  public String getLocalsAsString() {
+    return localVariableSlots.toString();
+  }
+
   @Override
   public void visitInsn(int opcode) {
     switch (opcode) {
@@ -476,8 +477,7 @@ public final class BytecodeTypeInference extends MethodVisitor {
       int argumentSize = (Type.getArgumentsAndReturnSizes(desc) >> 2);
       InferredType receiverType = getTypeOfOperandFromTop(argumentSize - 1);
       if (receiverType.isUninitialized()) {
-        InferredType realType = InferredType.create('L' + owner + ';');
-        uninitializedToConcreteTypeMap.put(receiverType, realType);
+        InferredType realType = InferredType.createNonUninitializedType('L' + owner + ';');
         replaceUninitializedTypeInStack(receiverType, realType);
       }
     }
@@ -642,7 +642,6 @@ public final class BytecodeTypeInference extends MethodVisitor {
     operandStack.addAll(previousFrame.stack());
     localVariableSlots.clear();
     localVariableSlots.addAll(previousFrame.locals());
-
     super.visitFrame(type, nLocal, local, nStack, stack);
   }
 
@@ -672,7 +671,7 @@ public final class BytecodeTypeInference extends MethodVisitor {
     checkArgument(oldType.isUninitialized(), "The old type is NOT uninitialized. %s", oldType);
     for (int i = 0, size = operandStack.size(); i < size; ++i) {
       InferredType type = operandStack.get(i);
-      if (type == oldType) {
+      if (type.equals(oldType)) {
         operandStack.set(i, newType);
       }
     }
@@ -703,7 +702,7 @@ public final class BytecodeTypeInference extends MethodVisitor {
         break;
       case 'L':
       case '[':
-        push(InferredType.create(desc.substring(index)));
+        push(InferredType.createNonUninitializedType(desc.substring(index)));
         break;
       default:
         throw new RuntimeException("Unhandled type: " + desc);
@@ -771,12 +770,6 @@ public final class BytecodeTypeInference extends MethodVisitor {
     return lastPopped;
   }
 
-  private static ImmutableList<InferredType> removeBackFromList(
-      ImmutableList<InferredType> list, int countToRemove) {
-    int newSize = list.size() - countToRemove;
-    return list.subList(0, newSize);
-  }
-
   /**
    * Create the types of local variables at the very beginning of the method with the information of
    * the declaring class and the method descriptor.
@@ -787,7 +780,7 @@ public final class BytecodeTypeInference extends MethodVisitor {
 
     if (!BitFlags.isSet(access, Opcodes.ACC_STATIC)) {
       // Instance method, and this is the receiver
-      types.add(InferredType.create(convertToDescriptor(ownerClass)));
+      types.add(InferredType.createNonUninitializedType(convertToDescriptor(ownerClass)));
     }
     Type[] argumentTypes = Type.getArgumentTypes(methodDescriptor);
     for (Type argumentType : argumentTypes) {
@@ -812,7 +805,7 @@ public final class BytecodeTypeInference extends MethodVisitor {
           break;
         case Type.ARRAY:
         case Type.OBJECT:
-          types.add(InferredType.create(argumentType.getDescriptor()));
+          types.add(InferredType.createNonUninitializedType(argumentType.getDescriptor()));
           break;
         default:
           throw new RuntimeException(
@@ -828,6 +821,28 @@ public final class BytecodeTypeInference extends MethodVisitor {
     return types;
   }
 
+  private static ImmutableList<InferredType> removeBackFromList(
+      ImmutableList<InferredType> list, int countToRemove) {
+    int origSize = list.size();
+    int index = origSize - 1;
+
+    while (index >= 0 && countToRemove > 0) {
+      InferredType type = list.get(index);
+      if (type.equals(InferredType.TOP) && index > 0 && list.get(index - 1).isCategory2()) {
+        --index; // A category 2 takes two slots.
+      }
+      --index; // Eat this local variable.
+      --countToRemove;
+    }
+    checkState(
+        countToRemove == 0,
+        "countToRemove is %s but not 0. index=%s, list=%s",
+        countToRemove,
+        index,
+        list);
+    return list.subList(0, index + 1);
+  }
+
   private ImmutableList<InferredType> appendArrayToList(
       ImmutableList<InferredType> list, int size, Object[] array) {
     ImmutableList.Builder<InferredType> builder = ImmutableList.builder();
@@ -861,13 +876,13 @@ public final class BytecodeTypeInference extends MethodVisitor {
     } else if (typeInStackMapFrame instanceof String) {
       String referenceTypeName = (String) typeInStackMapFrame;
       if (referenceTypeName.charAt(0) == '[') {
-        return InferredType.create(referenceTypeName);
+        return InferredType.createNonUninitializedType(referenceTypeName);
       } else {
-        return InferredType.create('L' + referenceTypeName + ';');
+        return InferredType.createNonUninitializedType('L' + referenceTypeName + ';');
       }
     } else if (typeInStackMapFrame instanceof Label) {
       Label label = (Label) typeInStackMapFrame;
-      return InferredType.createUninitialized(label.getOffset());
+      return InferredType.createUninitializedType(label);
     } else {
       throw new RuntimeException(
           "Cannot reach here. Unhandled element: value="
@@ -911,26 +926,45 @@ public final class BytecodeTypeInference extends MethodVisitor {
     public static final String UNINITIALIZED_PREFIX = "UNINIT@";
 
     public static final InferredType BOOLEAN =
-        new AutoValue_BytecodeTypeInference_InferredType("Z");
-    public static final InferredType BYTE = new AutoValue_BytecodeTypeInference_InferredType("B");
-    public static final InferredType INT = new AutoValue_BytecodeTypeInference_InferredType("I");
-    public static final InferredType FLOAT = new AutoValue_BytecodeTypeInference_InferredType("F");
-    public static final InferredType LONG = new AutoValue_BytecodeTypeInference_InferredType("J");
-    public static final InferredType DOUBLE = new AutoValue_BytecodeTypeInference_InferredType("D");
+        new AutoValue_BytecodeTypeInference_InferredType("Z", /*uninitializationLabel=*/ null);
+    public static final InferredType BYTE =
+        new AutoValue_BytecodeTypeInference_InferredType("B", /*uninitializationLabel=*/ null);
+    public static final InferredType INT =
+        new AutoValue_BytecodeTypeInference_InferredType("I", /*uninitializationLabel=*/ null);
+    public static final InferredType FLOAT =
+        new AutoValue_BytecodeTypeInference_InferredType("F", /*uninitializationLabel=*/ null);
+    public static final InferredType LONG =
+        new AutoValue_BytecodeTypeInference_InferredType("J", /*uninitializationLabel=*/ null);
+    public static final InferredType DOUBLE =
+        new AutoValue_BytecodeTypeInference_InferredType("D", /*uninitializationLabel=*/ null);
     /** Not a real value. */
-    public static final InferredType TOP = new AutoValue_BytecodeTypeInference_InferredType("TOP");
+    public static final InferredType TOP =
+        new AutoValue_BytecodeTypeInference_InferredType("TOP", /*uninitializationLabel=*/ null);
     /** The value NULL */
     public static final InferredType NULL =
-        new AutoValue_BytecodeTypeInference_InferredType("NULL");
+        new AutoValue_BytecodeTypeInference_InferredType("NULL", /*uninitializationLabel=*/ null);
 
     public static final InferredType UNINITIALIZED_THIS =
-        new AutoValue_BytecodeTypeInference_InferredType("UNINITIALIZED_THIS");
+        new AutoValue_BytecodeTypeInference_InferredType(
+            "UNINITIALIZED_THIS", /*uninitializationLabel=*/ null);
 
-    static InferredType create(String descriptor) {
+    /**
+     * Create a type for a value. This method is not intended to be called outside of this class.
+     */
+    private static InferredType create(String descriptor, @Nullable Label uninitializationLabel) {
+      if (UNINITIALIZED_PREFIX.equals(descriptor)) {
+        return new AutoValue_BytecodeTypeInference_InferredType(
+            UNINITIALIZED_PREFIX, checkNotNull(uninitializationLabel));
+      }
+      checkArgument(
+          uninitializationLabel == null,
+          "The uninitializationLabel should be null for non-uninitialized value. %s",
+          descriptor);
       char firstChar = descriptor.charAt(0);
-      if (firstChar == 'L' || firstChar == '[' || descriptor.startsWith(UNINITIALIZED_PREFIX)) {
-        // Reference, array, or uninitialized values.
-        return new AutoValue_BytecodeTypeInference_InferredType(descriptor);
+      if (firstChar == 'L' || firstChar == '[') {
+        // Reference, array.
+        return new AutoValue_BytecodeTypeInference_InferredType(
+            descriptor, /*uninitializationLabel=*/ null);
       }
       switch (descriptor) {
         case "Z":
@@ -956,12 +990,23 @@ public final class BytecodeTypeInference extends MethodVisitor {
       }
     }
 
-    /** Create a type for uninitialized value. The label is generated by ASM. */
-    static InferredType createUninitialized(int label) {
-      return create(UNINITIALIZED_PREFIX + label);
+    /** Creates all types except UNINITIALIZED. This method can also create UNINTIALIZED_THIS. */
+    static InferredType createNonUninitializedType(String descriptor) {
+      return create(descriptor, /*uninitializationLabel=*/ null);
+    }
+
+    /** Create a type for an UNINITIALIZED value. The uninitializationLabel is generated by ASM. */
+    static InferredType createUninitializedType(Label uninitializationLabel) {
+      return create(UNINITIALIZED_PREFIX, uninitializationLabel);
     }
 
     abstract String descriptor();
+    /**
+     * The label may be null. This field is meaningful if the current type is "UNINITIALIZED". For
+     * other types, this field is null.
+     */
+    @Nullable
+    abstract Label uninitializationLabel();
 
     @Override
     public String toString() {
@@ -978,7 +1023,7 @@ public final class BytecodeTypeInference extends MethodVisitor {
     public InferredType getElementTypeIfArrayOrThrow() {
       String descriptor = descriptor();
       checkState(descriptor.charAt(0) == '[', "This type %s is not an array.", this);
-      return create(descriptor.substring(1));
+      return createNonUninitializedType(descriptor.substring(1));
     }
 
     /** Is an uninitialized value? */
@@ -992,17 +1037,16 @@ public final class BytecodeTypeInference extends MethodVisitor {
     }
 
     /**
-     * If this type is a reference type, then return the internal name. Otherwise, throw an
-     * exception.
+     * If this type is a reference type, then return the internal name. Otherwise, returns empty.
      */
-    public String getInternalNameOrThrow() {
+    public Optional<String> getInternalName() {
       String descriptor = descriptor();
       int length = descriptor.length();
-      checkState(
-          descriptor.charAt(0) == 'L' && descriptor.charAt(length - 1) == ';',
-          "The type is expected to be either a class or an interface: %s",
-          descriptor);
-      return descriptor.substring(1, length - 1);
+      if (length > 0 && descriptor.charAt(0) == 'L' && descriptor.charAt(length - 1) == ';') {
+        return Optional.of(descriptor.substring(1, length - 1));
+      } else {
+        return Optional.empty();
+      }
     }
   }
 }
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java b/src/tools/android/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java
index e4d4da5246..8e6d6d558b 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java
@@ -31,6 +31,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.android.desugar.BytecodeTypeInference.InferredType;
 import java.util.Collections;
 import java.util.LinkedHashSet;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 import javax.annotation.Nullable;
@@ -291,10 +292,16 @@ public class TryWithResourcesRewriter extends ClassVisitor {
           // Check the exception type.
           InferredType exceptionClass = typeInference.getTypeOfOperandFromTop(1);
           if (!exceptionClass.isNull()) {
-            String exceptionClassInternalName = exceptionClass.getInternalNameOrThrow();
+            Optional<String> exceptionClassInternalName = exceptionClass.getInternalName();
+            checkState(
+                exceptionClassInternalName.isPresent(),
+                "The exception %s is not a reference type in %s.%s",
+                exceptionClass,
+                internalName,
+                methodSignature);
             checkState(
                 isAssignableFrom(
-                    "java.lang.Throwable", exceptionClassInternalName.replace('/', '.')),
+                    "java.lang.Throwable", exceptionClassInternalName.get().replace('/', '.')),
                 "The exception type %s in %s.%s should be a subclass of java.lang.Throwable.",
                 exceptionClassInternalName,
                 internalName,
@@ -302,20 +309,26 @@ public class TryWithResourcesRewriter extends ClassVisitor {
           }
         }
 
-        String resourceClassInternalName =
-            typeInference.getTypeOfOperandFromTop(0).getInternalNameOrThrow();
+        InferredType resourceType = typeInference.getTypeOfOperandFromTop(0);
+        Optional<String> resourceClassInternalName = resourceType.getInternalName();
+        checkState(
+            resourceClassInternalName.isPresent(),
+            "The resource class %s is not a reference type in %s.%s",
+            resourceType,
+            internalName,
+            methodSignature);
         checkState(
             isAssignableFrom(
-                "java.lang.AutoCloseable", resourceClassInternalName.replace('/', '.')),
+                "java.lang.AutoCloseable", resourceClassInternalName.get().replace('/', '.')),
             "The resource type should be a subclass of java.lang.AutoCloseable: %s",
             resourceClassInternalName);
 
-        resourceTypeInternalNames.add(resourceClassInternalName);
+        resourceTypeInternalNames.add(resourceClassInternalName.get());
         super.visitMethodInsn(
             opcode,
             owner,
             "$closeResource",
-            "(Ljava/lang/Throwable;L" + resourceClassInternalName + ";)V",
+            "(Ljava/lang/Throwable;L" + resourceClassInternalName.get() + ";)V",
             itf);
         return;
       }