From be29a42498f4ef1b50bc49d4e453ca543bc91489 Mon Sep 17 00:00:00 2001 From: "Marc R. Hoffmann" Date: Tue, 24 Feb 2015 22:21:23 +0100 Subject: GitHub #286: Fix missing probes Fix missing probes at the beginning of try/catch blocks when the block starts as a jump target. This should restore exec file compatibility with 0.7.2. Basically a revert of commit e4a474c --- .../core/internal/flow/LabelFlowAnalyzerTest.java | 11 ++++++++++- .../core/internal/flow/MethodProbesAdapterTest.java | 2 ++ .../jacoco/core/test/validation/ExceptionsTest.java | 18 ++++++++++++++---- .../core/test/validation/targets/Target03.java | 13 +++++++++++++ .../core/internal/flow/LabelFlowAnalyzer.java | 21 ++++++++++++++++++++- .../core/internal/flow/MethodProbesAdapter.java | 20 ++++++++------------ org.jacoco.doc/docroot/doc/changes.html | 8 ++++++++ 7 files changed, 75 insertions(+), 18 deletions(-) diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/flow/LabelFlowAnalyzerTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/flow/LabelFlowAnalyzerTest.java index 6d278743..440b4f38 100644 --- a/org.jacoco.core.test/src/org/jacoco/core/internal/flow/LabelFlowAnalyzerTest.java +++ b/org.jacoco.core.test/src/org/jacoco/core/internal/flow/LabelFlowAnalyzerTest.java @@ -107,6 +107,15 @@ public class LabelFlowAnalyzerTest { @Test public void testFlowScenario10() { + analyzer.visitTryCatchBlock(new Label(), new Label(), label, + "java/lang/Exception"); + analyzer.visitJumpInsn(GOTO, label); + assertTrue(LabelInfo.isMultiTarget(label)); + assertFalse(LabelInfo.isSuccessor(label)); + } + + @Test + public void testFlowScenario11() { // Even if the same label is referenced multiple times but from the same // source instruction this is only counted as one target. analyzer.visitLookupSwitchInsn(label, new int[] { 0, 1 }, new Label[] { @@ -116,7 +125,7 @@ public class LabelFlowAnalyzerTest { } @Test - public void testFlowScenario11() { + public void testFlowScenario12() { // Even if the same label is referenced multiple times but from the same // source instruction this is only counted as one target. analyzer.visitTableSwitchInsn(0, 1, label, new Label[] { label, label }); diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/flow/MethodProbesAdapterTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/flow/MethodProbesAdapterTest.java index af5e98bd..25da70cd 100644 --- a/org.jacoco.core.test/src/org/jacoco/core/internal/flow/MethodProbesAdapterTest.java +++ b/org.jacoco.core.test/src/org/jacoco/core/internal/flow/MethodProbesAdapterTest.java @@ -330,6 +330,7 @@ public class MethodProbesAdapterTest implements IProbeIdGenerator { public void testVisitTryCatchBlockWithProbe() { Label target = new Label(); LabelInfo.setSuccessor(target); + LabelInfo.setTarget(target); Label end = new Label(); Label handler = new Label(); Label start = new Label(); @@ -348,6 +349,7 @@ public class MethodProbesAdapterTest implements IProbeIdGenerator { public void testVisitMultipleTryCatchBlocksWithProbe() { Label target = new Label(); LabelInfo.setSuccessor(target); + LabelInfo.setTarget(target); Label end = new Label(); Label handler1 = new Label(); Label handler2 = new Label(); diff --git a/org.jacoco.core.test/src/org/jacoco/core/test/validation/ExceptionsTest.java b/org.jacoco.core.test/src/org/jacoco/core/test/validation/ExceptionsTest.java index a3bfa81d..03517826 100644 --- a/org.jacoco.core.test/src/org/jacoco/core/test/validation/ExceptionsTest.java +++ b/org.jacoco.core.test/src/org/jacoco/core/test/validation/ExceptionsTest.java @@ -61,7 +61,17 @@ public class ExceptionsTest extends ValidationTestBase { assertLine("implicitExceptionTryCatch.catchBlock", ICounter.FULLY_COVERED); - // 5. Try/Catch Block With Exception Thrown Explicitly + // 5. Try/Catch Block With Exception Thrown Implicitly After Condition + // As the try/catch block is entered at one branch of the condition + // should be marked as executed + assertLine("implicitExceptionTryCatchAfterCondition.condition", + ICounter.FULLY_COVERED, 1, 1); + assertLine("implicitExceptionTryCatchAfterCondition.exception", + ICounter.NOT_COVERED); + assertLine("implicitExceptionTryCatchAfterCondition.catchBlock", + ICounter.FULLY_COVERED); + + // 6. Try/Catch Block With Exception Thrown Explicitly assertLine("explicitExceptionTryCatch.beforeBlock", ICounter.FULLY_COVERED); assertLine("explicitExceptionTryCatch.before", ICounter.FULLY_COVERED); @@ -69,13 +79,13 @@ public class ExceptionsTest extends ValidationTestBase { assertLine("explicitExceptionTryCatch.catchBlock", ICounter.FULLY_COVERED); - // 6. Finally Block Without Exception Thrown + // 7. Finally Block Without Exception Thrown // Finally block is yellow as the exception path is missing. assertLine("noExceptionFinally.beforeBlock", ICounter.FULLY_COVERED); assertLine("noExceptionFinally.tryBlock", ICounter.FULLY_COVERED); assertLine("noExceptionFinally.finallyBlock", ICounter.PARTLY_COVERED); - // 7. Finally Block With Implicit Exception + // 8. Finally Block With Implicit Exception // Finally block is yellow as the non-exception path is missing. assertLine("implicitExceptionFinally.beforeBlock", ICounter.FULLY_COVERED); @@ -85,7 +95,7 @@ public class ExceptionsTest extends ValidationTestBase { assertLine("implicitExceptionFinally.finallyBlock", ICounter.PARTLY_COVERED); - // 8. Finally Block With Exception Thrown Explicitly + // 9. Finally Block With Exception Thrown Explicitly assertLine("explicitExceptionFinally.beforeBlock", ICounter.FULLY_COVERED); assertLine("explicitExceptionFinally.before", ICounter.FULLY_COVERED); diff --git a/org.jacoco.core.test/src/org/jacoco/core/test/validation/targets/Target03.java b/org.jacoco.core.test/src/org/jacoco/core/test/validation/targets/Target03.java index 0ea2d69e..41dfbe0b 100644 --- a/org.jacoco.core.test/src/org/jacoco/core/test/validation/targets/Target03.java +++ b/org.jacoco.core.test/src/org/jacoco/core/test/validation/targets/Target03.java @@ -12,6 +12,7 @@ package org.jacoco.core.test.validation.targets; import static org.jacoco.core.test.validation.targets.Stubs.ex; +import static org.jacoco.core.test.validation.targets.Stubs.f; import static org.jacoco.core.test.validation.targets.Stubs.nop; import org.jacoco.core.test.validation.targets.Stubs.StubException; @@ -32,6 +33,7 @@ public class Target03 implements Runnable { } noExceptionTryCatch(); implicitExceptionTryCatch(); + implicitExceptionTryCatchAfterCondition(); explicitExceptionTryCatch(); noExceptionFinally(); try { @@ -75,6 +77,17 @@ public class Target03 implements Runnable { } } + private void implicitExceptionTryCatchAfterCondition() { + if (f()) { // $line-implicitExceptionTryCatchAfterCondition.condition$ + return; + } + try { + ex(); // $line-implicitExceptionTryCatchAfterCondition.exception$ + } catch (StubException e) { + nop(); // $line-implicitExceptionTryCatchAfterCondition.catchBlock$ + } + } + private void explicitExceptionTryCatch() { nop(); // $line-explicitExceptionTryCatch.beforeBlock$ try { diff --git a/org.jacoco.core/src/org/jacoco/core/internal/flow/LabelFlowAnalyzer.java b/org.jacoco.core/src/org/jacoco/core/internal/flow/LabelFlowAnalyzer.java index ad1e7405..08176dfd 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/flow/LabelFlowAnalyzer.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/flow/LabelFlowAnalyzer.java @@ -32,7 +32,13 @@ public final class LabelFlowAnalyzer extends MethodVisitor { * Method to mark labels */ public static void markLabels(final MethodNode method) { - method.instructions.accept(new LabelFlowAnalyzer()); + // We do not use the accept() method as ASM resets labels after every + // call to accept() + final MethodVisitor lfa = new LabelFlowAnalyzer(); + for (int i = method.tryCatchBlocks.size(); --i >= 0;) { + method.tryCatchBlocks.get(i).accept(lfa); + } + method.instructions.accept(lfa); } /** @@ -54,6 +60,19 @@ public final class LabelFlowAnalyzer extends MethodVisitor { super(JaCoCo.ASM_API_VERSION); } + @Override + public void visitTryCatchBlock(final Label start, final Label end, + final Label handler, final String type) { + // Enforce probe at the beginning of the block. Assuming the start of + // the block already is successor of some other code, adding a target + // makes the start a multitarget. However, if the start of the block + // also is the start of the method, no probe will be added. + LabelInfo.setTarget(start); + + // Mark exception handler as possible target of the block + LabelInfo.setTarget(handler); + } + @Override public void visitJumpInsn(final int opcode, final Label label) { LabelInfo.setTarget(label); diff --git a/org.jacoco.core/src/org/jacoco/core/internal/flow/MethodProbesAdapter.java b/org.jacoco.core/src/org/jacoco/core/internal/flow/MethodProbesAdapter.java index 135e3eb9..32ef9f10 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/flow/MethodProbesAdapter.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/flow/MethodProbesAdapter.java @@ -64,13 +64,12 @@ public final class MethodProbesAdapter extends MethodVisitor { @Override public void visitTryCatchBlock(Label start, final Label end, final Label handler, final String type) { - // If it is not the direct start of a method we insert an extra probe at - // the beginning of every try/catch block. To ensure that the probe - // itsel is still within the try/catch block, we introduce a new start - // label. + // If a probe will be inserted before the start label, we'll need to use + // a different label for the try-catch block. if (tryCatchProbeLabels.containsKey(start)) { start = tryCatchProbeLabels.get(start); - } else if (LabelInfo.isSuccessor(start)) { + } else if (LabelInfo.isMultiTarget(start) + && LabelInfo.isSuccessor(start)) { final Label probeLabel = new Label(); LabelInfo.setSuccessor(probeLabel); tryCatchProbeLabels.put(start, probeLabel); @@ -81,14 +80,11 @@ public final class MethodProbesAdapter extends MethodVisitor { @Override public void visitLabel(final Label label) { - final Label tryCatchStartLabel = tryCatchProbeLabels.get(label); - if (tryCatchStartLabel != null) { - probesVisitor.visitLabel(tryCatchStartLabel); - probesVisitor.visitProbe(idGenerator.nextId()); - } else { - if (LabelInfo.isMultiTarget(label) && LabelInfo.isSuccessor(label)) { - probesVisitor.visitProbe(idGenerator.nextId()); + if (LabelInfo.isMultiTarget(label) && LabelInfo.isSuccessor(label)) { + if (tryCatchProbeLabels.containsKey(label)) { + probesVisitor.visitLabel(tryCatchProbeLabels.get(label)); } + probesVisitor.visitProbe(idGenerator.nextId()); } probesVisitor.visitLabel(label); } diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html index c4f3ffaa..0cbcddae 100644 --- a/org.jacoco.doc/docroot/doc/changes.html +++ b/org.jacoco.doc/docroot/doc/changes.html @@ -20,6 +20,14 @@

Snapshot Build @qualified.bundle.version@ (@build.date@)

+

Fixed Bugs

+ + +

Release 0.7.3 (2015/02/19)

New Features

-- cgit v1.2.3