Don't insert stackmap frames into class files with version < 1.6. (#667)
For certain probes additional frames needs to be inserted, but only from
class file version 1.6 on.
diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/InstrSupportTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/InstrSupportTest.java
index fb1cb3e..95de8bd 100644
--- a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/InstrSupportTest.java
+++ b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/InstrSupportTest.java
@@ -12,9 +12,13 @@
package org.jacoco.core.internal.instr;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import org.jacoco.core.internal.BytecodeVersion;
import org.junit.Before;
import org.junit.Test;
+import org.objectweb.asm.Opcodes;
import org.objectweb.asm.util.Printer;
import org.objectweb.asm.util.Textifier;
import org.objectweb.asm.util.TraceMethodVisitor;
@@ -34,6 +38,24 @@
}
@Test
+ public void needFrames_should_return_false_for_versions_less_than_1_6() {
+ assertFalse(InstrSupport.needsFrames(Opcodes.V1_1));
+ assertFalse(InstrSupport.needsFrames(Opcodes.V1_2));
+ assertFalse(InstrSupport.needsFrames(Opcodes.V1_3));
+ assertFalse(InstrSupport.needsFrames(Opcodes.V1_4));
+ assertFalse(InstrSupport.needsFrames(Opcodes.V1_5));
+ }
+
+ @Test
+ public void needFrames_should_return_true_for_versions_greater_than_or_equal_to_1_6() {
+ assertTrue(InstrSupport.needsFrames(Opcodes.V1_6));
+ assertTrue(InstrSupport.needsFrames(Opcodes.V1_7));
+ assertTrue(InstrSupport.needsFrames(Opcodes.V1_8));
+ assertTrue(InstrSupport.needsFrames(Opcodes.V9));
+ assertTrue(InstrSupport.needsFrames(BytecodeVersion.V10));
+ }
+
+ @Test
public void testAssertNotIntrumentedPositive() {
InstrSupport.assertNotInstrumented("run", "Foo");
}
diff --git a/org.jacoco.core.test/src/org/jacoco/core/test/validation/ClassFileVersionsTest.java b/org.jacoco.core.test/src/org/jacoco/core/test/validation/ClassFileVersionsTest.java
index d73fc64..87d78a2 100644
--- a/org.jacoco.core.test/src/org/jacoco/core/test/validation/ClassFileVersionsTest.java
+++ b/org.jacoco.core.test/src/org/jacoco/core/test/validation/ClassFileVersionsTest.java
@@ -15,7 +15,12 @@
import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
import static org.objectweb.asm.Opcodes.ACC_SUPER;
import static org.objectweb.asm.Opcodes.ALOAD;
+import static org.objectweb.asm.Opcodes.F_NEW;
+import static org.objectweb.asm.Opcodes.IFEQ;
+import static org.objectweb.asm.Opcodes.ILOAD;
import static org.objectweb.asm.Opcodes.INVOKESPECIAL;
+import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL;
+import static org.objectweb.asm.Opcodes.POP;
import static org.objectweb.asm.Opcodes.RETURN;
import static org.objectweb.asm.Opcodes.V1_1;
import static org.objectweb.asm.Opcodes.V1_2;
@@ -30,6 +35,7 @@
import java.io.IOException;
import org.jacoco.core.instr.Instrumenter;
+import org.jacoco.core.internal.BytecodeVersion;
import org.jacoco.core.internal.instr.InstrSupport;
import org.jacoco.core.runtime.IRuntime;
import org.jacoco.core.runtime.SystemPropertiesRuntime;
@@ -37,7 +43,9 @@
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
/**
* Test class inserted stackmap frames for different class file versions.
@@ -85,12 +93,17 @@
}
@Test
- public void test_1_9() throws IOException {
+ public void test_9() throws IOException {
testVersion(V9, true);
}
+ @Test
+ public void test_10() throws IOException {
+ testVersion(BytecodeVersion.V10, true);
+ }
+
private void testVersion(int version, boolean frames) throws IOException {
- final byte[] original = createClass(version);
+ final byte[] original = createClass(version, frames);
IRuntime runtime = new SystemPropertiesRuntime();
Instrumenter instrumenter = new Instrumenter(runtime);
@@ -99,30 +112,52 @@
assertFrames(instrumented, frames);
}
- private void assertFrames(byte[] source, boolean expected) {
- final boolean[] hasFrames = new boolean[] { false };
+ private void assertFrames(byte[] source, final boolean expected) {
+ int version = BytecodeVersion.get(source);
+ source = BytecodeVersion.downgradeIfNeeded(version, source);
new ClassReader(source)
.accept(new ClassVisitor(InstrSupport.ASM_API_VERSION) {
@Override
public MethodVisitor visitMethod(int access, String name,
- String desc, String signature, String[] exceptions) {
+ String desc, String signature,
+ String[] exceptions) {
return new MethodVisitor(InstrSupport.ASM_API_VERSION) {
+ boolean frames = false;
@Override
public void visitFrame(int type, int nLocal,
- Object[] local, int nStack, Object[] stack) {
- hasFrames[0] = true;
+ Object[] local, int nStack,
+ Object[] stack) {
+ frames = true;
}
+ @Override
+ public void visitEnd() {
+ assertEquals(Boolean.valueOf(expected),
+ Boolean.valueOf(frames));
+ }
};
}
-
}, 0);
- assertEquals(Boolean.valueOf(expected), Boolean.valueOf(hasFrames[0]));
}
- private byte[] createClass(int version) {
+ /**
+ * Creates a class that requires a frame before the return statement. Also
+ * for this class instrumentation should insert another frame.
+ *
+ * <code><pre>
+ * public class Sample {
+ * public Sample(boolean b){
+ * if(b){
+ * toString();
+ * }
+ * return;
+ * }
+ * }
+ * </pre></code>
+ */
+ private byte[] createClass(int version, boolean frames) {
ClassWriter cw = new ClassWriter(0);
MethodVisitor mv;
@@ -130,13 +165,26 @@
cw.visit(version, ACC_PUBLIC + ACC_SUPER, "org/jacoco/test/Sample",
null, "java/lang/Object", null);
- mv = cw.visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
+ mv = cw.visitMethod(ACC_PUBLIC, "<init>", "(Z)V", null, null);
mv.visitCode();
mv.visitVarInsn(ALOAD, 0);
mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V",
false);
+ mv.visitVarInsn(ILOAD, 1);
+ Label l1 = new Label();
+ mv.visitJumpInsn(IFEQ, l1);
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Object", "toString",
+ "()Ljava/lang/String;", false);
+ mv.visitInsn(POP);
+ mv.visitLabel(l1);
+ if (frames) {
+ mv.visitFrame(F_NEW, 2,
+ new Object[] { "org/jacoco/test/Sample", Opcodes.INTEGER },
+ 0, new Object[] {});
+ }
mv.visitInsn(RETURN);
- mv.visitMaxs(1, 1);
+ mv.visitMaxs(1, 2);
mv.visitEnd();
cw.visitEnd();
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 85cba59..da956fd 100644
--- a/org.jacoco.core/src/org/jacoco/core/instr/Instrumenter.java
+++ b/org.jacoco.core/src/org/jacoco/core/instr/Instrumenter.java
@@ -29,6 +29,7 @@
import org.jacoco.core.internal.flow.ClassProbesAdapter;
import org.jacoco.core.internal.instr.ClassInstrumenter;
import org.jacoco.core.internal.instr.IProbeArrayStrategy;
+import org.jacoco.core.internal.instr.InstrSupport;
import org.jacoco.core.internal.instr.ProbeArrayStrategyFactory;
import org.jacoco.core.internal.instr.SignatureRemover;
import org.jacoco.core.runtime.IExecutionDataAccessorGenerator;
@@ -97,7 +98,8 @@
final IProbeArrayStrategy strategy = ProbeArrayStrategyFactory
.createFor(classId, reader, accessorGenerator);
final ClassVisitor visitor = new ClassProbesAdapter(
- new ClassInstrumenter(strategy, writer), true);
+ new ClassInstrumenter(strategy, writer),
+ InstrSupport.needsFrames(originalVersion));
reader.accept(visitor, ClassReader.EXPAND_FRAMES);
final byte[] instrumented = writer.toByteArray();
BytecodeVersion.set(instrumented, originalVersion);
diff --git a/org.jacoco.core/src/org/jacoco/core/internal/instr/InstrSupport.java b/org.jacoco.core/src/org/jacoco/core/internal/instr/InstrSupport.java
index b6632c6..c7b7cf8 100644
--- a/org.jacoco.core/src/org/jacoco/core/internal/instr/InstrSupport.java
+++ b/org.jacoco.core/src/org/jacoco/core/internal/instr/InstrSupport.java
@@ -158,6 +158,18 @@
static final int CLINIT_ACC = Opcodes.ACC_SYNTHETIC | Opcodes.ACC_STATIC;
/**
+ * Determines whether the given class file version requires stackmap frames.
+ *
+ * @param version
+ * class file version
+ * @return <code>true</code> if frames are required
+ */
+ public static boolean needsFrames(final int version) {
+ // consider major version only (due to 1.1 anomaly)
+ return (version & 0xff) >= Opcodes.V1_6;
+ }
+
+ /**
* Ensures that the given member does not correspond to a internal member
* created by the instrumentation process. This would mean that the class is
* already instrumented.
@@ -173,8 +185,8 @@
public static void assertNotInstrumented(final String member,
final String owner) throws IllegalStateException {
if (member.equals(DATAFIELD_NAME) || member.equals(INITMETHOD_NAME)) {
- throw new IllegalStateException(format(
- "Class %s is already instrumented.", owner));
+ throw new IllegalStateException(
+ format("Class %s is already instrumented.", owner));
}
}
diff --git a/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeArrayStrategyFactory.java b/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeArrayStrategyFactory.java
index 38d572a..a4fb82b 100644
--- a/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeArrayStrategyFactory.java
+++ b/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeArrayStrategyFactory.java
@@ -45,7 +45,6 @@
final String className = reader.getClassName();
final int version = BytecodeVersion.get(reader.b);
- final boolean withFrames = version >= Opcodes.V1_6;
if (isInterfaceOrModule(reader)) {
final ProbeCounter counter = getProbeCounter(reader);
@@ -61,7 +60,7 @@
}
} else {
return new ClassFieldProbeArrayStrategy(className, classId,
- withFrames, accessorGenerator);
+ InstrSupport.needsFrames(version), accessorGenerator);
}
}
diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html
index ee53442..33fa7de 100644
--- a/org.jacoco.doc/docroot/doc/changes.html
+++ b/org.jacoco.doc/docroot/doc/changes.html
@@ -20,6 +20,13 @@
<h2>Snapshot Build @qualified.bundle.version@ (@build.date@)</h2>
+<h3>Fixed Bugs</h3>
+<ul>
+ <li>Don't insert stackmap frames into class files with version < 1.6,
+ this fixes regression which was introduced in version 0.6.5
+ (GitHub <a href="https://github.com/jacoco/jacoco/issues/667">#667</a>).</li>
+</ul>
+
<h2>Release 0.8.1 (2018/03/21)</h2>
<h3>New Features</h3>