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.

Sign up for GitHub

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

Jump to bottom

Fix range for debug symbols of method parameters #1246

Merged
merged 1 commit into from Nov 8, 2021
Merged

Fix range for debug symbols of method parameters #1246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
48 changes: 27 additions & 21 deletions org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeInserterTest.java
View file
Open in desktop
Expand Up @@ -59,7 +59,7 @@ public void verify() {
}

@Test
public void testVariableStatic() {
public void probevar_should_be_at_position_0_for_static_method_without_parameters() {
ProbeInserter pi = new ProbeInserter(Opcodes.ACC_STATIC, "m", "()V",
actualVisitor, arrayStrategy);
pi.insertProbe(0);
Expand All @@ -71,7 +71,7 @@ public void testVariableStatic() {
}

@Test
public void testVariableNonStatic() {
public void probevar_should_be_at_position_1_for_instance_method_without_parameters() {
ProbeInserter pi = new ProbeInserter(0, "m", "()V", actualVisitor,
arrayStrategy);
pi.insertProbe(0);
Expand All @@ -83,7 +83,7 @@ public void testVariableNonStatic() {
}

@Test
public void testVariableNonStatic_IZObject() {
public void probevar_should_be_at_position_4_for_instance_method_with_3_parameters() {
ProbeInserter pi = new ProbeInserter(0, "m", "(IZLjava/lang/Object;)V",
actualVisitor, arrayStrategy);
pi.insertProbe(0);
Expand All @@ -95,7 +95,7 @@ public void testVariableNonStatic_IZObject() {
}

@Test
public void testVariableNonStatic_JD() {
public void probevar_should_be_at_position_5_for_instance_method_with_2_wide_parameters() {
ProbeInserter pi = new ProbeInserter(0, "m", "(JD)V", actualVisitor,
arrayStrategy);
pi.insertProbe(0);
Expand All @@ -107,25 +107,27 @@ public void testVariableNonStatic_JD() {
}

@Test
public void testVisitCode() {
public void visitCode_should_call_IProbeArrayStrategy_for_any_methods() {
ProbeInserter pi = new ProbeInserter(0, "m", "()V", actualVisitor,
arrayStrategy);
pi.visitCode();

expectedVisitor.visitLabel(new Label());
expectedVisitor.visitLdcInsn("init");
}

@Test
public void testVisitClinit() {
public void visitCode_should_call_IProbeArrayStrategy_for_static_initializers() {
ProbeInserter pi = new ProbeInserter(0, "<clinit>", "()V",
actualVisitor, arrayStrategy);
pi.visitCode();

expectedVisitor.visitLabel(new Label());
expectedVisitor.visitLdcInsn("clinit");
}

@Test
public void testVisitVarIns() {
public void visitVarInsn_should_be_called_with_adjusted_variable_positions() {
ProbeInserter pi = new ProbeInserter(0, "m", "(II)V", actualVisitor,
arrayStrategy);

Expand All @@ -146,7 +148,7 @@ public void testVisitVarIns() {
}

@Test
public void testVisitIincInsn() {
public void visitIincInsn_should_be_called_with_adjusted_variable_positions() {
ProbeInserter pi = new ProbeInserter(0, "m", "(II)V", actualVisitor,
arrayStrategy);
pi.visitIincInsn(0, 100);
Expand All @@ -166,7 +168,7 @@ public void testVisitIincInsn() {
}

@Test
public void testVisitLocalVariable() {
public void visitLocalVariable_should_be_called_with_adjusted_variable_positions() {
ProbeInserter pi = new ProbeInserter(0, "m", "(II)V", actualVisitor,
arrayStrategy);

Expand All @@ -176,10 +178,12 @@ public void testVisitLocalVariable() {
pi.visitLocalVariable(null, null, null, null, null, 3);
pi.visitLocalVariable(null, null, null, null, null, 4);

Label begin = new Label();

// Argument variables stay at the same position:
expectedVisitor.visitLocalVariable(null, null, null, null, null, 0);
expectedVisitor.visitLocalVariable(null, null, null, null, null, 1);
expectedVisitor.visitLocalVariable(null, null, null, null, null, 2);
expectedVisitor.visitLocalVariable(null, null, null, begin, null, 0);
expectedVisitor.visitLocalVariable(null, null, null, begin, null, 1);
expectedVisitor.visitLocalVariable(null, null, null, begin, null, 2);

// Local variables are shifted by one:
expectedVisitor.visitLocalVariable(null, null, null, null, null, 4);
Expand Down Expand Up @@ -209,29 +213,31 @@ public void should_remap_LocalVariableAnnotation() {
}

@Test
public void testVisitMaxs1() {
public void new_stack_size_should_be_big_enought_to_store_probe_array() {
ProbeInserter pi = new ProbeInserter(0, "m", "(II)V", actualVisitor,
arrayStrategy);
pi.visitCode();
pi.visitMaxs(0, 8);

expectedVisitor.visitLabel(new Label());
expectedVisitor.visitLdcInsn("init");
expectedVisitor.visitMaxs(5, 9);
}

@Test
public void testVisitMaxs2() {
public void new_stack_size_should_be_increased_for_probes() {
ProbeInserter pi = new ProbeInserter(0, "m", "(II)V", actualVisitor,
arrayStrategy);
pi.visitCode();
pi.visitMaxs(10, 8);

expectedVisitor.visitLabel(new Label());
expectedVisitor.visitLdcInsn("init");
expectedVisitor.visitMaxs(13, 9);
}

@Test
public void testVisitFrame() {
public void visitFrame_should_insert_probe_variable_between_arguments_and_local_variables() {
ProbeInserter pi = new ProbeInserter(0, "m", "(J)V", actualVisitor,
arrayStrategy);

Expand All @@ -245,7 +251,7 @@ public void testVisitFrame() {
}

@Test
public void testVisitFrameNoLocals() {
public void visitFrame_should_only_insert_probe_variable_when_no_other_local_variables_exist() {
ProbeInserter pi = new ProbeInserter(Opcodes.ACC_STATIC, "m", "()V",
actualVisitor, arrayStrategy);

Expand All @@ -256,7 +262,7 @@ public void testVisitFrameNoLocals() {
}

@Test
public void testVisitFrameProbeAt0() {
public void visitFrame_should_insert_probe_variable_first_when_no_parameters_exist() {
ProbeInserter pi = new ProbeInserter(Opcodes.ACC_STATIC, "m", "()V",
actualVisitor, arrayStrategy);

Expand All @@ -268,7 +274,7 @@ public void testVisitFrameProbeAt0() {
}

@Test
public void testFillOneWord() {
public void visitFrame_should_fill_one_unused_slots_before_probe_variable_with_TOP() {
ProbeInserter pi = new ProbeInserter(Opcodes.ACC_STATIC, "m", "(I)V",
actualVisitor, arrayStrategy);

Expand All @@ -280,7 +286,7 @@ public void testFillOneWord() {
}

@Test
public void testFillTwoWord() {
public void visitFrame_should_fill_two_unused_slots_before_probe_variable_with_TOP_TOP() {
ProbeInserter pi = new ProbeInserter(Opcodes.ACC_STATIC, "m", "(J)V",
actualVisitor, arrayStrategy);

Expand All @@ -293,7 +299,7 @@ public void testFillTwoWord() {
}

@Test
public void testFillPartly() {
public void visitFrame_should_fill_three_unused_slots_before_probe_variable_with_TOP_TOP_TOP() {
ProbeInserter pi = new ProbeInserter(Opcodes.ACC_STATIC, "m", "(DIJ)V",
actualVisitor, arrayStrategy);

Expand All @@ -309,7 +315,7 @@ public void testFillPartly() {
}

@Test(expected = IllegalArgumentException.class)
public void testVisitFrame_invalidType() {
public void visitFrame_must_only_support_resolved_frames() {
ProbeInserter pi = new ProbeInserter(0, "m", "()V", actualVisitor,
arrayStrategy);
pi.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
Expand Down
14 changes: 13 additions & 1 deletion org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeInserter.java
View file
Open in desktop
Expand Up @@ -38,6 +38,9 @@ class ProbeInserter extends MethodVisitor implements IProbeInserter {
/** Position of the inserted variable. */
private final int variable;

/** Label for the new beginning of the method */
private final Label beginLabel;

/** Maximum stack usage of the code to access the probe array. */
private int accessorStackSize;

Expand Down Expand Up @@ -66,6 +69,7 @@ class ProbeInserter extends MethodVisitor implements IProbeInserter {
pos += t.getSize();
}
variable = pos;
beginLabel = new Label();
}

public void insertProbe(final int id) {
Expand Down Expand Up @@ -93,6 +97,7 @@ public void insertProbe(final int id) {

@Override
public void visitCode() {
mv.visitLabel(beginLabel);
accessorStackSize = arrayStrategy.storeInstance(mv, clinit, variable);
mv.visitCode();
}
Expand All @@ -111,7 +116,14 @@ public final void visitIincInsn(final int var, final int increment) {
public final void visitLocalVariable(final String name, final String desc,
final String signature, final Label start, final Label end,
final int index) {
mv.visitLocalVariable(name, desc, signature, start, end, map(index));
if (index < variable) {
// Method parameters are still valid from the very beginning
mv.visitLocalVariable(name, desc, signature, beginLabel, end,
index);
} else {
mv.visitLocalVariable(name, desc, signature, start, end,
map(index));
}
}

@Override
Expand Down
2 changes: 2 additions & 0 deletions org.jacoco.doc/docroot/doc/changes.html
View file
Open in desktop
Expand Up @@ -33,6 +33,8 @@ <h3>Fixed bugs</h3>
<ul>
<li>Fixed <code>NullPointerException</code> during filtering
(GitHub <a href="https://github.com/jacoco/jacoco/issues/1189">#1189</a>).</li>
<li>Fix range for debug symbols of method parameters
(GitHub <a href="https://github.com/jacoco/jacoco/issues/1246">#1246</a>).</li>
</ul>

<h3>Non-functional Changes</h3>
Expand Down