diff --git a/centipede.cc b/centipede.cc index 6dc29b0..d2e0de4 100644 --- a/centipede.cc +++ b/centipede.cc @@ -112,6 +112,12 @@ Centipede::Centipede(const Environment &env, CentipedeCallbacks &user_callbacks, input_filter_cmd_.StartForkServer(TemporaryLocalDirPath(), "input_filter"); } +Centipede::~Centipede() { + LOG_IF(WARNING, num_crash_reports_ > env_.max_num_crash_reports) + << "Skipped " << (num_crash_reports_ - env_.max_num_crash_reports) + << "crash reports"; +} + int Centipede::SaveCorpusToLocalDir(const Environment &env, std::string_view save_corpus_to_local_dir) { for (size_t shard = 0; shard < env.total_shards; shard++) { @@ -692,13 +698,23 @@ void Centipede::ReportCrash(std::string_view binary, const BatchResult &batch_result) { CHECK_EQ(input_vec.size(), batch_result.results().size()); - if (num_crash_reports_ >= env_.max_num_crash_reports) return; + ++num_crash_reports_; + if (num_crash_reports_ == env_.max_num_crash_reports) { + LOG(INFO) << "Reached --max_num_crash_reports: last report below; further " + "reports will be suppressed"; + } else if (num_crash_reports_ > env_.max_num_crash_reports) { + return; + } + // The runner adds results to the returned value as it successfully executes + // inputs. It doesn't add anything for a crashing input (there is nothing to + // add!), and immediately returns to the engine. Therefore, the suspect + // crashing input's index is equal to the number of the returned results. const size_t suspect_input_idx = std::clamp( batch_result.num_outputs_read(), 0, input_vec.size() - 1); - std::string log_prefix = - absl::StrCat("ReportCrash[", num_crash_reports_, "]: "); + const std::string log_prefix = + absl::StrCat("ReportCrash[", num_crash_reports_ - 1, "]: "); LOG(INFO) << log_prefix << "Batch execution failed:" << "\nBinary : " << binary @@ -714,37 +730,32 @@ void Centipede::ReportCrash(std::string_view binary, } LOG(INFO).NoPrefix() << "\n"; - LOG_IF(INFO, ++num_crash_reports_ == env_.max_num_crash_reports) - << log_prefix - << "Reached --max_num_crash_reports: further reports will be suppressed"; + if (batch_result.failure_description() == kExecutionFailurePerBatchTimeout) { + LOG(INFO) + << log_prefix + << "Failure applies to entire batch: not re-running inputs one-by-one"; + return; + } // Determine the optimal order of the inputs to try to maximize the chances of - // finding the reproducer fast. + // finding the crasher fast: insert it in front of everything else; however, + // do keep it at the old place, too, in case the target was primed for a crash + // by the sequence of inputs that preceded the crasher. // TODO(b/274705740): When the bug is fixed, set `input_idxs_to_try`'s size to // `suspect_input_idx + 1`. std::vector input_idxs_to_try(input_vec.size() + 1); input_idxs_to_try.front() = suspect_input_idx; std::iota(input_idxs_to_try.begin() + 1, input_idxs_to_try.end(), 0); - // Prioritize the presumed crasher by inserting it in front of everything - // else. However, do keep it at the old location, too, in case the target was - // primed for a crash by the sequence of inputs that preceded the crasher. - - if (batch_result.failure_description() == kExecutionFailurePerBatchTimeout) { - LOG(INFO) << log_prefix - << "Failure applies to entire batch: not executing inputs " - "one-by-one, trying to find the reproducer"; - return; - } // Try inputs one-by-one in the determined order. - LOG(INFO) << log_prefix - << "Executing inputs one-by-one, trying to find the reproducer"; + // TODO(ussuri): Only re-run sequentially; offload all I/O to a thread pool. + LOG(INFO) << log_prefix << "Re-running inputs one-by-one to find reproducer"; for (auto input_idx : input_idxs_to_try) { const auto &one_input = input_vec[input_idx]; BatchResult one_input_batch_result; if (!user_callbacks_.Execute(binary, {one_input}, one_input_batch_result)) { - auto hash = Hash(one_input); - auto crash_dir = env_.MakeCrashReproducerDirPath(); + const auto hash = Hash(one_input); + const auto crash_dir = env_.MakeCrashReproducerDirPath(); RemoteMkdir(crash_dir); std::string file_path = std::filesystem::path(crash_dir).append(hash); LOG(INFO) << log_prefix << "Detected crash-reproducing input:" @@ -754,16 +765,13 @@ void Centipede::ReportCrash(std::string_view binary, << "\nFailure : " << one_input_batch_result.failure_description() << "\nSaving input to: " << file_path; - auto *file = RemoteFileOpen(file_path, "w"); // overwrites existing file. - CHECK(file != nullptr) << log_prefix << "Failed to open " << file_path; - RemoteFileAppend(file, one_input); - RemoteFileClose(file); + RemoteFileSetContents(file_path, AsString(one_input)); return; } } LOG(INFO) << log_prefix - << "Crash was not observed when running inputs one-by-one"; + << "Crash was not observed when re-running inputs one-by-one"; // There will be cases when several inputs collectively cause a crash, but no // single input does. Handle this by writing out all inputs from the batch. diff --git a/centipede.h b/centipede.h index 5db32bc..c738541 100644 --- a/centipede.h +++ b/centipede.h @@ -42,7 +42,7 @@ class Centipede { Centipede(const Environment &env, CentipedeCallbacks &user_callbacks, const BinaryInfo &binary_info, CoverageLogger &coverage_logger, Stats &stats); - virtual ~Centipede() {} + ~Centipede(); // Main loop. void FuzzingLoop(); diff --git a/testing/centipede_test.cc b/testing/centipede_test.cc index b15f215..b7233da 100644 --- a/testing/centipede_test.cc +++ b/testing/centipede_test.cc @@ -12,11 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include - #include #include #include +#include #include #include #include @@ -47,7 +46,8 @@ namespace { // A mock for CentipedeCallbacks. class CentipedeMock : public CentipedeCallbacks { public: - CentipedeMock(const Environment &env) : CentipedeCallbacks(env) {} + explicit CentipedeMock(const Environment &env) : CentipedeCallbacks(env) {} + // Doesn't execute anything // Sets `batch_result.results()` based on the values of `inputs`: // Collects various stats about the inputs, to be checked in tests. @@ -58,7 +58,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 (const auto &input : inputs) { ByteArray counters(256); for (uint8_t byte : input) { counters[byte]++; @@ -84,6 +84,7 @@ class CentipedeMock : public CentipedeCallbacks { min_batch_size_ = std::min(min_batch_size_, inputs.size()); return true; } + // Makes predictable mutants: // first 255 mutations are 1-byte sequences {1} ... {255}. // (the value {0} is produced by DummyValidInput()). @@ -104,6 +105,8 @@ class CentipedeMock : public CentipedeCallbacks { } } + // TODO(ussuri): Make all of these private. + absl::flat_hash_set observed_1byte_inputs_; absl::flat_hash_set observed_2byte_inputs_; @@ -147,12 +150,16 @@ TEST(Centipede, MockTest) { EXPECT_EQ(mock.observed_2byte_inputs_.size(), 65536); // all 2-byte seqs. } -static size_t CountFilesInDir(std::string_view dir_path) { +namespace { + +size_t CountFilesInDir(std::string_view dir_path) { const std::filesystem::directory_iterator dir_iter{dir_path}; return std::distance(std::filesystem::begin(dir_iter), std::filesystem::end(dir_iter)); } +} // namespace + // Tests fuzzing and distilling in multiple shards. TEST(Centipede, ShardsAndDistillTest) { TempCorpusDir tmp_dir{test_info_->name()}; @@ -236,27 +243,31 @@ TEST(Centipede, InputFilter) { EXPECT_TRUE(corpus_set.count({'c'})); } +namespace { + // Callbacks for MutateViaExternalBinary test. class MutateCallbacks : public CentipedeCallbacks { public: explicit MutateCallbacks(const Environment &env) : CentipedeCallbacks(env) {} + // Will not be called. bool Execute(std::string_view binary, const std::vector &inputs, BatchResult &batch_result) override { - CHECK(false); - return false; + ABSL_UNREACHABLE(); } // Will not be called. void Mutate(const std::vector &inputs, size_t num_mutants, std::vector &mutants) override { - CHECK(false); + ABSL_UNREACHABLE(); } // Redeclare a protected member function as public so the tests can call it. using CentipedeCallbacks::MutateViaExternalBinary; }; +} // namespace + TEST(Centipede, MutateViaExternalBinary) { // This binary contains a test-friendly custom mutator. const std::string binary_with_custom_mutator = @@ -332,6 +343,8 @@ TEST(Centipede, MutateViaExternalBinary) { } } +namespace { + // A mock for MergeFromOtherCorpus test. class MergeMock : public CentipedeCallbacks { public: @@ -366,6 +379,8 @@ class MergeMock : public CentipedeCallbacks { size_t number_of_mutations_ = 0; }; +} // namespace + TEST(Centipede, MergeFromOtherCorpus) { using Corpus = std::vector; @@ -411,6 +426,8 @@ TEST(Centipede, MergeFromOtherCorpus) { EXPECT_EQ(work_tmp_dir.GetCorpus(1), Corpus({{3}, {4}, {5}, {6}, {7}})); } +namespace { + // A mock for FunctionFilter test. class FunctionFilterMock : public CentipedeCallbacks { public: @@ -428,7 +445,7 @@ class FunctionFilterMock : public CentipedeCallbacks { void Mutate(const std::vector &inputs, size_t num_mutants, std::vector &mutants) override { mutants.resize(num_mutants); - for (auto &input : inputs) { + for (const auto &input : inputs) { if (input != DummyValidInput()) { observed_inputs_.insert(input); } @@ -455,8 +472,8 @@ 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::vector RunWithFunctionFilter(std::string_view function_filter, + const TempDir &tmp_dir) { Environment env; env.workdir = tmp_dir.path(); env.seed = 1; // make the runs predictable. @@ -480,6 +497,8 @@ static std::vector RunWithFunctionFilter( return res; } +} // namespace + // Tests --function_filter. TEST(Centipede, FunctionFilter) { // Run with empty function filter. @@ -688,7 +707,9 @@ TEST(Centipede, UndetectedCrashingInput) { ASSERT_EQ(found_crash_file_names.size(), kCrashingInputIdxInBatch + 1); } -static void WriteBlobsToFile(const std::vector &blobs, +namespace{ + +void WriteBlobsToFile(const std::vector &blobs, const std::string_view path) { auto appender = DefaultBlobFileAppenderFactory(); CHECK_OK(appender->Open(path)); @@ -697,6 +718,8 @@ static void WriteBlobsToFile(const std::vector &blobs, } } +} // namespace + TEST(Centipede, ShardReader) { ByteArray data1 = {1, 2, 3}; ByteArray data2 = {3, 4, 5, 6};