Trac #64, #69, #81: Produce correct stackmap frames without using LocalVariableSorter.
diff --git a/org.jacoco.core.test/src/org/jacoco/core/instr/MethodInstrumenterTest.java b/org.jacoco.core.test/src/org/jacoco/core/instr/MethodInstrumenterTest.java
index 11de689..d8da1ff 100644
--- a/org.jacoco.core.test/src/org/jacoco/core/instr/MethodInstrumenterTest.java
+++ b/org.jacoco.core.test/src/org/jacoco/core/instr/MethodInstrumenterTest.java
@@ -53,7 +53,7 @@
}
};
- instrumenter = new MethodInstrumenter(actual, 0, "test", "()V",
+ instrumenter = new MethodInstrumenter(actual, 0, "()V",
probeArrayStrategy);
}
diff --git a/org.jacoco.core.test/src/org/jacoco/core/instr/ProbeVariableInserterTest.java b/org.jacoco.core.test/src/org/jacoco/core/instr/ProbeVariableInserterTest.java
new file mode 100644
index 0000000..281f501
--- /dev/null
+++ b/org.jacoco.core.test/src/org/jacoco/core/instr/ProbeVariableInserterTest.java
@@ -0,0 +1,195 @@
+/*******************************************************************************
+ * Copyright (c) 2009, 2010 Mountainminds GmbH & Co. KG and Contributors
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Marc R. Hoffmann - initial API and implementation
+ *
+ * $Id: $
+ *******************************************************************************/
+package org.jacoco.core.instr;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.Arrays;
+
+import org.junit.Test;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.commons.EmptyVisitor;
+
+/**
+ * Unit tests for {@link ProbeVariableInserter}.
+ *
+ * @author Marc R. Hoffmann
+ * @version $Revision: $
+ */
+public class ProbeVariableInserterTest {
+
+ private final MethodVisitor delegate = new EmptyVisitor();
+
+ private int var;
+
+ @Test
+ public void testVariableStatic() {
+ ProbeVariableInserter i = new ProbeVariableInserter(Opcodes.ACC_STATIC,
+ "()V", delegate);
+ assertEquals(0, i.variable);
+ }
+
+ @Test
+ public void testVariableNonStatic() {
+ ProbeVariableInserter i = new ProbeVariableInserter(0, "()V", delegate);
+ assertEquals(1, i.variable);
+ }
+
+ @Test
+ public void testVariableNonStatic_IZObject() {
+ ProbeVariableInserter i = new ProbeVariableInserter(0,
+ "(IZLjava/lang/Object;)V", delegate);
+ assertEquals(4, i.variable);
+ }
+
+ @Test
+ public void testVariableNonStatic_JD() {
+ ProbeVariableInserter i = new ProbeVariableInserter(0, "(JD)V",
+ delegate);
+ assertEquals(5, i.variable);
+ }
+
+ @Test
+ public void testVisitVarIns() {
+ ProbeVariableInserter i = new ProbeVariableInserter(0, "(II)V",
+ new EmptyVisitor() {
+ @Override
+ public void visitVarInsn(final int opcode, final int var) {
+ ProbeVariableInserterTest.this.var = var;
+ }
+ });
+ // Argument variables stay at the same position:
+ i.visitVarInsn(Opcodes.ALOAD, 0);
+ assertEquals(0, var);
+ i.visitVarInsn(Opcodes.ALOAD, 1);
+ assertEquals(1, var);
+ i.visitVarInsn(Opcodes.ALOAD, 2);
+ assertEquals(2, var);
+
+ assertEquals(3, i.variable);
+
+ // Local variables are shifted by one:
+ i.visitVarInsn(Opcodes.ALOAD, 3);
+ assertEquals(4, var);
+ i.visitVarInsn(Opcodes.ALOAD, 4);
+ assertEquals(5, var);
+ }
+
+ @Test
+ public void testVisitIincInsn() {
+ ProbeVariableInserter i = new ProbeVariableInserter(0, "(II)V",
+ new EmptyVisitor() {
+ @Override
+ public void visitIincInsn(final int var, final int increment) {
+ ProbeVariableInserterTest.this.var = var;
+ }
+ });
+ // Argument variables stay at the same position:
+ i.visitIincInsn(0, 999);
+ assertEquals(0, var);
+ i.visitIincInsn(1, 999);
+ assertEquals(1, var);
+ i.visitIincInsn(2, 999);
+ assertEquals(2, var);
+
+ assertEquals(3, i.variable);
+
+ // Local variables are shifted by one:
+ i.visitIincInsn(3, 999);
+ assertEquals(4, var);
+ i.visitIincInsn(4, 999);
+ assertEquals(5, var);
+ }
+
+ @Test
+ public void testVisitLocalVariable() {
+ ProbeVariableInserter i = new ProbeVariableInserter(0, "(II)V",
+ new EmptyVisitor() {
+ @Override
+ public void visitLocalVariable(final String name,
+ final String desc, final String signature,
+ final Label start, final Label end, final int index) {
+ ProbeVariableInserterTest.this.var = index;
+ }
+ });
+ // Argument variables stay at the same position:
+ i.visitLocalVariable(null, null, null, null, null, 0);
+ assertEquals(0, var);
+ i.visitLocalVariable(null, null, null, null, null, 1);
+ assertEquals(1, var);
+ i.visitLocalVariable(null, null, null, null, null, 2);
+ assertEquals(2, var);
+
+ assertEquals(3, i.variable);
+
+ // Local variables are shifted by one:
+ i.visitLocalVariable(null, null, null, null, null, 3);
+ assertEquals(4, var);
+ i.visitLocalVariable(null, null, null, null, null, 4);
+ assertEquals(5, var);
+ }
+
+ @Test
+ public void testVisitMaxs() {
+ ProbeVariableInserter i = new ProbeVariableInserter(0, "(II)V",
+ new EmptyVisitor() {
+ @Override
+ public void visitMaxs(final int maxStack,
+ final int maxLocals) {
+ ProbeVariableInserterTest.this.var = maxLocals;
+ }
+ });
+ i.visitMaxs(4, 8);
+ assertEquals(9, var);
+ }
+
+ @Test
+ public void testVisitFrame() {
+ class Recorder extends EmptyVisitor {
+ int nLocal;
+ Object[] local;
+
+ @Override
+ public void visitFrame(final int type, final int nLocal,
+ final Object[] local, final int nStack, final Object[] stack) {
+ this.nLocal = nLocal;
+ this.local = local;
+ }
+ }
+ final Recorder rec = new Recorder();
+ ProbeVariableInserter i = new ProbeVariableInserter(0, "(J)V", rec);
+
+ // The first (implicit) frame must not be modified:
+ i.visitFrame(Opcodes.F_NEW, 2, new Object[] { "LFoo;", Opcodes.LONG },
+ 0, null);
+ assertEquals(2, rec.nLocal);
+ assertEquals(Arrays.asList((Object) "LFoo;", Opcodes.LONG), Arrays
+ .asList(rec.local));
+
+ // Starting from the second frame on the probe variable is inserted:
+ i.visitFrame(Opcodes.F_NEW, 3, new Object[] { "LFoo;", Opcodes.LONG,
+ "Ljava/lang/String;" }, 0, null);
+ assertEquals(4, rec.nLocal);
+ assertEquals(Arrays.asList((Object) "LFoo;", Opcodes.LONG, "[Z",
+ "Ljava/lang/String;"), Arrays.asList(rec.local));
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void testVisitFrameNegative() {
+ ProbeVariableInserter i = new ProbeVariableInserter(0, "(J)V", delegate);
+ i.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
+ }
+
+}
diff --git a/org.jacoco.core/src/org/jacoco/core/instr/ClassInstrumenter.java b/org.jacoco.core/src/org/jacoco/core/instr/ClassInstrumenter.java
index d1b315d..59ee3aa 100644
--- a/org.jacoco.core/src/org/jacoco/core/instr/ClassInstrumenter.java
+++ b/org.jacoco.core/src/org/jacoco/core/instr/ClassInstrumenter.java
@@ -31,6 +31,9 @@
*/
public class ClassInstrumenter extends BlockClassAdapter {
+ private static final Object[] STACK_ARRZ = new Object[] { GeneratorConstants.PROBEDATA_TYPE
+ .getDescriptor() };
+
private final ClassVisitor delegate;
private final long id;
@@ -91,8 +94,7 @@
if (mv == null) {
return null;
}
- return new MethodInstrumenter(mv, access, name, desc,
- probeArrayStrategy);
+ return new MethodInstrumenter(mv, access, desc, probeArrayStrategy);
}
@Override
@@ -203,6 +205,7 @@
// Stack[0]: [Z
// Return the method's block array:
+ mv.visitFrame(Opcodes.F_FULL, 0, new Object[0], 1, STACK_ARRZ);
mv.visitLabel(alreadyInitialized);
mv.visitInsn(Opcodes.ARETURN);
diff --git a/org.jacoco.core/src/org/jacoco/core/instr/Instrumenter.java b/org.jacoco.core/src/org/jacoco/core/instr/Instrumenter.java
index 02d7a88..e3c8a60 100644
--- a/org.jacoco.core/src/org/jacoco/core/instr/Instrumenter.java
+++ b/org.jacoco.core/src/org/jacoco/core/instr/Instrumenter.java
@@ -19,7 +19,6 @@
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.ClassWriter;
-import org.objectweb.asm.Opcodes;
/**
* Several APIs to instrument Java class definitions for coverage tracing.
@@ -64,12 +63,7 @@
*
*/
public byte[] instrument(final ClassReader reader) {
- // TODO: Temporary hack to produce valid stackmap frames for Java > 1.6,
- // ideally the visitor chain for instrumentation would produce valid
- // frames directly.
- final int ver = reader.readUnsignedShort(6);
- final int flags = ver >= Opcodes.V1_6 ? ClassWriter.COMPUTE_FRAMES : 0;
- final ClassWriter writer = new ClassWriter(reader, flags);
+ final ClassWriter writer = new ClassWriter(reader, 0);
final ClassVisitor visitor = createInstrumentingVisitor(CRC64
.checksum(reader.b), writer);
reader.accept(visitor, ClassReader.EXPAND_FRAMES);
diff --git a/org.jacoco.core/src/org/jacoco/core/instr/MethodInstrumenter.java b/org.jacoco.core/src/org/jacoco/core/instr/MethodInstrumenter.java
index 34ac71d..8834f0c 100644
--- a/org.jacoco.core/src/org/jacoco/core/instr/MethodInstrumenter.java
+++ b/org.jacoco.core/src/org/jacoco/core/instr/MethodInstrumenter.java
@@ -14,7 +14,6 @@
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;
-import org.objectweb.asm.commons.GeneratorAdapter;
/**
* This method adapter instruments a method to record every block that gets
@@ -23,15 +22,13 @@
* @author Marc R. Hoffmann
* @version $Revision: $
*/
-class MethodInstrumenter extends GeneratorAdapter implements
+class MethodInstrumenter extends ProbeVariableInserter implements
IBlockMethodVisitor {
private final IProbeArrayStrategy probeArrayStrategy;
private int accessorStackSize;
- private int probeArray;
-
/**
* Create a new instrumenter instance for the given method.
*
@@ -39,17 +36,14 @@
* next method visitor in the chain
* @param access
* access flags for the method
- * @param name
- * name of the method
* @param desc
* description of the method
* @param probeArrayStrategy
* strategy to get access to the probe array
*/
public MethodInstrumenter(final MethodVisitor mv, final int access,
- final String name, final String desc,
- final IProbeArrayStrategy probeArrayStrategy) {
- super(mv, access, name, desc);
+ final String desc, final IProbeArrayStrategy probeArrayStrategy) {
+ super(access, desc, mv);
this.probeArrayStrategy = probeArrayStrategy;
}
@@ -62,8 +56,7 @@
// Stack[0]: [Z
- probeArray = newLocal(GeneratorConstants.PROBEDATA_TYPE);
- mv.visitVarInsn(Opcodes.ASTORE, probeArray);
+ mv.visitVarInsn(Opcodes.ASTORE, variable);
}
@Override
@@ -73,7 +66,7 @@
// stack size is an absolute maximum, as the accessor code is inserted
// at the very beginning of each method when the stack size is empty.
final int increasedStack = Math.max(maxStack + 3, accessorStackSize);
- super.visitMaxs(increasedStack, maxLocals + 1);
+ super.visitMaxs(increasedStack, maxLocals);
}
// === IBlockMethodVisitor ===
@@ -82,7 +75,7 @@
// At the end of every block we set the corresponding position in the
// boolean[] array to true.
- mv.visitVarInsn(Opcodes.ALOAD, probeArray);
+ mv.visitVarInsn(Opcodes.ALOAD, variable);
// Stack[0]: [Z
@@ -100,6 +93,18 @@
visitInsn(Opcodes.BASTORE);
}
+ private void push(final int value) {
+ if (value >= -1 && value <= 5) {
+ mv.visitInsn(Opcodes.ICONST_0 + value);
+ } else if (value >= Byte.MIN_VALUE && value <= Byte.MAX_VALUE) {
+ mv.visitIntInsn(Opcodes.BIPUSH, value);
+ } else if (value >= Short.MIN_VALUE && value <= Short.MAX_VALUE) {
+ mv.visitIntInsn(Opcodes.SIPUSH, value);
+ } else {
+ mv.visitLdcInsn(new Integer(value));
+ }
+ }
+
public void visitBlockEnd(final int id) {
// nothing to do here
}
diff --git a/org.jacoco.core/src/org/jacoco/core/instr/ProbeVariableInserter.java b/org.jacoco.core/src/org/jacoco/core/instr/ProbeVariableInserter.java
new file mode 100644
index 0000000..165c39e
--- /dev/null
+++ b/org.jacoco.core/src/org/jacoco/core/instr/ProbeVariableInserter.java
@@ -0,0 +1,126 @@
+/*******************************************************************************
+ * Copyright (c) 2009, 2010 Mountainminds GmbH & Co. KG and Contributors
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Marc R. Hoffmann - initial API and implementation
+ *
+ * $Id: $
+ *******************************************************************************/
+package org.jacoco.core.instr;
+
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodAdapter;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.Type;
+
+/**
+ * Simplified version of ASM's
+ * {@link org.objectweb.asm.commons.LocalVariablesSorter} that inserts a local
+ * variable at the very beginning of the method. This avoids maintaining mapping
+ * tables and prevents ASM bug #314563.
+ *
+ * @author Marc R. Hoffmann
+ * @version $Revision: $
+ */
+class ProbeVariableInserter extends MethodAdapter {
+
+ /** Position of the inserted variable. */
+ protected final int variable;
+
+ /** Index the inserted variable. */
+ private final int variableIdx;
+
+ private boolean firstFrame = true;
+
+ /**
+ * Creates a new {@link ProbeVariableInserter}.
+ *
+ * @param access
+ * access flags of the adapted method.
+ * @param desc
+ * the method's descriptor
+ * @param mv
+ * the method visitor to which this adapter delegates calls
+ */
+ ProbeVariableInserter(final int access, final String desc,
+ final MethodVisitor mv) {
+ super(mv);
+ int idx = (Opcodes.ACC_STATIC & access) == 0 ? 1 : 0;
+ int pos = idx;
+ for (final Type t : Type.getArgumentTypes(desc)) {
+ idx++;
+ pos += t.getSize();
+ }
+ variableIdx = idx;
+ variable = pos;
+ }
+
+ @Override
+ public void visitVarInsn(final int opcode, final int var) {
+ mv.visitVarInsn(opcode, map(var));
+ }
+
+ @Override
+ public void visitIincInsn(final int var, final int increment) {
+ mv.visitIincInsn(map(var), increment);
+ }
+
+ @Override
+ public void visitMaxs(final int maxStack, final int maxLocals) {
+ mv.visitMaxs(maxStack, maxLocals + 1);
+ }
+
+ @Override
+ public void visitLocalVariable(final String name, final String desc,
+ final String signature, final Label start, final Label end,
+ final int index) {
+ mv.visitLocalVariable(name, desc, signature, start, end, map(index));
+ }
+
+ private int map(final int var) {
+ if (var < variable) {
+ return var;
+ } else {
+ return var + 1;
+ }
+ }
+
+ @Override
+ public void visitFrame(final int type, final int nLocal,
+ final Object[] local, final int nStack, final Object[] stack) {
+
+ if (type != Opcodes.F_NEW) { // uncompressed frame
+ throw new IllegalStateException(
+ "ClassReader.accept() should be called with EXPAND_FRAMES flag");
+ }
+
+ if (firstFrame) {
+ // The first frame is generated by ASM and represents the implicit
+ // frame derived from the method signature. This frame must not be
+ // modified.
+ mv.visitFrame(type, nLocal, local, nStack, stack);
+ firstFrame = false;
+ return;
+ }
+
+ final Object[] newLocal = new Object[nLocal + 1];
+ for (int i = 0; i <= local.length; i++) {
+ if (i < variableIdx) {
+ newLocal[i] = local[i];
+ continue;
+ }
+ if (i > variableIdx) {
+ newLocal[i] = local[i - 1];
+ continue;
+ }
+ newLocal[i] = GeneratorConstants.PROBEDATA_TYPE.getDescriptor();
+ }
+ mv.visitFrame(type, nLocal + 1, newLocal, nStack, stack);
+ }
+
+}
diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html
index 92a2699..361a8d0 100644
--- a/org.jacoco.doc/docroot/doc/changes.html
+++ b/org.jacoco.doc/docroot/doc/changes.html
@@ -22,13 +22,15 @@
<h3>New Features</h3>
<ul>
<li>Support for different archives (jar, war, ear etc.) and nested archives
- (Trac #78).</li>
+ (Trac #78).</li>
<li>XML report with line level coverage information (requested for Sonar).</li>
</ul>
<h3>Fixed Bugs</h3>
<ul>
<li>Correct stackmap frames for Java 1.6 class files. (Track #81).</li>
+ <li>Avoid usage of <code>LocalVariableSorter</code> due to ASM bug #314563
+ (Track #69).</li>
</ul>
<h2>Release 0.3.2 (2010/04/01)</h2>
@@ -36,7 +38,7 @@
<h3>New Features</h3>
<ul>
<li>New HTML report option to directly create a zip file containing the report
- (Trac #12).</li>
+ (Trac #12).</li>
<li>Code coverage for static initializers in interfaces (Trac #21).</li>
<li>Better error handling for <code>report</code> Ant task (Trac #71).</li>
<li>Classes without instructions are excluded from reports (Trac #73).</li>
@@ -45,7 +47,7 @@
<h3>Fixed Bugs</h3>
<ul>
<li>XML and CSV report output now also works for structures without groups
- (Track #76).</li>
+ (Track #76).</li>
</ul>
<h3>API Changes</h3>