Restore exec file compatibility after upgrade of ASM to version 9.5 (#1492)

5 files changed, 179 insertions, 0 deletions

diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/MethodAnalyzerTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/MethodAnalyzerTest.java
index b8807525..69c1fb17 100644
--- a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/MethodAnalyzerTest.java
+++ b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/MethodAnalyzerTest.java
@@ -20,6 +20,7 @@ import java.util.Set;
 
 import org.jacoco.core.analysis.ILine;
 import org.jacoco.core.analysis.IMethodCoverage;
+import org.jacoco.core.data.ExecutionDataWriter;
 import org.jacoco.core.internal.analysis.filter.FilterContextMock;
 import org.jacoco.core.internal.analysis.filter.Filters;
 import org.jacoco.core.internal.analysis.filter.IFilter;
@@ -137,6 +138,62 @@ public class MethodAnalyzerTest implements IProbeIdGenerator {
 		assertLine(1002, 0, 1, 0, 0);
 	}
 
+	// === Scenario: method invocation after zero line number
+
+	/**
+	 * @see org.jacoco.core.internal.flow.LabelFlowAnalyzer#visitLineNumber(int,
+	 *      Label)
+	 * @see org.jacoco.core.internal.instr.ZeroLineNumberTest
+	 */
+	private void createZeroLineNumber() {
+		final Label l0 = new Label();
+		method.visitLabel(l0);
+		method.visitLineNumber(1001, l0);
+		method.visitInsn(Opcodes.NOP);
+		final Label l1 = new Label();
+		method.visitLabel(l1);
+		method.visitLineNumber(0, l1);
+		method.visitMethodInsn(Opcodes.INVOKESTATIC, "Foo", "foo", "()V",
+				false);
+		method.visitInsn(Opcodes.NOP);
+		final Label l2 = new Label();
+		method.visitLabel(l2);
+		method.visitLineNumber(1002, l2);
+		method.visitInsn(Opcodes.RETURN);
+	}
+
+	@Test
+	public void zero_line_number_should_create_1_probe() {
+		createZeroLineNumber();
+		runMethodAnalzer();
+		assertEquals(1, nextProbeId);
+
+		// workaround for zero line number can be removed if needed
+		// during change of exec file version
+		assertEquals(0x1007, ExecutionDataWriter.FORMAT_VERSION);
+	}
+
+	@Test
+	public void zero_line_number_should_show_missed_when_no_probes_are_executed() {
+		createZeroLineNumber();
+		runMethodAnalzer();
+
+		assertLine(1001, 1, 0, 0, 0);
+		assertLine(0, 2, 0, 0, 0);
+		assertLine(1002, 1, 0, 0, 0);
+	}
+
+	@Test
+	public void zero_line_number_should_show_covered_when_probe_is_executed() {
+		createZeroLineNumber();
+		probes[0] = true;
+		runMethodAnalzer();
+
+		assertLine(1001, 0, 1, 0, 0);
+		assertLine(0, 0, 2, 0, 0);
+		assertLine(1002, 0, 1, 0, 0);
+	}
+
 	// === Scenario: simple if branch ===
 
 	private void createIfBranch() {
diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/flow/LabelFlowAnalyzerTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/flow/LabelFlowAnalyzerTest.java
index 18fae43a..fcf31c55 100644
--- a/org.jacoco.core.test/src/org/jacoco/core/internal/flow/LabelFlowAnalyzerTest.java
+++ b/org.jacoco.core.test/src/org/jacoco/core/internal/flow/LabelFlowAnalyzerTest.java
@@ -12,12 +12,14 @@
  *******************************************************************************/
 package org.jacoco.core.internal.flow;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.objectweb.asm.Opcodes.*;
 
+import org.jacoco.core.data.ExecutionDataWriter;
 import org.junit.Before;
 import org.junit.Test;
 import org.objectweb.asm.Label;
@@ -304,6 +306,20 @@ public class LabelFlowAnalyzerTest {
 		assertSame(label, analyzer.lineStart);
 	}
 
+	/**
+	 * @see org.jacoco.core.internal.analysis.MethodAnalyzerTest#zero_line_number_should_create_1_probe()
+	 * @see org.jacoco.core.internal.instr.ZeroLineNumberTest
+	 */
+	@Test
+	public void visitLineNumber_should_skip_zero() {
+		analyzer.visitLineNumber(0, label);
+		assertNull(analyzer.lineStart);
+
+		// workaround for zero line number can be removed if needed
+		// during change of exec file version
+		assertEquals(0x1007, ExecutionDataWriter.FORMAT_VERSION);
+	}
+
 	@Test
 	public void testMethodInsn() {
 		analyzer.visitLineNumber(42, label);
diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ZeroLineNumberTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ZeroLineNumberTest.java
new file mode 100644
index 00000000..d87723b8
--- /dev/null
+++ b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ZeroLineNumberTest.java
@@ -0,0 +1,97 @@
+/*******************************************************************************
+ * Copyright (c) 2009, 2023 Mountainminds GmbH & Co. KG and Contributors
+ * This program and the accompanying materials are made available under
+ * the terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *    Evgeny Mandrikov - initial API and implementation
+ *
+ *******************************************************************************/
+package org.jacoco.core.internal.instr;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import org.jacoco.core.instr.Instrumenter;
+import org.jacoco.core.internal.data.CRC64;
+import org.jacoco.core.runtime.IRuntime;
+import org.jacoco.core.runtime.RuntimeData;
+import org.jacoco.core.runtime.SystemPropertiesRuntime;
+import org.jacoco.core.test.TargetLoader;
+import org.junit.Test;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+
+/**
+ * @see org.jacoco.core.internal.flow.LabelFlowAnalyzer#visitLineNumber(int,
+ *      Label)
+ * @see org.jacoco.core.internal.analysis.MethodAnalyzerTest#zero_line_number_should_create_1_probe()
+ */
+public class ZeroLineNumberTest {
+
+	@Test
+	public void zero_line_numbers_should_be_preserved_during_instrumentation_and_should_not_cause_insertion_of_additional_probes()
+			throws Exception {
+		final IRuntime runtime = new SystemPropertiesRuntime();
+		final RuntimeData data = new RuntimeData();
+		runtime.startup(data);
+
+		final byte[] original = createClass();
+		final byte[] instrumented = new Instrumenter(runtime)
+				.instrument(original, "Sample");
+
+		final Class<?> cls = new TargetLoader().add("Sample", instrumented);
+		try {
+			cls.newInstance();
+			fail("Exception expected");
+		} catch (final Exception e) {
+			assertEquals(0, e.getStackTrace()[1].getLineNumber());
+		}
+
+		data.getExecutionData(CRC64.classId(original), "Sample", 2);
+	}
+
+	private static byte[] createClass() {
+		final ClassWriter cv = new ClassWriter(ClassWriter.COMPUTE_MAXS);
+		cv.visit(Opcodes.V1_5, Opcodes.ACC_PUBLIC, "Sample", null,
+				"java/lang/Object", null);
+		cv.visitSource("Sample.java", null);
+
+		MethodVisitor mv = cv.visitMethod(Opcodes.ACC_PUBLIC, "<init>", "()V",
+				null, null);
+		mv.visitCode();
+		final Label label1 = new Label();
+		mv.visitLabel(label1);
+		mv.visitLineNumber(1, label1);
+		mv.visitVarInsn(Opcodes.ALOAD, 0);
+		mv.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/Object", "<init>",
+				"()V", false);
+		final Label label2 = new Label();
+		mv.visitLabel(label2);
+		mv.visitLineNumber(0, label2);
+		mv.visitMethodInsn(Opcodes.INVOKESTATIC, "Sample", "throw", "()V",
+				false);
+		mv.visitInsn(Opcodes.RETURN);
+		mv.visitMaxs(0, 0);
+		mv.visitEnd();
+
+		mv = cv.visitMethod(Opcodes.ACC_STATIC, "throw", "()V", null, null);
+		mv.visitCode();
+		mv.visitTypeInsn(Opcodes.NEW, "java/lang/RuntimeException");
+		mv.visitInsn(Opcodes.DUP);
+		mv.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/RuntimeException",
+				"<init>", "()V", false);
+		mv.visitInsn(Opcodes.ATHROW);
+		mv.visitMaxs(0, 0);
+		mv.visitEnd();
+
+		cv.visitEnd();
+		return cv.toByteArray();
+	}
+
+}
diff --git a/org.jacoco.core/src/org/jacoco/core/internal/flow/LabelFlowAnalyzer.java b/org.jacoco.core/src/org/jacoco/core/internal/flow/LabelFlowAnalyzer.java
index 0e08f7ee..e78373e9 100644
--- a/org.jacoco.core/src/org/jacoco/core/internal/flow/LabelFlowAnalyzer.java
+++ b/org.jacoco.core/src/org/jacoco/core/internal/flow/LabelFlowAnalyzer.java
@@ -101,6 +101,12 @@ public final class LabelFlowAnalyzer extends MethodVisitor {
 
 	@Override
 	public void visitLineNumber(final int line, final Label start) {
+		if (line == 0) {
+			// ASM versions prior to 9.5 were ignoring zero line numbers
+			// (https://gitlab.ow2.org/asm/asm/-/issues/317989)
+			// so we ignore them here to preserve exec file compatibility
+			return;
+		}
 		lineStart = start;
 	}
 
diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html
index b0c1e0f9..13aff734 100644
--- a/org.jacoco.doc/docroot/doc/changes.html
+++ b/org.jacoco.doc/docroot/doc/changes.html
@@ -40,6 +40,9 @@
       local variable of method parameters is overridden in the method body to
       store a value of type long or double
       (GitHub <a href="https://github.com/jacoco/jacoco/issues/893">#893</a>).</li>
+  <li>Restore exec file compatibility with versions from 0.7.5 to 0.8.8
+      in case of class files with zero line numbers
+      (GitHub <a href="https://github.com/jacoco/jacoco/issues/1492">#1492</a>).</li>
 </ul>
 
 <h3>Non-functional Changes</h3>