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.

2 files changed, 82 insertions, 12 deletions

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 fb1cb3e3..95de8bd5 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 @@ public class InstrSupportTest {
 	}
 
 	@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 d73fc642..87d78a21 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.junit.Assert.assertEquals;
 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 static org.objectweb.asm.Opcodes.V9;
 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.junit.Test;
 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 @@ public class ClassFileVersionsTest {
 	}
 
 	@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 @@ public class ClassFileVersionsTest {
 		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 @@ public class ClassFileVersionsTest {
 		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();