Expose WriteFailureDescription instead of passing it to OnFailure. #Centipede

This enables recording failures before RunnerMain (where OnFailure would be called), and simplifies the ownership/lifetime reasoning for OnFailure handler.

PiperOrigin-RevId: 753705766

Author: xinhaoyuan

centipede/runner.cc

@@ -100,31 +100,6 @@ GlobalRunnerState state __attribute__((init_priority(200)));
 // This avoids calls to __tls_init() in hot functions that use `tls`.
 __thread ThreadLocalRunnerState tls;
 
-// Tries to write `description` to `state.failure_description_path`.
-static void WriteFailureDescription(const char *description) {
-  // TODO(b/264715830): Remove I/O error logging once the bug is fixed?
-  if (state.failure_description_path == nullptr) return;
-  // Make sure that the write is atomic and only happens once.
-  [[maybe_unused]] static int write_once = [=] {
-    FILE *f = fopen(state.failure_description_path, "w");
-    if (f == nullptr) {
-      perror("FAILURE: fopen()");
-      return 0;
-    }
-    const auto len = strlen(description);
-    if (fwrite(description, 1, len, f) != len) {
-      perror("FAILURE: fwrite()");
-    }
-    if (fflush(f) != 0) {
-      perror("FAILURE: fflush()");
-    }
-    if (fclose(f) != 0) {
-      perror("FAILURE: fclose()");
-    }
-    return 0;
-  }();
-}
-
 void ThreadLocalRunnerState::TraceMemCmp(uintptr_t caller_pc, const uint8_t *s1,
                                          const uint8_t *s2, size_t n,
                                          bool is_equal) {
@@ -279,7 +254,7 @@ static void CheckWatchdogLimits() {
             "https://github.com/google/fuzztest/tree/main/doc/flags-reference.md"
             "\n",
             resource.what, resource.limit, resource.units, resource.value);
-        WriteFailureDescription(resource.failure);
+        CentipedeSetFailureDescription(resource.failure);
         std::abort();
       }
     }
@@ -315,7 +290,7 @@ __attribute__((noinline)) void CheckStackLimit(uintptr_t sp) {
             " > %zu"
             " (byte); aborting\n",
             tls.top_frame_sp - sp, stack_limit);
-    fuzztest::internal::WriteFailureDescription(
+    CentipedeSetFailureDescription(
         fuzztest::internal::kExecutionFailureStackLimitExceeded.data());
     std::abort();
   }
@@ -595,9 +570,6 @@ bool RunnerCallbacks::Mutate(
   return true;
 }
 
-void RunnerCallbacks::OnFailure(
-    std::function<void(std::string_view)> /*failure_description_callback*/) {}
-
 class LegacyRunnerCallbacks : public RunnerCallbacks {
  public:
   LegacyRunnerCallbacks(FuzzerTestOneInputCallback test_one_input_cb,
@@ -1171,10 +1143,6 @@ int RunnerMain(int argc, char **argv, RunnerCallbacks &callbacks) {
     return EXIT_SUCCESS;
   }
 
-  callbacks.OnFailure([](std::string_view failure_description) {
-    WriteFailureDescription(std::string(failure_description).c_str());
-  });
-
   // Inputs / outputs from shmem.
   if (state.HasFlag(":shmem:")) {
     if (!state.arg1 || !state.arg2) return EXIT_FAILURE;
@@ -1319,3 +1287,27 @@ extern "C" void CentipedeSetExecutionResult(const uint8_t *data, size_t size) {
       state.execution_result_override->num_outputs_read() == 1,
       "Failed to set execution result from CentipedeSetExecutionResult");
 }
+
+extern "C" void CentipedeSetFailureDescription(const char *description) {
+  using fuzztest::internal::state;
+  if (state.failure_description_path == nullptr) return;
+  // Make sure that the write is atomic and only happens once.
+  [[maybe_unused]] static int write_once = [=] {
+    FILE *f = fopen(state.failure_description_path, "w");
+    if (f == nullptr) {
+      perror("FAILURE: fopen()");
+      return 0;
+    }
+    const auto len = strlen(description);
+    if (fwrite(description, 1, len, f) != len) {
+      perror("FAILURE: fwrite()");
+    }
+    if (fflush(f) != 0) {
+      perror("FAILURE: fflush()");
+    }
+    if (fclose(f) != 0) {
+      perror("FAILURE: fclose()");
+    }
+    return 0;
+  }();
+}

centipede/runner_interface.h

@@ -126,6 +126,10 @@ extern "C" size_t CentipedeGetCoverageData(uint8_t *data, size_t capacity);
 // "empty" with no features or metadata.
 extern "C" void CentipedeSetExecutionResult(const uint8_t *data, size_t size);
 
+// Set the failure description for the runner to propagate further. Only the
+// description from the first call will be used.
+extern "C" void CentipedeSetFailureDescription(const char *description);
+
 namespace fuzztest::internal {
 
 // Callbacks interface implemented by the fuzzer and called by the runner.
@@ -153,11 +157,6 @@ class RunnerCallbacks {
   virtual bool Mutate(const std::vector<MutationInputRef> &inputs,
                       size_t num_mutants,
                       std::function<void(ByteSpan)> new_mutant_callback);
-  // Registers a function to be called when a failure happens. If the
-  // implementation supports this functionality, it will call the function with
-  // a description of the failure. Otherwise, it will do nothing.
-  virtual void OnFailure(
-      std::function<void(std::string_view)> failure_description_callback);
   virtual ~RunnerCallbacks() = default;
 };
 

fuzztest/internal/centipede_adaptor.cc

@@ -453,29 +453,6 @@ class CentipedeAdaptorRunnerCallbacks
     return configuration_.Serialize();
   }
 
-  void OnFailure(std::function<void(std::string_view)>
-                     failure_description_callback) override {
-    // We register the callback only once. This is because `runtime_` is a
-    // global singleton object, and hence previously registered callbacks remain
-    // in the registry. In normal circumstances, there should be only one
-    // runner callback object and a single call to this method, but there are
-    // corner cases when multiple runner callback objects are created, e.g.,
-    // when Centipede runs multiple fuzz tests in the multi-process mode.
-    [[maybe_unused]] static bool callback_registered =
-        [this, failure_description_callback =
-                   std::move(failure_description_callback)]() mutable {
-          runtime_.RegisterCrashMetadataListener(
-              [failure_description_callback =
-                   std::move(failure_description_callback)](
-                  absl::string_view crash_type,
-                  absl::Span<const std::string> /*stack_frames*/) {
-                failure_description_callback(
-                    {crash_type.data(), crash_type.size()});
-              });
-          return true;
-        }();
-  }
-
   bool HasCustomMutator() const override { return true; }
 
   bool Mutate(const std::vector<fuzztest::internal::MutationInputRef>& inputs,
@@ -739,6 +716,13 @@ bool CentipedeFuzzerAdaptor::Run(int* argc, char*** argv, RunMode mode,
     // enabled in the controller mode to handle test setup failures.
     runtime_.EnableReporter(&fuzzer_impl_.stats_, [] { return absl::Now(); });
   }
+  if (runner_mode) {
+    runtime_.RegisterCrashMetadataListener(
+        [](absl::string_view crash_type,
+           absl::Span<const std::string> /*stack_frames*/) {
+          CentipedeSetFailureDescription(std::string{crash_type}.c_str());
+        });
+  }
   if (!configuration.corpus_database.empty() &&
       configuration.crashing_input_to_reproduce.has_value() &&
       configuration.replay_in_single_process) {