Skip to content

Commit

Permalink
Fix KotlinDefaultArgumentsFilter for the case of more than 32 paramet…
Browse files Browse the repository at this point in the history 
…ers (#1556)
  • Loading branch information
@Godin
Godin committed Dec 18, 2023
1 parent b127971 commit eb30f17
Show file tree
Hide file tree
Showing 4 changed files with 224 additions and 11 deletions.
43 changes: 43 additions & 0 deletions ...kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinDefaultArgumentsTarget.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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 = ""
)
}

}
147 changes: 147 additions & 0 deletions ...e.test/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -219,4 +221,149 @@ public void should_filter_methods_with_parameters_that_consume_two_slots() {
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));
}

}
38 changes: 27 additions & 11 deletions ...acoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@
/**
* 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
Expand All @@ -38,11 +41,14 @@
* 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 {

Expand Down Expand Up @@ -132,19 +138,29 @@ public void match(final MethodNode methodNode,

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;
}

}
7 changes: 7 additions & 0 deletions org.jacoco.doc/docroot/doc/changes.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ <h3>New Features</h3>
(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>
Expand Down

0 comments on commit eb30f17

Please sign in to comment.