Skip to content

Commit 70d5b98

Browse files
marchofmarchof
authored
Fix range for debug symbols of method parameters (#1246)
By inserting the probe array access code at the beginning of the method we accidently moved the range of the debug symbols of the method parameters after the probe array access code. Actually the method parameters are still valid from the very beginning of the method. This offset in the method parameter's debug symbols may confuse tools like Jandex.
1 parent 472be4e commit 70d5b98

File tree

3 files changed

+42
-22
lines changed

3 files changed

+42
-22
lines changed

‎org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeInserterTest.java

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public void verify() {
5959
}
6060

6161
@Test
62-
public void testVariableStatic() {
62+
public void probevar_should_be_at_position_0_for_static_method_without_parameters() {
6363
ProbeInserter pi = new ProbeInserter(Opcodes.ACC_STATIC, "m", "()V",
6464
actualVisitor, arrayStrategy);
6565
pi.insertProbe(0);
@@ -71,7 +71,7 @@ public void testVariableStatic() {
7171
}
7272

7373
@Test
74-
public void testVariableNonStatic() {
74+
public void probevar_should_be_at_position_1_for_instance_method_without_parameters() {
7575
ProbeInserter pi = new ProbeInserter(0, "m", "()V", actualVisitor,
7676
arrayStrategy);
7777
pi.insertProbe(0);
@@ -83,7 +83,7 @@ public void testVariableNonStatic() {
8383
}
8484

8585
@Test
86-
public void testVariableNonStatic_IZObject() {
86+
public void probevar_should_be_at_position_4_for_instance_method_with_3_parameters() {
8787
ProbeInserter pi = new ProbeInserter(0, "m", "(IZLjava/lang/Object;)V",
8888
actualVisitor, arrayStrategy);
8989
pi.insertProbe(0);
@@ -95,7 +95,7 @@ public void testVariableNonStatic_IZObject() {
9595
}
9696

9797
@Test
98-
public void testVariableNonStatic_JD() {
98+
public void probevar_should_be_at_position_5_for_instance_method_with_2_wide_parameters() {
9999
ProbeInserter pi = new ProbeInserter(0, "m", "(JD)V", actualVisitor,
100100
arrayStrategy);
101101
pi.insertProbe(0);
@@ -107,25 +107,27 @@ public void testVariableNonStatic_JD() {
107107
}
108108

109109
@Test
110-
public void testVisitCode() {
110+
public void visitCode_should_call_IProbeArrayStrategy_for_any_methods() {
111111
ProbeInserter pi = new ProbeInserter(0, "m", "()V", actualVisitor,
112112
arrayStrategy);
113113
pi.visitCode();
114114

115+
expectedVisitor.visitLabel(new Label());
115116
expectedVisitor.visitLdcInsn("init");
116117
}
117118

118119
@Test
119-
public void testVisitClinit() {
120+
public void visitCode_should_call_IProbeArrayStrategy_for_static_initializers() {
120121
ProbeInserter pi = new ProbeInserter(0, "<clinit>", "()V",
121122
actualVisitor, arrayStrategy);
122123
pi.visitCode();
123124

125+
expectedVisitor.visitLabel(new Label());
124126
expectedVisitor.visitLdcInsn("clinit");
125127
}
126128

127129
@Test
128-
public void testVisitVarIns() {
130+
public void visitVarInsn_should_be_called_with_adjusted_variable_positions() {
129131
ProbeInserter pi = new ProbeInserter(0, "m", "(II)V", actualVisitor,
130132
arrayStrategy);
131133

@@ -146,7 +148,7 @@ public void testVisitVarIns() {
146148
}
147149

148150
@Test
149-
public void testVisitIincInsn() {
151+
public void visitIincInsn_should_be_called_with_adjusted_variable_positions() {
150152
ProbeInserter pi = new ProbeInserter(0, "m", "(II)V", actualVisitor,
151153
arrayStrategy);
152154
pi.visitIincInsn(0, 100);
@@ -166,7 +168,7 @@ public void testVisitIincInsn() {
166168
}
167169

168170
@Test
169-
public void testVisitLocalVariable() {
171+
public void visitLocalVariable_should_be_called_with_adjusted_variable_positions() {
170172
ProbeInserter pi = new ProbeInserter(0, "m", "(II)V", actualVisitor,
171173
arrayStrategy);
172174

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

181+
Label begin = new Label();
182+
179183
// Argument variables stay at the same position:
180-
expectedVisitor.visitLocalVariable(null, null, null, null, null, 0);
181-
expectedVisitor.visitLocalVariable(null, null, null, null, null, 1);
182-
expectedVisitor.visitLocalVariable(null, null, null, null, null, 2);
184+
expectedVisitor.visitLocalVariable(null, null, null, begin, null, 0);
185+
expectedVisitor.visitLocalVariable(null, null, null, begin, null, 1);
186+
expectedVisitor.visitLocalVariable(null, null, null, begin, null, 2);
183187

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

211215
@Test
212-
public void testVisitMaxs1() {
216+
public void new_stack_size_should_be_big_enought_to_store_probe_array() {
213217
ProbeInserter pi = new ProbeInserter(0, "m", "(II)V", actualVisitor,
214218
arrayStrategy);
215219
pi.visitCode();
216220
pi.visitMaxs(0, 8);
217221

222+
expectedVisitor.visitLabel(new Label());
218223
expectedVisitor.visitLdcInsn("init");
219224
expectedVisitor.visitMaxs(5, 9);
220225
}
221226

222227
@Test
223-
public void testVisitMaxs2() {
228+
public void new_stack_size_should_be_increased_for_probes() {
224229
ProbeInserter pi = new ProbeInserter(0, "m", "(II)V", actualVisitor,
225230
arrayStrategy);
226231
pi.visitCode();
227232
pi.visitMaxs(10, 8);
228233

234+
expectedVisitor.visitLabel(new Label());
229235
expectedVisitor.visitLdcInsn("init");
230236
expectedVisitor.visitMaxs(13, 9);
231237
}
232238

233239
@Test
234-
public void testVisitFrame() {
240+
public void visitFrame_should_insert_probe_variable_between_arguments_and_local_variables() {
235241
ProbeInserter pi = new ProbeInserter(0, "m", "(J)V", actualVisitor,
236242
arrayStrategy);
237243

@@ -245,7 +251,7 @@ public void testVisitFrame() {
245251
}
246252

247253
@Test
248-
public void testVisitFrameNoLocals() {
254+
public void visitFrame_should_only_insert_probe_variable_when_no_other_local_variables_exist() {
249255
ProbeInserter pi = new ProbeInserter(Opcodes.ACC_STATIC, "m", "()V",
250256
actualVisitor, arrayStrategy);
251257

@@ -256,7 +262,7 @@ public void testVisitFrameNoLocals() {
256262
}
257263

258264
@Test
259-
public void testVisitFrameProbeAt0() {
265+
public void visitFrame_should_insert_probe_variable_first_when_no_parameters_exist() {
260266
ProbeInserter pi = new ProbeInserter(Opcodes.ACC_STATIC, "m", "()V",
261267
actualVisitor, arrayStrategy);
262268

@@ -268,7 +274,7 @@ public void testVisitFrameProbeAt0() {
268274
}
269275

270276
@Test
271-
public void testFillOneWord() {
277+
public void visitFrame_should_fill_one_unused_slots_before_probe_variable_with_TOP() {
272278
ProbeInserter pi = new ProbeInserter(Opcodes.ACC_STATIC, "m", "(I)V",
273279
actualVisitor, arrayStrategy);
274280

@@ -280,7 +286,7 @@ public void testFillOneWord() {
280286
}
281287

282288
@Test
283-
public void testFillTwoWord() {
289+
public void visitFrame_should_fill_two_unused_slots_before_probe_variable_with_TOP_TOP() {
284290
ProbeInserter pi = new ProbeInserter(Opcodes.ACC_STATIC, "m", "(J)V",
285291
actualVisitor, arrayStrategy);
286292

@@ -293,7 +299,7 @@ public void testFillTwoWord() {
293299
}
294300

295301
@Test
296-
public void testFillPartly() {
302+
public void visitFrame_should_fill_three_unused_slots_before_probe_variable_with_TOP_TOP_TOP() {
297303
ProbeInserter pi = new ProbeInserter(Opcodes.ACC_STATIC, "m", "(DIJ)V",
298304
actualVisitor, arrayStrategy);
299305

@@ -309,7 +315,7 @@ public void testFillPartly() {
309315
}
310316

311317
@Test(expected = IllegalArgumentException.class)
312-
public void testVisitFrame_invalidType() {
318+
public void visitFrame_must_only_support_resolved_frames() {
313319
ProbeInserter pi = new ProbeInserter(0, "m", "()V", actualVisitor,
314320
arrayStrategy);
315321
pi.visitFrame(Opcodes.F_SAME, 0, null, 0, null);

‎org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeInserter.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ class ProbeInserter extends MethodVisitor implements IProbeInserter {
3838
/** Position of the inserted variable. */
3939
private final int variable;
4040

41+
/** Label for the new beginning of the method */
42+
private final Label beginLabel;
43+
4144
/** Maximum stack usage of the code to access the probe array. */
4245
private int accessorStackSize;
4346

@@ -66,6 +69,7 @@ class ProbeInserter extends MethodVisitor implements IProbeInserter {
6669
pos += t.getSize();
6770
}
6871
variable = pos;
72+
beginLabel = new Label();
6973
}
7074

7175
public void insertProbe(final int id) {
@@ -93,6 +97,7 @@ public void insertProbe(final int id) {
9397

9498
@Override
9599
public void visitCode() {
100+
mv.visitLabel(beginLabel);
96101
accessorStackSize = arrayStrategy.storeInstance(mv, clinit, variable);
97102
mv.visitCode();
98103
}
@@ -111,7 +116,14 @@ public final void visitIincInsn(final int var, final int increment) {
111116
public final void visitLocalVariable(final String name, final String desc,
112117
final String signature, final Label start, final Label end,
113118
final int index) {
114-
mv.visitLocalVariable(name, desc, signature, start, end, map(index));
119+
if (index < variable) {
120+
// Method parameters are still valid from the very beginning
121+
mv.visitLocalVariable(name, desc, signature, beginLabel, end,
122+
index);
123+
} else {
124+
mv.visitLocalVariable(name, desc, signature, start, end,
125+
map(index));
126+
}
115127
}
116128

117129
@Override

‎org.jacoco.doc/docroot/doc/changes.html

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ <h3>Fixed bugs</h3>
3333
<ul>
3434
<li>Fixed <code>NullPointerException</code> during filtering
3535
(GitHub <a href="https://github.com/jacoco/jacoco/issues/1189">#1189</a>).</li>
36+
<li>Fix range for debug symbols of method parameters
37+
(GitHub <a href="https://github.com/jacoco/jacoco/issues/1246">#1246</a>).</li>
3638
</ul>
3739

3840
<h3>Non-functional Changes</h3>

0 commit comments

Comments
 (0)