diff --git a/src/main/java/com/code_intelligence/jazzer/mutation/api/SerializingMutator.java b/src/main/java/com/code_intelligence/jazzer/mutation/api/SerializingMutator.java index 58b2a49b3..b4e53232f 100644 --- a/src/main/java/com/code_intelligence/jazzer/mutation/api/SerializingMutator.java +++ b/src/main/java/com/code_intelligence/jazzer/mutation/api/SerializingMutator.java @@ -17,6 +17,7 @@ package com.code_intelligence.jazzer.mutation.api; import com.google.errorprone.annotations.DoNotMock; +import com.google.errorprone.annotations.ForOverride; /** * Combines a {@link ValueMutator} with a {@link Serializer} for objects of type {@code T}. @@ -28,8 +29,36 @@ */ @DoNotMock("Use TestSupport#mockMutator instead") public abstract class SerializingMutator implements Serializer, ValueMutator { + private Boolean cachedHasFixedSize; + @Override public final String toString() { return Debuggable.getDebugString(this); } + + @Override + public boolean hasFixedSize() { + if (cachedHasFixedSize != null) { + return cachedHasFixedSize; + } + // If the type to mutate is recursive, computeHasFixedSize() may call back into hasFixedSize(). + // Ensure that the innermost call returns false to terminate the cycle and rely on all + // intermediate calls to propagate false up to the outermost call. This is safe since only the + // outermost call will ever reach this code (mutators are explicitly not thread-safe). + cachedHasFixedSize = false; + cachedHasFixedSize = computeHasFixedSize(); + return cachedHasFixedSize; + } + + /** + * Computes the value of {@link ValueMutator#hasFixedSize()} by inspecting the return value of + * that function for child mutators. + * + *

If the return value is a constant, override {@link ValueMutator#hasFixedSize()} directly. + */ + @ForOverride + protected boolean computeHasFixedSize() { + throw new UnsupportedOperationException( + "Subclasses of SerializingMutator must override hasFixedSize or computeHasFixedSize"); + } } diff --git a/src/main/java/com/code_intelligence/jazzer/mutation/api/ValueMutator.java b/src/main/java/com/code_intelligence/jazzer/mutation/api/ValueMutator.java index 2fc711204..3e37cacbd 100644 --- a/src/main/java/com/code_intelligence/jazzer/mutation/api/ValueMutator.java +++ b/src/main/java/com/code_intelligence/jazzer/mutation/api/ValueMutator.java @@ -87,6 +87,10 @@ public interface ValueMutator extends Debuggable { * *

Examples of types with fixed size include primitive types, enums, and classes with only * primitive types and enums as members. + * + *

Note: Implementing classes should only override this method if the result does not depend on + * the value of {@code hasFixedSize()} for any child mutators. If it would, instead override + * {@link SerializingMutator#computeHasFixedSize()} to prevent issues when encountering a cycle. */ boolean hasFixedSize(); } diff --git a/src/main/java/com/code_intelligence/jazzer/mutation/combinator/MutatorCombinators.java b/src/main/java/com/code_intelligence/jazzer/mutation/combinator/MutatorCombinators.java index 66fdeb2c3..11643bb04 100644 --- a/src/main/java/com/code_intelligence/jazzer/mutation/combinator/MutatorCombinators.java +++ b/src/main/java/com/code_intelligence/jazzer/mutation/combinator/MutatorCombinators.java @@ -182,9 +182,10 @@ public String toString() { }; } - boolean hasFixedSize = stream(partialMutators).allMatch(InPlaceMutator::hasFixedSize); final InPlaceMutator[] mutators = Arrays.copyOf(partialMutators, partialMutators.length); return new InPlaceMutator() { + private Boolean cachedHasFixedSize; + @Override public void initInPlace(T reference, PseudoRandom prng) { for (InPlaceMutator mutator : mutators) { @@ -204,9 +205,15 @@ public void crossOverInPlace(T reference, T otherReference, PseudoRandom prng) { } } + /** See comment on {@link SerializingMutator#hasFixedSize()}. */ @Override public boolean hasFixedSize() { - return hasFixedSize; + if (cachedHasFixedSize != null) { + return cachedHasFixedSize; + } + cachedHasFixedSize = false; + cachedHasFixedSize = stream(partialMutators).allMatch(InPlaceMutator::hasFixedSize); + return cachedHasFixedSize; } @Override @@ -482,39 +489,33 @@ public boolean hasFixedSize() { * @param serializer implementation of the {@link Serializer} part * @param lazyMutator supplies the implementation of the {@link InPlaceMutator} part. This is * guaranteed to be invoked exactly once and only after {@code registerSelf}. - * @param hasFixedSize the value to return from the resulting mutators {@link - * InPlaceMutator#hasFixedSize()} */ public static SerializingInPlaceMutator assemble( Consumer> registerSelf, Supplier makeDefaultInstance, Serializer serializer, - Supplier> lazyMutator, - boolean hasFixedSize) { + Supplier> lazyMutator) { return new DelegatingSerializingInPlaceMutator<>( - registerSelf, makeDefaultInstance, serializer, lazyMutator, hasFixedSize); + registerSelf, makeDefaultInstance, serializer, lazyMutator); } - private static class DelegatingSerializingInPlaceMutator extends SerializingInPlaceMutator { + private static final class DelegatingSerializingInPlaceMutator + extends SerializingInPlaceMutator { private final Supplier makeDefaultInstance; private final Serializer serializer; private final InPlaceMutator mutator; - private final boolean hasFixedSize; private DelegatingSerializingInPlaceMutator( Consumer> registerSelf, Supplier makeDefaultInstance, Serializer serializer, - Supplier> lazyMutator, - boolean hasFixedSize) { + Supplier> lazyMutator) { requireNonNull(makeDefaultInstance); requireNonNull(serializer); registerSelf.accept(this); this.makeDefaultInstance = makeDefaultInstance; this.serializer = serializer; - // Set before invoking the supplier as that can result in calls to hasFixedSize(). - this.hasFixedSize = hasFixedSize; this.mutator = lazyMutator.get(); } @@ -534,10 +535,8 @@ public void crossOverInPlace(T reference, T otherReference, PseudoRandom prng) { } @Override - public boolean hasFixedSize() { - // This uses a fixed value rather than calling mutator.hasFixedSize() as this method is called - // before the constructor has finished, which is necessary in the case of a cycle. - return hasFixedSize; + protected boolean computeHasFixedSize() { + return mutator.hasFixedSize(); } @Override diff --git a/src/main/java/com/code_intelligence/jazzer/mutation/combinator/PostComposedMutator.java b/src/main/java/com/code_intelligence/jazzer/mutation/combinator/PostComposedMutator.java index 4b79cb80a..4024f116e 100644 --- a/src/main/java/com/code_intelligence/jazzer/mutation/combinator/PostComposedMutator.java +++ b/src/main/java/com/code_intelligence/jazzer/mutation/combinator/PostComposedMutator.java @@ -62,7 +62,7 @@ public R crossOver(R value, R otherValue, PseudoRandom prng) { } @Override - public boolean hasFixedSize() { + protected boolean computeHasFixedSize() { return mutator.hasFixedSize(); } diff --git a/src/main/java/com/code_intelligence/jazzer/mutation/combinator/ProductMutator.java b/src/main/java/com/code_intelligence/jazzer/mutation/combinator/ProductMutator.java index ccd1ce026..38fdb838d 100644 --- a/src/main/java/com/code_intelligence/jazzer/mutation/combinator/ProductMutator.java +++ b/src/main/java/com/code_intelligence/jazzer/mutation/combinator/ProductMutator.java @@ -41,13 +41,11 @@ public final class ProductMutator extends SerializingInPlaceMutator { private static final int INVERSE_PICK_VALUE_SUPPLIER_FREQUENCY = 100; private final SerializingMutator[] mutators; - private final boolean hasFixedSize; ProductMutator(SerializingMutator[] mutators) { requireNonNullElements(mutators); require(mutators.length > 0, "mutators must not be empty"); this.mutators = Arrays.copyOf(mutators, mutators.length); - this.hasFixedSize = stream(mutators).allMatch(ValueMutator::hasFixedSize); } @Override @@ -128,8 +126,8 @@ public void crossOverInPlace(Object[] reference, Object[] otherReference, Pseudo } @Override - public boolean hasFixedSize() { - return hasFixedSize; + protected boolean computeHasFixedSize() { + return stream(mutators).allMatch(ValueMutator::hasFixedSize); } @Override diff --git a/src/main/java/com/code_intelligence/jazzer/mutation/mutator/lang/NullableMutatorFactory.java b/src/main/java/com/code_intelligence/jazzer/mutation/mutator/lang/NullableMutatorFactory.java index 093f4b6ab..389463f96 100644 --- a/src/main/java/com/code_intelligence/jazzer/mutation/mutator/lang/NullableMutatorFactory.java +++ b/src/main/java/com/code_intelligence/jazzer/mutation/mutator/lang/NullableMutatorFactory.java @@ -110,7 +110,7 @@ public T crossOver(T value, T otherValue, PseudoRandom prng) { } @Override - public boolean hasFixedSize() { + protected boolean computeHasFixedSize() { return mutator.hasFixedSize(); } diff --git a/src/main/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorFactory.java b/src/main/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorFactory.java index 7d49bc686..4ab5e0a26 100644 --- a/src/main/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorFactory.java +++ b/src/main/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorFactory.java @@ -30,7 +30,6 @@ import static com.code_intelligence.jazzer.mutation.mutator.proto.BuilderAdapters.setFieldWithPresence; import static com.code_intelligence.jazzer.mutation.mutator.proto.BuilderAdapters.setMapField; import static com.code_intelligence.jazzer.mutation.mutator.proto.TypeLibrary.getDefaultInstance; -import static com.code_intelligence.jazzer.mutation.mutator.proto.TypeLibrary.isRecursiveField; import static com.code_intelligence.jazzer.mutation.mutator.proto.TypeLibrary.withoutInitIfRecursive; import static com.code_intelligence.jazzer.mutation.support.InputStreamSupport.cap; import static com.code_intelligence.jazzer.mutation.support.TypeSupport.asAnnotatedType; @@ -65,7 +64,6 @@ import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.Message; import com.google.protobuf.Message.Builder; -import com.google.protobuf.MessageOrBuilder; import com.google.protobuf.UnknownFieldSet; import java.io.DataInputStream; import java.io.DataOutputStream; @@ -336,11 +334,6 @@ private SerializingMutator mutatorForAny( .boxed() .collect(toMap(i -> getTypeUrl(getDefaultInstance(anySource.value()[i])), identity())); - boolean hasFixedSize = - stream(anySource.value()) - .map(TypeLibrary::getDefaultInstance) - .map(MessageOrBuilder::getDescriptorForType) - .allMatch(BuilderMutatorFactory::hasFixedSize); return assemble( mutator -> internedMutators.put(new CacheKey(Any.getDescriptor(), anySource), mutator), Any.getDefaultInstance()::toBuilder, @@ -374,8 +367,7 @@ private SerializingMutator mutatorForAny( any.setValue(message.toByteString()); }); }) - .toArray(InPlaceMutator[]::new)), - hasFixedSize); + .toArray(InPlaceMutator[]::new))); } private static String getTypeUrl(Message message) { @@ -474,21 +466,7 @@ private SerializingMutator makeBuilderMutator( ? new Annotation[0] : new Annotation[] {anySource}, factory)) - .toArray(InPlaceMutator[]::new)), - hasFixedSize(descriptor)); - } - - private static boolean hasFixedSize(Descriptor descriptor) { - return descriptor.getFields().stream() - .noneMatch( - field -> - field.isMapField() - || field.isRepeated() - || isRecursiveField(field) - || field.getJavaType() == JavaType.STRING - || field.getJavaType() == JavaType.BYTE_STRING - || (field.getJavaType() == JavaType.MESSAGE - && !hasFixedSize(field.getMessageType()))); + .toArray(InPlaceMutator[]::new))); } private static final class CacheKey { diff --git a/src/test/java/com/code_intelligence/jazzer/mutation/combinator/MutatorCombinatorsTest.java b/src/test/java/com/code_intelligence/jazzer/mutation/combinator/MutatorCombinatorsTest.java index b1c0ab5bc..c928a2f08 100644 --- a/src/test/java/com/code_intelligence/jazzer/mutation/combinator/MutatorCombinatorsTest.java +++ b/src/test/java/com/code_intelligence/jazzer/mutation/combinator/MutatorCombinatorsTest.java @@ -323,8 +323,7 @@ public Foo detach(Foo value) { return null; } }, - () -> combine(valueMutator, listMutator), - false); + () -> combine(valueMutator, listMutator)); assertThat(mutator.toString()).isEqualTo("{Foo.Integer, Foo via List}"); @@ -381,8 +380,7 @@ public Foo detach(Foo value) { return null; } }, - () -> combine(valueMutator, listMutator), - false); + () -> combine(valueMutator, listMutator)); Foo foo = new Foo(0, singletonList(0)); Foo fooOther = new Foo(1, singletonList(1)); diff --git a/src/test/java/com/code_intelligence/jazzer/mutation/mutator/StressTest.java b/src/test/java/com/code_intelligence/jazzer/mutation/mutator/StressTest.java index 523d1cf57..2e0e5ce43 100644 --- a/src/test/java/com/code_intelligence/jazzer/mutation/mutator/StressTest.java +++ b/src/test/java/com/code_intelligence/jazzer/mutation/mutator/StressTest.java @@ -520,7 +520,7 @@ public static Stream protoStressTestCases() { "{Builder.Nullable Message |" + " Builder.{Builder.Nullable<(cycle) -> Message>} -> Message -> Message>} ->" + " Message", - false, + true, exactly( AnyField3.getDefaultInstance(), AnyField3.newBuilder() diff --git a/src/test/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorProto2Test.java b/src/test/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorProto2Test.java index b41744907..e227872df 100644 --- a/src/test/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorProto2Test.java +++ b/src/test/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorProto2Test.java @@ -50,6 +50,7 @@ void testPrimitiveField() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder.Nullable}"); + assertThat(mutator.hasFixedSize()).isTrue(); PrimitiveField2.Builder builder = PrimitiveField2.newBuilder(); @@ -103,6 +104,7 @@ void testRequiredPrimitiveField() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder.Boolean}"); + assertThat(mutator.hasFixedSize()).isTrue(); RequiredPrimitiveField2.Builder builder = RequiredPrimitiveField2.newBuilder(); @@ -124,6 +126,7 @@ void testRepeatedPrimitiveField() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder via List}"); + assertThat(mutator.hasFixedSize()).isFalse(); RepeatedPrimitiveField2.Builder builder = RepeatedPrimitiveField2.newBuilder(); @@ -175,6 +178,7 @@ void testMessageField() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder.Nullable<{Builder.Boolean} -> Message>}"); + assertThat(mutator.hasFixedSize()).isTrue(); MessageField2.Builder builder = MessageField2.newBuilder(); @@ -225,6 +229,7 @@ void testRepeatedOptionalMessageField() { RepeatedOptionalMessageField2.@NotNull Builder>() {}.annotatedType()); assertThat(mutator.toString()) .isEqualTo("{Builder via List<{Builder.Nullable} -> Message>}"); + assertThat(mutator.hasFixedSize()).isFalse(); RepeatedOptionalMessageField2.Builder builder = RepeatedOptionalMessageField2.newBuilder(); @@ -264,6 +269,7 @@ void testRepeatedRequiredMessageField() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder via List<{Builder.Boolean} -> Message>}"); + assertThat(mutator.hasFixedSize()).isFalse(); RepeatedMessageField2.Builder builder = RepeatedMessageField2.newBuilder(); @@ -328,6 +334,7 @@ void testRecursiveMessageField() { new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()) .isEqualTo("{Builder.Boolean, WithoutInit(Builder.Nullable<(cycle) -> Message>)}"); + assertThat(mutator.hasFixedSize()).isFalse(); RecursiveMessageField2.Builder builder = RecursiveMessageField2.newBuilder(); try (MockPseudoRandom prng = @@ -396,6 +403,7 @@ void testOneOfField2() { .isEqualTo( "{Builder.Boolean, Builder.Nullable, Builder.Nullable |" + " Builder.Nullable<{Builder.Boolean} -> Message>}"); + assertThat(mutator.hasFixedSize()).isTrue(); OneOfField2.Builder builder = OneOfField2.newBuilder(); try (MockPseudoRandom prng = diff --git a/src/test/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorProto3Test.java b/src/test/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorProto3Test.java index 67f940bea..25e117a67 100644 --- a/src/test/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorProto3Test.java +++ b/src/test/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorProto3Test.java @@ -63,6 +63,7 @@ void testPrimitiveField() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder.Boolean}"); + assertThat(mutator.hasFixedSize()).isTrue(); PrimitiveField3.Builder builder = PrimitiveField3.newBuilder(); @@ -84,6 +85,7 @@ void testEnumField() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder.Enum}"); + assertThat(mutator.hasFixedSize()).isTrue(); EnumField3.Builder builder = EnumField3.newBuilder(); try (MockPseudoRandom prng = mockPseudoRandom(0)) { mutator.initInPlace(builder, prng); @@ -102,6 +104,7 @@ void testEnumFieldOutside() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder.Enum}"); + assertThat(mutator.hasFixedSize()).isTrue(); EnumFieldOutside3.Builder builder = EnumFieldOutside3.newBuilder(); try (MockPseudoRandom prng = mockPseudoRandom(0)) { mutator.initInPlace(builder, prng); @@ -120,6 +123,7 @@ void testEnumFieldWithOneValue() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder.FixedValue(ONE)}"); + assertThat(mutator.hasFixedSize()).isTrue(); EnumFieldOne3.Builder builder = EnumFieldOne3.newBuilder(); try (MockPseudoRandom prng = mockPseudoRandom()) { mutator.initInPlace(builder, prng); @@ -138,6 +142,7 @@ void testRepeatedEnumField() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder via List>}"); + assertThat(mutator.hasFixedSize()).isFalse(); EnumFieldRepeated3.Builder builder = EnumFieldRepeated3.newBuilder(); try (MockPseudoRandom prng = mockPseudoRandom( @@ -173,6 +178,7 @@ void testOptionalPrimitiveField() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder.Nullable}"); + assertThat(mutator.hasFixedSize()).isTrue(); OptionalPrimitiveField3.Builder builder = OptionalPrimitiveField3.newBuilder(); @@ -226,6 +232,7 @@ void testRepeatedPrimitiveField() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder via List}"); + assertThat(mutator.hasFixedSize()).isFalse(); RepeatedPrimitiveField3.Builder builder = RepeatedPrimitiveField3.newBuilder(); @@ -277,6 +284,7 @@ void testMessageField() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder.Nullable<{Builder.Boolean} -> Message>}"); + assertThat(mutator.hasFixedSize()).isTrue(); MessageField3.Builder builder = MessageField3.newBuilder(); @@ -324,6 +332,7 @@ void testRepeatedMessageField() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{Builder via List<{Builder.Boolean} -> Message>}"); + assertThat(mutator.hasFixedSize()).isFalse(); RepeatedMessageField3.Builder builder = RepeatedMessageField3.newBuilder(); @@ -388,6 +397,7 @@ void testRecursiveMessageField() { new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()) .isEqualTo("{Builder.Boolean, WithoutInit(Builder.Nullable<(cycle) -> Message>)}"); + assertThat(mutator.hasFixedSize()).isFalse(); RecursiveMessageField3.Builder builder = RecursiveMessageField3.newBuilder(); try (MockPseudoRandom prng = @@ -456,6 +466,7 @@ void testOneOfField3() { .isEqualTo( "{Builder.Boolean, Builder.Boolean, Builder.Nullable |" + " Builder.Nullable<{Builder.Boolean} -> Message>}"); + assertThat(mutator.hasFixedSize()).isTrue(); OneOfField3.Builder builder = OneOfField3.newBuilder(); try (MockPseudoRandom prng = @@ -566,6 +577,7 @@ void testEmptyMessage3() { FACTORY.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()).isEqualTo("{}"); + assertThat(mutator.hasFixedSize()).isTrue(); EmptyMessage3.Builder builder = EmptyMessage3.newBuilder(); try (MockPseudoRandom prng = mockPseudoRandom()) { @@ -591,6 +603,7 @@ void testAnyField3() throws InvalidProtocolBufferException { .isEqualTo( "{Builder.Nullable Message |" + " Builder.{Builder.Nullable<(cycle) -> Message>} -> Message -> Message>}"); + assertThat(mutator.hasFixedSize()).isTrue(); AnyField3.Builder builder = AnyField3.newBuilder(); try (MockPseudoRandom prng =