Distinguish between undetected and failing custom mutator (fail on th…

…e latter).

I extended the IPC protocol for mutation via external binary: the binary now
first writes a marker to indicate whether it has a custom mutator. If it does,
it proceeds by writing mutants, otherwise it successfully exits.

On the Centipede side, this allows distinguishing between an undetected custom
mutator (indicated by the marker) and a failure (indicated by a failed
execution).

The protocol is implemented using a new type of runner result, `MutationResult`,
similarly to the `ExecutionResult` and `BatchResult`.

In addition, I cleaned up the `Mutate()` API so that the results are passed as
return values instead of using output parameters. This results in cleaner, less
error-prone, and more efficient code (e.g., the number of mallocs in the
malloc-counting test went down significantly, so I updated the expected count
there as well).

PiperOrigin-RevId: 731361992

centipede/BUILD

@@ -459,6 +459,7 @@ cc_library(
         ":feature",
         ":execution_metadata",
         ":shared_memory_blob_sequence",
+        "@com_google_fuzztest//common:defs",
     ],
 )
 
@@ -1468,6 +1469,7 @@ cc_test(
         ":feature",
         ":runner_result",
         ":shared_memory_blob_sequence",
+        "@com_google_fuzztest//common:defs",
         "@com_google_fuzztest//common:test_util",
         "@com_google_googletest//:gtest_main",
     ],
@@ -1829,6 +1831,7 @@ cc_test(
         "@com_google_fuzztest//centipede/testing:abort_fuzz_target",
         "@com_google_fuzztest//centipede/testing:expensive_startup_fuzz_target",
         "@com_google_fuzztest//centipede/testing:fuzz_target_with_config",
+        "@com_google_fuzztest//centipede/testing:fuzz_target_with_custom_mutator",
         "@com_google_fuzztest//centipede/testing:seeded_fuzz_target",
         "@com_google_fuzztest//centipede/testing:test_fuzz_target",
         "@com_google_fuzztest//centipede/testing:test_input_filter",

centipede/byte_array_mutator.cc

@@ -320,18 +320,18 @@ void ByteArrayMutator::CrossOver(ByteArray &data, const ByteArray &other) {
 // TODO(kcc): add tests with different values of knobs.
 const KnobId knob_mutate_or_crossover = Knobs::NewId("mutate_or_crossover");
 
-void ByteArrayMutator::MutateMany(const std::vector<MutationInputRef> &inputs,
-                                  size_t num_mutants,
-                                  std::vector<ByteArray> &mutants) {
+std::vector<ByteArray> ByteArrayMutator::MutateMany(
+    const std::vector<MutationInputRef> &inputs, size_t num_mutants) {
   if (inputs.empty()) abort();
   // TODO(xinhaoyuan): Consider metadata in other inputs instead of always the
   // first one.
   SetMetadata(inputs[0].metadata != nullptr ? *inputs[0].metadata
                                             : ExecutionMetadata());
   size_t num_inputs = inputs.size();
-  mutants.resize(num_mutants);
-  for (auto &mutant : mutants) {
-    mutant = inputs[rng_() % num_inputs].data;
+  std::vector<ByteArray> mutants;
+  mutants.reserve(num_mutants);
+  for (size_t i = 0; i < num_mutants; ++i) {
+    auto mutant = inputs[rng_() % num_inputs].data;
     if (mutant.size() <= max_len_ &&
         knobs_.GenerateBool(knob_mutate_or_crossover, rng_())) {
       // Do crossover only if the mutant is not over the max_len_.
@@ -342,7 +342,9 @@ void ByteArrayMutator::MutateMany(const std::vector<MutationInputRef> &inputs,
       // Perform mutation.
       Mutate(mutant);
     }
+    mutants.push_back(std::move(mutant));
   }
+  return mutants;
 }
 
 }  // namespace centipede

centipede/byte_array_mutator.h

@@ -107,10 +107,9 @@ class ByteArrayMutator {
     return cmp_dictionary_.SetFromMetadata(metadata);
   }
 
-  // Takes non-empty `inputs`, produces `num_mutants` mutations in `mutants`.
-  // Old contents of `mutants` are discarded.
-  void MutateMany(const std::vector<MutationInputRef> &inputs,
-                  size_t num_mutants, std::vector<ByteArray> &mutants);
+  // Takes non-empty `inputs` and produces `num_mutants` mutants.
+  std::vector<ByteArray> MutateMany(const std::vector<MutationInputRef> &inputs,
+                                    size_t num_mutants);
 
   using CrossOverFn = void (ByteArrayMutator::*)(ByteArray &,
                                                  const ByteArray &);

centipede/byte_array_mutator_test.cc

@@ -895,16 +895,16 @@ TEST(ByteArrayMutator, MutateManyWithAlignedInputs) {
   ByteArrayMutator mutator(knobs, /*seed=*/1);
   EXPECT_TRUE(mutator.set_size_alignment(kSizeAlignment));
   constexpr size_t kNumMutantsToGenerate = 10000;
-  std::vector<ByteArray> mutants;
 
   // If all inputs are aligned, expect all generated mutants to be aligned.
   const std::vector<ByteArray> aligned_inputs = {
       {0, 1, 2, 3},
       {0, 1, 2, 3, 4, 5, 6, 7},
       {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11},
   };
-  mutator.MutateMany(GetMutationInputRefsFromDataInputs(aligned_inputs),
-                     kNumMutantsToGenerate, mutants);
+  const std::vector<ByteArray> mutants =
+      mutator.MutateMany(GetMutationInputRefsFromDataInputs(aligned_inputs),
+                         kNumMutantsToGenerate);
   EXPECT_EQ(mutants.size(), kNumMutantsToGenerate);
   for (const ByteArray &mutant : mutants) {
     EXPECT_EQ(mutant.size() % kSizeAlignment, 0);
@@ -917,7 +917,6 @@ TEST(ByteArrayMutator, MutateManyWithUnalignedInputs) {
   ByteArrayMutator mutator(knobs, /*seed=*/1);
   EXPECT_TRUE(mutator.set_size_alignment(kSizeAlignment));
   constexpr size_t kNumMutantsToGenerate = 10000;
-  std::vector<ByteArray> mutants;
 
   // If there are unaligned inputs, most mutants should be aligned, but the ones
   // that are unaligned should be the same size as the unaligned inputs (as they
@@ -933,8 +932,9 @@ TEST(ByteArrayMutator, MutateManyWithUnalignedInputs) {
       {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
       {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10},
   };
-  mutator.MutateMany(GetMutationInputRefsFromDataInputs(unaligned_inputs),
-                     kNumMutantsToGenerate, mutants);
+  const std::vector<ByteArray> mutants =
+      mutator.MutateMany(GetMutationInputRefsFromDataInputs(unaligned_inputs),
+                         kNumMutantsToGenerate);
   EXPECT_EQ(mutants.size(), kNumMutantsToGenerate);
   for (const ByteArray &mutant : mutants) {
     if (mutant.size() % kSizeAlignment != 0) {
@@ -949,16 +949,15 @@ TEST(ByteArrayMutator, MutateManyWithMaxLen) {
   ByteArrayMutator mutator(knobs, /*seed=*/1);
   EXPECT_TRUE(mutator.set_max_len(kMaxLen));
   constexpr size_t kNumMutantsToGenerate = 10000;
-  std::vector<ByteArray> mutants;
 
   const std::vector<ByteArray> inputs = {
       {0},
       {0, 1},
       {0, 1, 2},
       {0, 1, 2, 3},
   };
-  mutator.MutateMany(GetMutationInputRefsFromDataInputs(inputs),
-                     kNumMutantsToGenerate, mutants);
+  const std::vector<ByteArray> mutants = mutator.MutateMany(
+      GetMutationInputRefsFromDataInputs(inputs), kNumMutantsToGenerate);
   EXPECT_EQ(mutants.size(), kNumMutantsToGenerate);
 
   for (const ByteArray &mutant : mutants) {
@@ -972,13 +971,12 @@ TEST(ByteArrayMutator, MutateManyWithMaxLenWithStartingLargeInput) {
   ByteArrayMutator mutator(knobs, /*seed=*/1);
   EXPECT_TRUE(mutator.set_max_len(kMaxLen));
   constexpr size_t kNumMutantsToGenerate = 10000;
-  std::vector<ByteArray> mutants;
 
   const std::vector<ByteArray> large_input = {
       {0, 1, 2, 3, 4, 5, 6, 7}, {0}, {0, 1}, {0, 1, 2}, {0, 1, 2, 3},
   };
-  mutator.MutateMany(GetMutationInputRefsFromDataInputs(large_input),
-                     kNumMutantsToGenerate, mutants);
+  const std::vector<ByteArray> mutants = mutator.MutateMany(
+      GetMutationInputRefsFromDataInputs(large_input), kNumMutantsToGenerate);
   EXPECT_EQ(mutants.size(), kNumMutantsToGenerate);
 
   for (const ByteArray &mutant : mutants) {

centipede/centipede.cc

@@ -769,7 +769,6 @@ void Centipede::FuzzingLoop() {
     auto remaining_runs = env_.num_runs - new_runs;
     auto batch_size = std::min(env_.batch_size, remaining_runs);
     std::vector<MutationInputRef> mutation_inputs;
-    std::vector<ByteArray> mutants;
     mutation_inputs.reserve(env_.mutate_batch_size);
     for (size_t i = 0; i < env_.mutate_batch_size; i++) {
       const auto &corpus_record = env_.use_corpus_weights
@@ -779,7 +778,8 @@ void Centipede::FuzzingLoop() {
           MutationInputRef{corpus_record.data, &corpus_record.metadata});
     }
 
-    user_callbacks_.Mutate(mutation_inputs, batch_size, mutants);
+    const std::vector<ByteArray> mutants =
+        user_callbacks_.Mutate(mutation_inputs, batch_size);
     bool gained_new_coverage =
         RunBatch(mutants, corpus_file.get(), features_file.get(), nullptr);
     new_runs += mutants.size();

centipede/centipede_callbacks.cc

@@ -336,9 +336,9 @@ bool CentipedeCallbacks::GetSerializedTargetConfigViaExternalBinary(
 }
 
 // See also: MutateInputsFromShmem().
-bool CentipedeCallbacks::MutateViaExternalBinary(
+MutationResult CentipedeCallbacks::MutateViaExternalBinary(
     std::string_view binary, const std::vector<MutationInputRef> &inputs,
-    std::vector<ByteArray> &mutants) {
+    size_t num_mutants) {
   CHECK(!env_.has_input_wildcards)
       << "Standalone binary does not support custom mutator";
 
@@ -347,7 +347,7 @@ bool CentipedeCallbacks::MutateViaExternalBinary(
   outputs_blobseq_.Reset();
 
   size_t num_inputs_written =
-      runner_request::RequestMutation(mutants.size(), inputs, inputs_blobseq_);
+      runner_request::RequestMutation(num_mutants, inputs, inputs_blobseq_);
   LOG_IF(INFO, num_inputs_written != inputs.size())
       << VV(num_inputs_written) << VV(inputs.size());
 
@@ -363,19 +363,13 @@ bool CentipedeCallbacks::MutateViaExternalBinary(
     PrintExecutionLog();
   }
 
-  // Read all mutants.
-  for (size_t i = 0; i < mutants.size(); ++i) {
-    auto blob = outputs_blobseq_.Read();
-    if (blob.size == 0) {
-      mutants.resize(i);
-      break;
-    }
-    mutants[i].assign(blob.data, blob.data + blob.size);
-  }
+  MutationResult result;
+  result.exit_code() = retval;
+  result.Read(num_mutants, outputs_blobseq_);
   outputs_blobseq_.ReleaseSharedMemory();  // Outputs are already consumed.
 
   VLOG(1) << __FUNCTION__ << " took " << (absl::Now() - start_time);
-  return retval == 0;
+  return result;
 }
 
 size_t CentipedeCallbacks::LoadDictionary(std::string_view dictionary_path) {

centipede/centipede_callbacks.h

@@ -69,14 +69,12 @@ class CentipedeCallbacks {
                        const std::vector<ByteArray> &inputs,
                        BatchResult &batch_result) = 0;
 
-  // Takes non-empty `inputs`, discards old contents of `mutants`,
-  // adds at least one and at most `num_mutants` mutated inputs to
-  // `mutants`.
-  virtual void Mutate(const std::vector<MutationInputRef> &inputs,
-                      size_t num_mutants, std::vector<ByteArray> &mutants) {
-    env_.use_legacy_default_mutator
-        ? byte_array_mutator_.MutateMany(inputs, num_mutants, mutants)
-        : fuzztest_mutator_.MutateMany(inputs, num_mutants, mutants);
+  // Takes non-empty `inputs` and returns at most `num_mutants` mutated inputs.
+  virtual std::vector<ByteArray> Mutate(
+      const std::vector<MutationInputRef> &inputs, size_t num_mutants) {
+    return env_.use_legacy_default_mutator
+               ? byte_array_mutator_.MutateMany(inputs, num_mutants)
+               : fuzztest_mutator_.MutateMany(inputs, num_mutants);
   }
 
   // Populates the BinaryInfo using the `symbolizer_path` and `coverage_binary`
@@ -142,17 +140,13 @@ class CentipedeCallbacks {
   // or implement the legacy Structure-Aware Fuzzing interface described here:
   // github.com/google/fuzzing/blob/master/docs/structure-aware-fuzzing.md
   //
-  // Produces at most `mutants.size()` non-empty mutants,
-  // replacing the existing elements of `mutants`,
-  // and shrinking `mutants` if needed.
-  //
-  // Returns true if the custom mutator in the binary is found and
-  // used, false otherwise. Note that mutants.size() may be 0 when
-  // returning true, if the mutator exists but refuses to mutate
-  // (hopefully occasionally).
-  bool MutateViaExternalBinary(std::string_view binary,
-                               const std::vector<MutationInputRef> &inputs,
-                               std::vector<ByteArray> &mutants);
+  // Returns a `MutationResult` instance where `exit_code` indicates whether
+  // the binary was executed successfully, `has_custom_mutator` indicates
+  // whether the binary has a custom mutator, and if it does, `mutants` contains
+  // at most `num_mutants` non-empty mutants.
+  MutationResult MutateViaExternalBinary(
+      std::string_view binary, const std::vector<MutationInputRef> &inputs,
+      size_t num_mutants);
 
   // Loads the dictionary from `dictionary_path`,
   // returns the number of dictionary entries loaded.

centipede/centipede_default_callbacks.cc

@@ -15,8 +15,10 @@
 #include "./centipede/centipede_default_callbacks.h"
 
 #include <cstddef>
+#include <cstdlib>
 #include <string>
 #include <string_view>
+#include <utility>
 #include <vector>
 
 #include "absl/log/check.h"
@@ -72,37 +74,45 @@ CentipedeDefaultCallbacks::GetSerializedTargetConfig() {
       "Failed to get serialized configuration from the target binary.");
 }
 
-void CentipedeDefaultCallbacks::Mutate(
-    const std::vector<MutationInputRef> &inputs, size_t num_mutants,
-    std::vector<ByteArray> &mutants) {
-  mutants.resize(num_mutants);
-  if (num_mutants == 0) return;
+std::vector<ByteArray> CentipedeDefaultCallbacks::Mutate(
+    const std::vector<MutationInputRef> &inputs, size_t num_mutants) {
+  if (num_mutants == 0) return {};
   // Try to use the custom mutator if it hasn't been disabled.
   if (custom_mutator_is_usable_.value_or(true)) {
-    if (MutateViaExternalBinary(env_.binary, inputs, mutants)) {
+    MutationResult result =
+        MutateViaExternalBinary(env_.binary, inputs, num_mutants);
+    if (result.exit_code() == EXIT_SUCCESS) {
       if (!custom_mutator_is_usable_.has_value()) {
-        LOG(INFO) << "Custom mutator detected: will use it";
-        custom_mutator_is_usable_ = true;
+        custom_mutator_is_usable_ = result.has_custom_mutator();
+        if (*custom_mutator_is_usable_) {
+          LOG(INFO) << "Custom mutator detected; will use it.";
+        } else {
+          LOG(INFO) << "Custom mutator not detected; falling back to the "
+                       "built-in mutator.";
+        }
+      }
+      if (*custom_mutator_is_usable_) {
+        // TODO(b/398261908): Exit with failure instead of crashing.
+        CHECK(result.has_custom_mutator())
+            << "Test binary no longer has a custom mutator, even though it was "
+               "previously detected.";
+        if (!result.mutants().empty()) return std::move(result).mutants();
+        LOG_FIRST_N(WARNING, 5) << "Custom mutator returned no mutants; will "
+                                   "generate some using the built-in mutator.";
       }
-      if (!mutants.empty()) return;
-      LOG_FIRST_N(WARNING, 5)
-          << "Custom mutator returned no mutants: falling back to internal "
-             "default mutator";
     } else if (ShouldStop()) {
       LOG(WARNING) << "Custom mutator failed, but ignored since the stop "
                       "condition it met. Possibly what triggered the stop "
                       "condition also interrupted the mutator.";
-      return;
+      // Returning whatever mutants we got before the failure.
+      return std::move(result).mutants();
     } else {
-      LOG(WARNING) << "Custom mutator undetected or misbehaving:";
-      CHECK(!custom_mutator_is_usable_.has_value())
-          << "Custom mutator is unreliable, aborting";
-      LOG(WARNING) << "Falling back to internal default mutator";
-      custom_mutator_is_usable_ = false;
+      // TODO(b/398261908): Exit with failure instead of crashing.
+      LOG(FATAL) << "Test binary failed when asked to mutate inputs.";
     }
   }
-  // Fallback of the internal mutator.
-  CentipedeCallbacks::Mutate(inputs, num_mutants, mutants);
+  // Fall back to the internal mutator.
+  return CentipedeCallbacks::Mutate(inputs, num_mutants);
 }
 
 }  // namespace centipede

centipede/centipede_default_callbacks.h

@@ -42,8 +42,8 @@ class CentipedeDefaultCallbacks : public CentipedeCallbacks {
   absl::StatusOr<std::string> GetSerializedTargetConfig() override;
   bool Execute(std::string_view binary, const std::vector<ByteArray> &inputs,
                BatchResult &batch_result) override;
-  void Mutate(const std::vector<MutationInputRef> &inputs, size_t num_mutants,
-              std::vector<ByteArray> &mutants) override;
+  std::vector<ByteArray> Mutate(const std::vector<MutationInputRef> &inputs,
+                                size_t num_mutants) override;
 
  private:
   std::optional<bool> custom_mutator_is_usable_ = std::nullopt;