From d727fce3a56f27940e73054fce574ce5c3802a15 Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Fri, 2 Jun 2023 09:24:56 +0100 Subject: [PATCH] Ensure mutators do not modify bytecode during scan stage Add in assertion for all tests using verifier to ensure bytecode remains unaltered. Fix applied to InlineConstantMutator which produced functionally equivalent but different bytecode. --- .../engine/gregor/MethodMutatorFactory.java | 6 +- .../mutators/InlineConstantMutator.java | 58 ++++--- .../mutants/MutatorVerifierStart.java | 45 +++++ .../mutants/ScanningClassVisitor.java | 157 ++++++++++++++++++ 4 files changed, 239 insertions(+), 27 deletions(-) create mode 100644 pitest/src/test/java/org/pitest/verifier/mutants/ScanningClassVisitor.java diff --git a/pitest/src/main/java/org/pitest/mutationtest/engine/gregor/MethodMutatorFactory.java b/pitest/src/main/java/org/pitest/mutationtest/engine/gregor/MethodMutatorFactory.java index 3ca0d07ec..9d77d904d 100644 --- a/pitest/src/main/java/org/pitest/mutationtest/engine/gregor/MethodMutatorFactory.java +++ b/pitest/src/main/java/org/pitest/mutationtest/engine/gregor/MethodMutatorFactory.java @@ -46,15 +46,15 @@ default MethodVisitor create(MutationContext context, } - default AnnotationVisitor createForAnnotation(NoMethodContext context, AnnotationInfo annotationInfo, AnnotationVisitor next) { + default AnnotationVisitor createForAnnotation(BasicContext context, AnnotationInfo annotationInfo, AnnotationVisitor next) { return null; } - default boolean skipAnnotation(NoMethodContext nonMethodContext, AnnotationInfo annotationInfo) { + default boolean skipAnnotation(BasicContext nonMethodContext, AnnotationInfo annotationInfo) { return false; } - default FieldVisitor createForField(NoMethodContext context, FieldInfo fieldInfo, FieldVisitor fieldVisitor) { + default FieldVisitor createForField(BasicContext context, FieldInfo fieldInfo, FieldVisitor fieldVisitor) { return null; } diff --git a/pitest/src/main/java/org/pitest/mutationtest/engine/gregor/mutators/InlineConstantMutator.java b/pitest/src/main/java/org/pitest/mutationtest/engine/gregor/mutators/InlineConstantMutator.java index 5334e6385..5219cdb46 100644 --- a/pitest/src/main/java/org/pitest/mutationtest/engine/gregor/mutators/InlineConstantMutator.java +++ b/pitest/src/main/java/org/pitest/mutationtest/engine/gregor/mutators/InlineConstantMutator.java @@ -43,31 +43,33 @@ private final class InlineConstantVisitor extends MethodVisitor { this.context = context; } - private void mutate(final Double constant) { + private boolean mutate(final Double constant) { // avoid addition to floating points as may yield same value final Double replacement = (constant == 1D) ? 2D : 1D; if (shouldMutate(constant, replacement)) { translateToByteCode(replacement); - } else { - translateToByteCode(constant); + return true; } + + return false; } - private void mutate(final Float constant) { + private boolean mutate(final Float constant) { // avoid addition to floating points as may yield same value final Float replacement = (constant == 1F) ? 2F : 1F; if (shouldMutate(constant, replacement)) { translateToByteCode(replacement); - } else { - translateToByteCode(constant); + return true; } + + return false; } - private void mutate(final Integer constant) { + private boolean mutate(final Integer constant) { final Integer replacement; switch (constant.intValue()) { @@ -87,33 +89,35 @@ private void mutate(final Integer constant) { if (shouldMutate(constant, replacement)) { translateToByteCode(replacement); - } else { - translateToByteCode(constant); + return true; } + + return false; } - private void mutate(final Long constant) { + private boolean mutate(final Long constant) { final Long replacement = constant + 1L; if (shouldMutate(constant, replacement)) { translateToByteCode(replacement); - } else { - translateToByteCode(constant); + return true; } + return false; + } - private void mutate(final Number constant) { + private boolean mutate(final Number constant) { if (constant instanceof Integer) { - mutate((Integer) constant); + return mutate((Integer) constant); } else if (constant instanceof Long) { - mutate((Long) constant); + return mutate((Long) constant); } else if (constant instanceof Float) { - mutate((Float) constant); + return mutate((Float) constant); } else if (constant instanceof Double) { - mutate((Double) constant); + return mutate((Double) constant); } else { throw new PitError("Unsupported subtype of Number found:" + constant.getClass()); @@ -251,7 +255,9 @@ public void visitInsn(final int opcode) { return; } - mutate(inlineConstant); + if (!mutate(inlineConstant) ) { + super.visitInsn(opcode); + } } /* @@ -262,10 +268,12 @@ public void visitInsn(final int opcode) { @Override public void visitIntInsn(final int opcode, final int operand) { if ((opcode == Opcodes.BIPUSH) || (opcode == Opcodes.SIPUSH)) { - mutate(operand); - } else { - super.visitIntInsn(opcode, operand); + if (mutate(operand) ) { + return; + } } + + super.visitIntInsn(opcode, operand); } /* @@ -277,10 +285,12 @@ public void visitIntInsn(final int opcode, final int operand) { public void visitLdcInsn(final Object constant) { // do not mutate strings or .class here if (constant instanceof Number) { - mutate((Number) constant); - } else { - super.visitLdcInsn(constant); + if (mutate((Number) constant)) { + return; + } } + + super.visitLdcInsn(constant); } } diff --git a/pitest/src/test/java/org/pitest/verifier/mutants/MutatorVerifierStart.java b/pitest/src/test/java/org/pitest/verifier/mutants/MutatorVerifierStart.java index 0aad86b6a..8f2a14eda 100644 --- a/pitest/src/test/java/org/pitest/verifier/mutants/MutatorVerifierStart.java +++ b/pitest/src/test/java/org/pitest/verifier/mutants/MutatorVerifierStart.java @@ -1,5 +1,9 @@ package org.pitest.verifier.mutants; +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.util.Textifier; +import org.objectweb.asm.util.TraceClassVisitor; import org.pitest.classinfo.ClassByteArraySource; import org.pitest.classinfo.ClassName; import org.pitest.classpath.ClassloaderByteArraySource; @@ -8,6 +12,8 @@ import org.pitest.mutationtest.engine.gregor.MethodInfo; import org.pitest.mutationtest.engine.gregor.MethodMutatorFactory; +import java.io.PrintWriter; +import java.io.StringWriter; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -21,6 +27,7 @@ import java.util.stream.Collectors; import static java.util.Arrays.asList; +import static org.assertj.core.api.Assertions.assertThat; public class MutatorVerifierStart { @@ -69,6 +76,7 @@ public MutatorVerifierStart notCheckingUnMutatedValues() { } public MutatorVerifier forClass(Class clazz) { + assertScanDoesNotAlterClass(clazz); GregorMutater engine = makeEngine(); return new MutatorVerifier(engine, clazz, mutantFilter, checkUnmutatedValues); } @@ -79,31 +87,37 @@ public MutatorVerifier forClass(String clazz) { } public CallableMutantVerifier forCallableClass(Class> clazz) { + assertScanDoesNotAlterClass(clazz); GregorMutater engine = makeEngine(); return new CallableMutantVerifier(engine, clazz, mutantFilter, checkUnmutatedValues); } public MutantVerifier forFunctionClass(Class> clazz) { + assertScanDoesNotAlterClass(clazz); GregorMutater engine = makeEngine(); return new MutantVerifier(engine, clazz, mutantFilter, checkUnmutatedValues); } public IntMutantVerifier forIntFunctionClass(Class> clazz) { + assertScanDoesNotAlterClass(clazz); GregorMutater engine = makeEngine(); return new IntMutantVerifier<>(engine, clazz, mutantFilter, checkUnmutatedValues); } public LongMutantVerifier forLongFunctionClass(Class> clazz) { + assertScanDoesNotAlterClass(clazz); GregorMutater engine = makeEngine(); return new LongMutantVerifier<>(engine, clazz, mutantFilter, checkUnmutatedValues); } public DoubleMutantVerifier forDoubleFunctionClass(Class> clazz) { + assertScanDoesNotAlterClass(clazz); GregorMutater engine = makeEngine(); return new DoubleMutantVerifier<>(engine, clazz, mutantFilter, checkUnmutatedValues); } public BiFunctionMutantVerifier forBiFunctionClass(Class> clazz) { + assertScanDoesNotAlterClass(clazz); GregorMutater engine = makeEngine(); return new BiFunctionMutantVerifier<>(engine, clazz, mutantFilter, checkUnmutatedValues); } @@ -118,4 +132,35 @@ private GregorMutater makeEngine() { return new GregorMutater(source, filter, mutators); } + public void assertScanDoesNotAlterClass(Class clazz) { + ClassByteArraySource source = ClassloaderByteArraySource.fromContext(); + byte[] bytes = source.getBytes(clazz.getName()).get(); + + ClassWriter writer = new ClassWriter(ClassWriter.COMPUTE_FRAMES); + final ClassReader reader = new ClassReader(bytes); + final ScanningClassVisitor nv = new ScanningClassVisitor(writer, mmfs); + + reader.accept(nv, ClassReader.EXPAND_FRAMES); + + assertThat(asString(writer.toByteArray())) + .isEqualTo(asString(plainTransform(bytes))) + .describedAs("Mutator modified class when mutation was not active"); + + } + + private byte[] plainTransform(byte[] bytes) { + ClassWriter writer = new ClassWriter(ClassWriter.COMPUTE_FRAMES); + final ClassReader reader = new ClassReader(bytes); + reader.accept(writer, ClassReader.EXPAND_FRAMES); + return writer.toByteArray(); + } + + private String asString(byte[] bytes) { + ClassReader reader = new ClassReader(bytes); + StringWriter writer = new StringWriter(); + reader.accept(new TraceClassVisitor(null, new Textifier(), new PrintWriter( + writer)), ClassReader.EXPAND_FRAMES); + return writer.toString(); + } + } diff --git a/pitest/src/test/java/org/pitest/verifier/mutants/ScanningClassVisitor.java b/pitest/src/test/java/org/pitest/verifier/mutants/ScanningClassVisitor.java new file mode 100644 index 000000000..92c5a76d5 --- /dev/null +++ b/pitest/src/test/java/org/pitest/verifier/mutants/ScanningClassVisitor.java @@ -0,0 +1,157 @@ +package org.pitest.verifier.mutants; + +import org.objectweb.asm.AnnotationVisitor; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.FieldVisitor; +import org.objectweb.asm.MethodVisitor; +import org.pitest.bytecode.ASMVersion; +import org.pitest.classinfo.ClassName; +import org.pitest.mutationtest.engine.Location; +import org.pitest.mutationtest.engine.MutationIdentifier; +import org.pitest.mutationtest.engine.gregor.AnnotationInfo; +import org.pitest.mutationtest.engine.gregor.BasicContext; +import org.pitest.mutationtest.engine.gregor.ClassInfo; +import org.pitest.mutationtest.engine.gregor.FieldInfo; +import org.pitest.mutationtest.engine.gregor.MethodInfo; +import org.pitest.mutationtest.engine.gregor.MethodMutatorFactory; +import org.pitest.mutationtest.engine.gregor.MutationContext; +import org.pitest.mutationtest.engine.gregor.NoMethodContext; + +import java.util.List; + +public class ScanningClassVisitor extends ClassVisitor { + + private final List mmfs; + + private ClassInfo classInfo; + + protected ScanningClassVisitor(ClassVisitor classVisitor, List mmfs) { + super(ASMVersion.asmVersion(), classVisitor); + this.mmfs = mmfs; + } + + @Override + public void visit(final int version, final int access, final String name, + final String signature, final String superName, final String[] interfaces) { + super.visit(version, access, name, signature, superName, interfaces); + this.classInfo = new ClassInfo(access, name, superName); + } + + @Override + public MethodVisitor visitMethod(final int access, final String methodName, + final String methodDescriptor, final String signature, + final String[] exceptions) { + + MutationContext context = fakeContext(classInfo); + + MethodInfo methodInfo = new MethodInfo() + .withOwner(classInfo).withAccess(access) + .withMethodName(methodName).withMethodDescriptor(methodDescriptor); + + MethodVisitor next = super.visitMethod(access, methodName, methodDescriptor, signature, exceptions); + for (final MethodMutatorFactory each : this.mmfs) { + MethodVisitor mv = each.create(context, methodInfo, next); + if (mv != null) { + next = mv; + } + } + + return next; + } + + @Override + public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) { + AnnotationVisitor next = super.visitAnnotation(descriptor, visible); + AnnotationInfo annotationInfo = new AnnotationInfo(descriptor, visible); + + BasicContext context = fakeMethodContext(); + + for (final MethodMutatorFactory each : this.mmfs) { + if (each.skipAnnotation(context, annotationInfo)) { + return null; + } + } + + for (final MethodMutatorFactory each : this.mmfs) { + AnnotationVisitor fv = each.createForAnnotation(context, annotationInfo, next); + if (fv != null) { + next = fv; + } + } + return next; + } + + @Override + public FieldVisitor visitField(int access, String name, String descriptor, String signature, Object value) { + FieldVisitor next = super.visitField(access, name, descriptor, signature, value); + FieldInfo fieldInfo = new FieldInfo(access, name, descriptor, signature, value); + + for (final MethodMutatorFactory each : this.mmfs) { + FieldVisitor fv = each.createForField(fakeMethodContext(), fieldInfo, next); + if (fv != null) { + next = fv; + } + } + + return next; + } + + private BasicContext fakeMethodContext() { + return new BasicContext() { + @Override + public ClassInfo getClassInfo() { + return classInfo; + } + + @Override + public boolean shouldMutate(MutationIdentifier id) { + return false; + } + + @Override + public void registerMutation(MutationIdentifier id, String description) { + + } + }; + } + + + private MutationContext fakeContext(ClassInfo classInfo) { + return new MutationContext() { + @Override + public void registerCurrentLine(int line) { + + } + + @Override + public MutationIdentifier registerMutation(MethodMutatorFactory factory, String description) { + return new MutationIdentifier(Location.location(ClassName.fromString("fake"), "fake", "fake"), 1, ""); + } + + @Override + public ClassInfo getClassInfo() { + return classInfo; + } + + @Override + public void registerMutation(MutationIdentifier id, String description) { + + } + + @Override + public boolean shouldMutate(MutationIdentifier newId) { + return false; + } + + @Override + public void registerNewBlock() { + + } + + @Override + public void registerNewMethodStart() { + + } + }; + } +}