fix: fuzz test invocation with referring protobufs

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

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<Any.Builder> 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<? extends Message> messageClass;
 
-    private CacheKey(Descriptor descriptor, AnySource anySource) {
+    private CacheKey(
+        Descriptor descriptor, AnySource anySource, Class<? extends Message> 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);
     }
   }
 }

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<ArgumentsMutator> maybeMutator =
+        ArgumentsMutator.forMethod(Mutators.newFactory(), method);
+    assertThat(maybeMutator).isPresent();
+    ArgumentsMutator mutator = maybeMutator.get();
+    mutator.init(12345);
+    mutator.invoke(null, false);
+  }
 }

src/test/java/com/code_intelligence/jazzer/mutation/mutator/StressTest.java

@@ -928,7 +928,8 @@ public static Stream<Arguments> 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<Arguments> protoStressTestCases() {
                 + " Builder.Nullable<Double>, Builder.Nullable<String>,"
                 + " Builder.Nullable<Enum<Enum>>,"
                 + " WithoutInit(Builder.Nullable<{Builder.Nullable<Integer>, Builder via"
-                + " List<Integer>, WithoutInit(Builder.Nullable<(cycle) -> Message>)} -> Message>),"
-                + " Builder via List<Boolean>, Builder via List<Integer>, Builder via"
-                + " List<Integer>, Builder via List<Long>, Builder via List<Long>, Builder via"
-                + " List<Float>, Builder via List<Double>, Builder via List<String>, Builder via"
-                + " List<Enum<Enum>>, WithoutInit(Builder via List<(cycle) -> Message>),"
-                + " Builder.Map<Integer, Integer>, Builder.Nullable<FixedValue(OnlyLabel)>,"
-                + " Builder.Nullable<{<empty>} -> Message>, Builder.Nullable<Integer> |"
-                + " Builder.Nullable<Long> | Builder.Nullable<Integer>} -> Message",
+                + " List<Integer>, WithoutInit(Builder.Nullable<{Builder.Nullable<Boolean>,"
+                + " Builder.Nullable<Integer>, Builder.Nullable<Integer>, Builder.Nullable<Long>,"
+                + " Builder.Nullable<Long>, Builder.Nullable<Float>, Builder.Nullable<Double>,"
+                + " Builder.Nullable<String>, Builder.Nullable<Enum<Enum>>,"
+                + " WithoutInit(Builder.Nullable<(cycle) -> Message>), Builder via List<Boolean>,"
+                + " Builder via List<Integer>, Builder via List<Integer>, Builder via List<Long>,"
+                + " Builder via List<Long>, Builder via List<Float>, Builder via List<Double>,"
+                + " Builder via List<String>, Builder via List<Enum<Enum>>, WithoutInit(Builder via"
+                + " List<(cycle) -> Message>), Builder.Map<Integer, Integer>,"
+                + " Builder.Nullable<FixedValue(OnlyLabel)>, Builder.Nullable<{<empty>} ->"
+                + " Message>, Builder.Nullable<Integer> | Builder.Nullable<Long> |"
+                + " Builder.Nullable<Integer>} -> Message>)} -> Message>), Builder via"
+                + " List<Boolean>, Builder via List<Integer>, Builder via List<Integer>, Builder"
+                + " via List<Long>, Builder via List<Long>, Builder via List<Float>, Builder via"
+                + " List<Double>, Builder via List<String>, Builder via List<Enum<Enum>>,"
+                + " WithoutInit(Builder via List<(cycle) -> Message>), Builder.Map<Integer,"
+                + " Integer>, Builder.Nullable<FixedValue(OnlyLabel)>, Builder.Nullable<(cycle) ->"
+                + " Message>, Builder.Nullable<Integer> | Builder.Nullable<Long> |"
+                + " Builder.Nullable<Integer>} -> Message",
             false,
             manyDistinctElements(),
             manyDistinctElements()),
@@ -1063,14 +1075,25 @@ public static Stream<Arguments> protoStressTestCases() {
                 + " Builder.Nullable<Double>, Builder.Nullable<String>,"
                 + " Builder.Nullable<Enum<Enum>>,"
                 + " WithoutInit(Builder.Nullable<{Builder.Nullable<Integer>, Builder via"
-                + " List<Integer>, WithoutInit(Builder.Nullable<(cycle) -> Message>)} -> Message>),"
-                + " Builder via List<Boolean>, Builder via List<Integer>, Builder via"
-                + " List<Integer>, Builder via List<Long>, Builder via List<Long>, Builder via"
-                + " List<Float>, Builder via List<Double>, Builder via List<String>, Builder via"
-                + " List<Enum<Enum>>, WithoutInit(Builder via List<(cycle) -> Message>),"
-                + " Builder.Map<Integer, Integer>, Builder.Nullable<FixedValue(OnlyLabel)>,"
-                + " Builder.Nullable<{<empty>} -> Message>, Builder.Nullable<Integer> |"
-                + " Builder.Nullable<Long> | Builder.Nullable<Integer>} -> Message",
+                + " List<Integer>, WithoutInit(Builder.Nullable<{Builder.Nullable<Boolean>,"
+                + " Builder.Nullable<Integer>, Builder.Nullable<Integer>, Builder.Nullable<Long>,"
+                + " Builder.Nullable<Long>, Builder.Nullable<Float>, Builder.Nullable<Double>,"
+                + " Builder.Nullable<String>, Builder.Nullable<Enum<Enum>>,"
+                + " WithoutInit(Builder.Nullable<(cycle) -> Message>), Builder via List<Boolean>,"
+                + " Builder via List<Integer>, Builder via List<Integer>, Builder via List<Long>,"
+                + " Builder via List<Long>, Builder via List<Float>, Builder via List<Double>,"
+                + " Builder via List<String>, Builder via List<Enum<Enum>>, WithoutInit(Builder via"
+                + " List<(cycle) -> Message>), Builder.Map<Integer, Integer>,"
+                + " Builder.Nullable<FixedValue(OnlyLabel)>, Builder.Nullable<{<empty>} ->"
+                + " Message>, Builder.Nullable<Integer> | Builder.Nullable<Long> |"
+                + " Builder.Nullable<Integer>} -> Message>)} -> Message>), Builder via"
+                + " List<Boolean>, Builder via List<Integer>, Builder via List<Integer>, Builder"
+                + " via List<Long>, Builder via List<Long>, Builder via List<Float>, Builder via"
+                + " List<Double>, Builder via List<String>, Builder via List<Enum<Enum>>,"
+                + " WithoutInit(Builder via List<(cycle) -> Message>), Builder.Map<Integer,"
+                + " Integer>, Builder.Nullable<FixedValue(OnlyLabel)>, Builder.Nullable<(cycle) ->"
+                + " Message>, Builder.Nullable<Integer> | Builder.Nullable<Long> |"
+                + " Builder.Nullable<Integer>} -> Message",
             false,
             manyDistinctElements(),
             manyDistinctElements()),
@@ -1079,8 +1102,8 @@ public static Stream<Arguments> protoStressTestCases() {
                 @NotNull @AnySource({PrimitiveField3.class, MessageField3.class})
                 AnyField3>() {}.annotatedType(),
             "{Builder.Nullable<Builder.{Builder.Boolean} -> Message |"
-                + " Builder.{Builder.Nullable<(cycle) -> Message>} -> Message -> Message>} ->"
-                + " Message",
+                + " Builder.{Builder.Nullable<{Builder.Boolean} -> Message>} -> Message ->"
+                + " Message>} -> Message",
             true,
             exactly(
                 AnyField3.getDefaultInstance(),

src/test/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorProto2Test.java

@@ -336,7 +336,9 @@ void testRecursiveMessageField() {
             factory.createInPlaceOrThrow(
                 new TypeHolder<RecursiveMessageField2.@NotNull Builder>() {}.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();
 

src/test/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorProto3Test.java

@@ -403,7 +403,9 @@ void testRecursiveMessageField() {
             factory.createInPlaceOrThrow(
                 new TypeHolder<RecursiveMessageField3.@NotNull Builder>() {}.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<Builder.{Builder.Boolean} -> 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();