Send SIGABRT to the runtime thread in the legacy FuzzTest runtime. I also made it so that we no longer spawn a new watchdog thread that we never terminate whenever we run a test. The watchdog threads are now properly cleaned up at the end of each test. PiperOrigin-RevId: 726626429
diff --git a/fuzztest/BUILD b/fuzztest/BUILD index 7e3835a..b9eb849 100644 --- a/fuzztest/BUILD +++ b/fuzztest/BUILD
@@ -604,6 +604,11 @@ name = "runtime", srcs = ["internal/runtime.cc"], hdrs = ["internal/runtime.h"], + linkopts = select({ + "@platforms//os:linux": ["-lpthread"], + "@platforms//os:macos": ["-lpthread"], + "//conditions:default": [], + }), deps = [ ":configuration", ":corpus_database", @@ -618,6 +623,7 @@ ":seed_seq", ":serialization", ":status", + "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/functional:any_invocable", "@com_google_absl//absl/functional:bind_front", "@com_google_absl//absl/functional:function_ref",
diff --git a/fuzztest/CMakeLists.txt b/fuzztest/CMakeLists.txt index 1c3c925..3d41c8b 100644 --- a/fuzztest/CMakeLists.txt +++ b/fuzztest/CMakeLists.txt
@@ -578,6 +578,7 @@ fuzztest::seed_seq fuzztest::serialization fuzztest::status + absl::core_headers absl::any_invocable absl::bind_front absl::function_ref @@ -591,6 +592,8 @@ absl::str_format absl::time absl::span + LINKOPTS + pthread ) fuzztest_cc_test(
diff --git a/fuzztest/internal/runtime.cc b/fuzztest/internal/runtime.cc index fb47977..7ac064c 100644 --- a/fuzztest/internal/runtime.cc +++ b/fuzztest/internal/runtime.cc
@@ -19,6 +19,8 @@ #include <sys/resource.h> #endif +#include <signal.h> + #include <algorithm> #include <atomic> #include <cerrno> @@ -346,25 +348,62 @@ absl::Format(out, "%s", GetSeparator()); } -void Runtime::StartWatchdog() { - // Centipede runner has its own watchdog. #ifndef FUZZTEST_USE_CENTIPEDE - auto watchdog_thread = std::thread(std::bind(&Runtime::Watchdog, this)); - while (!watchdog_thread_started) std::this_thread::yield(); - watchdog_thread.detach(); -#endif -} -void Runtime::Watchdog() { - watchdog_thread_started = true; - while (true) { - while (watchdog_spinlock_.test_and_set()) std::this_thread::yield(); - if (test_iteration_started_) CheckWatchdogLimits(); - watchdog_spinlock_.clear(); - absl::SleepFor(absl::Seconds(1)); +class Runtime::Watchdog { + public: + explicit Watchdog(Runtime& runtime) : runtime_{runtime} { + watchdog_thread_ = + std::thread(absl::bind_front(&Runtime::Watchdog::WatchdogLoop, this)); + while (!watchdog_thread_started_) std::this_thread::yield(); + } + + ~Watchdog() { + stop_requested_ = true; + watchdog_thread_.join(); + } + + private: + void WatchdogLoop() { + watchdog_thread_started_ = true; + while (!stop_requested_) { + runtime_.CheckWatchdogLimits(); + runtime_.watchdog_spinlock_.Lock(); + if (runtime_.watchdog_limit_exceeded_) { + runtime_.watchdog_spinlock_.Unlock(); + break; + } + runtime_.watchdog_spinlock_.Unlock(); + absl::SleepFor(absl::Seconds(1)); + } + } + + std::atomic<bool> watchdog_thread_started_ = false; + std::atomic<bool> stop_requested_ = false; + std::thread watchdog_thread_; + Runtime& runtime_; +}; + +#else // FUZZTEST_USE_CENTIPEDE + +// Centipede runner has its own watchdog. +class Runtime::Watchdog { + public: + explicit Watchdog(Runtime&) {} +}; + +#endif // FUZZTEST_USE_CENTIPEDE + +Runtime::Watchdog Runtime::CreateWatchdog() { return Watchdog{*this}; } + +void Runtime::Spinlock::Lock() { + while (locked_.test_and_set(std::memory_order_acq_rel)) { + std::this_thread::yield(); } } +void Runtime::Spinlock::Unlock() { locked_.clear(std::memory_order_release); } + static size_t GetPeakRSSBytes() { #ifndef FUZZTEST_HAS_RUSAGE return 0; @@ -379,7 +418,12 @@ void Runtime::CheckWatchdogLimits() { // Centipede runner has its own watchdog. #ifndef FUZZTEST_USE_CENTIPEDE - if (current_configuration_ == nullptr) return; + watchdog_spinlock_.Lock(); + if (current_configuration_ == nullptr || !test_iteration_started_ || + watchdog_limit_exceeded_) { + watchdog_spinlock_.Unlock(); + return; + } const absl::Duration run_duration = clock_fn_() - current_iteration_start_time_; if (current_configuration_->time_limit_per_input > absl::ZeroDuration() && @@ -388,7 +432,14 @@ GetStderr(), "[!] Per-input timeout exceeded: %s > %s - aborting\n", absl::FormatDuration(run_duration), absl::FormatDuration(current_configuration_->time_limit_per_input)); + watchdog_limit_exceeded_ = true; + watchdog_spinlock_.Unlock(); +#if defined(__linux__) || defined(__APPLE__) + pthread_kill(reporting_thread_, SIGABRT); + return; +#else std::abort(); +#endif } const size_t rss_usage = GetPeakRSSBytes(); if (current_configuration_->rss_limit > 0 && @@ -396,9 +447,17 @@ absl::FPrintF(GetStderr(), "[!] RSS limit exceeded: %zu > %zu (bytes) - aborting\n", rss_usage, current_configuration_->rss_limit); + watchdog_limit_exceeded_ = true; + watchdog_spinlock_.Unlock(); +#if defined(__linux__) || defined(__APPLE__) + pthread_kill(reporting_thread_, SIGABRT); + return; +#else std::abort(); - } #endif + } + watchdog_spinlock_.Unlock(); +#endif // FUZZTEST_USE_CENTIPEDE } void Runtime::SetCurrentTest(const FuzzTest* test, @@ -422,11 +481,18 @@ ++test_counter_; } +void Runtime::OnTestIterationStart(const absl::Time& start_time) { + watchdog_spinlock_.Lock(); + current_iteration_start_time_ = start_time; + test_iteration_started_ = true; + watchdog_spinlock_.Unlock(); +} + void Runtime::OnTestIterationEnd() { - test_iteration_started_ = false; - while (watchdog_spinlock_.test_and_set()) std::this_thread::yield(); CheckWatchdogLimits(); - watchdog_spinlock_.clear(); + watchdog_spinlock_.Lock(); + test_iteration_started_ = false; + watchdog_spinlock_.Unlock(); } #if defined(__linux__) || defined(__APPLE__) @@ -971,7 +1037,7 @@ test_.full_name()); return; } - runtime_.StartWatchdog(); + [[maybe_unused]] auto watchdog = runtime_.CreateWatchdog(); PopulateLimits(configuration, execution_coverage_); // TODO(sbenzaquen): Currently, some infrastructure code assumes that replay @@ -1143,7 +1209,7 @@ test_.full_name()); return true; } - runtime_.StartWatchdog(); + [[maybe_unused]] auto watchdog = runtime_.CreateWatchdog(); PopulateLimits(configuration, execution_coverage_); runtime_.SetRunMode(RunMode::kFuzz);
diff --git a/fuzztest/internal/runtime.h b/fuzztest/internal/runtime.h index 3ac8dd2..479c837 100644 --- a/fuzztest/internal/runtime.h +++ b/fuzztest/internal/runtime.h
@@ -15,6 +15,10 @@ #ifndef FUZZTEST_FUZZTEST_INTERNAL_RUNTIME_H_ #define FUZZTEST_FUZZTEST_INTERNAL_RUNTIME_H_ +#if defined(__linux__) || defined(__APPLE__) +#include <pthread.h> +#endif + #include <atomic> #include <cstddef> #include <cstdio> @@ -28,6 +32,7 @@ #include <utility> #include <vector> +#include "absl/base/thread_annotations.h" #include "absl/functional/any_invocable.h" #include "absl/functional/function_ref.h" #include "absl/random/bit_gen_ref.h" @@ -36,6 +41,7 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" +#include "absl/time/clock.h" #include "absl/time/time.h" #include "absl/types/span.h" #include "./fuzztest/internal/configuration.h" @@ -156,8 +162,6 @@ return termination_requested_.load(std::memory_order_relaxed); } - void StartWatchdog(); - void SetRunMode(RunMode run_mode) { run_mode_ = run_mode; } RunMode run_mode() const { return run_mode_; } @@ -167,6 +171,11 @@ reporter_enabled_ = true; stats_ = stats; clock_fn_ = clock_fn; +#if defined(__linux__) || defined(__APPLE__) + // Set this just in case that the function is called in a thread other than + // the one that created the runtime. + reporting_thread_ = pthread_self(); +#endif // In case we have not installed them yet, do so now. InstallSignalHandlers(GetStderr()); ResetCrashType(); @@ -183,10 +192,7 @@ // calling `DisableReporter()`. void SetCurrentTest(const FuzzTest* test, const Configuration* configuration); - void OnTestIterationStart(const absl::Time& start_time) { - current_iteration_start_time_ = start_time; - test_iteration_started_ = true; - } + void OnTestIterationStart(const absl::Time& start_time); void OnTestIterationEnd(); void SetCurrentArgs(Args* args) { current_args_ = args; } @@ -209,11 +215,18 @@ } void ResetCrashType() { crash_type_ = std::nullopt; } + class Watchdog; + // Returns a watchdog that periodically checks the time and memory limits in a + // separate thread. The watchdog handles the logic of starting and joining the + // thread. The runtime must outlive the watchdog. + Watchdog CreateWatchdog(); + private: Runtime(); + // Checks time and memory limits. If any limit is exceeded, sends SIGABRT to + // the runtime thread. void CheckWatchdogLimits(); - void Watchdog(); // Returns the file path of the reproducer. // Returns empty string if writing the file failed. @@ -243,7 +256,6 @@ std::atomic<bool> termination_requested_ = false; RunMode run_mode_ = RunMode::kUnitTest; - std::atomic<bool> watchdog_thread_started = false; absl::Time creation_time_ = absl::Now(); size_t test_counter_ = 0; @@ -252,12 +264,34 @@ Args* current_args_ = nullptr; const FuzzTest* current_test_ = nullptr; const Configuration* current_configuration_; - absl::Time current_iteration_start_time_; - std::atomic<bool> test_iteration_started_ = false; - std::atomic_flag watchdog_spinlock_ = ATOMIC_FLAG_INIT; const RuntimeStats* stats_ = nullptr; absl::Time (*clock_fn_)() = nullptr; + // We use a simple custom spinlock instead of absl::Mutex to reduce + // dependencies and avoid potential issues with code instrumentation. + class ABSL_LOCKABLE Spinlock { + public: + Spinlock() = default; + Spinlock(const Spinlock&) = delete; + Spinlock& operator=(const Spinlock&) = delete; + + void Lock() ABSL_EXCLUSIVE_LOCK_FUNCTION(); + void Unlock() ABSL_UNLOCK_FUNCTION(); + + private: + std::atomic_flag locked_ = ATOMIC_FLAG_INIT; + }; + + Spinlock watchdog_spinlock_; + absl::Time current_iteration_start_time_ ABSL_GUARDED_BY(watchdog_spinlock_); + bool test_iteration_started_ ABSL_GUARDED_BY(watchdog_spinlock_) = false; + bool watchdog_limit_exceeded_ ABSL_GUARDED_BY(watchdog_spinlock_) = false; + +#if defined(__linux__) || defined(__APPLE__) + // The thread to which the watchdog sends SIGABRT. + pthread_t reporting_thread_ = pthread_self(); +#endif + // A registry of crash metadata listeners. std::vector<CrashMetadataListener> crash_metadata_listeners_; // In case of a crash, contains the crash type.