Improve error handling when getting serialized configuration from tes…

…t binary. Instead of having two outcomes from `GetSerializedTargetConfig()` (the config is empty or non-empty), we now have three outcomes: - The config is non-empty, which means it should be de-serializable config from a FuzzTest binary. - The config is empty. - There was an error while obtaining the config from the binary. #Centipede PiperOrigin-RevId: 718431844
google · Jan 22, 2025 · ce1164d · ce1164d
1 parent 528033d
commit ce1164d
Show file tree

Hide file tree

Showing 10 changed files with 50 additions and 26 deletions.
diff --git a/centipede/BUILD b/centipede/BUILD
@@ -680,6 +680,7 @@ cc_library(
         "@com_google_absl//absl/base:nullability",
         "@com_google_absl//absl/log",
         "@com_google_absl//absl/log:check",
+        "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/time",
         "@com_google_fuzztest//common:blob_file",
@@ -827,6 +828,7 @@ cc_library(
         "@com_google_absl//absl/log",
         "@com_google_absl//absl/log:check",
         "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
         "@com_google_absl//absl/time",
@@ -916,6 +918,8 @@ cc_library(
         ":stop",
         "@com_google_absl//absl/log",
         "@com_google_absl//absl/log:check",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
         "@com_google_fuzztest//common:defs",
         "@com_google_fuzztest//common:logging",
     ],

diff --git a/centipede/centipede_callbacks.cc b/centipede/centipede_callbacks.cc
@@ -307,6 +307,9 @@ bool CentipedeCallbacks::GetSerializedTargetConfigViaExternalBinary(
       serialized_config = "";
     }
   }
+  if (env_.print_runner_log || !is_success) {
+    PrintExecutionLog();
+  }
   std::error_code error;
   std::filesystem::remove(config_file_path, error);
   CHECK(!error);

diff --git a/centipede/centipede_callbacks.h b/centipede/centipede_callbacks.h
@@ -23,6 +23,7 @@
 
 #include "absl/base/nullability.h"
 #include "absl/log/check.h"
+#include "absl/status/statusor.h"
 #include "./centipede/binary_info.h"
 #include "./centipede/byte_array_mutator.h"
 #include "./centipede/command.h"
@@ -93,7 +94,7 @@ class CentipedeCallbacks {
 
   // Returns the configuration from the test target in the serialized form.
   // Returns an empty string if the test target doesn't provide configuration.
-  virtual std::string GetSerializedTargetConfig() { return ""; }
+  virtual absl::StatusOr<std::string> GetSerializedTargetConfig() { return ""; }
 
  protected:
   // Helpers that the user-defined class may use if needed.

diff --git a/centipede/centipede_default_callbacks.cc b/centipede/centipede_default_callbacks.cc
@@ -21,6 +21,8 @@
 
 #include "absl/log/check.h"
 #include "absl/log/log.h"
+#include "absl/status/status.h"
+#include "absl/status/statusor.h"
 #include "./centipede/centipede_callbacks.h"
 #include "./centipede/environment.h"
 #include "./centipede/mutation_input.h"
@@ -53,19 +55,21 @@ bool CentipedeDefaultCallbacks::Execute(std::string_view binary,
 size_t CentipedeDefaultCallbacks::GetSeeds(size_t num_seeds,
                                            std::vector<ByteArray> &seeds) {
   seeds.resize(num_seeds);
-  if (GetSeedsViaExternalBinary(env_.binary, num_seeds, seeds))
+  if (GetSeedsViaExternalBinary(env_.binary, num_seeds, seeds)) {
     return num_seeds;
-  else
-    return CentipedeCallbacks::GetSeeds(num_seeds, seeds);
+  }
+  return CentipedeCallbacks::GetSeeds(num_seeds, seeds);
 }
 
-std::string CentipedeDefaultCallbacks::GetSerializedTargetConfig() {
+absl::StatusOr<std::string>
+CentipedeDefaultCallbacks::GetSerializedTargetConfig() {
   std::string serialized_target_config;
   if (GetSerializedTargetConfigViaExternalBinary(env_.binary,
                                                  serialized_target_config)) {
     return serialized_target_config;
   }
-  return CentipedeCallbacks::GetSerializedTargetConfig();
+  return absl::InternalError(
+      "Failed to get serialized configuration from the target binary.");
 }
 
 void CentipedeDefaultCallbacks::Mutate(

diff --git a/centipede/centipede_default_callbacks.h b/centipede/centipede_default_callbacks.h
@@ -25,6 +25,7 @@
 #include <string_view>
 #include <vector>
 
+#include "absl/status/statusor.h"
 #include "./centipede/centipede_callbacks.h"
 #include "./centipede/environment.h"
 #include "./centipede/mutation_input.h"
@@ -38,7 +39,7 @@ class CentipedeDefaultCallbacks : public CentipedeCallbacks {
  public:
   explicit CentipedeDefaultCallbacks(const Environment &env);
   size_t GetSeeds(size_t num_seeds, std::vector<ByteArray> &seeds) override;
-  std::string GetSerializedTargetConfig() override;
+  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,

diff --git a/centipede/centipede_interface.cc b/centipede/centipede_interface.cc
@@ -35,6 +35,7 @@
 #include "absl/log/check.h"
 #include "absl/log/log.h"
 #include "absl/status/status.h"
+#include "absl/status/statusor.h"
 #include "absl/strings/ascii.h"
 #include "absl/strings/numbers.h"
 #include "absl/strings/str_cat.h"
@@ -708,14 +709,17 @@ int CentipedeMain(const Environment &env,
 
   // Enter the update corpus database mode only if we have a binary to invoke
   // and a corpus database to update.
-  if (!env.binary.empty()) {
-    const std::string serialized_target_config = [&] {
+  // We don't update the corpus database for standalone binaries (i.e., when
+  // `env.has_input_wildcards` is true).
+  if (!env.binary.empty() && !env.has_input_wildcards) {
+    const absl::StatusOr<std::string> serialized_target_config = [&] {
       ScopedCentipedeCallbacks scoped_callbacks(callbacks_factory, env);
       return scoped_callbacks.callbacks()->GetSerializedTargetConfig();
     }();
-    if (!serialized_target_config.empty()) {
+    CHECK_OK(serialized_target_config.status());
+    if (!serialized_target_config->empty()) {
       const auto target_config = fuzztest::internal::Configuration::Deserialize(
-          serialized_target_config);
+          *serialized_target_config);
       CHECK_OK(target_config.status())
           << "Failed to deserialize target configuration";
       if (!target_config->corpus_database.empty()) {
@@ -729,11 +733,6 @@ int CentipedeMain(const Environment &env,
         return UpdateCorpusDatabaseForFuzzTests(env, *target_config,
                                                 callbacks_factory);
       }
-    } else if (std::getenv("CENTIPEDE_NO_FUZZ_IF_NO_CONFIG") != nullptr) {
-      // TODO(fniksic): Improve the GetSerializedTargetConfig interface to avoid
-      // using the environment variable.
-      LOG(INFO) << "Failed to get target config!";
-      return EXIT_FAILURE;
     }
   }
 

diff --git a/centipede/centipede_test.cc b/centipede/centipede_test.cc
@@ -628,14 +628,6 @@ TEST(Centipede, FunctionFilter) {
   }
 }
 
-TEST(Centipede, SkipsFuzzingWhenNoFuzzIfNoConfigIsSet) {
-  TempDir tmp_dir{test_info_->name(), "no_config"};
-  setenv("CENTIPEDE_NO_FUZZ_IF_NO_CONFIG", "true", /*replace=*/1);
-  auto observed_no_config = RunWithFunctionFilter("", tmp_dir);
-  EXPECT_TRUE(observed_no_config.empty());
-  unsetenv("CENTIPEDE_NO_FUZZ_IF_NO_CONFIG");
-}
-
 namespace {
 
 struct Crash {
@@ -908,7 +900,20 @@ TEST(Centipede, GetsSerializedTargetConfig) {
   env.binary =
       GetDataDependencyFilepath("centipede/testing/fuzz_target_with_config");
   CentipedeDefaultCallbacks callbacks(env);
-  EXPECT_EQ(callbacks.GetSerializedTargetConfig(), "fake serialized config");
+  const auto serialized_config = callbacks.GetSerializedTargetConfig();
+  ASSERT_TRUE(serialized_config.ok());
+  EXPECT_EQ(*serialized_config, "fake serialized config");
+}
+
+TEST(Centipede, GetSerializedTargetConfigProducesFailure) {
+  Environment env;
+  env.binary = absl::StrCat(
+      GetDataDependencyFilepath("centipede/testing/fuzz_target_with_config")
+          .c_str(),
+      " --simulate_failure");
+  CentipedeDefaultCallbacks callbacks(env);
+  const auto serialized_config = callbacks.GetSerializedTargetConfig();
+  EXPECT_FALSE(serialized_config.ok());
 }
 
 TEST(Centipede, CleansUpMetadataAfterStartup) {

diff --git a/centipede/testing/fuzz_target_with_config.cc b/centipede/testing/fuzz_target_with_config.cc
@@ -13,6 +13,8 @@
 // limitations under the License.
 
 #include <cstddef>
+#include <cstdlib>
+#include <cstring>
 #include <functional>
 #include <string>
 #include <vector>
@@ -40,6 +42,9 @@ class FakeSerializedConfigRunnerCallbacks : public centipede::RunnerCallbacks {
 };
 
 int main(int argc, absl::Nonnull<char **> argv) {
+  if (argc >= 2 && std::strcmp(argv[1], "--simulate_failure") == 0) {
+    return EXIT_FAILURE;
+  }
   FakeSerializedConfigRunnerCallbacks runner_callbacks;
   return centipede::RunnerMain(argc, argv, runner_callbacks);
 }
diff --git a/fuzztest/BUILD b/fuzztest/BUILD
@@ -224,6 +224,7 @@ cc_library(
         "@com_google_absl//absl/memory",
         "@com_google_absl//absl/random",
         "@com_google_absl//absl/random:distributions",
+        "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
         "@com_google_absl//absl/strings:string_view",

diff --git a/fuzztest/internal/centipede_adaptor.cc b/fuzztest/internal/centipede_adaptor.cc
@@ -41,6 +41,7 @@
 #include "absl/memory/memory.h"
 #include "absl/random/distributions.h"
 #include "absl/random/random.h"
+#include "absl/status/statusor.h"
 #include "absl/strings/match.h"
 #include "absl/strings/numbers.h"
 #include "absl/strings/str_cat.h"
@@ -381,7 +382,7 @@ class CentipedeAdaptorEngineCallbacks : public centipede::CentipedeCallbacks {
     return num_avail_seeds;
   }
 
-  std::string GetSerializedTargetConfig() override {
+  absl::StatusOr<std::string> GetSerializedTargetConfig() override {
     return runner_callbacks_.GetSerializedTargetConfig();
   }