diff --git a/selffuzz/src/test/java/com/code_intelligence/selffuzz/mutation/ArgumentsMutatorFuzzTest.java b/selffuzz/src/test/java/com/code_intelligence/selffuzz/mutation/ArgumentsMutatorFuzzTest.java index 68d3b9d55..41164949e 100644 --- a/selffuzz/src/test/java/com/code_intelligence/selffuzz/mutation/ArgumentsMutatorFuzzTest.java +++ b/selffuzz/src/test/java/com/code_intelligence/selffuzz/mutation/ArgumentsMutatorFuzzTest.java @@ -22,6 +22,7 @@ import com.code_intelligence.jazzer.junit.FuzzTest; import com.code_intelligence.jazzer.mutation.annotation.WithSize; import com.code_intelligence.jazzer.mutation.annotation.WithUtf8Length; +import com.code_intelligence.jazzer.protobuf.Proto2; import com.code_intelligence.jazzer.protobuf.Proto3; import com.code_intelligence.selffuzz.jazzer.mutation.ArgumentsMutator; import com.code_intelligence.selffuzz.jazzer.mutation.annotation.NotNull; @@ -246,11 +247,9 @@ void fuzz_ProtoBufsNotNull( @SelfFuzzTest void fuzz_MapField3(Proto3.MapField3 o1) {} - // BUG: causes java.lang.IllegalArgumentException: argument type mismatch - // no problem when testing the two types separately - // @SelfFuzzTest - // public static void fuzz_MutuallyReferringProtobufs( - // Proto2.TestProtobuf o1, Proto2.TestSubProtobuf o2) {} + @SelfFuzzTest + public static void fuzz_MutuallyReferringProtobufs( + Proto2.TestProtobuf o1, Proto2.TestSubProtobuf o2) {} /** * @return all methods in this class annotated by @SelfFuzzTest. If any of those methods are 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 60fda5c9c..33914d694 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 @@ -199,18 +199,15 @@ private ExtendedMutatorFactory withDescriptorDependentMutatorFactoryIfNeeded( .flatMap( clazz -> { // BuilderMutatorFactory only handles concrete subclasses of - // Message.Builder - // and requests Message.Builder itself for message fields, which we - // handle - // here. + // Message.Builder and requests Message.Builder itself for message + // fields, which we handle here. if (clazz != Message.Builder.class) { return Optional.empty(); } // It is important that we use originalFactory here instead of factory: // factory has this field-specific message mutator appended, but this // mutator should only be used for this particular field and not any - // message - // subfields. + // message subfields. return Optional.of( makeBuilderMutator( type, @@ -352,7 +349,8 @@ private SerializingMutator mutatorForAny( .collect(toMap(i -> getTypeUrl(getDefaultInstance(anySource.value()[i])), identity())); return assemble( - mutator -> internedMutators.put(new CacheKey(Any.getDescriptor(), anySource), mutator), + mutator -> + internedMutators.put(new CacheKey(Any.getDescriptor(), anySource, Any.class), mutator), Any.getDefaultInstance()::toBuilder, makeBuilderSerializer(Any.getDefaultInstance()), () -> @@ -442,7 +440,7 @@ private SerializingMutator makeBuilderMutator( "@AnySource must list a non-empty list of classes"); Descriptor descriptor = defaultInstance.getDescriptorForType(); - CacheKey cacheKey = new CacheKey(descriptor, anySource); + CacheKey cacheKey = new CacheKey(descriptor, anySource, defaultInstance.getClass()); if (internedMutators.containsKey(cacheKey)) { return internedMutators.get(cacheKey); } @@ -494,10 +492,13 @@ private SerializingMutator makeBuilderMutator( private static final class CacheKey { private final Descriptor descriptor; private final AnySource anySource; + private final Class messageClass; - private CacheKey(Descriptor descriptor, AnySource anySource) { + private CacheKey( + Descriptor descriptor, AnySource anySource, Class messageClass) { this.descriptor = descriptor; this.anySource = anySource; + this.messageClass = messageClass; } @Override @@ -509,12 +510,16 @@ public boolean equals(Object o) { return false; } CacheKey cacheKey = (CacheKey) o; - return descriptor == cacheKey.descriptor && Objects.equals(anySource, cacheKey.anySource); + return descriptor == cacheKey.descriptor + && Objects.equals(anySource, cacheKey.anySource) + && Objects.equals(messageClass, cacheKey.messageClass); } @Override public int hashCode() { - return 31 * System.identityHashCode(descriptor) + Objects.hashCode(anySource); + return 31 * System.identityHashCode(descriptor) + + Objects.hashCode(anySource) + + Objects.hashCode(messageClass); } } } diff --git a/src/test/java/com/code_intelligence/jazzer/mutation/ArgumentsMutatorTest.java b/src/test/java/com/code_intelligence/jazzer/mutation/ArgumentsMutatorTest.java index fe9cd895e..a46ddf0cd 100644 --- a/src/test/java/com/code_intelligence/jazzer/mutation/ArgumentsMutatorTest.java +++ b/src/test/java/com/code_intelligence/jazzer/mutation/ArgumentsMutatorTest.java @@ -23,6 +23,7 @@ import com.code_intelligence.jazzer.mutation.annotation.NotNull; import com.code_intelligence.jazzer.mutation.mutator.Mutators; import com.code_intelligence.jazzer.mutation.support.TestSupport.MockPseudoRandom; +import com.code_intelligence.jazzer.protobuf.Proto2; import com.code_intelligence.jazzer.protobuf.Proto2.TestProtobuf; import com.google.protobuf.DescriptorProtos; import java.io.ByteArrayInputStream; @@ -371,4 +372,28 @@ void testProtoAndDescriptorParameters() throws NoSuchMethodException { // because "productMutator" is null assertThat(maybeMutator.toString()).contains("-> Message"); } + + // Regression: ensure that the mutator can invoke a method with two protobuf messages where one + // message is a submessage of the other. This would fail with an "argument type mismatch" error + // because the created Mutator for `proto2.TestProtobuf` inserted a mutator for the type + // `DynamicMessage` with proto descriptor for `Proto2.TestSubProtobuf` into the + // `BuilderMutatorsFactory.internedMutators` cache. This cache was then used for the second + // argument in this function leading to a `DynamicMessage != Proto2.TestSubProtobuf` argument type + // mismatch error. + @SuppressWarnings("unused") + public static void protoWithSubmessage( + @NotNull Proto2.TestProtobuf proto, @NotNull Proto2.TestSubProtobuf subproto) {} + + @Test + void testProtoWithSubmessage() throws Throwable { + Method method = + ArgumentsMutatorTest.class.getMethod( + "protoWithSubmessage", Proto2.TestProtobuf.class, Proto2.TestSubProtobuf.class); + Optional maybeMutator = + ArgumentsMutator.forMethod(Mutators.newFactory(), method); + assertThat(maybeMutator).isPresent(); + ArgumentsMutator mutator = maybeMutator.get(); + mutator.init(12345); + mutator.invoke(null, false); + } } 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 5c59ed8ad..091879654 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 @@ -928,7 +928,8 @@ public static Stream protoStressTestCases() { OptionalPrimitiveField3.newBuilder().setSomeField(true).build())), arguments( new TypeHolder<@NotNull RepeatedRecursiveMessageField3>() {}.annotatedType(), - "{Builder.Boolean, WithoutInit(Builder via List<(cycle) -> Message>)} -> Message", + "{Builder.Boolean, WithoutInit(Builder via List<{Builder.Boolean, WithoutInit(Builder" + + " via List<(cycle) -> Message>)} -> Message>)} -> Message", false, // The message field is recursive and thus not initialized. exactly( @@ -1041,14 +1042,25 @@ public static Stream protoStressTestCases() { + " Builder.Nullable, Builder.Nullable," + " Builder.Nullable>," + " WithoutInit(Builder.Nullable<{Builder.Nullable, Builder via" - + " List, WithoutInit(Builder.Nullable<(cycle) -> Message>)} -> Message>)," - + " Builder via List, Builder via List, Builder via" - + " List, Builder via List, Builder via List, Builder via" - + " List, Builder via List, Builder via List, Builder via" - + " List>, WithoutInit(Builder via List<(cycle) -> Message>)," - + " Builder.Map, Builder.Nullable," - + " Builder.Nullable<{} -> Message>, Builder.Nullable |" - + " Builder.Nullable | Builder.Nullable} -> Message", + + " List, WithoutInit(Builder.Nullable<{Builder.Nullable," + + " Builder.Nullable, Builder.Nullable, Builder.Nullable," + + " Builder.Nullable, Builder.Nullable, Builder.Nullable," + + " Builder.Nullable, Builder.Nullable>," + + " WithoutInit(Builder.Nullable<(cycle) -> Message>), Builder via List," + + " Builder via List, Builder via List, Builder via List," + + " Builder via List, Builder via List, Builder via List," + + " Builder via List, Builder via List>, WithoutInit(Builder via" + + " List<(cycle) -> Message>), Builder.Map," + + " Builder.Nullable, Builder.Nullable<{} ->" + + " Message>, Builder.Nullable | Builder.Nullable |" + + " Builder.Nullable} -> Message>)} -> Message>), Builder via" + + " List, Builder via List, Builder via List, Builder" + + " via List, Builder via List, Builder via List, Builder via" + + " List, Builder via List, Builder via List>," + + " WithoutInit(Builder via List<(cycle) -> Message>), Builder.Map, Builder.Nullable, Builder.Nullable<(cycle) ->" + + " Message>, Builder.Nullable | Builder.Nullable |" + + " Builder.Nullable} -> Message", false, manyDistinctElements(), manyDistinctElements()), @@ -1063,14 +1075,25 @@ public static Stream protoStressTestCases() { + " Builder.Nullable, Builder.Nullable," + " Builder.Nullable>," + " WithoutInit(Builder.Nullable<{Builder.Nullable, Builder via" - + " List, WithoutInit(Builder.Nullable<(cycle) -> Message>)} -> Message>)," - + " Builder via List, Builder via List, Builder via" - + " List, Builder via List, Builder via List, Builder via" - + " List, Builder via List, Builder via List, Builder via" - + " List>, WithoutInit(Builder via List<(cycle) -> Message>)," - + " Builder.Map, Builder.Nullable," - + " Builder.Nullable<{} -> Message>, Builder.Nullable |" - + " Builder.Nullable | Builder.Nullable} -> Message", + + " List, WithoutInit(Builder.Nullable<{Builder.Nullable," + + " Builder.Nullable, Builder.Nullable, Builder.Nullable," + + " Builder.Nullable, Builder.Nullable, Builder.Nullable," + + " Builder.Nullable, Builder.Nullable>," + + " WithoutInit(Builder.Nullable<(cycle) -> Message>), Builder via List," + + " Builder via List, Builder via List, Builder via List," + + " Builder via List, Builder via List, Builder via List," + + " Builder via List, Builder via List>, WithoutInit(Builder via" + + " List<(cycle) -> Message>), Builder.Map," + + " Builder.Nullable, Builder.Nullable<{} ->" + + " Message>, Builder.Nullable | Builder.Nullable |" + + " Builder.Nullable} -> Message>)} -> Message>), Builder via" + + " List, Builder via List, Builder via List, Builder" + + " via List, Builder via List, Builder via List, Builder via" + + " List, Builder via List, Builder via List>," + + " WithoutInit(Builder via List<(cycle) -> Message>), Builder.Map, Builder.Nullable, Builder.Nullable<(cycle) ->" + + " Message>, Builder.Nullable | Builder.Nullable |" + + " Builder.Nullable} -> Message", false, manyDistinctElements(), manyDistinctElements()), @@ -1079,8 +1102,8 @@ public static Stream protoStressTestCases() { @NotNull @AnySource({PrimitiveField3.class, MessageField3.class}) AnyField3>() {}.annotatedType(), "{Builder.Nullable Message |" - + " Builder.{Builder.Nullable<(cycle) -> Message>} -> Message -> Message>} ->" - + " Message", + + " Builder.{Builder.Nullable<{Builder.Boolean} -> Message>} -> Message ->" + + " Message>} -> Message", true, exactly( AnyField3.getDefaultInstance(), 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 9ffe9cca4..fca9ade26 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 @@ -336,7 +336,9 @@ void testRecursiveMessageField() { factory.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()) - .isEqualTo("{Builder.Boolean, WithoutInit(Builder.Nullable<(cycle) -> Message>)}"); + .isEqualTo( + "{Builder.Boolean, WithoutInit(Builder.Nullable<{Builder.Boolean," + + " WithoutInit(Builder.Nullable<(cycle) -> Message>)} -> Message>)}"); assertThat(mutator.hasFixedSize()).isFalse(); RecursiveMessageField2.Builder builder = RecursiveMessageField2.newBuilder(); 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 6e4b082dd..93e032bae 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 @@ -403,7 +403,9 @@ void testRecursiveMessageField() { factory.createInPlaceOrThrow( new TypeHolder() {}.annotatedType()); assertThat(mutator.toString()) - .isEqualTo("{Builder.Boolean, WithoutInit(Builder.Nullable<(cycle) -> Message>)}"); + .isEqualTo( + "{Builder.Boolean, WithoutInit(Builder.Nullable<{Builder.Boolean," + + " WithoutInit(Builder.Nullable<(cycle) -> Message>)} -> Message>)}"); assertThat(mutator.hasFixedSize()).isFalse(); RecursiveMessageField3.Builder builder = RecursiveMessageField3.newBuilder(); @@ -609,7 +611,8 @@ void testAnyField3() throws InvalidProtocolBufferException { assertThat(mutator.toString()) .isEqualTo( "{Builder.Nullable Message |" - + " Builder.{Builder.Nullable<(cycle) -> Message>} -> Message -> Message>}"); + + " Builder.{Builder.Nullable<{Builder.Boolean} -> Message>} -> Message ->" + + " Message>}"); assertThat(mutator.hasFixedSize()).isTrue(); AnyField3.Builder builder = AnyField3.newBuilder();