From e93053e8a54540f5394fe0938752f7421e2222ad Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov Date: Sat, 22 Apr 2017 12:39:30 +0200 Subject: Add filter for try-with-resources statement (#500) --- .../core/internal/analysis/MethodAnalyzer.java | 3 + .../internal/analysis/filter/AbstractMatcher.java | 83 +++++++ .../analysis/filter/SynchronizedFilter.java | 39 +-- .../analysis/filter/TryWithResourcesEcjFilter.java | 265 +++++++++++++++++++++ .../filter/TryWithResourcesJavacFilter.java | 253 ++++++++++++++++++++ 5 files changed, 615 insertions(+), 28 deletions(-) create mode 100644 org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java create mode 100644 org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/TryWithResourcesEcjFilter.java create mode 100644 org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/TryWithResourcesJavacFilter.java (limited to 'org.jacoco.core/src/org/jacoco') diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/MethodAnalyzer.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/MethodAnalyzer.java index 993fe88a..ee051436 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/MethodAnalyzer.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/MethodAnalyzer.java @@ -25,6 +25,8 @@ import org.jacoco.core.internal.analysis.filter.IFilterOutput; import org.jacoco.core.internal.analysis.filter.LombokGeneratedFilter; import org.jacoco.core.internal.analysis.filter.SynchronizedFilter; import org.jacoco.core.internal.analysis.filter.SyntheticFilter; +import org.jacoco.core.internal.analysis.filter.TryWithResourcesEcjFilter; +import org.jacoco.core.internal.analysis.filter.TryWithResourcesJavacFilter; import org.jacoco.core.internal.flow.IFrame; import org.jacoco.core.internal.flow.Instruction; import org.jacoco.core.internal.flow.LabelInfo; @@ -45,6 +47,7 @@ public class MethodAnalyzer extends MethodProbesVisitor private static final IFilter[] FILTERS = new IFilter[] { new EnumFilter(), new SyntheticFilter(), new SynchronizedFilter(), + new TryWithResourcesJavacFilter(), new TryWithResourcesEcjFilter(), new LombokGeneratedFilter() }; private final String className; diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java new file mode 100644 index 00000000..e4958bdb --- /dev/null +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java @@ -0,0 +1,83 @@ +/******************************************************************************* + * Copyright (c) 2009, 2017 Mountainminds GmbH & Co. KG and Contributors + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Evgeny Mandrikov - initial API and implementation + * + *******************************************************************************/ +package org.jacoco.core.internal.analysis.filter; + +import java.util.HashMap; +import java.util.Map; + +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.MethodInsnNode; +import org.objectweb.asm.tree.VarInsnNode; + +abstract class AbstractMatcher { + + final Map vars = new HashMap(); + + AbstractInsnNode cursor; + + final void nextIsAddSuppressed() { + nextIs(Opcodes.INVOKEVIRTUAL); + if (cursor == null) { + return; + } + final MethodInsnNode m = (MethodInsnNode) cursor; + if ("java/lang/Throwable".equals(m.owner) + && "addSuppressed".equals(m.name)) { + return; + } + cursor = null; + } + + final void nextIsVar(final int opcode, final String name) { + nextIs(opcode); + if (cursor == null) { + return; + } + final VarInsnNode actual = (VarInsnNode) cursor; + final VarInsnNode expected = vars.get(name); + if (expected == null) { + vars.put(name, actual); + } else if (expected.var != actual.var) { + cursor = null; + } + } + + /** + * Moves {@link #cursor} to next instruction if it has given opcode, + * otherwise sets it to null. + */ + final void nextIs(final int opcode) { + next(); + if (cursor == null) { + return; + } + if (cursor.getOpcode() != opcode) { + cursor = null; + } + } + + /** + * Moves {@link #cursor} to next instruction. + */ + final void next() { + if (cursor == null) { + return; + } + do { + cursor = cursor.getNext(); + } while (cursor != null && (cursor.getType() == AbstractInsnNode.FRAME + || cursor.getType() == AbstractInsnNode.LABEL + || cursor.getType() == AbstractInsnNode.LINE)); + } + +} diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/SynchronizedFilter.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/SynchronizedFilter.java index 655226e7..eec00948 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/SynchronizedFilter.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/SynchronizedFilter.java @@ -38,9 +38,8 @@ public final class SynchronizedFilter implements IFilter { } } - private static class Matcher { + private static class Matcher extends AbstractMatcher { private final AbstractInsnNode start; - private AbstractInsnNode cursor; private Matcher(final AbstractInsnNode start) { this.start = start; @@ -55,36 +54,20 @@ public final class SynchronizedFilter implements IFilter { private boolean nextIsJavac() { cursor = start; - return nextIs(Opcodes.ASTORE) && nextIs(Opcodes.ALOAD) - && nextIs(Opcodes.MONITOREXIT) && nextIs(Opcodes.ALOAD) - && nextIs(Opcodes.ATHROW); + nextIsVar(Opcodes.ASTORE, "t"); + nextIs(Opcodes.ALOAD); + nextIs(Opcodes.MONITOREXIT); + nextIsVar(Opcodes.ALOAD, "t"); + nextIs(Opcodes.ATHROW); + return cursor != null; } private boolean nextIsEcj() { cursor = start; - return nextIs(Opcodes.ALOAD) && nextIs(Opcodes.MONITOREXIT) - && nextIs(Opcodes.ATHROW); - } - - /** - * Moves {@link #cursor} to next instruction and returns - * true if it has given opcode. - */ - private boolean nextIs(int opcode) { - next(); - return cursor != null && cursor.getOpcode() == opcode; - } - - /** - * Moves {@link #cursor} to next instruction. - */ - private void next() { - do { - cursor = cursor.getNext(); - } while (cursor != null - && (cursor.getType() == AbstractInsnNode.FRAME - || cursor.getType() == AbstractInsnNode.LABEL - || cursor.getType() == AbstractInsnNode.LINE)); + nextIs(Opcodes.ALOAD); + nextIs(Opcodes.MONITOREXIT); + nextIs(Opcodes.ATHROW); + return cursor != null; } } diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/TryWithResourcesEcjFilter.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/TryWithResourcesEcjFilter.java new file mode 100644 index 00000000..7d7d396c --- /dev/null +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/TryWithResourcesEcjFilter.java @@ -0,0 +1,265 @@ +/******************************************************************************* + * Copyright (c) 2009, 2017 Mountainminds GmbH & Co. KG and Contributors + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Evgeny Mandrikov - initial API and implementation + * + *******************************************************************************/ +package org.jacoco.core.internal.analysis.filter; + +import java.util.HashMap; +import java.util.Map; + +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.JumpInsnNode; +import org.objectweb.asm.tree.LabelNode; +import org.objectweb.asm.tree.MethodInsnNode; +import org.objectweb.asm.tree.MethodNode; +import org.objectweb.asm.tree.TryCatchBlockNode; + +/** + * Filters code that ECJ generates for try-with-resources statement. + */ +public final class TryWithResourcesEcjFilter implements IFilter { + + public void filter(final String className, final String superClassName, + final MethodNode methodNode, final IFilterOutput output) { + if (methodNode.tryCatchBlocks.isEmpty()) { + return; + } + final Matcher matcher = new Matcher(output); + for (TryCatchBlockNode t : methodNode.tryCatchBlocks) { + if (t.type == null) { + matcher.start(t.handler); + if (!matcher.matchEcj()) { + matcher.start(t.handler); + matcher.matchEcjNoFlowOut(); + } + } + } + } + + static class Matcher extends AbstractMatcher { + + private final IFilterOutput output; + + private final Map owners = new HashMap(); + private final Map labels = new HashMap(); + + private AbstractInsnNode start; + + Matcher(final IFilterOutput output) { + this.output = output; + } + + private void start(final AbstractInsnNode start) { + this.start = start; + cursor = start.getPrevious(); + vars.clear(); + labels.clear(); + owners.clear(); + } + + private boolean matchEcj() { + // "catch (any primaryExc)" + nextIsVar(Opcodes.ASTORE, "primaryExc"); + nextIsEcjCloseAndThrow("r0"); + + AbstractInsnNode c; + int resources = 1; + String r = "r" + resources; + c = cursor; + while (nextIsEcjClose(r)) { + nextIsJump(Opcodes.GOTO, r + ".end"); + nextIsEcjSuppress(r); + nextIsEcjCloseAndThrow(r); + resources++; + r = "r" + resources; + c = cursor; + } + cursor = c; + nextIsEcjSuppress("last"); + // "throw primaryExc" + nextIsVar(Opcodes.ALOAD, "primaryExc"); + nextIs(Opcodes.ATHROW); + if (cursor == null) { + return false; + } + final AbstractInsnNode end = cursor; + + AbstractInsnNode startOnNonExceptionalPath = start.getPrevious(); + cursor = startOnNonExceptionalPath; + while (!nextIsEcjClose("r0")) { + startOnNonExceptionalPath = startOnNonExceptionalPath + .getPrevious(); + cursor = startOnNonExceptionalPath; + if (cursor == null) { + return false; + } + } + startOnNonExceptionalPath = startOnNonExceptionalPath.getNext(); + + next(); + if (cursor == null || cursor.getOpcode() != Opcodes.GOTO) { + return false; + } + + output.ignore(startOnNonExceptionalPath, cursor); + output.ignore(start, end); + return true; + } + + private boolean matchEcjNoFlowOut() { + // "catch (any primaryExc)" + nextIsVar(Opcodes.ASTORE, "primaryExc"); + + AbstractInsnNode c; + int resources = 0; + String r = "r" + resources; + c = cursor; + while (nextIsEcjCloseAndThrow(r) && nextIsEcjSuppress(r)) { + resources++; + r = "r" + resources; + c = cursor; + } + cursor = c; + // "throw primaryExc" + nextIsVar(Opcodes.ALOAD, "primaryExc"); + nextIs(Opcodes.ATHROW); + if (cursor == null) { + return false; + } + final AbstractInsnNode end = cursor; + + AbstractInsnNode startOnNonExceptionalPath = start.getPrevious(); + cursor = startOnNonExceptionalPath; + while (!nextIsEcjClose("r0")) { + startOnNonExceptionalPath = startOnNonExceptionalPath + .getPrevious(); + cursor = startOnNonExceptionalPath; + if (cursor == null) { + return false; + } + } + startOnNonExceptionalPath = startOnNonExceptionalPath.getNext(); + for (int i = 1; i < resources; i++) { + if (!nextIsEcjClose("r" + i)) { + return false; + } + } + + output.ignore(startOnNonExceptionalPath, cursor); + output.ignore(start, end); + return true; + } + + private boolean nextIsEcjClose(final String name) { + nextIsVar(Opcodes.ALOAD, name); + // "if (r != null)" + nextIsJump(Opcodes.IFNULL, name + ".end"); + // "r.close()" + nextIsClose(name); + return cursor != null; + } + + private boolean nextIsEcjCloseAndThrow(final String name) { + nextIsVar(Opcodes.ALOAD, name); + // "if (r != null)" + nextIsJump(Opcodes.IFNULL, name); + // "r.close()" + nextIsClose(name); + nextIsLabel(name); + nextIsVar(Opcodes.ALOAD, "primaryExc"); + nextIs(Opcodes.ATHROW); + return cursor != null; + } + + private boolean nextIsEcjSuppress(final String name) { + final String suppressedExc = name + ".t"; + final String startLabel = name + ".suppressStart"; + final String endLabel = name + ".suppressEnd"; + nextIsVar(Opcodes.ASTORE, suppressedExc); + // "suppressedExc = t" + // "if (primaryExc != null)" + nextIsVar(Opcodes.ALOAD, "primaryExc"); + nextIsJump(Opcodes.IFNONNULL, startLabel); + // "primaryExc = suppressedExc" + nextIsVar(Opcodes.ALOAD, suppressedExc); + nextIsVar(Opcodes.ASTORE, "primaryExc"); + nextIsJump(Opcodes.GOTO, endLabel); + // "if (primaryExc == suppressedExc)" + nextIsLabel(startLabel); + nextIsVar(Opcodes.ALOAD, "primaryExc"); + nextIsVar(Opcodes.ALOAD, suppressedExc); + nextIsJump(Opcodes.IF_ACMPEQ, endLabel); + // "primaryExc.addSuppressed(suppressedExc)" + nextIsVar(Opcodes.ALOAD, "primaryExc"); + nextIsVar(Opcodes.ALOAD, suppressedExc); + nextIsAddSuppressed(); + nextIsLabel(endLabel); + return cursor != null; + } + + private void nextIsClose(final String name) { + nextIsVar(Opcodes.ALOAD, name); + next(); + if (cursor == null) { + return; + } + if (cursor.getOpcode() != Opcodes.INVOKEINTERFACE + && cursor.getOpcode() != Opcodes.INVOKEVIRTUAL) { + cursor = null; + return; + } + final MethodInsnNode m = (MethodInsnNode) cursor; + if (!"close".equals(m.name) || !"()V".equals(m.desc)) { + cursor = null; + return; + } + final String actual = m.owner; + final String expected = owners.get(name); + if (expected == null) { + owners.put(name, actual); + } else if (!expected.equals(actual)) { + cursor = null; + } + } + + private void nextIsJump(final int opcode, final String name) { + nextIs(opcode); + if (cursor == null) { + return; + } + final LabelNode actual = ((JumpInsnNode) cursor).label; + final LabelNode expected = labels.get(name); + if (expected == null) { + labels.put(name, actual); + } else if (expected != actual) { + cursor = null; + } + } + + private void nextIsLabel(final String name) { + if (cursor == null) { + return; + } + cursor = cursor.getNext(); + if (cursor.getType() != AbstractInsnNode.LABEL) { + cursor = null; + return; + } + final LabelNode actual = (LabelNode) cursor; + final LabelNode expected = labels.get(name); + if (expected != actual) { + cursor = null; + } + } + + } + +} diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/TryWithResourcesJavacFilter.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/TryWithResourcesJavacFilter.java new file mode 100644 index 00000000..02ed47e5 --- /dev/null +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/TryWithResourcesJavacFilter.java @@ -0,0 +1,253 @@ +/******************************************************************************* + * Copyright (c) 2009, 2017 Mountainminds GmbH & Co. KG and Contributors + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Evgeny Mandrikov - initial API and implementation + * + *******************************************************************************/ +package org.jacoco.core.internal.analysis.filter; + +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.MethodInsnNode; +import org.objectweb.asm.tree.MethodNode; +import org.objectweb.asm.tree.TryCatchBlockNode; + +/** + * Filters code that javac generates for try-with-resources statement. + */ +public final class TryWithResourcesJavacFilter implements IFilter { + + public void filter(final String className, final String superClassName, + final MethodNode methodNode, final IFilterOutput output) { + if (methodNode.tryCatchBlocks.isEmpty()) { + return; + } + final Matcher matcher = new Matcher(output); + for (TryCatchBlockNode t : methodNode.tryCatchBlocks) { + if ("java/lang/Throwable".equals(t.type)) { + for (Matcher.JavacPattern p : Matcher.JavacPattern.values()) { + matcher.start(t.handler); + if (matcher.matchJavac(p)) { + break; + } + } + } + } + } + + /** + * javac from JDK 7 and 8 generates bytecode that is equivalent to the + * compilation of source code that is described in JLS + * 14.20.3. try-with-resources: + * + * 
+	 *     Resource r = ...;
+	 *     Throwable primaryExc = null;
+	 *     try {
+	 *         ...
+	 *     } finally {
+	 *         if (r != null) {
+	 *             if (primaryExc != null) {
+	 *                 try {
+	 *                     r.close();
+	 *                 } catch (Throwable suppressedExc) {
+	 *                     primaryExc.addSuppressed(suppressedExc);
+	 *                 }
+	 *             } else {
+	 *                 r.close();
+	 *             }
+	 *         }
+	 *     }
+	 *
+ * + * Case of multiple resources looks like multiple nested try-with-resources + * statements. javac from JDK 9 EA b160 does the same, but with some + * optimizations (see JDK-7020499): + *
    + *
  • null check for resource is omitted when it is + * initialized using new
    • + *
  • synthetic method $closeResource containing + * null check of primaryExc and calls to methods + * addSuppressed and close is used when number of + * copies of closing logic reaches threshold, null check of + * resource (if present) is done before call of this method
    • + *
+ * During matching association between resource and slot of variable is done + * on exceptional path and is used to find close of resource on normal path. + * Order of loading variables primaryExc and r is different in different + * cases, which implies that this order should be determined before + * association. So {@link JavacPattern} defines all possible variants that + * will be tried sequentially. + */ + static class Matcher extends AbstractMatcher { + private final IFilterOutput output; + + private String expectedOwner; + + private AbstractInsnNode start; + + Matcher(final IFilterOutput output) { + this.output = output; + } + + private enum JavacPattern { + /** + * resource is loaded after primaryExc, null check of + * resource is omitted, method $closeResource is used + */ + OPTIMAL, + /** + * resource is loaded before primaryExc and both are checked on + * null + */ + FULL, + /** + * resource is loaded after primaryExc, null check of + * resource is omitted + */ + OMITTED_NULL_CHECK, + /** + * resource is loaded before primaryExc and checked on + * null, method $closeResource is used + */ + METHOD, + } + + private void start(final AbstractInsnNode start) { + this.start = start; + cursor = start.getPrevious(); + vars.clear(); + expectedOwner = null; + } + + private boolean matchJavac(final JavacPattern p) { + // "catch (Throwable t)" + nextIsVar(Opcodes.ASTORE, "t1"); + // "primaryExc = t" + nextIsVar(Opcodes.ALOAD, "t1"); + nextIsVar(Opcodes.ASTORE, "primaryExc"); + // "throw t" + nextIsVar(Opcodes.ALOAD, "t1"); + nextIs(Opcodes.ATHROW); + + // "catch (any t)" + nextIsVar(Opcodes.ASTORE, "t2"); + nextIsJavacClose(p, "e"); + // "throw t" + nextIsVar(Opcodes.ALOAD, "t2"); + nextIs(Opcodes.ATHROW); + if (cursor == null) { + return false; + } + final AbstractInsnNode end = cursor; + + AbstractInsnNode startOnNonExceptionalPath = start.getPrevious(); + cursor = startOnNonExceptionalPath; + while (!nextIsJavacClose(p, "n")) { + startOnNonExceptionalPath = startOnNonExceptionalPath + .getPrevious(); + cursor = startOnNonExceptionalPath; + if (cursor == null) { + return false; + } + } + startOnNonExceptionalPath = startOnNonExceptionalPath.getNext(); + + final AbstractInsnNode m = cursor; + next(); + if (cursor.getOpcode() != Opcodes.GOTO) { + cursor = m; + } + + output.ignore(startOnNonExceptionalPath, cursor); + output.ignore(start, end); + return true; + } + + /** + * On a first invocation will associate variables with names "r" and + * "primaryExc", on subsequent invocations will use those associations + * for checks. + */ + private boolean nextIsJavacClose(final JavacPattern p, + final String ctx) { + switch (p) { + case METHOD: + case FULL: + // "if (r != null)" + nextIsVar(Opcodes.ALOAD, "r"); + nextIs(Opcodes.IFNULL); + } + switch (p) { + case METHOD: + case OPTIMAL: + nextIsVar(Opcodes.ALOAD, "primaryExc"); + nextIsVar(Opcodes.ALOAD, "r"); + nextIs(Opcodes.INVOKESTATIC); + if (cursor != null) { + final MethodInsnNode m = (MethodInsnNode) cursor; + if ("$closeResource".equals(m.name) + && "(Ljava/lang/Throwable;Ljava/lang/AutoCloseable;)V" + .equals(m.desc)) { + return true; + } + cursor = null; + } + return false; + case FULL: + case OMITTED_NULL_CHECK: + nextIsVar(Opcodes.ALOAD, "primaryExc"); + // "if (primaryExc != null)" + nextIs(Opcodes.IFNULL); + // "r.close()" + nextIsClose(); + nextIs(Opcodes.GOTO); + // "catch (Throwable t)" + nextIsVar(Opcodes.ASTORE, ctx + "t"); + // "primaryExc.addSuppressed(t)" + nextIsVar(Opcodes.ALOAD, "primaryExc"); + nextIsVar(Opcodes.ALOAD, ctx + "t"); + nextIsAddSuppressed(); + nextIs(Opcodes.GOTO); + // "r.close()" + nextIsClose(); + return cursor != null; + default: + throw new AssertionError(); + } + } + + private void nextIsClose() { + nextIsVar(Opcodes.ALOAD, "r"); + next(); + if (cursor == null) { + return; + } + if (cursor.getOpcode() != Opcodes.INVOKEINTERFACE + && cursor.getOpcode() != Opcodes.INVOKEVIRTUAL) { + cursor = null; + return; + } + final MethodInsnNode m = (MethodInsnNode) cursor; + if (!"close".equals(m.name) || !"()V".equals(m.desc)) { + cursor = null; + return; + } + final String actual = m.owner; + if (expectedOwner == null) { + expectedOwner = actual; + } else if (!expectedOwner.equals(actual)) { + cursor = null; + } + } + + } + +} -- cgit v1.2.3