From 70d5b98d8c438126e9e95f186017ef972b031ef8 Mon Sep 17 00:00:00 2001 From: "Marc R. Hoffmann" Date: Mon, 8 Nov 2021 17:08:51 +0100 Subject: [PATCH] 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. --- .../internal/instr/ProbeInserterTest.java | 48 +++++++++++-------- .../core/internal/instr/ProbeInserter.java | 14 +++++- org.jacoco.doc/docroot/doc/changes.html | 2 + 3 files changed, 42 insertions(+), 22 deletions(-) diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeInserterTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeInserterTest.java index fa42be8229..4f5c23e50a 100644 --- a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeInserterTest.java +++ b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeInserterTest.java @@ -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); @@ -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); @@ -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); @@ -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); @@ -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, "", "()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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); diff --git a/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeInserter.java b/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeInserter.java index 0f5b99ff55..dfa7df0f93 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeInserter.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeInserter.java @@ -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; @@ -66,6 +69,7 @@ class ProbeInserter extends MethodVisitor implements IProbeInserter { pos += t.getSize(); } variable = pos; + beginLabel = new Label(); } public void insertProbe(final int id) { @@ -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(); } @@ -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 diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html index 128455ffda..873039ab62 100644 --- a/org.jacoco.doc/docroot/doc/changes.html +++ b/org.jacoco.doc/docroot/doc/changes.html @@ -33,6 +33,8 @@

Fixed bugs

Non-functional Changes