Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 = ""
)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -219,4 +219,113 @@ 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(1 << 31);
Godin marked this conversation as resolved.
Show resolved Hide resolved
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)));
}

}
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,15 +138,21 @@ public void match(final MethodNode methodNode,

private static int maskVar(final String desc,
final boolean constructor) {
final Type[] argumentTypes = Type.getMethodType(desc)
.getArgumentTypes();
int masks;
for (masks = 1; masks <= 8; masks++) {
if (argumentTypes.length <= masks * 32 + masks + 1) {
break;
}
}
marchof marked this conversation as resolved.
Show resolved Hide resolved
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 - masks;
for (int i = 0; i < firstMaskArgument; i++) {
slot += argumentTypes[i].getSize();
}
return slot;
Expand Down
7 changes: 7 additions & 0 deletions org.jacoco.doc/docroot/doc/changes.html
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