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

Merge branch '317823-fix-compute-maxs-with-jsr'

parents b4491054 fcb74d56
......@@ -79,6 +79,10 @@ public class AnalyzerTest {
methodVisitor.visitInsn(Opcodes.ICONST_0);
}
private void POP() {
methodVisitor.visitInsn(Opcodes.POP);
}
private void ICONST_0() {
methodVisitor.visitInsn(Opcodes.ICONST_0);
}
......@@ -816,6 +820,78 @@ public class AnalyzerTest {
assertMaxs(4, 6);
}
/**
* Tests an example coming from distilled down version of
* com/sun/corba/ee/impl/protocol/CorbaClientDelegateImpl from GlassFish 2. See issue #317823.
*/
@Test
public void testGlassFish2CorbaClientDelegateImplExample() {
Label l0 = new Label();
Label l1 = new Label();
Label l2 = new Label();
Label l3 = new Label();
Label l4 = new Label();
Label l5 = new Label();
Label l6 = new Label();
Label l7 = new Label();
Label l8 = new Label();
Label l9 = new Label();
Label l10 = new Label();
Label l11 = new Label();
Label l12 = new Label();
LABEL(l0);
JSR(l9);
LABEL(l1);
GOTO(l10);
LABEL(l2);
POP();
JSR(l9);
LABEL(l3);
ACONST_NULL();
ATHROW();
LABEL(l9);
ASTORE(1);
RET(1);
LABEL(l10);
ACONST_NULL();
ACONST_NULL();
ACONST_NULL();
POP();
POP();
POP();
LABEL(l4);
GOTO(l11);
LABEL(l5);
POP();
GOTO(l11);
ACONST_NULL();
ATHROW();
LABEL(l11);
ICONST_0();
IFNE(l0);
JSR(l12);
LABEL(l6);
RETURN();
LABEL(l7);
POP();
JSR(l12);
LABEL(l8);
ACONST_NULL();
ATHROW();
LABEL(l12);
ASTORE(2);
RET(2);
TRYCATCH(l0, l1, l2);
TRYCATCH(l2, l3, l2);
TRYCATCH(l0, l4, l5);
TRYCATCH(l0, l6, l7);
TRYCATCH(l7, l8, l7);
assertMaxs(3, 3);
}
protected void assertMaxs(final int maxStack, final int maxLocals) {
methodVisitor.visitMaxs(maxStack, maxLocals);
methodVisitor.visitEnd();
......
......@@ -1373,6 +1373,175 @@ public class JSRInlinerAdapterTest extends AsmTest {
assertMethodEquals(expectedMethod, inlinedMethod);
}
/**
* Tests an example coming from distilled down version of
* com/sun/corba/ee/impl/protocol/CorbaClientDelegateImpl from GlassFish 2. See issue #317823.
*/
@Test
public void testGlassFish2CorbaClientDelegateImplExample() {
Label l0 = new Label();
Label l1 = new Label();
Label l2 = new Label();
Label l3 = new Label();
Label l4 = new Label();
Label l5 = new Label();
Label l6 = new Label();
Label l7 = new Label();
Label l8 = new Label();
Label l9 = new Label();
Label l10 = new Label();
Label l11 = new Label();
Label l12 = new Label();
new Generator(inlinedMethod)
.LABEL(l0)
.JSR(l9)
.LABEL(l1)
.GOTO(l10)
.LABEL(l2)
.POP()
.JSR(l9)
.LABEL(l3)
.ACONST_NULL()
.ATHROW()
.LABEL(l9)
.ASTORE(1)
.RET(1)
.LABEL(l10)
.ACONST_NULL()
.ACONST_NULL()
.ACONST_NULL()
.POP()
.POP()
.POP()
.LABEL(l4)
.GOTO(l11)
.LABEL(l5)
.POP()
.GOTO(l11)
.ACONST_NULL()
.ATHROW()
.LABEL(l11)
.ICONST_0()
.IFNE(l0)
.JSR(l12)
.LABEL(l6)
.RETURN()
.LABEL(l7)
.POP()
.JSR(l12)
.LABEL(l8)
.ACONST_NULL()
.ATHROW()
.LABEL(l12)
.ASTORE(2)
.RET(2)
.TRYCATCH(l0, l1, l2)
.TRYCATCH(l2, l3, l2)
.TRYCATCH(l0, l4, l5)
.TRYCATCH(l0, l6, l7)
.TRYCATCH(l7, l8, l7)
.END(3, 3);
Label L0 = new Label();
Label L1 = new Label();
Label L2 = new Label();
Label L3 = new Label();
Label L4 = new Label();
Label L5 = new Label();
Label L6 = new Label();
Label L7 = new Label();
Label L8 = new Label();
Label L9_1a = new Label();
Label L9_1b = new Label();
Label L9_1c = new Label();
Label L9_2a = new Label();
Label L9_2b = new Label();
Label L10 = new Label();
Label L11 = new Label();
Label L12_1a = new Label();
Label L12_1b = new Label();
Label L12_1c = new Label();
Label L12_2a = new Label();
Label L12_2b = new Label();
new Generator(expectedMethod)
// --- Main Subroutine ---
.LABEL(L0)
.ACONST_NULL()
.GOTO(L9_1a)
.LABEL(L9_1b)
.LABEL(L1)
.GOTO(L10)
.LABEL(L2)
.POP()
.ACONST_NULL()
.GOTO(L9_2a)
.LABEL(L9_2b)
.LABEL(L3)
.ACONST_NULL()
.ATHROW()
.LABEL(L10)
.ACONST_NULL()
.ACONST_NULL()
.ACONST_NULL()
.POP()
.POP()
.POP()
.LABEL(L4)
.GOTO(L11)
.LABEL(L5)
.POP()
.GOTO(L11)
// [some dead code skipped here]
.LABEL(L11)
.ICONST_0()
.IFNE(L0)
.ACONST_NULL()
.GOTO(L12_1a)
.LABEL(L12_1b)
.LABEL(L6)
.RETURN()
.LABEL(L7)
.POP()
.ACONST_NULL()
.GOTO(L12_2a)
.LABEL(L12_2b)
.LABEL(L8)
.ACONST_NULL()
.ATHROW()
// --- First instantiation of first subroutine ---
.LABEL()
.LABEL(L9_1a)
.ASTORE(1)
.GOTO(L9_1b)
.LABEL(L9_1c)
// --- Second instantiation of first subroutine ---
.LABEL(L9_2a)
.ASTORE(1)
.GOTO(L9_2b)
.LABEL(L12_1c)
// --- First instantiation of second subroutine ---
.LABEL(L12_1a)
.ASTORE(2)
.GOTO(L12_1b)
// --- Second instantiation of second subroutine ---
.LABEL(L12_2a)
.ASTORE(2)
.GOTO(L12_2b)
.TRYCATCH(L0, L1, L2)
.TRYCATCH(L2, L3, L2)
.TRYCATCH(L0, L4, L5)
.TRYCATCH(L0, L6, L7)
.TRYCATCH(L7, L8, L7)
.TRYCATCH(L9_1a, L9_1c, L5)
.TRYCATCH(L9_1a, L9_1c, L7)
.TRYCATCH(L9_2a, L12_1c, L5)
.TRYCATCH(L9_2a, L12_1c, L7)
.END(3, 3);
assertMethodEquals(expectedMethod, inlinedMethod);
}
/**
* Tests a method which has line numbers and local variable declarations.
*
......@@ -1492,6 +1661,11 @@ public class JSRInlinerAdapterTest extends AsmTest {
return this;
}
Generator POP() {
methodNode.visitInsn(Opcodes.POP);
return this;
}
Generator ISTORE(final int var) {
methodNode.visitVarInsn(Opcodes.ISTORE, var);
return this;
......
......@@ -78,11 +78,8 @@ public class Label {
*/
static final int FLAG_SUBROUTINE_START = 32;
/** A flag indicating that the basic block corresponding to a label is part of a subroutine. */
static final int FLAG_SUBROUTINE_BODY = 64;
/** A flag indicating that the basic block corresponding to a label is the end of a subroutine. */
static final int FLAG_SUBROUTINE_END = 128;
static final int FLAG_SUBROUTINE_END = 64;
/**
* The number of elements to add to the {@link #otherLineNumbers} array when it needs to be
......@@ -91,16 +88,16 @@ public class Label {
static final int LINE_NUMBERS_CAPACITY_INCREMENT = 4;
/**
* The number of elements to add to the {@link #values} array when it needs to be resized to store
* a new value.
* The number of elements to add to the {@link #forwardReferences} array when it needs to be
* resized to store a new forward reference.
*/
static final int VALUES_CAPACITY_INCREMENT = 6;
static final int FORWARD_REFERENCES_CAPACITY_INCREMENT = 6;
/**
* The bit mask to extract the type of a forward reference to this label. The extracted type is
* either {@link #FORWARD_REFERENCE_TYPE_SHORT} or {@link #FORWARD_REFERENCE_TYPE_WIDE}.
*
* @see #values
* @see #forwardReferences
*/
static final int FORWARD_REFERENCE_TYPE_MASK = 0xF0000000;
......@@ -121,7 +118,7 @@ public class Label {
* is the bytecode offset where the forward reference value is stored (using either 2 or 4 bytes,
* as indicated by the {@link #FORWARD_REFERENCE_TYPE_MASK}).
*
* @see #values
* @see #forwardReferences
*/
static final int FORWARD_REFERENCE_HANDLE_MASK = 0x0FFFFFFF;
......@@ -143,7 +140,7 @@ public class Label {
* The type and status of this label or its corresponding basic block. Must be zero or more of
* {@link #FLAG_DEBUG_ONLY}, {@link #FLAG_JUMP_TARGET}, {@link #FLAG_RESOLVED}, {@link
* #FLAG_REACHABLE}, {@link #FLAG_SUBROUTINE_CALLER}, {@link #FLAG_SUBROUTINE_START}, {@link
* #FLAG_SUBROUTINE_BODY}, {@link #FLAG_SUBROUTINE_END}.
* #FLAG_SUBROUTINE_END}.
*/
short flags;
......@@ -167,38 +164,30 @@ public class Label {
*/
int bytecodeOffset;
/** The number of elements actually used in the {@link #values} array. */
private short valueCount;
/**
* The additional values associated with this label.
* The forward references to this label. The first element is the number of forward references,
* times 2 (this corresponds to the index of the last element actually used in this array). Then,
* each forward reference is described with two consecutive integers noted
* 'sourceInsnBytecodeOffset' and 'reference':
*
* <ul>
* <li>before {@link MethodWriter#computeMaxStackAndLocal}, this array contains the forward
* references to this label. Each forward reference is described with two consecutive
* integers noted 'sourceInsnBytecodeOffset' and 'reference':
* <ul>
* <li>'sourceInsnBytecodeOffset' is the bytecode offset of the instruction that contains
* the forward reference,
* <li>'reference' contains the type and the offset in the bytecode where the forward
* reference value must be stored, which can be extracted with {@link
* #FORWARD_REFERENCE_TYPE_MASK} and {@link #FORWARD_REFERENCE_HANDLE_MASK}.
* </ul>
* For instance, for an ifnull instruction at bytecode offset x, 'source' is equal to x, and
* 'reference' is of type {@link #FORWARD_REFERENCE_TYPE_SHORT} with value x + 1 (because
* the ifnull instruction uses a 2 bytes bytecode offset operand stored one byte after the
* start of the instruction itself). For the default case of a lookupswitch instruction at
* bytecode offset x, 'source' is equal to x, and 'reference' is of type {@link
* #FORWARD_REFERENCE_TYPE_WIDE} with value between x + 1 and x + 4 (because the
* lookupswitch instruction uses a 4 bytes bytecode offset operand stored one to four bytes
* after the start of the instruction itself).
* <li>during {@link MethodWriter#computeMaxStackAndLocal}, this array is used as a bit field in
* order to store, for each basic block, the set of subroutines to which it belongs
* (subroutines are numbered from 0 to n, and a basic block belongs to subroutine i if and
* only if the bit number i of this bit field is set).
* <li>'sourceInsnBytecodeOffset' is the bytecode offset of the instruction that contains the
* forward reference,
* <li>'reference' contains the type and the offset in the bytecode where the forward reference
* value must be stored, which can be extracted with {@link #FORWARD_REFERENCE_TYPE_MASK}
* and {@link #FORWARD_REFERENCE_HANDLE_MASK}.
* </ul>
*
* For instance, for an ifnull instruction at bytecode offset x, 'sourceInsnBytecodeOffset' is
* equal to x, and 'reference' is of type {@link #FORWARD_REFERENCE_TYPE_SHORT} with value x + 1
* (because the ifnull instruction uses a 2 bytes bytecode offset operand stored one byte after
* the start of the instruction itself). For the default case of a lookupswitch instruction at
* bytecode offset x, 'sourceInsnBytecodeOffset' is equal to x, and 'reference' is of type {@link
* #FORWARD_REFERENCE_TYPE_WIDE} with value between x + 1 and x + 4 (because the lookupswitch
* instruction uses a 4 bytes bytecode offset operand stored one to four bytes after the start of
* the instruction itself).
*/
private int[] values;
private int[] forwardReferences;
// -----------------------------------------------------------------------------------------------
......@@ -238,10 +227,19 @@ public class Label {
/**
* The maximum height reached by the output stack, relatively to the top of the input stack, in
* the basick block corresponding to this label. This maximum is always positive or null.
* the basic block corresponding to this label. This maximum is always positive or null.
*/
short outputStackMax;
/**
* The id of the subroutine to which this basic block belongs, or 0. If the basic block belongs to
* several subroutines, this is the id of the "oldest" subroutine that contains it (with the
* convention that a subroutine calling another one is "older" than the callee). This field is
* computed in {@link MethodWriter#computeMaxStackAndLocal}, if the method contains JSR
* instructions.
*/
short subroutineId;
/**
* The input and output stack map frames of the basic block corresponding to this label. This
* field is only used when the {@link MethodWriter#COMPUTE_ALL_FRAMES} or {@link
......@@ -341,7 +339,7 @@ public class Label {
}
int otherLineNumberIndex = ++otherLineNumbers[0];
if (otherLineNumberIndex >= otherLineNumbers.length) {
int[] newLineNumbers = new int[otherLineNumbers.length + VALUES_CAPACITY_INCREMENT];
int[] newLineNumbers = new int[otherLineNumbers.length + LINE_NUMBERS_CAPACITY_INCREMENT];
System.arraycopy(otherLineNumbers, 0, newLineNumbers, 0, otherLineNumbers.length);
otherLineNumbers = newLineNumbers;
}
......@@ -415,16 +413,18 @@ public class Label {
*/
private void addForwardReference(
final int sourceInsnBytecodeOffset, final int referenceType, final int referenceHandle) {
if (values == null) {
values = new int[VALUES_CAPACITY_INCREMENT];
if (forwardReferences == null) {
forwardReferences = new int[FORWARD_REFERENCES_CAPACITY_INCREMENT];
}
if (valueCount >= values.length) {
int[] newValues = new int[values.length + VALUES_CAPACITY_INCREMENT];
System.arraycopy(values, 0, newValues, 0, values.length);
values = newValues;
int lastElementIndex = forwardReferences[0];
if (lastElementIndex + 2 >= forwardReferences.length) {
int[] newValues = new int[forwardReferences.length + FORWARD_REFERENCES_CAPACITY_INCREMENT];
System.arraycopy(forwardReferences, 0, newValues, 0, forwardReferences.length);
forwardReferences = newValues;
}
values[valueCount++] = sourceInsnBytecodeOffset;
values[valueCount++] = referenceType | referenceHandle;
forwardReferences[++lastElementIndex] = sourceInsnBytecodeOffset;
forwardReferences[++lastElementIndex] = referenceType | referenceHandle;
forwardReferences[0] = lastElementIndex;
}
/**
......@@ -444,10 +444,13 @@ public class Label {
final boolean resolve(final byte[] code, final int bytecodeOffset) {
this.flags |= FLAG_RESOLVED;
this.bytecodeOffset = bytecodeOffset;
if (forwardReferences == null) {
return false;
}
boolean hasAsmInstructions = false;
for (int i = 0; i < valueCount; i += 2) {
final int sourceInsnBytecodeOffset = values[i];
final int reference = values[i + 1];
for (int i = forwardReferences[0]; i > 0; i -= 2) {
final int sourceInsnBytecodeOffset = forwardReferences[i - 1];
final int reference = forwardReferences[i];
final int relativeOffset = bytecodeOffset - sourceInsnBytecodeOffset;
int handle = reference & FORWARD_REFERENCE_HANDLE_MASK;
if ((reference & FORWARD_REFERENCE_TYPE_MASK) == FORWARD_REFERENCE_TYPE_SHORT) {
......@@ -493,9 +496,8 @@ public class Label {
*
* @param subroutineId the id of the subroutine starting with the basic block corresponding to
* this label.
* @param numSubroutine the total number of subroutines in the method.
*/
final void markSubroutine(final int subroutineId, final int numSubroutine) {
final void markSubroutine(final short subroutineId) {
// Data flow algorithm: put this basic block in a list of blocks to process (which are blocks
// belonging to subroutine subroutineId) and, while there are blocks to process, remove one from
// the list, mark it as belonging to the subroutine, and add its successor basic blocks in the
......@@ -508,10 +510,10 @@ public class Label {
listOfBlocksToProcess = listOfBlocksToProcess.nextListElement;
basicBlock.nextListElement = null;
// If it is not already marked as belonging to the subroutine subroutineId, mark it and add
// its successors to the list of blocks to process (unless already done).
if (!basicBlock.isInSubroutine(subroutineId)) {
basicBlock.addToSubroutine(subroutineId, numSubroutine);
// If it is not already marked as belonging to a subroutine, mark it as belonging to
// subroutineId and add its successors to the list of blocks to process (unless already done).
if (basicBlock.subroutineId == 0) {
basicBlock.subroutineId = subroutineId;
listOfBlocksToProcess = basicBlock.pushSuccessors(listOfBlocksToProcess);
}
}
......@@ -547,10 +549,10 @@ public class Label {
listOfProcessedBlocks = basicBlock;
// Add an edge from this block to the successor of the caller basic block, if this block is
// the end of a subroutine and if this block and subroutineCaller do not belong to a common
// the end of a subroutine and if this block and subroutineCaller do not belong to the same
// subroutine.
if ((basicBlock.flags & FLAG_SUBROUTINE_END) != 0
&& !basicBlock.isInSameSubroutine(subroutineCaller)) {
&& basicBlock.subroutineId != subroutineCaller.subroutineId) {
basicBlock.outgoingEdges =
new Edge(
basicBlock.outputStackSize,
......@@ -603,48 +605,6 @@ public class Label {
return newListOfLabelsToProcess;
}
/**
* @param subroutineId a subroutine id, between 0 and the total number of subroutines in the
* method.
* @return whether this basic block belongs to the given subroutine.
*/
private boolean isInSubroutine(final int subroutineId) {
if ((flags & Label.FLAG_SUBROUTINE_BODY) != 0) {
return (values[subroutineId / 32] & (1 << (subroutineId % 32))) != 0;
}
return false;
}
/**
* @param basicBlock another basic block.
* @return whether this basic block and the given one belong to a common subroutine.
*/
private boolean isInSameSubroutine(final Label basicBlock) {
if ((flags & FLAG_SUBROUTINE_BODY) == 0 || (basicBlock.flags & FLAG_SUBROUTINE_BODY) == 0) {
return false;
}
for (int i = 0; i < values.length; ++i) {
if ((values[i] & basicBlock.values[i]) != 0) {
return true;
}
}
return false;
}
/**
* Marks the basic block corresponding to this label as belonging to the given subroutine.
*
* @param subroutineId a subroutine id, between 0 and numSubroutine (inclusive).
* @param numSubroutine the total number of subroutines in the method.
*/
private void addToSubroutine(final int subroutineId, final int numSubroutine) {
if ((flags & FLAG_SUBROUTINE_BODY) == 0) {
flags |= FLAG_SUBROUTINE_BODY;
values = new int[numSubroutine / 32 + 1];
}
values[subroutineId / 32] |= (1 << (subroutineId % 32));
}
// -----------------------------------------------------------------------------------------------
// Overridden Object methods
// -----------------------------------------------------------------------------------------------
......
......@@ -519,8 +519,8 @@ final class MethodWriter extends MethodVisitor {
*/
private int[] currentFrame;
/** The number of subroutines in this method. */
private int numSubroutines;
/** Whether this method contains subroutines. */
private boolean hasSubroutines;
// -----------------------------------------------------------------------------------------------
// Other miscellaneous status fields
......@@ -1140,7 +1140,7 @@ final class MethodWriter extends MethodVisitor {
// Record the fact that 'label' designates a subroutine, if not already done.
if ((label.flags & Label.FLAG_SUBROUTINE_START) == 0) {
label.flags |= Label.FLAG_SUBROUTINE_START;
++numSubroutines;
hasSubroutines = true;
}
currentBasicBlock.flags |= Label.FLAG_SUBROUTINE_CALLER;
// Note that, by construction in this method, a block which calls a subroutine has at
......@@ -1645,19 +1645,18 @@ final class MethodWriter extends MethodVisitor {
}
// Complete the control flow graph with the successor blocks of subroutines, if needed.
if (numSubroutines > 0) {
if (hasSubroutines) {
// First step: find the subroutines. This step determines, for each basic block, to which
// subroutine(s) it belongs. Start with the main "subroutine":
int subroutineId = 0;
firstBasicBlock.markSubroutine(subroutineId, numSubroutines);
short subroutineId = 1;
firstBasicBlock.markSubroutine(subroutineId);
// Then, loop over all the basic blocks to find those that belong to real subroutines.
Label basicBlock = firstBasicBlock;
while (basicBlock != null) {
if ((basicBlock.flags & Label.FLAG_SUBROUTINE_START) != 0
&& (basicBlock.flags & Label.FLAG_SUBROUTINE_BODY) == 0) {
if ((basicBlock.flags & Label.FLAG_SUBROUTINE_START) != 0 && basicBlock.subroutineId == 0) {
// If this subroutine has not been marked yet, find its basic blocks.
subroutineId += 1;
basicBlock.markSubroutine(subroutineId, numSubroutines);
basicBlock.markSubroutine(subroutineId);
}