From 54334f20cfd15dbd62929ed77464a7fcf3353ef8 Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov Date: Tue, 16 Aug 2016 21:29:36 +0200 Subject: Do not add members into interfaces with only abstract and clinit methods (#441) --- .../instr/ProbeArrayStrategyFactoryTest.java | 91 ++++++++++++++++------ .../core/internal/instr/ProbeCounterTest.java | 7 ++ .../jacoco/core/internal/instr/ProbeCounter.java | 8 +- org.jacoco.doc/docroot/doc/changes.html | 3 + 4 files changed, 84 insertions(+), 25 deletions(-) diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeArrayStrategyFactoryTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeArrayStrategyFactoryTest.java index 3ecbd3ad..8dbcc253 100644 --- a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeArrayStrategyFactoryTest.java +++ b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeArrayStrategyFactoryTest.java @@ -48,56 +48,72 @@ public class ProbeArrayStrategyFactoryTest { @Test public void testClass1() { - test(Opcodes.V1_1, 0, false, true); + final IProbeArrayStrategy strategy = test(Opcodes.V1_1, 0, false, true, + true); + assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass()); assertDataField(InstrSupport.DATAFIELD_ACC); assertInitMethod(false); } @Test public void testClass2() { - test(Opcodes.V1_2, 0, false, true); + final IProbeArrayStrategy strategy = test(Opcodes.V1_2, 0, false, true, + true); + assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass()); assertDataField(InstrSupport.DATAFIELD_ACC); assertInitMethod(false); } @Test public void testClass3() { - test(Opcodes.V1_3, 0, false, true); + final IProbeArrayStrategy strategy = test(Opcodes.V1_3, 0, false, true, + true); + assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass()); assertDataField(InstrSupport.DATAFIELD_ACC); assertInitMethod(false); } @Test public void testClass4() { - test(Opcodes.V1_4, 0, false, true); + final IProbeArrayStrategy strategy = test(Opcodes.V1_4, 0, false, true, + true); + assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass()); assertDataField(InstrSupport.DATAFIELD_ACC); assertInitMethod(false); } @Test public void testClass5() { - test(Opcodes.V1_5, 0, false, true); + final IProbeArrayStrategy strategy = test(Opcodes.V1_5, 0, false, true, + true); + assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass()); assertDataField(InstrSupport.DATAFIELD_ACC); assertInitMethod(false); } @Test public void testClass6() { - test(Opcodes.V1_6, 0, false, true); + final IProbeArrayStrategy strategy = test(Opcodes.V1_6, 0, false, true, + true); + assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass()); assertDataField(InstrSupport.DATAFIELD_ACC); assertInitMethod(true); } @Test public void testClass7() { - test(Opcodes.V1_7, 0, false, true); + final IProbeArrayStrategy strategy = test(Opcodes.V1_7, 0, false, true, + true); + assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass()); assertDataField(InstrSupport.DATAFIELD_ACC); assertInitMethod(true); } @Test public void testClass8() { - final IProbeArrayStrategy strategy = test(Opcodes.V1_8, 0, false, true); + final IProbeArrayStrategy strategy = test(Opcodes.V1_8, 0, false, true, + true); + assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass()); assertDataField(InstrSupport.DATAFIELD_ACC); assertInitMethod(true); @@ -107,14 +123,18 @@ public class ProbeArrayStrategyFactoryTest { @Test public void testInterface7() { - test(Opcodes.V1_7, Opcodes.ACC_INTERFACE, true, false); + final IProbeArrayStrategy strategy = test(Opcodes.V1_7, + Opcodes.ACC_INTERFACE, true, false, true); + assertEquals(LocalProbeArrayStrategy.class, strategy.getClass()); assertNoDataField(); assertNoInitMethod(); } @Test public void testEmptyInterface7() { - test(Opcodes.V1_7, Opcodes.ACC_INTERFACE, false, false); + final IProbeArrayStrategy strategy = test(Opcodes.V1_7, + Opcodes.ACC_INTERFACE, false, false, false); + assertEquals(NoneProbeArrayStrategy.class, strategy.getClass()); assertNoDataField(); assertNoInitMethod(); } @@ -122,7 +142,7 @@ public class ProbeArrayStrategyFactoryTest { @Test(expected = UnsupportedOperationException.class) public void testEmptyInterface7StoreInstance() { IProbeArrayStrategy strategy = test(Opcodes.V1_7, Opcodes.ACC_INTERFACE, - false, false); + false, false, false); strategy.storeInstance(null, false, 0); } @@ -130,7 +150,9 @@ public class ProbeArrayStrategyFactoryTest { public void testInterface8() { cv.isInterface = true; final IProbeArrayStrategy strategy = test(Opcodes.V1_8, - Opcodes.ACC_INTERFACE, false, true); + Opcodes.ACC_INTERFACE, false, true, true); + assertEquals(InterfaceFieldProbeArrayStrategy.class, + strategy.getClass()); assertDataField(InstrSupport.DATAFIELD_INTF_ACC); assertInitAndClinitMethods(); @@ -140,7 +162,9 @@ public class ProbeArrayStrategyFactoryTest { @Test public void testEmptyInterface8() { - test(Opcodes.V1_8, Opcodes.ACC_INTERFACE, false, false); + final IProbeArrayStrategy strategy = test(Opcodes.V1_8, + Opcodes.ACC_INTERFACE, false, false, false); + assertEquals(NoneProbeArrayStrategy.class, strategy.getClass()); assertNoDataField(); assertNoInitMethod(); } @@ -148,13 +172,27 @@ public class ProbeArrayStrategyFactoryTest { @Test(expected = UnsupportedOperationException.class) public void testEmptyInterface8StoreInstance() { final IProbeArrayStrategy strategy = test(Opcodes.V1_8, - Opcodes.ACC_INTERFACE, false, false); + Opcodes.ACC_INTERFACE, false, false, false); strategy.storeInstance(null, false, 0); } + @Test + public void testClinitAndAbstractMethodsInterface8() { + final IProbeArrayStrategy strategy = test(Opcodes.V1_8, + Opcodes.ACC_INTERFACE, true, false, true); + assertEquals(LocalProbeArrayStrategy.class, strategy.getClass()); + assertNoDataField(); + assertNoInitMethod(); + + strategy.storeInstance(cv.visitMethod(0, null, null, null, null), false, + 0); + } + @Test public void testClinitInterface8() { - test(Opcodes.V1_8, Opcodes.ACC_INTERFACE, true, false); + final IProbeArrayStrategy strategy = test(Opcodes.V1_8, + Opcodes.ACC_INTERFACE, true, false, false); + assertEquals(LocalProbeArrayStrategy.class, strategy.getClass()); assertNoDataField(); assertNoInitMethod(); } @@ -163,7 +201,9 @@ public class ProbeArrayStrategyFactoryTest { public void testClinitAndMethodsInterface8() { cv.isInterface = true; final IProbeArrayStrategy strategy = test(Opcodes.V1_8, - Opcodes.ACC_INTERFACE, true, true); + Opcodes.ACC_INTERFACE, true, true, true); + assertEquals(InterfaceFieldProbeArrayStrategy.class, + strategy.getClass()); assertDataField(InstrSupport.DATAFIELD_INTF_ACC); assertInitAndClinitMethods(); @@ -172,25 +212,32 @@ public class ProbeArrayStrategyFactoryTest { } private IProbeArrayStrategy test(int version, int access, boolean clinit, - boolean method) { - ClassWriter writer = new ClassWriter(0); + boolean method, boolean abstractMethod) { + final ClassWriter writer = new ClassWriter(0); writer.visit(version, access, "Foo", "java/lang/Object", null, null); if (clinit) { - MethodVisitor mv = writer.visitMethod(Opcodes.ACC_PUBLIC - | Opcodes.ACC_STATIC, "", "()V", null, null); + final MethodVisitor mv = writer.visitMethod( + Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, "", "()V", + null, null); mv.visitCode(); mv.visitInsn(Opcodes.RETURN); mv.visitMaxs(0, 0); mv.visitEnd(); } if (method) { - MethodVisitor mv = writer.visitMethod(Opcodes.ACC_PUBLIC - | Opcodes.ACC_STATIC, "doit", "()V", null, null); + final MethodVisitor mv = writer.visitMethod( + Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, "doit", "()V", + null, null); mv.visitCode(); mv.visitInsn(Opcodes.RETURN); mv.visitMaxs(0, 0); mv.visitEnd(); } + if (abstractMethod) { + final MethodVisitor mv = writer.visitMethod(Opcodes.ACC_ABSTRACT, + "foo", "()V", null, null); + mv.visitEnd(); + } writer.visitEnd(); final IProbeArrayStrategy strategy = ProbeArrayStrategyFactory diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeCounterTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeCounterTest.java index ed304d27..0b831be5 100644 --- a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeCounterTest.java +++ b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeCounterTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertTrue; import org.junit.Before; import org.junit.Test; +import org.objectweb.asm.Opcodes; /** * Unit tests for {@link ProbeCounter}. @@ -49,6 +50,12 @@ public class ProbeCounterTest { assertFalse(counter.hasMethods()); } + @Test + public void testVisitAbstractMethod() { + counter.visitMethod(Opcodes.ACC_ABSTRACT, "foo", null, null, null); + assertFalse(counter.hasMethods()); + } + @Test public void testVisitMethod() { assertNull(counter.visitMethod(0, "foo", null, null, null)); diff --git a/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeCounter.java b/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeCounter.java index d45a8f9d..66adf029 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeCounter.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeCounter.java @@ -13,6 +13,7 @@ package org.jacoco.core.internal.instr; import org.jacoco.core.internal.flow.ClassProbesVisitor; import org.jacoco.core.internal.flow.MethodProbesVisitor; +import org.objectweb.asm.Opcodes; /** * Internal class to remember the total number of probes required for a class. @@ -30,7 +31,8 @@ class ProbeCounter extends ClassProbesVisitor { @Override public MethodProbesVisitor visitMethod(final int access, final String name, final String desc, final String signature, final String[] exceptions) { - if (!InstrSupport.CLINIT_NAME.equals(name)) { + if (!InstrSupport.CLINIT_NAME.equals(name) + && (access & Opcodes.ACC_ABSTRACT) == 0) { methods = true; } return null; @@ -46,8 +48,8 @@ class ProbeCounter extends ClassProbesVisitor { } /** - * @return true if the class has other methods than a static - * initializer + * @return true if the class has non-abstract methods other + * than a static initializer */ boolean hasMethods() { return methods; diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html index e62b4a80..51a58ed3 100644 --- a/org.jacoco.doc/docroot/doc/changes.html +++ b/org.jacoco.doc/docroot/doc/changes.html @@ -29,6 +29,9 @@

Fixed Bugs

    +
  • Do not add useless members into Java 8 interfaces that have only interface + initialization and abstract methods + (GitHub #441).
  • Fix instrumentation to not violate Java Virtual Machine Specification regarding initialization of final fields, otherwise IllegalAccessError will be thrown starting from OpenJDK 9 EA b127 -- cgit v1.2.3