From d0dbfe784f9e5bfa44d8b8e631471dd052d2acc4 Mon Sep 17 00:00:00 2001 From: Xinhao Yuan Date: Fri, 10 Jul 2026 07:23:53 -0700 Subject: [PATCH] Always perform exec in command. This avoids the shell process between the parent and the actual command when running without the fork server. Otherwise when running without fork server but in persistent mode, cleaning up the execution may end up getting the exit status of the shell process instead of the command process. PiperOrigin-RevId: 945700512 --- centipede/BUILD | 2 - centipede/centipede_callbacks.cc | 23 +- centipede/centipede_test.cc | 42 +-- centipede/command.cc | 12 +- centipede/command_test.cc | 8 +- centipede/engine_worker.cc | 283 ++++++++++++++++-- centipede/puzzles/run_puzzle.sh | 1 - centipede/runner.cc | 24 +- centipede/runner.h | 6 - .../testing/test_binary_for_engine_testing.cc | 8 +- 10 files changed, 313 insertions(+), 96 deletions(-) diff --git a/centipede/BUILD b/centipede/BUILD index 607811f8d..683b2afde 100644 --- a/centipede/BUILD +++ b/centipede/BUILD @@ -991,8 +991,6 @@ cc_library( ":runner_result", ":shared_memory_blob_sequence", "@abseil-cpp//absl/base:nullability", - "@abseil-cpp//absl/random", - "@abseil-cpp//absl/random:bit_gen_ref", "@com_google_fuzztest//common:defs", ], ) diff --git a/centipede/centipede_callbacks.cc b/centipede/centipede_callbacks.cc index b03b48aad..aae910449 100644 --- a/centipede/centipede_callbacks.cc +++ b/centipede/centipede_callbacks.cc @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -194,8 +195,11 @@ class CentipedeCallbacks::PersistentModeServer { int poll_ret = -1; do { poll_fd = {fd, static_cast(event)}; - const int poll_timeout_ms = static_cast(absl::ToInt64Milliseconds( - std::max(deadline - absl::Now(), kPollMinimalTimeout))); + const int poll_timeout_ms = + deadline == absl::InfiniteFuture() + ? -1 + : static_cast(std::ceil(absl::ToDoubleMilliseconds( + std::max(deadline - absl::Now(), kPollMinimalTimeout)))); poll_ret = poll(&poll_fd, 1, poll_timeout_ms); } while (poll_ret < 0 && errno == EINTR); if (poll_ret == 1 && (poll_fd.revents & (event | POLLHUP)) == event) { @@ -343,7 +347,6 @@ std::string CentipedeCallbacks::ConstructRunnerFlags( std::vector flags = { "CENTIPEDE_RUNNER_FLAGS=", absl::StrCat("timeout_per_input=", env_.timeout_per_input), - absl::StrCat("timeout_per_batch=", env_.timeout_per_batch), absl::StrCat("address_space_limit_mb=", env_.address_space_limit_mb), absl::StrCat("rss_limit_mb=", env_.rss_limit_mb), absl::StrCat("stack_limit_kb=", env_.stack_limit_kb), @@ -565,8 +568,12 @@ int CentipedeCallbacks::ExecuteCentipedeSancovBinaryWithShmem( } // Run. + const auto batch_start_time = absl::Now(); const int exit_code = RunBatchForBinary(binary); inputs_blobseq_.ReleaseSharedMemory(); // Inputs are already consumed. + const bool batch_timed_out = + env_.timeout_per_batch > 0 && + absl::Now() - batch_start_time > absl::Seconds(env_.timeout_per_batch); // Get results. batch_result.exit_code() = exit_code; @@ -591,6 +598,16 @@ int CentipedeCallbacks::ExecuteCentipedeSancovBinaryWithShmem( if (env_.print_runner_log) PrintExecutionLog(); + if (batch_timed_out) { + FUZZTEST_LOG(INFO) + << "Batch timeout detected. Replacing the original exit code: " + << exit_code + << ", failure description: " << batch_result.failure_description(); + batch_result.failure_description() = kExecutionFailurePerBatchTimeout; + batch_result.exit_code() = EXIT_FAILURE; + return EXIT_FAILURE; + } + // TODO: b/467103298 - Handle failures when the exit code is zero, e.g., when // the target exits via `std::_Exit(0)`. if (exit_code != EXIT_SUCCESS) { diff --git a/centipede/centipede_test.cc b/centipede/centipede_test.cc index 17383e8f3..37e0854d2 100644 --- a/centipede/centipede_test.cc +++ b/centipede/centipede_test.cc @@ -86,7 +86,7 @@ class CentipedeMock : public CentipedeCallbacks { // i-th element is the number of bytes with the value 'i' in the input. // `counters` is converted to FeatureVec and added to // `batch_result.results()`. - for (auto &input : inputs) { + for (auto& input : inputs) { ByteArray counters(256); for (uint8_t byte : input) { counters[byte]++; @@ -496,7 +496,7 @@ TEST_F(CentipedeWithTemporaryLocalDir, MutateViaExternalBinary) { // Must contain normal mutants, but not the ones from crossover. const auto mutant_data = GetDataFromMutants(result.mutants()); EXPECT_THAT(mutant_data, IsSupersetOf(some_of_expected_mutants)); - for (const auto &crossover_mutant : expected_crossover_mutants) { + for (const auto& crossover_mutant : expected_crossover_mutants) { EXPECT_THAT(mutant_data, Not(Contains(crossover_mutant))); } } @@ -525,7 +525,7 @@ class MergeMock : public CentipedeCallbacks { std::vector Mutate(absl::Span inputs, size_t num_mutants) override { std::vector mutants(num_mutants); - for (auto &mutant : mutants) { + for (auto& mutant : mutants) { mutant.data.resize(1); mutant.data[0] = ++number_of_mutations_; mutant.origin = Mutant::kOriginNone; @@ -611,7 +611,7 @@ class FunctionFilterMock : public CentipedeCallbacks { // Sets the inputs to one of 3 pre-defined values. std::vector Mutate(absl::Span inputs, size_t num_mutants) override { - for (auto &input : inputs) { + for (auto& input : inputs) { if (!seed_inputs_.contains(input.data)) { observed_inputs_.insert(input.data); } @@ -628,8 +628,8 @@ class FunctionFilterMock : public CentipedeCallbacks { // Returns one of 3 pre-defined values, that trigger different code paths in // the test target. static ByteArray GetMutant(size_t idx) { - const char *mutants[3] = {"func1", "func2-A", "foo"}; - const char *mutant = mutants[idx % 3]; + const char* mutants[3] = {"func1", "func2-A", "foo"}; + const char* mutant = mutants[idx % 3]; return {mutant, mutant + strlen(mutant)}; } @@ -646,7 +646,7 @@ class FunctionFilterMock : public CentipedeCallbacks { // Runs a short fuzzing session with the provided `function_filter`. // Returns a sorted array of observed inputs. static std::vector RunWithFunctionFilter( - std::string_view function_filter, const TempDir &tmp_dir) { + std::string_view function_filter, const TempDir& tmp_dir) { Environment env; env.workdir = tmp_dir.path(); env.seed = 1; // make the runs predictable. @@ -717,9 +717,9 @@ class ExtraBinariesMock : public CentipedeCallbacks { bool Execute(std::string_view binary, absl::Span inputs, BatchResult& batch_result) override { bool res = true; - for (const auto &input : inputs) { + for (const auto& input : inputs) { if (input.size() != 1) continue; - for (const Crash &crash : crashes_) { + for (const Crash& crash : crashes_) { if (binary == crash.binary && input[0] == crash.input) { batch_result.exit_code() = EXIT_FAILURE; batch_result.failure_description() = crash.description; @@ -736,7 +736,7 @@ class ExtraBinariesMock : public CentipedeCallbacks { std::vector Mutate(absl::Span inputs, size_t num_mutants) override { std::vector mutants(num_mutants); - for (auto &mutant : mutants) { + for (auto& mutant : mutants) { mutant.data.resize(1); mutant.data[0] = ++number_of_mutations_; mutant.origin = Mutant::kOriginNone; @@ -754,20 +754,20 @@ struct FileAndContents { std::string file; std::string contents; - bool operator==(const FileAndContents &other) const { + bool operator==(const FileAndContents& other) const { return file == other.file && contents == other.contents; } template - friend void AbslStringify(Sink &sink, const FileAndContents &f) { + friend void AbslStringify(Sink& sink, const FileAndContents& f) { absl::Format(&sink, "FileAndContents{%s, \"%s\"}", f.file, f.contents); } }; MATCHER_P(HasFilesWithContents, expected_files_and_contents, "") { - const std::string &dir_path = arg; + const std::string& dir_path = arg; std::vector files_and_contents; - for (const auto &dir_ent : std::filesystem::directory_iterator(dir_path)) { + for (const auto& dir_ent : std::filesystem::directory_iterator(dir_path)) { auto file_and_contents = FileAndContents{dir_ent.path().filename()}; ReadFromLocalFile(dir_ent.path().c_str(), file_and_contents.contents); files_and_contents.push_back(std::move(file_and_contents)); @@ -843,7 +843,7 @@ class UndetectedCrashingInputMock : public CentipedeCallbacks { if (!first_pass_) { num_inputs_triaged_ += inputs.size(); } - for (const auto &input : inputs) { + for (const auto& input : inputs) { FUZZTEST_CHECK_EQ(input.size(), 1); // By construction in `Mutate()`. // The contents of each mutant is its sequential number. if (input[0] == crashing_input_idx_) { @@ -933,7 +933,7 @@ TEST(Centipede, UndetectedCrashingInput) { absl::StrCat("crashing_batch-", crashing_input_hash); EXPECT_TRUE(std::filesystem::exists(crashes_dir_path)) << crashes_dir_path; std::vector found_crash_file_names; - for (auto const &dir_ent : + for (auto const& dir_ent : std::filesystem::directory_iterator(crashes_dir_path)) { found_crash_file_names.push_back(dir_ent.path().filename()); } @@ -1400,14 +1400,14 @@ TEST_F(CentipedeWithTemporaryLocalDir, EngineWorksInWorkerMode) { env.test_name = "some_test"; env.populate_binary_info = false; env.fork_server = false; - env.persistent_mode = false; + env.persistent_mode = true; env.exit_on_crash = true; + env.stop_at = absl::Now() + absl::Seconds(10); fuzztest::internal::DefaultCallbacksFactory< fuzztest::internal::CentipedeDefaultCallbacks> callbacks; - EXPECT_DEATH( - [&] { std::exit(CentipedeMain(env, callbacks)); }(), - ContainsRegex("Failure *: INPUT FAILURE: some_failure_description")); + EXPECT_DEATH([&] { std::exit(CentipedeMain(env, callbacks)); }(), + ContainsRegex("Failure *: some_failure_description")); } TEST_F(CentipedeWithTemporaryLocalDir, EngineWorksInStandaloneMode) { @@ -1427,7 +1427,7 @@ TEST_F(CentipedeWithTemporaryLocalDir, EngineWorksInStandaloneMode) { const int status = std::system(test_command.c_str()); std::exit(WEXITSTATUS(status)); }(), - ContainsRegex("Failure *: INPUT FAILURE: some_failure_description")); + ContainsRegex("Failure *: some_failure_description")); } } // namespace diff --git a/centipede/command.cc b/centipede/command.cc index 4afab0abb..308839468 100644 --- a/centipede/command.cc +++ b/centipede/command.cc @@ -30,6 +30,7 @@ #include #include +#include #include #include #include @@ -146,8 +147,11 @@ struct Command::ForkServerProps { /*fd=*/pipe_[1], /*events=*/POLLIN, }; - const int poll_timeout_ms = static_cast(absl::ToInt64Milliseconds( - std::max(deadline - absl::Now(), absl::Milliseconds(1)))); + const int poll_timeout_ms = + deadline == absl::InfiniteFuture() + ? -1 + : static_cast(std::ceil(absl::ToDoubleMilliseconds( + std::max(deadline - absl::Now(), absl::Milliseconds(1))))); poll_ret = poll(&poll_fd, 1, poll_timeout_ms); // The `poll()` syscall can get interrupted: it sets errno==EINTR in that // case. We should tolerate that. @@ -207,7 +211,7 @@ std::string Command::ToString() const { ss.reserve(/*env*/ 1 + options_.env_diff.size() + /*path*/ 1 + /*args*/ options_.args.size() + /*out/err*/ 2); // env. - ss.push_back("env"); + ss.push_back("exec env"); std::vector env_to_set; env_to_set.reserve(options_.env_diff.size()); // Arguments that unset environment variables must appear first. @@ -287,7 +291,7 @@ bool Command::StartForkServer(std::string_view temp_dir_path, { CENTIPEDE_FORK_SERVER_FIFO0="%s" \ CENTIPEDE_FORK_SERVER_FIFO1="%s" \ - exec %s + %s } & printf "%%s" $! > "%s" )sh"; diff --git a/centipede/command_test.cc b/centipede/command_test.cc index 4e638b4b2..48f4d0730 100644 --- a/centipede/command_test.cc +++ b/centipede/command_test.cc @@ -39,18 +39,18 @@ namespace { using ::testing::Optional; TEST(CommandTest, ToString) { - EXPECT_EQ(Command{"x"}.ToString(), "env \\\nx"); + EXPECT_EQ(Command{"x"}.ToString(), "exec env \\\nx"); { Command::Options cmd_options; cmd_options.args = {"arg1", "arg2"}; EXPECT_EQ((Command{"path", std::move(cmd_options)}.ToString()), - "env \\\npath \\\narg1 \\\narg2"); + "exec env \\\npath \\\narg1 \\\narg2"); } { Command::Options cmd_options; cmd_options.env_diff = {"K1=V1", "K2=V2", "-K3"}; EXPECT_EQ((Command{"x", std::move(cmd_options)}.ToString()), - "env \\\n-u K3 \\\nK1=V1 \\\nK2=V2 \\\nx"); + "exec env \\\n-u K3 \\\nK1=V1 \\\nK2=V2 \\\nx"); } } @@ -80,7 +80,7 @@ TEST(CommandTest, InputFileWildCard) { Command::Options cmd_options; cmd_options.temp_file_path = "TEMP_FILE"; Command cmd{"foo bar @@ baz", std::move(cmd_options)}; - EXPECT_EQ(cmd.ToString(), "env \\\nfoo bar TEMP_FILE baz"); + EXPECT_EQ(cmd.ToString(), "exec env \\\nfoo bar TEMP_FILE baz"); } TEST(CommandTest, ForkServer) { diff --git a/centipede/engine_worker.cc b/centipede/engine_worker.cc index 01030d1eb..f5e653607 100644 --- a/centipede/engine_worker.cc +++ b/centipede/engine_worker.cc @@ -12,6 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. #include +#include +#include +#include +#include #include #include @@ -23,14 +27,13 @@ #include #include #include +#include #include #include #include #include #include "absl/base/nullability.h" -#include "absl/random/bit_gen_ref.h" -#include "absl/random/random.h" #include "./centipede/engine_abi.h" #include "./centipede/engine_worker_abi.h" #include "./centipede/execution_metadata.h" @@ -105,7 +108,7 @@ struct WorkerFlags { }; // The first call of this function must be outside of signal handlers since it -// allocates memory (enforced by `GetWorkerFlagsEarly`). After that it would be +// allocates memory (enforced by `WorkerInitEarly`). After that it would be // signal-safe. // // The worker flags format is `:(NAME=VALUE|SWITCH:)+`. `GetWorkerFlags` @@ -137,10 +140,6 @@ const WorkerFlags& GetWorkerFlags() { return worker_flags; } -__attribute__((constructor(200))) void GetWorkerFlagsEarly() { - (void)GetWorkerFlags(); -} - // `header` should be in the form of `FLAG_NAME=`. // // Extracts "value" as a null-terminated string from "\0FLAG_NAME=value\0" in @@ -220,6 +219,7 @@ enum class WorkerAction { kTestGetSeeds, kTestMutate, kTestExecute, + kNoOp, }; constexpr std::string_view kWorkerBinaryIdOutputFlagHeader = @@ -237,6 +237,10 @@ constexpr std::string_view kWorkerInputsBlobSequencePathFlagHeader = "arg1="; // TODO: Use better flag names when standardizing the protocol. constexpr std::string_view kWorkerOutputsBlobSequencePathFlagHeader = "arg2="; // TODO: Use better flag names when standardizing the protocol. +constexpr std::string_view kWorkerPersistentModeSocketPathFlagHeader = + "persistent_mode_socket="; // TODO: Use better flag names when + // standardizing the protocol. +constexpr std::string_view kWorkerCrossOverLevel = "crossover_level="; struct WorkerState { std::atomic has_failure_output = false; @@ -244,6 +248,11 @@ struct WorkerState { std::atomic in_adapter_execute = false; std::atomic has_finding = false; std::atomic saved_binary_id = false; + + void ResetForPersistentMode() { + has_failure_output.store(false, std::memory_order_relaxed); + has_finding.store(false, std::memory_order_relaxed); + } }; WorkerState& GetWorkerState() { @@ -290,14 +299,27 @@ void WorkerEmitError(std::string_view message) { WorkerEmitFailureOutput("SETUP FAILURE: ", message); } +// TODO(xinhaoyuan): Some of them are planned to be removed. +static constexpr std::array kNonFindingPrefixes = { + "SETUP FAILURE:", + "IGNORED FAILURE:", + "SKIPPED TEST:", +}; + void WorkerEmitFinding(std::string_view description, std::string_view signature) { WorkerCheck( GetWorkerState().in_adapter_execute.load(std::memory_order_relaxed), "Must emit finding in adapter execute"); + for (auto bad_prefix : kNonFindingPrefixes) { + if (description.substr(0, bad_prefix.size()) == bad_prefix) { + WorkerEmitError("Bad prefix in the error description"); + return; + } + } const bool ignored = GetWorkerState().has_finding.exchange(true); if (!ignored) { - WorkerCheck(WorkerEmitFailureOutput("INPUT FAILURE: ", description), + WorkerCheck(WorkerEmitFailureOutput("", description), "Failed to emit failure output for the finding"); if (const char* finding_signature_path = GetWorkerFlag(kWorkerFailureSignaturePathFlagHeader); @@ -323,6 +345,64 @@ inline std::string_view ToStringView(const std::vector& bytes) { return {reinterpret_cast(bytes.data()), bytes.size()}; } +// Zero initialized. +static int persistent_mode_socket; + +__attribute__((constructor(200))) void WorkerInitEarly() { + const char* persistent_mode_socket_path = + GetWorkerFlag(kWorkerPersistentModeSocketPathFlagHeader); + if (persistent_mode_socket_path == nullptr) return; + persistent_mode_socket = socket(AF_UNIX, SOCK_STREAM, 0); + if (persistent_mode_socket < 0) { + WorkerLog( + "Failed to create persistent mode socket - not running persistent " + "mode.", + LogLnSync{}); + return; + } + + struct sockaddr_un addr{}; + addr.sun_family = AF_UNIX; + const size_t socket_path_len = strlen(persistent_mode_socket_path); + WorkerCheck( + socket_path_len < sizeof(addr.sun_path), + "persistent mode socket path string must be fit in sockaddr_un.sun_path"); + std::memcpy(addr.sun_path, persistent_mode_socket_path, socket_path_len); + + int connect_ret = 0; + do { + connect_ret = + connect(persistent_mode_socket, (struct sockaddr*)&addr, sizeof(addr)); + } while (connect_ret == -1 && errno == EINTR); + if (connect_ret == -1) { + WorkerLog("Failed to connect the persistent mode socket to ", + persistent_mode_socket_path, LogLnSync{}); + (void)close(persistent_mode_socket); + persistent_mode_socket = -1; + } + + int flags = fcntl(persistent_mode_socket, F_GETFD); + if (flags == -1) { + WorkerLog( + "fcntl(F_GETFD) failed on the persistent mode socket - exiting " + "persistent mode", + LogLnSync{}); + (void)close(persistent_mode_socket); + persistent_mode_socket = -1; + } + flags |= FD_CLOEXEC; + if (fcntl(persistent_mode_socket, F_SETFD, flags) == -1) { + WorkerLog( + "fcntl(F_SETFD) failed on the persistent mode socket - exiting " + "persistent mode", + LogLnSync{}); + (void)close(persistent_mode_socket); + persistent_mode_socket = -1; + } + WorkerLog("Persistent mode: connected to ", persistent_mode_socket_path, + LogLnSync{}); +} + BlobSequence* GetInputsBlobSequence() { static auto result = []() -> BlobSequence* { if (!HasWorkerSwitchFlag("shmem")) { @@ -349,8 +429,25 @@ BlobSequence* GetOutputsBlobSequence() { return result; } -WorkerAction GetWorkerAction() { - static WorkerAction worker_action = [] { +int GetCrossOverLevel() { + static int result = []() { + const char* cross_over_level_str = GetWorkerFlag(kWorkerCrossOverLevel); + if (cross_over_level_str != nullptr) { + const int parsed = + atoi(cross_over_level_str); // NOLINT: can't use strto64, etc. + if (0 <= parsed && parsed <= 100) return parsed; + } + // Default + return 50; + }(); + return result; +} + +std::optional GetWorkerAction() { + static auto worker_action = []() -> std::optional { + if (HasWorkerSwitchFlag("dump_configuration")) { + return WorkerAction::kNoOp; + } if (HasWorkerSwitchFlag("dump_binary_id")) { return WorkerAction::kGetBinaryId; } @@ -361,7 +458,9 @@ WorkerAction GetWorkerAction() { return WorkerAction::kTestGetSeeds; } auto* inputs_blobseq = GetInputsBlobSequence(); - WorkerCheck(inputs_blobseq != nullptr, "input blob sequence is not found"); + if (inputs_blobseq == nullptr) { + return std::nullopt; + } auto request_type_blob = inputs_blobseq->Read(); if (IsMutationRequest(request_type_blob)) { inputs_blobseq->Reset(); @@ -371,9 +470,7 @@ WorkerAction GetWorkerAction() { inputs_blobseq->Reset(); return WorkerAction::kTestExecute; } - WorkerCheck(false, "unknown worker action from the flags"); - // should not reach here. - std::abort(); + return std::nullopt; }(); return worker_action; } @@ -457,14 +554,6 @@ void WorkerDoGetSeeds(const FuzzTestAdapter& adapter) { } } -absl::BitGenRef GetBitGen() { - static thread_local std::unique_ptr bitgen; - if (bitgen == nullptr) { - bitgen = std::make_unique(); - } - return *bitgen; -} - void WorkerDoMutate(const FuzzTestAdapter& adapter) { auto* inputs_blobseq = GetInputsBlobSequence(); auto* outputs_blobseq = GetOutputsBlobSequence(); @@ -516,15 +605,25 @@ void WorkerDoMutate(const FuzzTestAdapter& adapter) { std::vector mutant_bytes; const auto mutant_bytes_sink = GetBytesSinkTo(mutant_bytes); + static unsigned int seed = 0; for (size_t i = 0; i < num_mutants; ++i) { - const auto origin = - absl::Uniform(GetBitGen(), 0, origin_inputs.size()); + // Assume RAND_MAX is large enough and not worry about fairness for now. + const auto origin = rand_r(&seed) % origin_inputs.size(); emitted_inputs.clear(); - adapter.Mutate(adapter.ctx, origin_inputs[origin], /*shrink=*/0, - &input_sink); + const bool do_cross_over = adapter.CrossOver != nullptr && + rand_r(&seed) % 100 < GetCrossOverLevel(); + if (do_cross_over) { + const auto other = rand_r(&seed) % origin_inputs.size(); + adapter.CrossOver(adapter.ctx, origin_inputs[origin], + origin_inputs[other], &input_sink); + } else { + adapter.Mutate(adapter.ctx, origin_inputs[origin], /*shrink=*/0, + &input_sink); + } WORKER_CHECK_FOR_ERROR(); WorkerCheck(emitted_inputs.size() == 1, - "Mutate must emit exactly one input"); + do_cross_over ? "CrossOver must emit exactly one input" + : "Mutate must emit exactly one input"); mutant_bytes.clear(); adapter.SerializeInputContent(adapter.ctx, emitted_inputs[0], &mutant_bytes_sink); @@ -596,6 +695,32 @@ void WorkerDoExecute(const FuzzTestAdapter& adapter) { return true; }(); + // Utils to get execution stats. + + size_t last_time_usec = 0; + auto UsecSinceLast = [&last_time_usec]() { + struct timeval tv = {}; + constexpr size_t kUsecInSec = 1000000; + gettimeofday(&tv, nullptr); + uint64_t t = tv.tv_sec * kUsecInSec + tv.tv_usec; + uint64_t ret_val = t - last_time_usec; + last_time_usec = t; + return ret_val; + }; + + auto GetPeakRSSMb = []() -> size_t { + struct rusage usage = {}; + if (getrusage(RUSAGE_SELF, &usage) != 0) return 0; +#ifdef __APPLE__ + // On MacOS, the unit seems to be byte according to experiment, while some + // documents mentioned KiB. This could depend on OS variants. + return usage.ru_maxrss >> 20; +#else // __APPLE__ + // On Linux, ru_maxrss is in KiB + return usage.ru_maxrss >> 10; +#endif // __APPLE__ + }; + // In-loop variables declared outside to save allocations. std::vector features; std::vector serialized_metadata; @@ -603,6 +728,16 @@ void WorkerDoExecute(const FuzzTestAdapter& adapter) { const auto input_sink = GetInputSinkTo(emitted_inputs); for (size_t i = 0; i < num_inputs; i++) { + // The stats gathered here are slightly different than + // the original definition: Here the exec_time_usec will cover the time used + // to process coverage feedback, which is usually fine for the purpose of + // corpus scheduling, but coverage processing can be non-deterministic due + // to optimization. E.g. Execute() may perform deep and slow cleanup on + // the beginning of consecutive Execute() calls to reduce noise. + // + // TODO(xinhaoyuan): Consider improving it. E.g. let user override stats. + ExecutionResult::Stats stats = {}; + UsecSinceLast(); auto blob = inputs_blobseq->Read(); if (!blob.IsValid()) return; // no more blobs to read. WorkerCheck(IsDataInput(blob), "Must read data input"); @@ -631,7 +766,9 @@ void WorkerDoExecute(const FuzzTestAdapter& adapter) { }}; GetWorkerState().in_adapter_execute = true; + stats.prep_time_usec = UsecSinceLast(); adapter.Execute(adapter.ctx, input, &feedback_sink); + stats.exec_time_usec = UsecSinceLast(); GetWorkerState().in_adapter_execute = false; WORKER_CHECK_FOR_ERROR(); @@ -681,6 +818,12 @@ void WorkerDoExecute(const FuzzTestAdapter& adapter) { WorkerLog("failed to write input metadata"); break; } + stats.post_time_usec = UsecSinceLast(); + stats.peak_rss_mb = GetPeakRSSMb(); + if (!BatchResult::WriteStats(stats, *outputs_blobseq)) { + WorkerLog("failed to write stats"); + break; + } if (!BatchResult::WriteInputEnd(*outputs_blobseq)) { WorkerLog("failed to write input end"); break; @@ -695,6 +838,72 @@ const char* FuzzTestWorkerGetTestName() { return test_name; } +void HandlePersistentMode(const FuzzTestAdapter& adapter) { + auto* inputs_blobseq = GetInputsBlobSequence(); + auto* outputs_blobseq = GetOutputsBlobSequence(); + bool first = true; + while (true) { + PersistentModeRequest req; + if (!ReadAll(persistent_mode_socket, reinterpret_cast(&req), 1)) { + WorkerLog("Failed to read request from persistent mode socket: ", + LogErrNo{}, LogLnSync{}); + return; + } + if (first) { + first = false; + WorkerLog("FuzzTest engine worker enter persistent mode", LogLnSync{}); + } else { + // Reset stdout/stderr. + for (int fd = 1; fd <= 2; fd++) { + lseek(fd, 0, SEEK_SET); + // NOTE: Allow ftruncate() to fail by ignoring its return; that's okay + // to happen when the stdout/stderr are not redirected to a file. + (void)ftruncate(fd, 0); + } + WorkerLog( + "FuzzTest engine worker (", + req == PersistentModeRequest::kExit ? "exiting persistent mode" + : "persistent mode batch", + "); flags: ", + GetWorkerFlags().present + ? std::string_view{GetWorkerFlags().str, GetWorkerFlags().len} + : "", + LogLnSync{}); + } + if (req == PersistentModeRequest::kExit) break; + WorkerCheck(req == PersistentModeRequest::kRunBatch, + "Unknown persistent mode request"); + + inputs_blobseq->Reset(); + outputs_blobseq->Reset(); + + GetWorkerState().ResetForPersistentMode(); + + // Read the first blob. It indicates what further actions to take. + auto request_type_blob = inputs_blobseq->Read(); + if (IsMutationRequest(request_type_blob)) { + inputs_blobseq->Reset(); + WorkerDoMutate(adapter); + } else if (IsExecutionRequest(request_type_blob)) { + inputs_blobseq->Reset(); + WorkerDoExecute(adapter); + } else { + WorkerCheck(false, "Unknown shmem request"); + } + + const int result = + GetWorkerState().has_finding.load(std::memory_order_relaxed) + ? EXIT_FAILURE + : EXIT_SUCCESS; + if (!WriteAll(persistent_mode_socket, + reinterpret_cast(&result), sizeof(result))) { + WorkerLog("Failed to write response to the persistent mode socket: ", + LogErrNo{}, LogLnSync{}); + return; + } + } +} + FuzzTestWorkerStatus WorkerRun(const FuzzTestAdapterManager& manager) { const auto& flags = GetWorkerFlags(); WorkerCheck(flags.present, "worker flags must present"); @@ -704,6 +913,12 @@ FuzzTestWorkerStatus WorkerRun(const FuzzTestAdapterManager& manager) { } const auto action = GetWorkerAction(); + WorkerCheck(action.has_value(), "No worker action to run"); + + if (action == WorkerAction::kNoOp) { + return kFuzzTestWorkerSuccess; + } + if (action == WorkerAction::kGetBinaryId) { WorkerDoGetBinaryId(manager); return kFuzzTestWorkerSuccess; @@ -748,8 +963,8 @@ FuzzTestWorkerStatus WorkerRun(const FuzzTestAdapterManager& manager) { WorkerCheck(manager.ConstructAdapter != nullptr, "ConstructAdapter is not defined"); FuzzTestAdapter adapter = {}; - manager.ConstructAdapter(manager.ctx, /*diagnostic_sink=*/&diagnostic_sink, - &adapter); + manager.ConstructAdapter(manager.ctx, + /*diagnostic_sink=*/&diagnostic_sink, &adapter); WORKER_CHECK_FOR_ERROR(); WorkerCheck(adapter.SetUpCoverageDomains != nullptr, "SetUpCoverageDomains must be defined"); @@ -764,7 +979,9 @@ FuzzTestWorkerStatus WorkerRun(const FuzzTestAdapterManager& manager) { WorkerCheck(adapter.FreeInput != nullptr, "FreeInput must be defined"); WorkerCheck(adapter.FreeCtx != nullptr, "FreeCtx must be defined"); - if (action == WorkerAction::kTestGetSeeds) { + if (persistent_mode_socket > 0) { + HandlePersistentMode(adapter); + } else if (action == WorkerAction::kTestGetSeeds) { WorkerDoGetSeeds(adapter); } else if (action == WorkerAction::kTestMutate) { WorkerDoMutate(adapter); @@ -794,7 +1011,11 @@ using ::fuzztest::internal::WorkerRun; } // namespace -int FuzzTestWorkerIsRequired() { return GetWorkerFlags().present; } +int FuzzTestWorkerIsRequired() { + static int result = GetWorkerFlags().present && + fuzztest::internal::GetWorkerAction().has_value(); + return result; +} FuzzTestWorkerStatus FuzzTestWorkerMaybeRun( const FuzzTestAdapterManager* manager) { diff --git a/centipede/puzzles/run_puzzle.sh b/centipede/puzzles/run_puzzle.sh index 0aee6243c..d6d12c853 100755 --- a/centipede/puzzles/run_puzzle.sh +++ b/centipede/puzzles/run_puzzle.sh @@ -94,7 +94,6 @@ function ExpectPerInputTimeout() { # Expects that Centipede found a per-batch timeout. function ExpectPerBatchTimeout() { echo "======= ${FUNCNAME[0]}" - fuzztest::internal::assert_regex_in_file "Per-batch timeout exceeded" "${log}" fuzztest::internal::assert_regex_in_file "Failure.*: per-batch-timeout-exceeded" "${log}" fuzztest::internal::assert_regex_in_file \ "Failure applies to entire batch: not executing inputs one-by-one" "${log}" diff --git a/centipede/runner.cc b/centipede/runner.cc index e085b54e9..82b2b4920 100644 --- a/centipede/runner.cc +++ b/centipede/runner.cc @@ -132,8 +132,7 @@ static void CheckWatchdogLimits() { const char *failure; }; const uint64_t input_start_time = state->input_start_time; - const uint64_t batch_start_time = state->batch_start_time; - if (input_start_time == 0 || batch_start_time == 0) return; + if (input_start_time == 0) return; const Resource resources[] = { {Resource{ /*what=*/"Per-input timeout", @@ -144,15 +143,6 @@ static void CheckWatchdogLimits() { state->run_time_flags.ignore_timeout_reports != 0, /*failure=*/kExecutionFailurePerInputTimeout.data(), }}, - {Resource{ - /*what=*/"Per-batch timeout", - /*units=*/"sec", - /*value=*/curr_time - batch_start_time, - /*limit=*/state->run_time_flags.timeout_per_batch, - /*ignore_report=*/ - state->run_time_flags.ignore_timeout_reports != 0, - /*failure=*/kExecutionFailurePerBatchTimeout.data(), - }}, {Resource{ /*what=*/"RSS limit", /*units=*/"MB", @@ -240,10 +230,10 @@ __attribute__((noinline)) void CheckStackLimit(size_t stack_usage, void GlobalRunnerState::StartWatchdogThread() { fprintf(stderr, "Starting watchdog thread: timeout_per_input: %" PRIu64 - " sec; timeout_per_batch: %" PRIu64 " sec; rss_limit_mb: %" PRIu64 - " MB; stack_limit_kb: %" PRIu64 " KB\n", + " sec; rss_limit_mb: %" PRIu64 " MB; stack_limit_kb: %" PRIu64 + " KB\n", run_time_flags.timeout_per_input.load(), - run_time_flags.timeout_per_batch, run_time_flags.rss_limit_mb.load(), + run_time_flags.rss_limit_mb.load(), state->run_time_flags.stack_limit_kb.load()); pthread_t watchdog_thread; pthread_create(&watchdog_thread, nullptr, WatchdogThread, nullptr); @@ -257,11 +247,6 @@ void GlobalRunnerState::StartWatchdogThread() { void GlobalRunnerState::ResetTimers() { const auto curr_time = time(nullptr); state->input_start_time = curr_time; - // batch_start_time is set only once -- just before the first input of the - // batch is about to start running. - if (batch_start_time == 0) { - batch_start_time = curr_time; - } } // Byte array mutation fallback for a custom mutator, as defined here: @@ -994,7 +979,6 @@ extern "C" void CentipedeEndExecutionBatch() { } in_execution_batch = false; fuzztest::internal::state->input_start_time = 0; - fuzztest::internal::state->batch_start_time = 0; } extern "C" void CentipedePrepareProcessing() { diff --git a/centipede/runner.h b/centipede/runner.h index fbe094166..5ddc3b3e5 100644 --- a/centipede/runner.h +++ b/centipede/runner.h @@ -33,7 +33,6 @@ namespace fuzztest::internal { // Flags derived from CENTIPEDE_RUNNER_FLAGS. struct RunTimeFlags { std::atomic timeout_per_input; - uint64_t timeout_per_batch; std::atomic rss_limit_mb; uint64_t crossover_level; uint64_t ignore_timeout_reports : 1; @@ -67,7 +66,6 @@ struct GlobalRunnerState { // flags can change later (if wrapped with std::atomic). RunTimeFlags run_time_flags = { /*timeout_per_input=*/flag_helper.HasIntFlag(":timeout_per_input=", 0), - /*timeout_per_batch=*/flag_helper.HasIntFlag(":timeout_per_batch=", 0), /*rss_limit_mb=*/flag_helper.HasIntFlag(":rss_limit_mb=", 0), /*crossover_level=*/flag_helper.HasIntFlag(":crossover_level=", 50), /*ignore_timeout_reports=*/ @@ -112,10 +110,6 @@ struct GlobalRunnerState { // time. std::atomic input_start_time; - // Per-batch timer. Initially, zero. ResetInputTimer() sets it to the current - // time before the first input and never resets it. - std::atomic batch_start_time; - // The Watchdog thread sets this to true. std::atomic watchdog_thread_started; }; diff --git a/centipede/testing/test_binary_for_engine_testing.cc b/centipede/testing/test_binary_for_engine_testing.cc index 75c1a42a5..071f2232c 100644 --- a/centipede/testing/test_binary_for_engine_testing.cc +++ b/centipede/testing/test_binary_for_engine_testing.cc @@ -202,10 +202,10 @@ int main(int argc, char** argv) { return worker_status == kFuzzTestWorkerSuccess ? EXIT_SUCCESS : EXIT_FAILURE; } - return ControllerRun(&manager, {absl::StrCat("--binary=", argv[0]), - "--test_name=some_test", - "--populate_binary_info=0", "--fork_server=0", - "--persistent_mode=0", "--exit_on_crash"}) == + return ControllerRun(&manager, + {absl::StrCat("--binary=", argv[0]), + "--test_name=some_test", "--populate_binary_info=0", + "--fork_server=0", "--exit_on_crash"}) == kFuzzTestControllerSuccess ? EXIT_SUCCESS : EXIT_FAILURE;