Commit b4491054 authored by Eric Bruneton's avatar Eric Bruneton
Browse files

Merge branch '317822-fix-mostly-add-optimization' into 'master'

Resolve "ACC_DEPRECATED flag (or corresponding attribute) is lost if the constant pool is copied"

Closes #317822 and #317824

See merge request asm/asm!162
parents 23337ecf baab1599
......@@ -976,8 +976,8 @@ public class ClassReader {
int exceptionsOffset = 0;
// - The strings corresponding to the Exceptions attribute, or null.
String[] exceptions = null;
// - The string corresponding to the Signature attribute, or null.
int signature = 0;
// - The constant pool index contained in the Signature attribute, or 0.
int signatureIndex = 0;
// - The offset of the RuntimeVisibleAnnotations attribute, or 0.
int runtimeVisibleAnnotationsOffset = 0;
// - The offset of the RuntimeInvisibleAnnotations attribute, or 0.
......@@ -1020,7 +1020,7 @@ public class ClassReader {
currentExceptionOffset += 2;
}
} else if (Constants.SIGNATURE.equals(attributeName)) {
signature = readUnsignedShort(currentOffset);
signatureIndex = readUnsignedShort(currentOffset);
} else if (Constants.DEPRECATED.equals(attributeName)) {
context.currentMethodAccessFlags |= Opcodes.ACC_DEPRECATED;
} else if (Constants.RUNTIME_VISIBLE_ANNOTATIONS.equals(attributeName)) {
......@@ -1063,42 +1063,26 @@ public class ClassReader {
context.currentMethodAccessFlags,
context.currentMethodName,
context.currentMethodDescriptor,
signature == 0 ? null : readUTF(signature, charBuffer),
signatureIndex == 0 ? null : readUTF(signatureIndex, charBuffer),
exceptions);
if (methodVisitor == null) {
return currentOffset;
}
// If the returned MethodVisitor is in fact a MethodWriter, it means there is no method
// adapter between the reader and the writer. If, in addition, the writer's constant pool was
// copied from this reader (mw.getSource() == this), and the signature and exceptions of the
// method have not been changed, then it is possible to skip all visit events and just copy the
// original code of the method to the writer (the access_flags, name_index and descriptor_index
// can have been changed, this is not important since they are not copied from the reader).
// adapter between the reader and the writer. In this case, it might be possible to copy
// the method attributes directly into the writer. If so, return early without visiting
// the content of these attributes.
if (methodVisitor instanceof MethodWriter) {
MethodWriter methodWriter = (MethodWriter) methodVisitor;
if (methodWriter.getSource() == this && signature == methodWriter.signatureIndex) {
boolean sameExceptions = false;
if (exceptions == null) {
sameExceptions = methodWriter.numberOfExceptions == 0;
} else if (exceptions.length == methodWriter.numberOfExceptions) {
sameExceptions = true;
int currentExceptionOffset = exceptionsOffset + 2;
for (int i = 0; i < exceptions.length; ++i) {
if (methodWriter.exceptionIndexTable[i] != readUnsignedShort(currentExceptionOffset)) {
sameExceptions = false;
break;
}
currentExceptionOffset += 2;
}
}
if (sameExceptions) {
// We do not copy directly the code into methodWriter to save a byte array copy operation.
// The real copy will be done in {@link MethodWriter#putMethodInfo}.
methodWriter.sourceOffset = methodInfoOffset + 6;
methodWriter.sourceLength = currentOffset - methodWriter.sourceOffset;
return currentOffset;
}
if (methodWriter.canCopyMethodAttributes(
this,
methodInfoOffset,
currentOffset - methodInfoOffset,
context.currentMethodAccessFlags,
signatureIndex,
exceptionsOffset)) {
return currentOffset;
}
}
......
......@@ -375,13 +375,13 @@ final class MethodWriter extends MethodVisitor {
// Other method_info attributes:
/** The number_of_exceptions field of the Exceptions attribute. */
final int numberOfExceptions;
private final int numberOfExceptions;
/** The exception_index_table array of the Exceptions attribute, or <tt>null</tt>. */
final int[] exceptionIndexTable;
private final int[] exceptionIndexTable;
/** The signature_index field of the Signature attribute. */
final int signatureIndex;
private final int signatureIndex;
/**
* The last runtime visible annotation of this method. The previous ones can be accessed with the
......@@ -538,16 +538,17 @@ final class MethodWriter extends MethodVisitor {
private int lastBytecodeOffset;
/**
* The offset in bytes in {@link #getSource} from which the method_info for this method (excluding
* its first 6 bytes) must be copied, or 0.
* The offset in bytes in {@link SymbolTable#getSource} from which the method_info for this method
* (excluding its first 6 bytes) must be copied, or 0.
*/
int sourceOffset;
private int sourceOffset;
/**
* The length in bytes in {@link #getSource} which must be copied to get the method_info for this
* method (excluding its first 6 bytes for access_flags, name_index and descriptor_index).
* The length in bytes in {@link SymbolTable#getSource} which must be copied to get the
* method_info for this method (excluding its first 6 bytes for access_flags, name_index and
* descriptor_index).
*/
int sourceLength;
private int sourceLength;
// -----------------------------------------------------------------------------------------------
// Constructor and accessors
......@@ -604,10 +605,6 @@ final class MethodWriter extends MethodVisitor {
}
}
ClassReader getSource() {
return symbolTable.getSource();
}
boolean hasFrames() {
return stackMapTableNumberOfEntries > 0;
}
......@@ -1944,6 +1941,67 @@ final class MethodWriter extends MethodVisitor {
// Utility methods
// -----------------------------------------------------------------------------------------------
/**
* Returns whether the attributes of this method can be copied from the attributes of the given
* method (assuming there is no method visitor between the given ClassReader and this
* MethodWriter). This method should only be called just after this MethodWriter has been created,
* and before any content is visited. It returns true if the attributes corresponding to the
* constructor arguments (at most a Signature, an Exception, a Deprecated and a Synthetic
* attribute) are the same as the corresponding attributes in the given method.
*
* @param source the source ClassReader from which the attributes of this method might be copied.
* @param methodInfoOffset the offset in 'source.b' of the method_info JVMS structure from which
* the attributes of this method might be copied.
* @param methodInfoLength the length in 'source.b' of the method_info JVMS structure from which
* the attributes of this method might be copied.
* @param access the access flags (including the ASM specific ones) 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
* JVMS structure from which the attributes of this method might be copied, or 0.
* @return whether the attributes of this method can be copied from the attributes of the
* method_info JVMS structure in 'source.b', between 'methodInfoOffset' and 'methodInfoOffset'
* + 'methodInfoLength'.
*/
boolean canCopyMethodAttributes(
final ClassReader source,
final int methodInfoOffset,
final int methodInfoLength,
final int access,
final int signatureIndex,
final int exceptionsOffset) {
if (source != symbolTable.getSource()
|| signatureIndex != this.signatureIndex
|| (access & Opcodes.ACC_DEPRECATED) != (accessFlags & Opcodes.ACC_DEPRECATED)) {
return false;
}
boolean useSyntheticAttribute = symbolTable.getMajorVersion() < Opcodes.V1_5;
if (useSyntheticAttribute
&& (access & Opcodes.ACC_SYNTHETIC) != (accessFlags & Opcodes.ACC_SYNTHETIC)) {
return false;
}
if (exceptionsOffset == 0) {
if (numberOfExceptions != 0) {
return false;
}
} else if (source.readUnsignedShort(exceptionsOffset) == numberOfExceptions) {
int currentExceptionOffset = exceptionsOffset + 2;
for (int i = 0; i < numberOfExceptions; ++i) {
if (source.readUnsignedShort(currentExceptionOffset) != exceptionIndexTable[i]) {
return false;
}
currentExceptionOffset += 2;
}
}
// Don't copy the attributes yet, instead store their location in the source class reader so
// they can be copied later, in {@link #putMethodInfo}. Note that we skip the 6 header bytes
// of the method_info JVMS structure.
this.sourceOffset = methodInfoOffset + 6;
this.sourceLength = methodInfoLength - 6;
return true;
}
/**
* Returns the size of the method_info JVMS structure generated by this MethodWriter. Also add the
* names of the attributes of this method in the constant pool.
......@@ -2086,7 +2144,7 @@ final class MethodWriter extends MethodVisitor {
output.putShort(accessFlags & ~mask).putShort(nameIndex).putShort(descriptorIndex);
// If this method_info must be copied from an existing one, copy it now and return early.
if (sourceOffset != 0) {
output.putByteArray(getSource().b, sourceOffset, sourceLength);
output.putByteArray(symbolTable.getSource().b, sourceOffset, sourceLength);
return;
}
// For ease of reference, we use here the same attribute order as in Section 4.7 of the JVMS.
......
......@@ -86,6 +86,42 @@ public class ClassVisitorTest extends AsmTest {
assertThatClass(classWriterWithCopyPool.toByteArray()).isEqualTo(classWriter.toByteArray());
}
/**
* Tests that a ClassReader -> class adapter -> ClassWriter chain give the same result with or
* without the copy pool option.
*/
@ParameterizedTest
@MethodSource(ALL_CLASSES_AND_ALL_APIS)
public void testReadAndWriteWithCopyPoolAndDeprecatedAdapter(
final PrecompiledClass classParameter, final Api apiParameter) {
byte[] classFile = classParameter.getBytes();
ClassReader classReader = new ClassReader(classFile);
ClassWriter classWriter = new ClassWriter(0);
ClassWriter classWriterWithCopyPool = new ClassWriter(classReader, 0);
int access = Opcodes.ACC_DEPRECATED;
classReader.accept(new ChangeAccessAdapter(classWriter, access), attributes(), 0);
classReader.accept(new ChangeAccessAdapter(classWriterWithCopyPool, access), attributes(), 0);
assertThatClass(classWriterWithCopyPool.toByteArray()).isEqualTo(classWriter.toByteArray());
}
/**
* Tests that a ClassReader -> class adapter -> ClassWriter chain give the same result with or
* without the copy pool option.
*/
@ParameterizedTest
@MethodSource(ALL_CLASSES_AND_ALL_APIS)
public void testReadAndWriteWithCopyPoolAndSyntheticAdapter(
final PrecompiledClass classParameter, final Api apiParameter) {
byte[] classFile = classParameter.getBytes();
ClassReader classReader = new ClassReader(classFile);
ClassWriter classWriter = new ClassWriter(0);
ClassWriter classWriterWithCopyPool = new ClassWriter(classReader, 0);
int access = Opcodes.ACC_SYNTHETIC;
classReader.accept(new ChangeAccessAdapter(classWriter, access), attributes(), 0);
classReader.accept(new ChangeAccessAdapter(classWriterWithCopyPool, access), attributes(), 0);
assertThatClass(classWriterWithCopyPool.toByteArray()).isEqualTo(classWriter.toByteArray());
}
/** Test that classes with only visible or only invisible annotations can be read correctly. */
@ParameterizedTest
@ValueSource(strings = {"true", "false"})
......@@ -281,6 +317,26 @@ public class ClassVisitorTest extends AsmTest {
}
}
private static class ChangeAccessAdapter extends ClassVisitor {
private final int accessFlags;
ChangeAccessAdapter(final ClassVisitor classVisitor, final int accessFlags) {
super(Opcodes.ASM6, classVisitor);
this.accessFlags = accessFlags;
}
@Override
public MethodVisitor visitMethod(
final int access,
final String name,
final String descriptor,
final String signature,
final String[] exceptions) {
return super.visitMethod(access ^ accessFlags, name, descriptor, signature, exceptions);
}
}
/** A class visitor which removes either all visible or all invisible [type] annotations. */
private static class RemoveAnnotationAdapter extends ClassVisitor {
......
// ASM: a very small and fast Java bytecode manipulation framework
// Copyright (c) 2000-2011 INRIA, France Telecom
// All rights reserved.
//
// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions
// are met:
// 1. Redistributions of source code must retain the above copyright
// notice, this list of conditions and the following disclaimer.
// 2. Redistributions in binary form must reproduce the above copyright
// notice, this list of conditions and the following disclaimer in the
// documentation and/or other materials provided with the distribution.
// 3. Neither the name of the copyright holders nor the names of its
// contributors may be used to endorse or promote products derived from
// this software without specific prior written permission.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
// THE POSSIBILITY OF SUCH DAMAGE.
package org.objectweb.asm;
import static java.util.stream.Collectors.toSet;
import static org.junit.jupiter.api.Assertions.assertEquals;
import java.util.Arrays;
import java.util.HashSet;
import org.junit.jupiter.api.Test;
/**
* MethodWriter tests.
*
* @author Eric Bruneton
*/
public class MethodWriterTest {
/**
* Tests that the attribute name fields of Constants are the expected ones. This test is designed
* to fail each time new attributes are added to Constants, and serves as a reminder to update the
* {@link MethodWriter#canCopyMethodAttributes} method, if needed.
*/
@Test
public void testCanCopyMethodAttributesUpdated() {
// IMPORTANT: if this fails, update the list AND update MethodWriter.canCopyMethodAttributes(),
// if needed.
assertEquals(
new HashSet<String>(
Arrays.asList(
Constants.CONSTANT_VALUE,
Constants.CODE,
Constants.STACK_MAP_TABLE,
Constants.EXCEPTIONS,
Constants.INNER_CLASSES,
Constants.ENCLOSING_METHOD,
Constants.SYNTHETIC,
Constants.SIGNATURE,
Constants.SOURCE_FILE,
Constants.SOURCE_DEBUG_EXTENSION,
Constants.LINE_NUMBER_TABLE,
Constants.LOCAL_VARIABLE_TABLE,
Constants.LOCAL_VARIABLE_TYPE_TABLE,
Constants.DEPRECATED,
Constants.RUNTIME_VISIBLE_ANNOTATIONS,
Constants.RUNTIME_INVISIBLE_ANNOTATIONS,
Constants.RUNTIME_VISIBLE_PARAMETER_ANNOTATIONS,
Constants.RUNTIME_INVISIBLE_PARAMETER_ANNOTATIONS,
Constants.RUNTIME_VISIBLE_TYPE_ANNOTATIONS,
Constants.RUNTIME_INVISIBLE_TYPE_ANNOTATIONS,
Constants.ANNOTATION_DEFAULT,
Constants.BOOTSTRAP_METHODS,
Constants.METHOD_PARAMETERS,
Constants.MODULE,
Constants.MODULE_PACKAGES,
Constants.MODULE_MAIN_CLASS)),
Arrays.stream(Constants.class.getDeclaredFields())
.filter(field -> field.getType() == String.class)
.map(
field -> {
try {
return field.get(null);
} catch (IllegalArgumentException | IllegalAccessException e) {
throw new RuntimeException("Can't get field value", e);
}
})
.collect(toSet()));
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment