Fix KotlinDefaultArgumentsFilter for the case of more than 32 parameters (#1556)

4 files changed, 224 insertions, 11 deletions

diff --git a/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinDefaultArgumentsTarget.kt b/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinDefaultArgumentsTarget.kt
index d619bc34..9befa9c4 100644
--- a/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinDefaultArgumentsTarget.kt
+++ b/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinDefaultArgumentsTarget.kt
@@ -35,6 +35,42 @@ object KotlinDefaultArgumentsTarget {
         constructor(a: Boolean, b: String = if (a) "a" else "b") : this() // assertFullyCovered(0, 2)
     }
 
+    class MoreThan32Parameters( // assertFullyCovered()
+        p1: String, // assertEmpty()
+        p2: String, // assertEmpty()
+        p3: String, // assertEmpty()
+        p4: String, // assertEmpty()
+        p5: String, // assertEmpty()
+        p6: String, // assertEmpty()
+        p7: String, // assertEmpty()
+        p8: String, // assertEmpty()
+        p9: String, // assertEmpty()
+        p10: String, // assertEmpty()
+        p11: String, // assertEmpty()
+        p12: String, // assertEmpty()
+        p13: String, // assertEmpty()
+        p14: String, // assertEmpty()
+        p15: String, // assertEmpty()
+        p16: String, // assertEmpty()
+        p17: String, // assertEmpty()
+        p18: String, // assertEmpty()
+        p19: String, // assertEmpty()
+        p20: String, // assertEmpty()
+        p21: String, // assertEmpty()
+        p22: String, // assertEmpty()
+        p23: String, // assertEmpty()
+        p24: String, // assertEmpty()
+        p25: String, // assertEmpty()
+        p26: String, // assertEmpty()
+        p27: String, // assertEmpty()
+        p28: String, // assertEmpty()
+        p29: String, // assertEmpty()
+        p30: String, // assertEmpty()
+        p31: String, // assertEmpty()
+        p32: String = "", // assertFullyCovered()
+        p33: String, // assertEmpty()
+    ) // assertFullyCovered()
+
     @JvmStatic
     fun main(args: Array<String>) {
         f(a = "a")
@@ -52,6 +88,13 @@ object KotlinDefaultArgumentsTarget {
 
         Constructor(false)
         Constructor(true)
+
+        MoreThan32Parameters(
+            "1", "2", "3", "4", "5", "6", "7", "8", "9", "10",
+            "11", "12", "13", "14", "15", "16", "17", "18", "19", "20",
+            "21", "22", "23", "24", "25", "26", "27", "28", "29", "30",
+            "31", p33 = ""
+        )
     }
 
 }
diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilterTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilterTest.java
index cc00e688..aeaa4885 100644
--- a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilterTest.java
+++ b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilterTest.java
@@ -12,6 +12,8 @@
  *******************************************************************************/
 package org.jacoco.core.internal.analysis.filter;
 
+import static org.junit.Assert.assertEquals;
+
 import org.jacoco.core.internal.instr.InstrSupport;
 import org.junit.Test;
 import org.objectweb.asm.Label;
@@ -219,4 +221,149 @@ public class KotlinDefaultArgumentsFilterTest extends FilterTestBase {
 		assertIgnored(new Range(m.instructions.get(3), m.instructions.get(3)));
 	}
 
+	/**
+	 * <pre>
+	 * class C(
+	 *   p1: Int,
+	 *   ...
+	 *   p31: Int,
+	 *   p32: Int = 42,
+	 *   p33: Int,
+	 * )
+	 * </pre>
+	 *
+	 * This constructor will have 2 additional parameters containing the mask.
+	 */
+	@Test
+	public void should_filter_methods_with_more_than_32_parameters() {
+		final StringBuilder paramTypes = new StringBuilder();
+		for (int i = 1; i <= 33; i++) {
+			paramTypes.append("I");
+		}
+		final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION,
+				Opcodes.ACC_SYNTHETIC, "<init>",
+				"(" + paramTypes
+						+ "IILkotlin/jvm/internal/DefaultConstructorMarker;)V",
+				null, null);
+		context.classAnnotations
+				.add(KotlinGeneratedFilter.KOTLIN_METADATA_DESC);
+
+		m.visitVarInsn(Opcodes.ILOAD, 34);
+		m.visitLdcInsn(Integer.valueOf(1 << 31));
+		m.visitInsn(Opcodes.IAND);
+		final Label label = new Label();
+		m.visitJumpInsn(Opcodes.IFEQ, label);
+		// default argument
+		m.visitLdcInsn(Integer.valueOf(42));
+		m.visitVarInsn(Opcodes.ISTORE, 32);
+		m.visitLabel(label);
+
+		m.visitVarInsn(Opcodes.ALOAD, 0);
+		for (int i = 1; i <= 33; i++) {
+			m.visitVarInsn(Opcodes.ILOAD, i);
+		}
+		m.visitMethodInsn(Opcodes.INVOKESPECIAL, "Owner", "<init>",
+				"(" + paramTypes + ")V", false);
+		m.visitInsn(Opcodes.RETURN);
+
+		filter.filter(m, context, output);
+
+		assertIgnored(new Range(m.instructions.get(3), m.instructions.get(3)));
+	}
+
+	/**
+	 * <pre>
+	 * class C(
+	 *   p1: Int = 42,
+	 *   ...
+	 *   p225: Int,
+	 * )
+	 * </pre>
+	 *
+	 * This constructor will have 8 additional parameters containing the mask.
+	 *
+	 * 9 additional parameters will be required for methods with more than 256
+	 * parameters, but according to Java Virtual Machine Specification <a href=
+	 * "https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.11">
+	 * §4.11</a>:
+	 *
+	 * <blockquote>
+	 * <p>
+	 * The number of method parameters is limited to 255
+	 * </p>
+	 * </blockquote>
+	 */
+	@Test
+	public void should_filter_methods_with_more_than_224_parameters() {
+		final StringBuilder paramTypes = new StringBuilder();
+		for (int i = 1; i <= 225; i++) {
+			paramTypes.append("I");
+		}
+		final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION,
+				Opcodes.ACC_SYNTHETIC, "<init>",
+				"(" + paramTypes
+						+ "IIIIIIIILkotlin/jvm/internal/DefaultConstructorMarker;)V",
+				null, null);
+		context.classAnnotations
+				.add(KotlinGeneratedFilter.KOTLIN_METADATA_DESC);
+
+		m.visitVarInsn(Opcodes.ILOAD, 226);
+		m.visitInsn(Opcodes.ICONST_1);
+		m.visitInsn(Opcodes.IAND);
+		final Label label = new Label();
+		m.visitJumpInsn(Opcodes.IFEQ, label);
+		// default argument
+		m.visitLdcInsn(Integer.valueOf(42));
+		m.visitVarInsn(Opcodes.ISTORE, 1);
+		m.visitLabel(label);
+
+		m.visitVarInsn(Opcodes.ALOAD, 0);
+		for (int i = 1; i <= 225; i++) {
+			m.visitVarInsn(Opcodes.ILOAD, i);
+		}
+		m.visitMethodInsn(Opcodes.INVOKESPECIAL, "Owner", "<init>",
+				"(" + paramTypes + ")V", false);
+		m.visitInsn(Opcodes.RETURN);
+
+		filter.filter(m, context, output);
+
+		assertIgnored(new Range(m.instructions.get(3), m.instructions.get(3)));
+	}
+
+	@Test
+	public void computeNumberOfMaskArguments() {
+		assertEquals(1,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(3));
+		assertEquals(1,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(34));
+		assertEquals(2,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(36));
+		assertEquals(2,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(67));
+		assertEquals(3,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(69));
+		assertEquals(3,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(100));
+		assertEquals(4,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(102));
+		assertEquals(4,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(133));
+		assertEquals(5,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(135));
+		assertEquals(5,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(166));
+		assertEquals(6,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(168));
+		assertEquals(6,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(199));
+		assertEquals(7,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(201));
+		assertEquals(7,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(232));
+		assertEquals(8,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(234));
+		assertEquals(8,
+				KotlinDefaultArgumentsFilter.computeNumberOfMaskArguments(255));
+	}
+
 }
diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilter.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilter.java
index f83167c3..e9d53d51 100644
--- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilter.java
+++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilter.java
@@ -26,8 +26,11 @@ import org.objectweb.asm.tree.VarInsnNode;
 /**
  * Filters branches that Kotlin compiler generates for default arguments.
  *
- * For each default argument Kotlin compiler generates following bytecode to
- * determine if it should be used or not:
+ * For methods and constructors with default arguments Kotlin compiler generates
+ * synthetic method with suffix "$default" or a synthetic constructor with last
+ * argument "kotlin.jvm.internal.DefaultConstructorMarker" respectively. And in
+ * this synthetic method for each default argument Kotlin compiler generates
+ * following bytecode to determine if it should be used or not:
  *
  * <pre>
  * ILOAD maskVar
@@ -38,11 +41,14 @@ import org.objectweb.asm.tree.VarInsnNode;
  * label:
  * </pre>
  *
- * Where <code>maskVar</code> is penultimate argument of synthetic method with
- * suffix "$default" or of synthetic constructor with last argument
- * "kotlin.jvm.internal.DefaultConstructorMarker". And its value can't be zero -
- * invocation with all arguments uses original non synthetic method, thus
- * <code>IFEQ</code> instructions should be ignored.
+ * If original method has <code>X</code> arguments, then in synthetic method
+ * <code>maskVar</code> is one of arguments from <code>X+1</code> to
+ * <code>X+1+(X/32)</code>.
+ *
+ * At least one of such arguments is not zero - invocation without default
+ * arguments uses original non synthetic method.
+ *
+ * This filter marks <code>IFEQ</code> instructions as ignored.
  */
 public final class KotlinDefaultArgumentsFilter implements IFilter {
 
@@ -132,19 +138,29 @@ public final class KotlinDefaultArgumentsFilter implements IFilter {
 
 		private static int maskVar(final String desc,
 				final boolean constructor) {
+			final Type[] argumentTypes = Type.getMethodType(desc)
+					.getArgumentTypes();
 			int slot = 0;
 			if (constructor) {
 				// one slot for reference to current object
 				slot++;
 			}
-			final Type[] argumentTypes = Type.getMethodType(desc)
-					.getArgumentTypes();
-			final int penultimateArgument = argumentTypes.length - 2;
-			for (int i = 0; i < penultimateArgument; i++) {
+			final int firstMaskArgument = argumentTypes.length - 1
+					- computeNumberOfMaskArguments(argumentTypes.length);
+			for (int i = 0; i < firstMaskArgument; i++) {
 				slot += argumentTypes[i].getSize();
 			}
 			return slot;
 		}
 	}
 
+	/**
+	 * @param arguments
+	 *            number of arguments of synthetic method
+	 * @return number of arguments holding mask
+	 */
+	static int computeNumberOfMaskArguments(final int arguments) {
+		return (arguments - 2) / 33 + 1;
+	}
+
 }
diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html
index 1d0b56df..90b0340f 100644
--- a/org.jacoco.doc/docroot/doc/changes.html
+++ b/org.jacoco.doc/docroot/doc/changes.html
@@ -26,6 +26,13 @@
       (GitHub <a href="https://github.com/jacoco/jacoco/issues/1553">#1553</a>).</li>
 </ul>
 
+<h3>Fixed bugs</h3>
+<ul>
+  <li>Branches added by the Kotlin compiler for functions with default arguments and
+      having more than 32 parameters are filtered out during generation of report
+      (GitHub <a href="https://github.com/jacoco/jacoco/issues/1556">#1556</a>).</li>
+</ul>
+
 <h2>Release 0.8.11 (2023/10/14)</h2>
 
 <h3>New Features</h3>