From fbc26c0d2a6b87f648ad2daa497d5719ca1447e2 Mon Sep 17 00:00:00 2001 From: Simon Resch Date: Thu, 23 Oct 2025 16:05:55 +0200 Subject: [PATCH] fix: fuzz test invocation with referring protobufs When constructing a mutator for a protobuf message the mutator is cached based on the protobuf descriptor. This led to issues for fuzz tests that take two protobuf messages where one message is referring to the other. While building the mutator for the top level message nested messages are constructed with the type `DynamicMessage` which lead to 'argument type mismatch' exceptions when invoking the fuzz test method. This is fixed by including the message class in the mutator cache key. --- .../mutation/ArgumentsMutatorFuzzTest.java | 9 ++- .../mutator/proto/BuilderMutatorFactory.java | 27 ++++---- .../jazzer/mutation/ArgumentsMutatorTest.java | 25 ++++++++ .../jazzer/mutation/mutator/StressTest.java | 61 +++++++++++++------ .../proto/BuilderMutatorProto2Test.java | 4 +- .../proto/BuilderMutatorProto3Test.java | 7 ++- 6 files changed, 95 insertions(+), 38 deletions(-) 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();