Merge branch '317833-fix-copy-pool-optimization-with-descriptor-change' into 'master'

Resolve "When ClassWriter copies constant pool of class reader, ClassWriter.COMPUTE_MAX does not consider changes in argument sizes"

Closes #317833

See merge request !185

asm/src/main/java/org/objectweb/asm/ClassReader.java

@@ -1132,6 +1132,7 @@ public class ClassReader {
           currentOffset - methodInfoOffset,
           synthetic,
           (context.currentMethodAccessFlags & Opcodes.ACC_DEPRECATED) != 0,
+          readUnsignedShort(methodInfoOffset + 4),
           signatureIndex,
           exceptionsOffset)) {
         return currentOffset;

asm/src/main/java/org/objectweb/asm/MethodWriter.java

@@ -2003,6 +2003,8 @@ final class MethodWriter extends MethodVisitor {
    *     of this method might be copied contains a Synthetic attribute.
    * @param hasDeprecatedAttribute whether the method_info JVMS structure from which the attributes
    *     of this method might be copied contains a Deprecated attribute.
+   * @param descriptorIndex the descriptor_index field of the method_info JVMS structure from which
+   *     the attributes of this method might be copied.
    * @param signatureIndex the constant pool index contained in the Signature attribute of the
    *     method_info JVMS structure from which the attributes of this method might be copied, or 0.
    * @param exceptionsOffset the offset in 'source.b' of the Exceptions attribute of the method_info
@@ -2017,9 +2019,16 @@ final class MethodWriter extends MethodVisitor {
       final int methodInfoLength,
       final boolean hasSyntheticAttribute,
       final boolean hasDeprecatedAttribute,
+      final int descriptorIndex,
       final int signatureIndex,
       final int exceptionsOffset) {
+    // If the method descriptor has changed, with more locals than the max_locals field of the
+    // original Code attribute, if any, then the original method attributes can't be copied. A
+    // conservative check on the descriptor changes alone ensures this (being more precise is not
+    // worth the additional complexity, because these cases should be rare -- if a transform changes
+    // a method descriptor, most of the time it needs to change the method's code too).
     if (source != symbolTable.getSource()
+        || descriptorIndex != this.descriptorIndex
         || signatureIndex != this.signatureIndex
         || hasDeprecatedAttribute != ((accessFlags & Opcodes.ACC_DEPRECATED) != 0)) {
       return false;

asm/src/test/java/org/objectweb/asm/ClassVisitorTest.java

@@ -29,6 +29,9 @@ package org.objectweb.asm;
 
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
@@ -157,6 +160,34 @@ public class ClassVisitorTest extends AsmTest {
     assertThatClass(copyPoolClassWriter.toByteArray()).isEqualTo(classWriter.toByteArray());
   }
 
+  /**
+   * Tests that a ClassReader -> class adapter -> ClassWriter chain give the same result when the
+   * descriptor of a method is changed.
+   */
+  @Test
+  public void testReadAndWriteWithCopyPoolAndChangeDescriptor() {
+    ClassWriter sourceClassWriter = new ClassWriter(ClassWriter.COMPUTE_MAXS);
+    sourceClassWriter.visit(
+        Opcodes.V1_7, Opcodes.ACC_ABSTRACT, "C", null, "java/lang/Object", null);
+    MethodVisitor methodVisitor =
+        sourceClassWriter.visitMethod(Opcodes.ACC_PUBLIC, "<init>", "()V", null, null);
+    methodVisitor.visitCode();
+    methodVisitor.visitVarInsn(Opcodes.ALOAD, 0);
+    methodVisitor.visitMethodInsn(
+        Opcodes.INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
+    methodVisitor.visitInsn(Opcodes.RETURN);
+    methodVisitor.visitMaxs(0, 0);
+    methodVisitor.visitEnd();
+    sourceClassWriter.visitEnd();
+
+    ClassReader classReader = new ClassReader(sourceClassWriter.toByteArray());
+    ClassWriter classWriter = new ClassWriter(ClassWriter.COMPUTE_MAXS);
+    ClassWriter copyPoolClassWriter = new ClassWriter(classReader, ClassWriter.COMPUTE_MAXS);
+    classReader.accept(new AddParameterAdapter(classWriter), 0);
+    classReader.accept(new AddParameterAdapter(copyPoolClassWriter), 0);
+    assertThatClass(copyPoolClassWriter.toByteArray()).isEqualTo(classWriter.toByteArray());
+  }
+
   /** Test that classes with only visible or only invisible annotations can be read correctly. */
   @ParameterizedTest
   @ValueSource(strings = {"true", "false"})
@@ -535,4 +566,31 @@ public class ClassVisitorTest extends AsmTest {
       };
     }
   }
+
+  /** A class visitor which adds a parameter to the declared method descriptors. */
+  private static class AddParameterAdapter extends ClassVisitor {
+
+    public AddParameterAdapter(final ClassVisitor classVisitor) {
+      super(Opcodes.ASM7_EXPERIMENTAL, classVisitor);
+    }
+
+    @Override
+    public MethodVisitor visitMethod(
+        final int access,
+        final String name,
+        final String descriptor,
+        final String signature,
+        final String[] exceptions) {
+      List<Type> argumentTypes = new ArrayList<>(Arrays.asList(Type.getArgumentTypes(descriptor)));
+      argumentTypes.add(Type.INT_TYPE);
+      Type returnType = Type.getReturnType(descriptor);
+      return super.visitMethod(
+          access,
+          name,
+          Type.getMethodDescriptor(
+              returnType, argumentTypes.toArray(new Type[argumentTypes.size()])),
+          signature,
+          exceptions);
+    }
+  }
 }