From 3794506b849c8a72cf0b29313260140dfa00d625 Mon Sep 17 00:00:00 2001 From: henry Date: Mon, 12 Feb 2024 18:20:20 +0000 Subject: [PATCH 1/2] filter mutations to defensive unmodifiable collection returns --- .../ReturnUnmodifiableCollection.java | 57 +++++++++++ .../ReturnUnmodifiableCollectionFactory.java | 25 +++++ ...ationtest.build.MutationInterceptorFactory | 2 + ...turnUnmodifiableCollectionFactoryTest.java | 97 +++++++++++++++++++ .../EqualsPerformanceShortcutFilterTest.java | 1 - .../main/java/org/pitest/util/StreamUtil.java | 3 +- 6 files changed, 183 insertions(+), 2 deletions(-) create mode 100644 pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollection.java create mode 100644 pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactory.java create mode 100644 pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactoryTest.java diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollection.java b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollection.java new file mode 100644 index 000000000..cb96139b3 --- /dev/null +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollection.java @@ -0,0 +1,57 @@ +package org.pitest.mutationtest.build.intercept.defensive; + +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.LabelNode; +import org.pitest.bytecode.analysis.MethodTree; +import org.pitest.classinfo.ClassName; +import org.pitest.mutationtest.build.intercept.Region; +import org.pitest.mutationtest.build.intercept.RegionInterceptor; +import org.pitest.sequence.Context; +import org.pitest.sequence.Match; +import org.pitest.sequence.QueryParams; +import org.pitest.sequence.QueryStart; +import org.pitest.sequence.SequenceMatcher; +import org.pitest.sequence.Slot; +import org.pitest.sequence.SlotWrite; + +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +import static org.pitest.bytecode.analysis.InstructionMatchers.anyInstruction; +import static org.pitest.bytecode.analysis.InstructionMatchers.isA; +import static org.pitest.bytecode.analysis.InstructionMatchers.methodCallTo; +import static org.pitest.bytecode.analysis.InstructionMatchers.notAnInstruction; +import static org.pitest.bytecode.analysis.OpcodeMatchers.ARETURN; +import static org.pitest.bytecode.analysis.OpcodeMatchers.INVOKESTATIC; +import static org.pitest.sequence.Result.result; + +public class ReturnUnmodifiableCollection extends RegionInterceptor { + + private static final ClassName COLLECTIONS = ClassName.fromClass(Collections.class); + + static final Slot MUTATED_INSTRUCTION = Slot.create(AbstractInsnNode.class); + + static final SequenceMatcher DEFENSIVE_RETURN = QueryStart + .any(AbstractInsnNode.class) + .then(INVOKESTATIC.and(methodCallTo(COLLECTIONS, "unmodifiableSet")).and(store(MUTATED_INSTRUCTION.write()))) + .then(ARETURN) + .zeroOrMore(QueryStart.match(anyInstruction())) + .compile(QueryParams.params(AbstractInsnNode.class) + .withIgnores(notAnInstruction().or(isA(LabelNode.class))) + ); + + + @Override + protected List computeRegions(MethodTree method) { + Context context = Context.start(); + return DEFENSIVE_RETURN.contextMatches(method.instructions(), context).stream() + .map(c -> c.retrieve(MUTATED_INSTRUCTION.read()).get()) + .map(n -> new Region(n, n)) + .collect(Collectors.toList()); + } + + private static Match store(SlotWrite slot) { + return (c,n) -> result(true, c.store(slot, n)); + } +} diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactory.java b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactory.java new file mode 100644 index 000000000..0543537d4 --- /dev/null +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactory.java @@ -0,0 +1,25 @@ +package org.pitest.mutationtest.build.intercept.defensive; + +import org.pitest.mutationtest.build.InterceptorParameters; +import org.pitest.mutationtest.build.MutationInterceptor; +import org.pitest.mutationtest.build.MutationInterceptorFactory; +import org.pitest.plugin.Feature; + +public class ReturnUnmodifiableCollectionFactory implements MutationInterceptorFactory { + @Override + public MutationInterceptor createInterceptor(InterceptorParameters params) { + return new ReturnUnmodifiableCollection(); + } + + @Override + public Feature provides() { + return Feature.named("DEFENSIVERETURN") + .withOnByDefault(true) + .withDescription(description()); + } + + @Override + public String description() { + return "Filter mutations to defensive return wrappers such as unmodifiableCollection"; + } +} diff --git a/pitest-entry/src/main/resources/META-INF/services/org.pitest.mutationtest.build.MutationInterceptorFactory b/pitest-entry/src/main/resources/META-INF/services/org.pitest.mutationtest.build.MutationInterceptorFactory index 3a8988333..441e2915d 100755 --- a/pitest-entry/src/main/resources/META-INF/services/org.pitest.mutationtest.build.MutationInterceptorFactory +++ b/pitest-entry/src/main/resources/META-INF/services/org.pitest.mutationtest.build.MutationInterceptorFactory @@ -24,5 +24,7 @@ org.pitest.mutationtest.build.intercept.equivalent.EquivalentReturnMutationFilte org.pitest.mutationtest.build.intercept.exclude.FirstLineInterceptorFactory org.pitest.mutationtest.build.intercept.equivalent.DivisionByMinusOneFilterFactory org.pitest.mutationtest.build.intercept.lombok.LombokFilter +org.pitest.mutationtest.build.intercept.defensive.ReturnUnmodifiableCollectionFactory + org.pitest.plugin.export.MutantExportFactory diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactoryTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactoryTest.java new file mode 100644 index 000000000..9ed6cced7 --- /dev/null +++ b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactoryTest.java @@ -0,0 +1,97 @@ +package org.pitest.mutationtest.build.intercept.defensive; + +import org.junit.Test; +import org.pitest.mutationtest.build.InterceptorType; +import org.pitest.mutationtest.build.MutationInterceptorFactory; +import org.pitest.mutationtest.engine.gregor.mutators.NullMutateEverything; +import org.pitest.verifier.interceptors.FactoryVerifier; +import org.pitest.verifier.interceptors.InterceptorVerifier; +import org.pitest.verifier.interceptors.VerifierStart; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +import static org.pitest.bytecode.analysis.OpcodeMatchers.INVOKESTATIC; + + +public class ReturnUnmodifiableCollectionFactoryTest { + private final MutationInterceptorFactory underTest = new ReturnUnmodifiableCollectionFactory(); + InterceptorVerifier v = VerifierStart.forInterceptorFactory(underTest) + .usingMutator(new NullMutateEverything()); + + @Test + public void isOnChain() { + FactoryVerifier.confirmFactory(underTest) + .isOnChain(); + } + + @Test + public void isOnByDefault() { + FactoryVerifier.confirmFactory(underTest) + .isOnByDefault(); + } + + @Test + public void featureIsCalledLombok() { + FactoryVerifier.confirmFactory(underTest) + .featureName().isEqualTo("defensivereturn"); + } + + @Test + public void createsFilters() { + FactoryVerifier.confirmFactory(underTest) + .createsInterceptorsOfType(InterceptorType.FILTER); + } + + + @Test + public void filtersMutationsToReturnUnmodifiableSet() { + v.forClass(HasUnmodifiableSetReturn.class) + .forCodeMatching(INVOKESTATIC.asPredicate()) + .allMutantsAreFiltered() + .verify(); + } + + @Test + public void doesNotFilterOtherCode() { + v.forClass(HasUnmodifiableSetReturn.class) + .forCodeMatching(INVOKESTATIC.asPredicate().negate()) + .noMutantsAreFiltered() + .verify(); + } + + @Test + public void doesNotFilterOtherCallsToUnModifiableSet() { + v.forClass(HasUnmodifiableSetNonReturn.class) + .forAnyCode() + .noMutantsAreFiltered() + .verify(); + } +} + +class HasUnmodifiableSetReturn { + private final Set s = new HashSet<>(); + + public Set mutateMe(int i) { + if (i != 1) { + return Collections.unmodifiableSet(s); + } + + return s; + } +} + +class HasUnmodifiableSetNonReturn { + private final Set s = new HashSet<>(); + private Set copy; + + + public Set dontMutateME(int i) { + if (i != 1) { + copy = Collections.unmodifiableSet(s); + } + + return s; + } +} \ No newline at end of file diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/equivalent/EqualsPerformanceShortcutFilterTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/equivalent/EqualsPerformanceShortcutFilterTest.java index 6a14186c4..57059f686 100644 --- a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/equivalent/EqualsPerformanceShortcutFilterTest.java +++ b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/equivalent/EqualsPerformanceShortcutFilterTest.java @@ -17,7 +17,6 @@ import org.pitest.mutationtest.engine.gregor.MethodMutatorFactory; import org.pitest.mutationtest.engine.gregor.config.Mutator; import org.pitest.mutationtest.engine.gregor.mutators.returns.BooleanFalseReturnValsMutator; -import org.pitest.mutationtest.engine.gregor.mutators.returns.PrimitiveReturnsMutator; public class EqualsPerformanceShortcutFilterTest { diff --git a/pitest/src/main/java/org/pitest/util/StreamUtil.java b/pitest/src/main/java/org/pitest/util/StreamUtil.java index f0eae4455..17cc25d90 100644 --- a/pitest/src/main/java/org/pitest/util/StreamUtil.java +++ b/pitest/src/main/java/org/pitest/util/StreamUtil.java @@ -5,6 +5,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.Buffer; import java.nio.ByteBuffer; import java.nio.channels.Channels; import java.nio.channels.ReadableByteChannel; @@ -35,7 +36,7 @@ private static void copy(final InputStream input, final OutputStream output) final WritableByteChannel dest = Channels.newChannel(output); final ByteBuffer buffer = ByteBuffer.allocateDirect(16 * 1024); while (src.read(buffer) != -1) { - buffer.flip(); + ((Buffer)buffer).flip(); dest.write(buffer); buffer.compact(); } From e80f8243c5db93dd9e469aee16400eadcbee5a10 Mon Sep 17 00:00:00 2001 From: henry Date: Mon, 12 Feb 2024 18:55:16 +0000 Subject: [PATCH 2/2] filter all returns of unmodifiable wrappers --- .../analysis/InstructionMatchers.java | 7 +++- .../ReturnUnmodifiableCollection.java | 4 +- ...turnUnmodifiableCollectionFactoryTest.java | 38 +++++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/pitest-entry/src/main/java/org/pitest/bytecode/analysis/InstructionMatchers.java b/pitest-entry/src/main/java/org/pitest/bytecode/analysis/InstructionMatchers.java index d8b2a8daa..8061d3b1c 100644 --- a/pitest-entry/src/main/java/org/pitest/bytecode/analysis/InstructionMatchers.java +++ b/pitest-entry/src/main/java/org/pitest/bytecode/analysis/InstructionMatchers.java @@ -188,16 +188,19 @@ public static Match methodDescEquals(final String desc) { } public static Match methodCallTo(final ClassName owner, final String name) { + return methodCallTo(owner, c -> c.equals(name)); + } + + public static Match methodCallTo(final ClassName owner, Predicate name) { return (c, t) -> { if ( t instanceof MethodInsnNode ) { final MethodInsnNode call = (MethodInsnNode) t; - return result( call.name.equals(name) && call.owner.equals(owner.asInternalName()), c); + return result( name.test(call.name) && call.owner.equals(owner.asInternalName()), c); } return result(false, c); }; } - public static Match isInstruction(final SlotRead target) { return (c, t) -> result(c.retrieve(target).get() == t, c); } diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollection.java b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollection.java index cb96139b3..efe9b2b5a 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollection.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollection.java @@ -28,13 +28,11 @@ public class ReturnUnmodifiableCollection extends RegionInterceptor { - private static final ClassName COLLECTIONS = ClassName.fromClass(Collections.class); - static final Slot MUTATED_INSTRUCTION = Slot.create(AbstractInsnNode.class); static final SequenceMatcher DEFENSIVE_RETURN = QueryStart .any(AbstractInsnNode.class) - .then(INVOKESTATIC.and(methodCallTo(COLLECTIONS, "unmodifiableSet")).and(store(MUTATED_INSTRUCTION.write()))) + .then(INVOKESTATIC.and(methodCallTo(ClassName.fromClass(Collections.class), n -> n.startsWith("unmodifiable"))).and(store(MUTATED_INSTRUCTION.write()))) .then(ARETURN) .zeroOrMore(QueryStart.match(anyInstruction())) .compile(QueryParams.params(AbstractInsnNode.class) diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactoryTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactoryTest.java index 9ed6cced7..b53d1086e 100644 --- a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactoryTest.java +++ b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactoryTest.java @@ -8,8 +8,11 @@ import org.pitest.verifier.interceptors.InterceptorVerifier; import org.pitest.verifier.interceptors.VerifierStart; +import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; +import java.util.List; +import java.util.Map; import java.util.Set; import static org.pitest.bytecode.analysis.OpcodeMatchers.INVOKESTATIC; @@ -53,6 +56,22 @@ public void filtersMutationsToReturnUnmodifiableSet() { .verify(); } + @Test + public void filtersMutationsToReturnUnmodifiableList() { + v.forClass(HasUnmodifiableListReturn.class) + .forCodeMatching(INVOKESTATIC.asPredicate()) + .allMutantsAreFiltered() + .verify(); + } + + @Test + public void filtersMutationsToReturnUnmodifiableMap() { + v.forClass(HasUnmodifiableMapReturn.class) + .forCodeMatching(INVOKESTATIC.asPredicate()) + .allMutantsAreFiltered() + .verify(); + } + @Test public void doesNotFilterOtherCode() { v.forClass(HasUnmodifiableSetReturn.class) @@ -82,6 +101,25 @@ public Set mutateMe(int i) { } } +class HasUnmodifiableListReturn { + private final List s = new ArrayList<>(); + + public List mutateMe(int i) { + if (i != 1) { + return Collections.unmodifiableList(s); + } + + return s; + } +} + +class HasUnmodifiableMapReturn { + + public Map mutateMe(Map m) { + return Collections.unmodifiableMap(m); + } +} + class HasUnmodifiableSetNonReturn { private final Set s = new HashSet<>(); private Set copy;