From 1c35fe51c265b91108fb78ed067f018c9d2072de Mon Sep 17 00:00:00 2001 From: Markus Kusano Date: Mon, 22 Jul 2024 08:58:13 -0700 Subject: [PATCH] No public description PiperOrigin-RevId: 654772464 --- centipede/BUILD | 6 + centipede/analyze_corpora.cc | 9 +- centipede/analyze_corpora_test.cc | 2 +- centipede/binary_info.cc | 18 +-- centipede/blob_file_converter.cc | 2 +- centipede/centipede.cc | 32 ++--- centipede/centipede_interface.cc | 47 +++---- centipede/config_file.cc | 8 +- centipede/corpus.cc | 13 +- centipede/coverage.cc | 49 ++++---- centipede/distill.cc | 7 +- centipede/environment.cc | 7 +- centipede/seed_corpus_maker_lib.cc | 18 +-- centipede/stats.cc | 16 +-- centipede/util.cc | 5 +- common/BUILD | 19 +++ common/blob_file.cc | 28 +++-- common/remote_file.cc | 70 +++++++---- common/remote_file.h | 53 ++++---- common/remote_file_oss.cc | 192 ++++++++++++++++++++--------- common/remote_file_test.cc | 28 +++-- common/status_macros.h | 64 ++++++++++ fuzztest/internal/io.cc | 6 +- 23 files changed, 468 insertions(+), 231 deletions(-) create mode 100644 common/status_macros.h diff --git a/centipede/BUILD b/centipede/BUILD index 13535102..cfa08753 100644 --- a/centipede/BUILD +++ b/centipede/BUILD @@ -515,6 +515,7 @@ cc_library( "@com_google_absl//absl/strings", "@com_google_absl//absl/synchronization", "@com_google_fuzztest//common:remote_file", + "@com_google_fuzztest//common:status_macros", ], ) @@ -597,6 +598,7 @@ cc_library( "@com_google_fuzztest//common:defs", "@com_google_fuzztest//common:logging", "@com_google_fuzztest//common:remote_file", + "@com_google_fuzztest//common:status_macros", ], ) @@ -770,6 +772,7 @@ cc_library( "@com_google_fuzztest//common:hash", "@com_google_fuzztest//common:logging", "@com_google_fuzztest//common:remote_file", + "@com_google_fuzztest//common:status_macros", ], ) @@ -815,6 +818,7 @@ cc_library( "@com_google_fuzztest//common:hash", "@com_google_fuzztest//common:logging", "@com_google_fuzztest//common:remote_file", + "@com_google_fuzztest//common:status_macros", "@com_google_fuzztest//fuzztest:configuration", ], ) @@ -843,6 +847,7 @@ cc_library( "@com_google_fuzztest//common:defs", "@com_google_fuzztest//common:logging", "@com_google_fuzztest//common:remote_file", + "@com_google_fuzztest//common:status_macros", ], ) @@ -874,6 +879,7 @@ cc_library( "@com_google_fuzztest//common:hash", "@com_google_fuzztest//common:logging", "@com_google_fuzztest//common:remote_file", + "@com_google_fuzztest//common:status_macros", ], ) diff --git a/centipede/analyze_corpora.cc b/centipede/analyze_corpora.cc index 6297d846..e4a8f6c3 100644 --- a/centipede/analyze_corpora.cc +++ b/centipede/analyze_corpora.cc @@ -48,9 +48,11 @@ std::vector ReadCorpora(std::string_view binary_name, WorkDir workdir(std::string(workdir_path), std::string(binary_name), std::string(binary_hash), /*my_shard_index=*/0); std::vector corpus_paths; - RemoteGlobMatch(workdir.CorpusFiles().AllShardsGlob(), corpus_paths); + CHECK_OK( + RemoteGlobMatch(workdir.CorpusFiles().AllShardsGlob(), corpus_paths)); std::vector features_paths; - RemoteGlobMatch(workdir.FeaturesFiles().AllShardsGlob(), features_paths); + CHECK_OK( + RemoteGlobMatch(workdir.FeaturesFiles().AllShardsGlob(), features_paths)); CHECK_EQ(corpus_paths.size(), features_paths.size()); std::vector corpus; @@ -193,7 +195,8 @@ void DumpCoverageReport(const CoverageResults &coverage_results, std::ostringstream symbol_table_stream; coverage_symbol_table.WriteToLLVMSymbolizer(symbol_table_stream); - RemoteFileSetContents(coverage_report_path, symbol_table_stream.str()); + CHECK_OK( + RemoteFileSetContents(coverage_report_path, symbol_table_stream.str())); } AnalyzeCorporaResults AnalyzeCorpora(std::string_view binary_name, diff --git a/centipede/analyze_corpora_test.cc b/centipede/analyze_corpora_test.cc index 055b053f..f72effa6 100644 --- a/centipede/analyze_corpora_test.cc +++ b/centipede/analyze_corpora_test.cc @@ -102,7 +102,7 @@ TEST(DumpCoverageReport, SimpleCoverageResults) { std::filesystem::path{test_tmpdir} / "covered_symbol_table"; DumpCoverageReport(coverage_results, coverage_report_path); std::string symbol_table_contents; - RemoteFileGetContents(coverage_report_path, symbol_table_contents); + ASSERT_OK(RemoteFileGetContents(coverage_report_path, symbol_table_contents)); std::istringstream symbol_table_stream(symbol_table_contents); SymbolTable symbols; diff --git a/centipede/binary_info.cc b/centipede/binary_info.cc index 073e508a..e822dca5 100644 --- a/centipede/binary_info.cc +++ b/centipede/binary_info.cc @@ -123,15 +123,16 @@ void BinaryInfo::InitializeFromSanCovBinary( void BinaryInfo::Read(std::string_view dir) { std::string symbol_table_contents; // TODO(b/295978603): move calculation of paths into WorkDir class. - RemoteFileGetContents( + CHECK_OK(RemoteFileGetContents( (std::filesystem::path(dir) / kSymbolTableFileName).c_str(), - symbol_table_contents); + symbol_table_contents)); std::istringstream symbol_table_stream(symbol_table_contents); symbols.ReadFromLLVMSymbolizer(symbol_table_stream); std::string pc_table_contents; - RemoteFileGetContents((std::filesystem::path(dir) / kPCTableFileName).c_str(), - pc_table_contents); + CHECK_OK(RemoteFileGetContents( + (std::filesystem::path(dir) / kPCTableFileName).c_str(), + pc_table_contents)); std::istringstream pc_table_stream(pc_table_contents); pc_table = ReadPcTable(pc_table_stream); } @@ -140,14 +141,15 @@ void BinaryInfo::Write(std::string_view dir) { std::ostringstream symbol_table_stream; symbols.WriteToLLVMSymbolizer(symbol_table_stream); // TODO(b/295978603): move calculation of paths into WorkDir class. - RemoteFileSetContents( + CHECK_OK(RemoteFileSetContents( (std::filesystem::path(dir) / kSymbolTableFileName).c_str(), - symbol_table_stream.str()); + symbol_table_stream.str())); std::ostringstream pc_table_stream; WritePcTable(pc_table, pc_table_stream); - RemoteFileSetContents((std::filesystem::path(dir) / kPCTableFileName).c_str(), - pc_table_stream.str()); + CHECK_OK(RemoteFileSetContents( + (std::filesystem::path(dir) / kPCTableFileName).c_str(), + pc_table_stream.str())); } } // namespace centipede diff --git a/centipede/blob_file_converter.cc b/centipede/blob_file_converter.cc index 93af3c7c..226e7412 100644 --- a/centipede/blob_file_converter.cc +++ b/centipede/blob_file_converter.cc @@ -99,7 +99,7 @@ void Convert( // // Verify and prepare source and destination. CHECK(RemotePathExists(in)) << VV(in); - RemoteMkdir(std::filesystem::path{out}.parent_path().c_str()); + CHECK_OK(RemoteMkdir(std::filesystem::path{out}.parent_path().c_str())); // Open blob file reader and writer. diff --git a/centipede/centipede.cc b/centipede/centipede.cc index 1f97cdc0..ac408d26 100644 --- a/centipede/centipede.cc +++ b/centipede/centipede.cc @@ -96,6 +96,7 @@ #include "./common/hash.h" #include "./common/logging.h" #include "./common/remote_file.h" +#include "./common/status_macros.h" namespace centipede { @@ -135,8 +136,8 @@ Centipede::Centipede(const Environment &env, CentipedeCallbacks &user_callbacks, void Centipede::CorpusToFiles(const Environment &env, std::string_view dir) { std::vector sharded_corpus_files; - RemoteGlobMatch(WorkDir{env}.CorpusFiles().AllShardsGlob(), - sharded_corpus_files); + CHECK_OK(RemoteGlobMatch(WorkDir{env}.CorpusFiles().AllShardsGlob(), + sharded_corpus_files)); ExportCorpus(sharded_corpus_files, dir); } @@ -147,7 +148,9 @@ void Centipede::CorpusFromFiles(const Environment &env, std::string_view dir) { std::vector> sharded_paths(env.total_shards); std::vector paths; size_t total_paths = 0; - for (const std::string &path : RemoteListFiles(dir, /*recursively=*/true)) { + const std::vector listed_paths = + ValueOrDie(RemoteListFiles(dir, /*recursively=*/true)); + for (const std::string &path : listed_paths) { size_t filename_hash = std::hash{}(path); sharded_paths[filename_hash % env.total_shards].push_back(path); ++total_paths; @@ -177,7 +180,7 @@ void Centipede::CorpusFromFiles(const Environment &env, std::string_view dir) { ByteArray shard_data; for (const auto &path : sharded_paths[shard]) { std::string input; - RemoteFileGetContents(path, input); + CHECK_OK(RemoteFileGetContents(path, input)); if (input.empty() || existing_hashes.contains(Hash(input))) { ++inputs_ignored; continue; @@ -533,7 +536,7 @@ void Centipede::GenerateSourceBasedCoverageReport( auto report_path = wd_.SourceBasedCoverageReportPath(filename_annotation); LOG(INFO) << "Generate source based coverage report [" << description << "]; " << VV(report_path); - RemoteMkdir(report_path); + CHECK_OK(RemoteMkdir(report_path)); std::vector raw_profiles = wd_.EnumerateRawCoverageProfiles(); @@ -574,15 +577,16 @@ void Centipede::GenerateRUsageReport(std::string_view filename_annotation, class ReportDumper : public RUsageProfiler::ReportSink { public: explicit ReportDumper(std::string_view path) - : file_{RemoteFileOpen(path, "w")} { + : file_{*RemoteFileOpen(path, "w")} { CHECK(file_ != nullptr) << VV(path); - RemoteFileSetWriteBufferSize(file_, 10UL * 1024 * 1024); + CHECK_OK(RemoteFileSetWriteBufferSize(file_, 10UL * 1024 * 1024)); } - ~ReportDumper() override { RemoteFileClose(file_); } + ~ReportDumper() override { CHECK_OK(RemoteFileClose(file_)); } ReportDumper &operator<<(std::string_view fragment) override { - RemoteFileAppend(file_, ByteArray{fragment.cbegin(), fragment.cend()}); + CHECK_OK(RemoteFileAppend(file_, + ByteArray{fragment.cbegin(), fragment.cend()})); return *this; } @@ -870,7 +874,7 @@ void Centipede::ReportCrash(std::string_view binary, if (!user_callbacks_.Execute(binary, {one_input}, one_input_batch_result)) { auto hash = Hash(one_input); auto crash_dir = wd_.CrashReproducerDirPath(); - RemoteMkdir(crash_dir); + CHECK_OK(RemoteMkdir(crash_dir)); std::string file_path = std::filesystem::path(crash_dir).append(hash); LOG(INFO) << log_prefix << "Detected crash-reproducing input:" << "\nInput index : " << input_idx << "\nInput bytes : " @@ -879,7 +883,7 @@ void Centipede::ReportCrash(std::string_view binary, << "\nFailure : " << one_input_batch_result.failure_description() << "\nSaving input to: " << file_path; - RemoteFileSetContents(file_path, one_input); + CHECK_OK(RemoteFileSetContents(file_path, one_input)); return; } } @@ -898,18 +902,18 @@ void Centipede::ReportCrash(std::string_view binary, const auto &suspect_input = input_vec[suspect_input_idx]; auto suspect_hash = Hash(suspect_input); auto crash_dir = wd_.CrashReproducerDirPath(); - RemoteMkdir(crash_dir); + CHECK_OK(RemoteMkdir(crash_dir)); std::string save_dir = std::filesystem::path(crash_dir) .append("crashing_batch-") .concat(suspect_hash); - RemoteMkdir(save_dir); + CHECK_OK(RemoteMkdir(save_dir)); LOG(INFO) << log_prefix << "Saving used inputs from batch to: " << save_dir; for (int i = 0; i <= suspect_input_idx; ++i) { const auto &one_input = input_vec[i]; auto hash = Hash(one_input); std::string file_path = std::filesystem::path(save_dir).append( absl::StrFormat("input-%010d-%s", i, hash)); - RemoteFileSetContents(file_path, one_input); + CHECK_OK(RemoteFileSetContents(file_path, one_input)); } } diff --git a/centipede/centipede_interface.cc b/centipede/centipede_interface.cc index 73dcb09b..1d970ddd 100644 --- a/centipede/centipede_interface.cc +++ b/centipede/centipede_interface.cc @@ -66,6 +66,7 @@ #include "./common/hash.h" #include "./common/logging.h" // IWYU pragma: keep #include "./common/remote_file.h" +#include "./common/status_macros.h" #include "./fuzztest/internal/configuration.h" namespace centipede { @@ -178,7 +179,7 @@ BinaryInfo PopulateBinaryInfoAndSavePCsIfNecessary( } if (env.save_binary_info) { const std::string binary_info_dir = WorkDir{env}.BinaryInfoDirPath(); - RemoteMkdir(binary_info_dir); + CHECK_OK(RemoteMkdir(binary_info_dir)); LOG(INFO) << "Serializing binary info to: " << binary_info_dir; binary_info.Write(binary_info_dir); } @@ -329,11 +330,11 @@ int PruneNonreproducibleAndCountRemainingCrashes( for (const std::string &crashing_input_file : crashing_input_files) { ByteArray crashing_input; - RemoteFileGetContents(crashing_input_file, crashing_input); + CHECK_OK(RemoteFileGetContents(crashing_input_file, crashing_input)); if (scoped_callbacks.callbacks()->Execute(env.binary, {crashing_input}, batch_result)) { // The crash is not reproducible. - RemotePathDelete(crashing_input_file, /*recursively=*/false); + CHECK_OK(RemotePathDelete(crashing_input_file, /*recursively=*/false)); } else { ++num_remaining_crashes; } @@ -435,10 +436,11 @@ int UpdateCorpusDatabaseForFuzzTests( // This could be a workdir from a failed run that used a different version // of the binary. We delete it so that we don't have to deal with the // assumptions under which it is safe to reuse an old workdir. - RemotePathDelete(env.workdir, /*recursively=*/true); + CHECK_OK(RemotePathDelete(env.workdir, /*recursively=*/true)); } const WorkDir workdir{env}; - RemoteMkdir(workdir.CoverageDirPath()); // Implicitly creates the workdir. + CHECK_OK(RemoteMkdir( + workdir.CoverageDirPath())); // Implicitly creates the workdir // Seed the fuzzing session with the latest coverage corpus from the // previous fuzzing session. @@ -465,11 +467,11 @@ int UpdateCorpusDatabaseForFuzzTests( Fuzz(env, binary_info, pcs_file_path, callbacks_factory); if (!stats_root_path.empty()) { const auto stats_dir = stats_root_path / fuzztest_config.fuzz_tests[i]; - RemoteMkdir(stats_dir.c_str()); - RemotePathRename( + CHECK_OK(RemoteMkdir(stats_dir.c_str())); + CHECK_OK(RemotePathRename( workdir.FuzzingStatsPath(), (stats_dir / absl::StrCat("fuzzing_stats_", execution_stamp)) - .c_str()); + .c_str())); } // Distill and store the coverage corpus. @@ -477,23 +479,25 @@ int UpdateCorpusDatabaseForFuzzTests( if (RemotePathExists(coverage_dir.c_str())) { // In the future, we will store k latest coverage corpora for some k, but // for now we only keep the latest one. - RemotePathDelete(coverage_dir.c_str(), /*recursively=*/true); + CHECK_OK(RemotePathDelete(coverage_dir.c_str(), /*recursively=*/true)); } - RemoteMkdir(coverage_dir.c_str()); + CHECK_OK(RemoteMkdir(coverage_dir.c_str())); std::vector distilled_corpus_files; - RemoteGlobMatch(workdir.DistilledCorpusFiles().AllShardsGlob(), - distilled_corpus_files); + CHECK_OK(RemoteGlobMatch(workdir.DistilledCorpusFiles().AllShardsGlob(), + distilled_corpus_files)); for (const std::string &corpus_file : distilled_corpus_files) { const std::string file_name = std::filesystem::path(corpus_file).filename(); - RemotePathRename(corpus_file, (coverage_dir / file_name).c_str()); + CHECK_OK( + RemotePathRename(corpus_file, (coverage_dir / file_name).c_str())); } const std::filesystem::path crashing_dir = fuzztest_db_path / "crashing"; const std::vector crashing_input_files = // The corpus database layout assumes the crash input files are located // directly in the crashing subdirectory, so we don't list recursively. - RemoteListFiles(crashing_dir.c_str(), /*recursively=*/false); + ValueOrDie( + RemoteListFiles(crashing_dir.c_str(), /*recursively=*/false)); const int num_remaining_crashes = PruneNonreproducibleAndCountRemainingCrashes(env, crashing_input_files, callbacks_factory); @@ -505,18 +509,19 @@ int UpdateCorpusDatabaseForFuzzTests( // The crash reproducer directory may contain subdirectories with // input files that don't individually cause a crash. We ignore those // for now and don't list the files recursively. - RemoteListFiles(workdir.CrashReproducerDirPath(), - /*recursively=*/false); + *RemoteListFiles(workdir.CrashReproducerDirPath(), + /*recursively=*/false); if (!new_crashing_input_files.empty()) { const std::string crashing_input_file_name = std::filesystem::path(new_crashing_input_files[0]).filename(); - RemoteMkdir(crashing_dir.c_str()); - RemotePathRename(new_crashing_input_files[0], - (crashing_dir / crashing_input_file_name).c_str()); + CHECK_OK(RemoteMkdir(crashing_dir.c_str())); + CHECK_OK(RemotePathRename( + new_crashing_input_files[0], + (crashing_dir / crashing_input_file_name).c_str())); } } } - RemotePathDelete(base_workdir_path.c_str(), /*recursively=*/true); + CHECK_OK(RemotePathDelete(base_workdir_path.c_str(), /*recursively=*/true)); return EXIT_SUCCESS; } @@ -589,7 +594,7 @@ int CentipedeMain(const Environment &env, // Create the remote coverage dirs once, before creating any threads. const auto coverage_dir = WorkDir{env}.CoverageDirPath(); - RemoteMkdir(coverage_dir); + CHECK_OK(RemoteMkdir(coverage_dir)); LOG(INFO) << "Coverage dir: " << coverage_dir << "; temporary dir: " << tmpdir; diff --git a/centipede/config_file.cc b/centipede/config_file.cc index c03df113..2c1e29cf 100644 --- a/centipede/config_file.cc +++ b/centipede/config_file.cc @@ -162,15 +162,15 @@ AugmentedArgvWithCleanup LocalizeConfigFilesInArgv( if (!path.empty() && !std::filesystem::exists(path)) { // assume remote // Read the remote file. std::string contents; - RemoteFileGetContents(path.c_str(), contents); + CHECK_OK(RemoteFileGetContents(path.c_str(), contents)); // Save a temporary local copy. const std::filesystem::path tmp_dir = TemporaryLocalDirPath(); const std::filesystem::path local_path = tmp_dir / path.filename(); LOG(INFO) << "Localizing remote config: " << VV(path) << VV(local_path); // NOTE: Ignore "Remote" in the API names here: the paths are always local. - RemoteMkdir(tmp_dir.c_str()); - RemoteFileSetContents(local_path.c_str(), contents); + CHECK_OK(RemoteMkdir(tmp_dir.c_str())); + CHECK_OK(RemoteFileSetContents(local_path.c_str(), contents)); // Augment the argv to point at the local copy and ensure it is cleaned up. replacements.emplace_back(path.c_str(), local_path.c_str()); @@ -244,7 +244,7 @@ set -x } else { file_contents = flags_str; } - RemoteFileSetContents(path.c_str(), file_contents); + CHECK_OK(RemoteFileSetContents(path.c_str(), file_contents)); } return path; diff --git a/centipede/corpus.cc b/centipede/corpus.cc index ba6cf444..a6c956b3 100644 --- a/centipede/corpus.cc +++ b/centipede/corpus.cc @@ -37,6 +37,7 @@ #include "./common/defs.h" #include "./common/logging.h" // IWYU pragma: keep #include "./common/remote_file.h" +#include "./common/status_macros.h" namespace centipede { @@ -136,9 +137,9 @@ const CorpusRecord &Corpus::UniformRandom(size_t random) const { void Corpus::DumpStatsToFile(const FeatureSet &fs, std::string_view filepath, std::string_view description) { - auto *file = RemoteFileOpen(filepath, "w"); + auto *file = ValueOrDie(RemoteFileOpen(filepath, "w")); CHECK(file != nullptr) << "Failed to open file: " << filepath; - RemoteFileSetWriteBufferSize(file, 100UL * 1024 * 1024); + CHECK_OK(RemoteFileSetWriteBufferSize(file, 100UL * 1024 * 1024)); static constexpr std::string_view kHeaderStub = R"(# $0 { "num_inputs": $1, @@ -151,7 +152,7 @@ void Corpus::DumpStatsToFile(const FeatureSet &fs, std::string_view filepath, )"; const std::string header_str = absl::Substitute(kHeaderStub, description, records_.size()); - RemoteFileAppend(file, header_str); + CHECK_OK(RemoteFileAppend(file, header_str)); std::string before_record; for (const auto &record : records_) { std::vector frequencies; @@ -162,11 +163,11 @@ void Corpus::DumpStatsToFile(const FeatureSet &fs, std::string_view filepath, const std::string frequencies_str = absl::StrJoin(frequencies, ", "); const std::string record_str = absl::Substitute( kRecordStub, before_record, record.data.size(), frequencies_str); - RemoteFileAppend(file, record_str); + CHECK_OK(RemoteFileAppend(file, record_str)); before_record = ","; } - RemoteFileAppend(file, std::string{kFooter}); - RemoteFileClose(file); + CHECK_OK(RemoteFileAppend(file, std::string{kFooter})); + CHECK_OK(RemoteFileClose(file)); } std::string Corpus::MemoryUsageString() const { diff --git a/centipede/coverage.cc b/centipede/coverage.cc index 1f91e565..66aa328b 100644 --- a/centipede/coverage.cc +++ b/centipede/coverage.cc @@ -33,6 +33,7 @@ #include "./centipede/pc_info.h" #include "./centipede/symbol_table.h" #include "./common/remote_file.h" +#include "./common/status_macros.h" namespace centipede { @@ -90,47 +91,47 @@ Coverage::Coverage(const PCTable &pc_table, const PCIndexVec &pci_vec) void Coverage::DumpReportToFile(const SymbolTable &symbols, std::string_view filepath, std::string_view description) { - auto *file = RemoteFileOpen(filepath, "w"); + auto *file = ValueOrDie(RemoteFileOpen(filepath, "w")); CHECK(file != nullptr) << "Failed to open file: " << filepath; - RemoteFileSetWriteBufferSize(file, 100UL * 1024 * 1024); + CHECK_OK(RemoteFileSetWriteBufferSize(file, 100UL * 1024 * 1024)); if (!description.empty()) { - RemoteFileAppend(file, "# "); - RemoteFileAppend(file, std::string{description}); - RemoteFileAppend(file, ":\n\n"); + CHECK_OK(RemoteFileAppend(file, "# ")); + CHECK_OK(RemoteFileAppend(file, std::string{description})); + CHECK_OK(RemoteFileAppend(file, ":\n\n")); } // Print symbolized function names for all covered functions. for (auto pc_index : fully_covered_funcs) { - RemoteFileAppend(file, "FULL: "); - RemoteFileAppend(file, symbols.full_description(pc_index)); - RemoteFileAppend(file, "\n"); + CHECK_OK(RemoteFileAppend(file, "FULL: ")); + CHECK_OK(RemoteFileAppend(file, symbols.full_description(pc_index))); + CHECK_OK(RemoteFileAppend(file, "\n")); } - RemoteFileFlush(file); + CHECK_OK(RemoteFileFlush(file)); // Same for uncovered functions. for (auto pc_index : uncovered_funcs) { - RemoteFileAppend(file, "NONE: "); - RemoteFileAppend(file, symbols.full_description(pc_index)); - RemoteFileAppend(file, "\n"); + CHECK_OK(RemoteFileAppend(file, "NONE: ")); + CHECK_OK(RemoteFileAppend(file, symbols.full_description(pc_index))); + CHECK_OK(RemoteFileAppend(file, "\n")); } - RemoteFileFlush(file); + CHECK_OK(RemoteFileFlush(file)); // For every partially covered function, first print its name, // then print its covered edges, then uncovered edges. for (auto &pcf : partially_covered_funcs) { - RemoteFileAppend(file, "PARTIAL: "); - RemoteFileAppend(file, symbols.full_description(pcf.covered[0])); - RemoteFileAppend(file, "\n"); + CHECK_OK(RemoteFileAppend(file, "PARTIAL: ")); + CHECK_OK(RemoteFileAppend(file, symbols.full_description(pcf.covered[0]))); + CHECK_OK(RemoteFileAppend(file, "\n")); for (auto pc_index : pcf.covered) { - RemoteFileAppend(file, " + "); - RemoteFileAppend(file, symbols.full_description(pc_index)); - RemoteFileAppend(file, "\n"); + CHECK_OK(RemoteFileAppend(file, " + ")); + CHECK_OK(RemoteFileAppend(file, symbols.full_description(pc_index))); + CHECK_OK(RemoteFileAppend(file, "\n")); } for (auto pc_index : pcf.uncovered) { - RemoteFileAppend(file, " - "); - RemoteFileAppend(file, symbols.full_description(pc_index)); - RemoteFileAppend(file, "\n"); + CHECK_OK(RemoteFileAppend(file, " - ")); + CHECK_OK(RemoteFileAppend(file, symbols.full_description(pc_index))); + CHECK_OK(RemoteFileAppend(file, "\n")); } } - RemoteFileFlush(file); - RemoteFileClose(file); + CHECK_OK(RemoteFileFlush(file)); + CHECK_OK(RemoteFileClose(file)); } std::string CoverageLogger::ObserveAndDescribeIfNew(PCIndex pc_index) { diff --git a/centipede/distill.cc b/centipede/distill.cc index 5f797054..84e8c137 100644 --- a/centipede/distill.cc +++ b/centipede/distill.cc @@ -52,6 +52,7 @@ #include "./common/hash.h" #include "./common/logging.h" #include "./common/remote_file.h" +#include "./common/status_macros.h" namespace centipede { @@ -116,8 +117,10 @@ class InputCorpusShardReader { perf::MemSize EstimateRamFootprint(size_t shard_idx) const { const auto corpus_path = workdir_.CorpusFiles().ShardPath(shard_idx); const auto features_path = workdir_.FeaturesFiles().ShardPath(shard_idx); - const perf::MemSize corpus_file_size = RemoteFileGetSize(corpus_path); - const perf::MemSize features_file_size = RemoteFileGetSize(features_path); + const perf::MemSize corpus_file_size = + ValueOrDie(RemoteFileGetSize(corpus_path)); + const perf::MemSize features_file_size = + ValueOrDie(RemoteFileGetSize(features_path)); // Conservative compression factors for the two file types. These have been // observed empirically for the Riegeli blob format. The legacy format is // approximately 1:1, but use the stricter Riegeli numbers, as the legacy diff --git a/centipede/environment.cc b/centipede/environment.cc index f1fba174..9b1e3094 100644 --- a/centipede/environment.cc +++ b/centipede/environment.cc @@ -33,6 +33,7 @@ #include "./common/defs.h" #include "./common/logging.h" #include "./common/remote_file.h" +#include "./common/status_macros.h" namespace centipede { @@ -192,10 +193,10 @@ void Environment::ReadKnobsFileIfSpecified() { const std::string_view knobs_file_path = knobs_file; if (knobs_file_path.empty()) return; ByteArray knob_bytes; - auto *f = RemoteFileOpen(knobs_file, "r"); + auto *f = ValueOrDie(RemoteFileOpen(knobs_file, "r")); CHECK(f) << "Failed to open remote file " << knobs_file; - RemoteFileRead(f, knob_bytes); - RemoteFileClose(f); + CHECK_OK(RemoteFileRead(f, knob_bytes)); + CHECK_OK(RemoteFileClose(f)); VLOG(1) << "Knobs: " << knob_bytes.size() << " knobs read from " << knobs_file; knobs.Set(knob_bytes); diff --git a/centipede/seed_corpus_maker_lib.cc b/centipede/seed_corpus_maker_lib.cc index b94f5160..c0754656 100644 --- a/centipede/seed_corpus_maker_lib.cc +++ b/centipede/seed_corpus_maker_lib.cc @@ -92,7 +92,7 @@ SeedCorpusConfig ResolveSeedCorpusConfig( // LOG(INFO) << "Config spec points at an existing file; trying to parse " "textproto config from it: " << VV(config_spec); - RemoteFileGetContents(config_spec, config_str); + CHECK_OK(RemoteFileGetContents(config_spec, config_str)); LOG(INFO) << "Raw config read from file:\n" << config_str; base_dir = std::filesystem::path{config_spec}.parent_path(); } else { @@ -161,7 +161,7 @@ void SampleSeedCorpusElementsFromSource( // // `source.num_recent_dirs()` most recent ones. std::vector src_dirs; - RemoteGlobMatch(source.dir_glob(), src_dirs); + CHECK_OK(RemoteGlobMatch(source.dir_glob(), src_dirs)); LOG(INFO) << "Found " << src_dirs.size() << " corpus dir(s) matching " << source.dir_glob(); // Sort in the ascending lexicographical order. We expect that dir names @@ -179,7 +179,7 @@ void SampleSeedCorpusElementsFromSource( // const std::string shards_glob = fs::path{dir} / source.shard_rel_glob(); // NOTE: `RemoteGlobMatch` appends to the output list. const auto prev_num_shards = corpus_shard_fnames.size(); - RemoteGlobMatch(shards_glob, corpus_shard_fnames); + CHECK_OK(RemoteGlobMatch(shards_glob, corpus_shard_fnames)); LOG(INFO) << "Found " << (corpus_shard_fnames.size() - prev_num_shards) << " shard(s) matching " << shards_glob; } @@ -422,7 +422,9 @@ void WriteSeedCorpusElementsToDestination( // for (const auto& fname : {corpus_fname, features_fname}) { if (!fname.empty()) { const auto dir = fs::path{fname}.parent_path().string(); - if (!RemotePathExists(dir)) RemoteMkdir(dir); + if (!RemotePathExists(dir)) { + CHECK_OK(RemoteMkdir(dir)); + } } } @@ -502,7 +504,7 @@ void GenerateSeedCorpusFromConfig( // std::string_view override_out_dir) { // Pre-create the destination dir early to catch possible misspellings etc. if (!RemotePathExists(config.destination().dir_path())) { - RemoteMkdir(config.destination().dir_path()); + CHECK_OK(RemoteMkdir(config.destination().dir_path())); } // Dump the config to the debug info dir in the destination. @@ -513,9 +515,9 @@ void GenerateSeedCorpusFromConfig( // /*my_shard_index=*/0, }; const std::filesystem::path debug_info_dir = workdir.DebugInfoDirPath(); - RemoteMkdir(debug_info_dir.c_str()); - RemoteFileSetContents((debug_info_dir / "seeding.cfg").c_str(), - absl::StrCat(config)); + CHECK_OK(RemoteMkdir(debug_info_dir.c_str())); + CHECK_OK(RemoteFileSetContents((debug_info_dir / "seeding.cfg").c_str(), + absl::StrCat(config))); InputAndFeaturesVec elements; diff --git a/centipede/stats.cc b/centipede/stats.cc index 2f968017..dbffda21 100644 --- a/centipede/stats.cc +++ b/centipede/stats.cc @@ -172,7 +172,7 @@ void StatsLogger::DoneFieldSamplesBatch() { StatsCsvFileAppender::~StatsCsvFileAppender() { for (const auto &[group_name, file] : files_) { - RemoteFileClose(file.file); + CHECK_OK(RemoteFileClose(file.file)); } } @@ -206,20 +206,20 @@ void StatsCsvFileAppender::SetCurrGroup(const Environment &master_env) { bool append = false; if (RemotePathExists(filename)) { std::string contents; - RemoteFileGetContents(filename, contents); + CHECK_OK(RemoteFileGetContents(filename, contents)); // NOTE: `csv_header_` ends with '\n', so the match is exact. if (absl::StartsWith(contents, csv_header_)) { append = true; } else { append = false; - RemoteFileSetContents(GetBackupFilename(filename), contents); + CHECK_OK(RemoteFileSetContents(GetBackupFilename(filename), contents)); } } - file.file = RemoteFileOpen(filename, append ? "a" : "w"); + file.file = *RemoteFileOpen(filename, append ? "a" : "w"); CHECK(file.file != nullptr) << VV(filename); if (!append) { - RemoteFileAppend(file.file, csv_header_); - RemoteFileFlush(file.file); + CHECK_OK(RemoteFileAppend(file.file, csv_header_)); + CHECK_OK(RemoteFileFlush(file.file)); } } curr_file_ = &file; @@ -260,8 +260,8 @@ void StatsCsvFileAppender::ReportFlags(const GroupToFlags &group_to_flags) { void StatsCsvFileAppender::DoneFieldSamplesBatch() { for (auto &&[group_name, file] : files_) { - RemoteFileAppend(file.file, absl::StrCat(file.buffer, "\n")); - RemoteFileFlush(file.file); + CHECK_OK(RemoteFileAppend(file.file, absl::StrCat(file.buffer, "\n"))); + CHECK_OK(RemoteFileFlush(file.file)); file.buffer.clear(); } } diff --git a/centipede/util.cc b/centipede/util.cc index fca98319..09ecb6c2 100644 --- a/centipede/util.cc +++ b/centipede/util.cc @@ -140,13 +140,14 @@ void WriteToLocalHashedFileInDir(std::string_view dir_path, ByteSpan data) { void WriteToRemoteHashedFileInDir(std::string_view dir_path, ByteSpan data) { if (dir_path.empty()) return; std::string file_path = std::filesystem::path(dir_path).append(Hash(data)); - RemoteFileSetContents(file_path, std::string(data.begin(), data.end())); + CHECK_OK( + RemoteFileSetContents(file_path, std::string(data.begin(), data.end()))); } std::string HashOfFileContents(std::string_view file_path) { if (file_path.empty()) return ""; std::string file_contents; - RemoteFileGetContents(file_path, file_contents); + CHECK_OK(RemoteFileGetContents(file_path, file_contents)); return Hash(file_contents); } diff --git a/common/BUILD b/common/BUILD index 34c69f4b..136a8114 100644 --- a/common/BUILD +++ b/common/BUILD @@ -49,6 +49,7 @@ cc_library( ":hash", ":logging", ":remote_file", + ":status_macros", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/log", "@com_google_absl//absl/log:check", @@ -107,8 +108,12 @@ cc_library( deps = [ ":defs", ":logging", + ":status_macros", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/log:check", + "@com_google_absl//absl/status", + "@com_google_absl//absl/status:statusor", + "@com_google_absl//absl/strings", ] + select({ "//conditions:default": [":remote_file_oss"], }) + @@ -135,9 +140,13 @@ cc_library( deps = [ ":defs", ":logging", + ":status_macros", "@com_google_absl//absl/base:nullability", "@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", ] + select({ "@com_google_fuzztest//fuzztest:disable_riegeli": [], "//conditions:default": [ @@ -149,6 +158,16 @@ cc_library( }), ) +cc_library( + name = "status_macros", + hdrs = ["status_macros.h"], + deps = [ + "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/log", + "@com_google_absl//absl/types:source_location", + ], +) + cc_library( name = "test_util", testonly = True, diff --git a/common/blob_file.cc b/common/blob_file.cc index 9fafe8e5..50110518 100644 --- a/common/blob_file.cc +++ b/common/blob_file.cc @@ -37,6 +37,7 @@ #include "./common/hash.h" #include "./common/logging.h" #include "./common/remote_file.h" +#include "./common/status_macros.h" #ifndef CENTIPEDE_DISABLE_RIEGELI #include "riegeli/base/object.h" #include "riegeli/base/types.h" @@ -76,14 +77,15 @@ class SimpleBlobFileReader : public BlobFileReader { absl::Status Open(std::string_view path) override { if (closed_) return absl::FailedPreconditionError("already closed"); if (file_) return absl::FailedPreconditionError("already open"); - file_ = RemoteFileOpen(path, "r"); + ASSIGN_OR_RETURN_IF_NOT_OK(file_, RemoteFileOpen(path, "r")); if (file_ == nullptr) return absl::UnknownError("can't open file"); // Read the entire file at once. // It may be useful to read the file in chunks, but if we are going // to migrate to something else, it's not important here. ByteArray raw_bytes; - RemoteFileRead(file_, raw_bytes); - RemoteFileClose(file_); // close the file here, we won't need it. + RETURN_IF_NOT_OK(RemoteFileRead(file_, raw_bytes)); + RETURN_IF_NOT_OK( + RemoteFileClose(file_)); // close the file here, we won't need it. UnpackBytesFromAppendFile(raw_bytes, &unpacked_blobs_); return absl::OkStatus(); } @@ -130,9 +132,9 @@ class SimpleBlobFileWriter : public BlobFileWriter { CHECK(mode == "w" || mode == "a") << VV(mode); if (closed_) return absl::FailedPreconditionError("already closed"); if (file_) return absl::FailedPreconditionError("already open"); - file_ = RemoteFileOpen(path, mode.data()); + ASSIGN_OR_RETURN_IF_NOT_OK(file_, RemoteFileOpen(path, mode.data())); if (file_ == nullptr) return absl::UnknownError("can't open file"); - RemoteFileSetWriteBufferSize(file_, kMaxBufferedBytes); + RETURN_IF_NOT_OK(RemoteFileSetWriteBufferSize(file_, kMaxBufferedBytes)); return absl::OkStatus(); } @@ -140,8 +142,8 @@ class SimpleBlobFileWriter : public BlobFileWriter { if (closed_) return absl::FailedPreconditionError("already closed"); if (!file_) return absl::FailedPreconditionError("was not open"); ByteArray packed = PackBytesForAppendFile(blob); - RemoteFileAppend(file_, packed); - RemoteFileFlush(file_); + RETURN_IF_NOT_OK(RemoteFileAppend(file_, packed)); + RETURN_IF_NOT_OK(RemoteFileFlush(file_)); return absl::OkStatus(); } @@ -149,7 +151,7 @@ class SimpleBlobFileWriter : public BlobFileWriter { if (closed_) return absl::FailedPreconditionError("already closed"); if (!file_) return absl::FailedPreconditionError("was not open"); closed_ = true; - RemoteFileClose(file_); + RETURN_IF_NOT_OK(RemoteFileClose(file_)); return absl::OkStatus(); } @@ -174,7 +176,9 @@ class DefaultBlobFileReader : public BlobFileReader { if (absl::Status s = Close(); !s.ok()) return s; #ifndef CENTIPEDE_DISABLE_RIEGELI - riegeli_reader_.Reset(CreateRiegeliFileReader(path)); + ASSIGN_OR_RETURN_IF_NOT_OK(std::unique_ptr new_reader, + CreateRiegeliFileReader(path)); + riegeli_reader_.Reset(std::move(new_reader)); if (riegeli_reader_.CheckFileFormat()) [[likely]] { // File could be opened and is in the Riegeli format. return absl::OkStatus(); @@ -277,8 +281,10 @@ class RiegeliWriter : public BlobFileWriter { if (absl::Status s = Close(); !s.ok()) return s; const auto kWriterOpts = riegeli::RecordWriterBase::Options{}.set_chunk_size(kMaxBufferedBytes); - writer_.Reset(CreateRiegeliFileWriter(path, mode == "a"), kWriterOpts); - if (!writer_.ok()) return writer_.status(); + ASSIGN_OR_RETURN_IF_NOT_OK(std::unique_ptr new_writer, + CreateRiegeliFileWriter(path, mode == "a")); + writer_.Reset(std::move(new_writer), kWriterOpts); + RETURN_IF_NOT_OK(writer_.status()); path_ = path; opened_at_ = absl::Now(); flushed_at_ = absl::Now(); diff --git a/common/remote_file.cc b/common/remote_file.cc index eb761e6a..633cd0a6 100644 --- a/common/remote_file.cc +++ b/common/remote_file.cc @@ -22,49 +22,69 @@ #include "absl/base/nullability.h" #include "absl/log/check.h" +#include "absl/status/status.h" #include "./common/defs.h" #include "./common/logging.h" +#include "./common/status_macros.h" namespace centipede { -void RemoteFileAppend(absl::Nonnull f, - const std::string &contents) { +absl::Status RemoteFileAppend(absl::Nonnull f, + const std::string &contents) { ByteArray contents_ba{contents.cbegin(), contents.cend()}; - RemoteFileAppend(f, contents_ba); + return RemoteFileAppend(f, contents_ba); } -void RemoteFileRead(absl::Nonnull f, std::string &contents) { +absl::Status RemoteFileRead(absl::Nonnull f, + std::string &contents) { ByteArray contents_ba; - RemoteFileRead(f, contents_ba); + RETURN_IF_NOT_OK(RemoteFileRead(f, contents_ba)); contents.assign(contents_ba.cbegin(), contents_ba.cend()); + return absl::OkStatus(); } -void RemoteFileSetContents(std::string_view path, const ByteArray &contents) { - auto *file = RemoteFileOpen(path, "w"); - CHECK(file != nullptr) << VV(path); - RemoteFileAppend(file, contents); - RemoteFileClose(file); +absl::Status RemoteFileSetContents(std::string_view path, + const ByteArray &contents) { + ASSIGN_OR_RETURN_IF_NOT_OK(RemoteFile * file, RemoteFileOpen(path, "w")); + if (file == nullptr) { + return absl::UnknownError( + "RemoteFileOpen returned an OK status but a nullptr RemoteFile*"); + } + RETURN_IF_NOT_OK(RemoteFileAppend(file, contents)); + return RemoteFileClose(file); } -void RemoteFileSetContents(std::string_view path, const std::string &contents) { - auto *file = RemoteFileOpen(path, "w"); - CHECK(file != nullptr) << VV(path); - RemoteFileAppend(file, contents); - RemoteFileClose(file); +absl::Status RemoteFileSetContents(std::string_view path, + const std::string &contents) { + ASSIGN_OR_RETURN_IF_NOT_OK(RemoteFile * file, RemoteFileOpen(path, "w")); + if (file == nullptr) { + return absl::UnknownError( + "RemoteFileOpen returned an OK status but a nullptr RemoteFile*"); + } + RETURN_IF_NOT_OK(RemoteFileAppend(file, contents)); + RETURN_IF_NOT_OK(RemoteFileClose(file)); + return absl::OkStatus(); } -void RemoteFileGetContents(std::string_view path, ByteArray &contents) { - auto *file = RemoteFileOpen(path, "r"); - CHECK(file != nullptr) << VV(path); - RemoteFileRead(file, contents); - RemoteFileClose(file); +absl::Status RemoteFileGetContents(std::string_view path, ByteArray &contents) { + ASSIGN_OR_RETURN_IF_NOT_OK(RemoteFile * file, RemoteFileOpen(path, "r")); + if (file == nullptr) { + return absl::UnknownError( + "RemoteFileOpen returned an OK status but a nullptr RemoteFile*"); + } + RETURN_IF_NOT_OK(RemoteFileRead(file, contents)); + return RemoteFileClose(file); } -void RemoteFileGetContents(std::string_view path, std::string &contents) { - auto *file = RemoteFileOpen(path, "r"); - CHECK(file != nullptr) << VV(path); - RemoteFileRead(file, contents); - RemoteFileClose(file); +absl::Status RemoteFileGetContents(std::string_view path, + std::string &contents) { + ASSIGN_OR_RETURN_IF_NOT_OK(RemoteFile * file, RemoteFileOpen(path, "r")); + if (file == nullptr) { + return absl::UnknownError( + "RemoteFileOpen returned an OK status but a nullptr RemoteFile*"); + } + RETURN_IF_NOT_OK(RemoteFileRead(file, contents)); + return RemoteFileClose(file); } } // namespace centipede diff --git a/common/remote_file.h b/common/remote_file.h index 6e54190e..627109e2 100644 --- a/common/remote_file.h +++ b/common/remote_file.h @@ -28,6 +28,8 @@ #include #include "absl/base/nullability.h" +#include "absl/status/status.h" +#include "absl/status/statusor.h" #include "./common/defs.h" #ifndef CENTIPEDE_DISABLE_RIEGELI #include "riegeli/bytes/reader.h" @@ -52,52 +54,58 @@ struct RemoteFile {}; // Opens a (potentially remote) file 'file_path' and returns a handle to it. // Supported modes: "r", "a", "w", same as in C FILE API. -absl::Nullable RemoteFileOpen(std::string_view file_path, +absl::StatusOr RemoteFileOpen(std::string_view file_path, const char *mode); // Closes the file previously opened by RemoteFileOpen. -void RemoteFileClose(absl::Nonnull f); +absl::Status RemoteFileClose(absl::Nonnull f); // Adjusts the buffered I/O capacity for a file opened for writing. By default, // the internal buffer of size `BUFSIZ` is used. May only be used after opening // a file, but before performing any other operations on it. Violating this // requirement in general can cause undefined behavior. -void RemoteFileSetWriteBufferSize(absl::Nonnull f, size_t size); +absl::Status RemoteFileSetWriteBufferSize(absl::Nonnull f, + size_t size); // Appends bytes from 'ba' to 'f'. -void RemoteFileAppend(absl::Nonnull f, const ByteArray &ba); +absl::Status RemoteFileAppend(absl::Nonnull f, + const ByteArray &ba); // Appends characters from 'contents' to 'f'. -void RemoteFileAppend(absl::Nonnull f, - const std::string &contents); +absl::Status RemoteFileAppend(absl::Nonnull f, + const std::string &contents); // Flushes the file's internal buffer. Some dynamic results of a running // pipeline are consumed by itself (e.g. shard cross-pollination) and can be // consumed by external processes (e.g. monitoring): for such files, call this // API after every write to ensure that they are in a valid state. -void RemoteFileFlush(absl::Nonnull f); +absl::Status RemoteFileFlush(absl::Nonnull f); // Reads all current contents of 'f' into 'ba'. -void RemoteFileRead(absl::Nonnull f, ByteArray &ba); +absl::Status RemoteFileRead(absl::Nonnull f, ByteArray &ba); // Reads all current contents of 'f' into 'contents'. -void RemoteFileRead(absl::Nonnull f, std::string &contents); +absl::Status RemoteFileRead(absl::Nonnull f, + std::string &contents); // Creates a (potentially remote) directory 'dir_path', as well as any missing // parent directories. No-op if the directory already exists. -void RemoteMkdir(std::string_view dir_path); +absl::Status RemoteMkdir(std::string_view dir_path); // Sets the contents of the file at 'path' to 'contents'. -void RemoteFileSetContents(std::string_view path, const ByteArray &contents); +absl::Status RemoteFileSetContents(std::string_view path, + const ByteArray &contents); // Sets the contents of the file at 'path' to 'contents'. -void RemoteFileSetContents(std::string_view path, const std::string &contents); +absl::Status RemoteFileSetContents(std::string_view path, + const std::string &contents); // Reads the contents of the file at 'path' into 'contents'. -void RemoteFileGetContents(std::string_view path, ByteArray &contents); +absl::Status RemoteFileGetContents(std::string_view path, ByteArray &contents); // Reads the contents of the file at 'path' into 'contents'. -void RemoteFileGetContents(std::string_view path, std::string &contents); +absl::Status RemoteFileGetContents(std::string_view path, + std::string &contents); // Returns true if `path` exists. bool RemotePathExists(std::string_view path); @@ -106,7 +114,7 @@ bool RemotePathExists(std::string_view path); bool RemotePathIsDirectory(std::string_view path); // Returns the size of the file at `path` in bytes. The file must exist. -int64_t RemoteFileGetSize(std::string_view path); +absl::StatusOr RemoteFileGetSize(std::string_view path); // Finds all files matching `glob` and appends them to `matches`. // @@ -115,31 +123,32 @@ int64_t RemoteFileGetSize(std::string_view path); // be solved in a more specific way, e.g., by listing files in a directory and // filtering based on some criterion. [[deprecated("Do not use in new code.")]] -void RemoteGlobMatch(std::string_view glob, std::vector &matches); +absl::Status RemoteGlobMatch(std::string_view glob, + std::vector &matches); // Lists all files within `path`, recursively expanding subdirectories if // `recursively` is true. Does not return any directories. Returns an empty // vector if `path` is an empty directory, or `path` does not exist. Returns // `{path}` if `path` is a non-directory. -std::vector RemoteListFiles(std::string_view path, - bool recursively); +absl::StatusOr> RemoteListFiles(std::string_view path, + bool recursively); // Renames `from` to `to`. -void RemotePathRename(std::string_view from, std::string_view to); +absl::Status RemotePathRename(std::string_view from, std::string_view to); // Deletes `path`. If `path` is a directory and `recursively` is true, // recursively deletes all files and subdirectories within `path`. -void RemotePathDelete(std::string_view path, bool recursively); +absl::Status RemotePathDelete(std::string_view path, bool recursively); #ifndef CENTIPEDE_DISABLE_RIEGELI // Returns a reader for the file at `file_path`. -std::unique_ptr CreateRiegeliFileReader( +absl::StatusOr> CreateRiegeliFileReader( std::string_view file_path); // Returns a writer for the file at `file_path`. // If `append` is `true`, writes will append to the end of the file if it // exists. If `false, the file will be truncated to empty if it exists. -std::unique_ptr CreateRiegeliFileWriter( +absl::StatusOr> CreateRiegeliFileWriter( std::string_view file_path, bool append); #endif // CENTIPEDE_DISABLE_RIEGELI diff --git a/common/remote_file_oss.cc b/common/remote_file_oss.cc index a49c7c2e..38fcaa6a 100644 --- a/common/remote_file_oss.cc +++ b/common/remote_file_oss.cc @@ -20,8 +20,10 @@ #define FUZZTEST_HAS_OSS_GLOB #endif // !defined(_MSC_VER) && !defined(__ANDROID__) && !defined(__Fuchsia__) +#include #include #include +#include #include // NOLINT #include #include @@ -33,9 +35,12 @@ #include "absl/base/nullability.h" #include "absl/log/check.h" #include "absl/log/log.h" +#include "absl/status/status.h" +#include "absl/strings/str_cat.h" #include "./common/defs.h" #include "./common/logging.h" #include "./common/remote_file.h" +#include "./common/status_macros.h" #ifndef CENTIPEDE_DISABLE_RIEGELI #include "riegeli/bytes/fd_reader.h" #include "riegeli/bytes/fd_writer.h" @@ -48,11 +53,12 @@ namespace { class LocalRemoteFile : public RemoteFile { public: - static LocalRemoteFile *Create(std::string path, std::string_view mode) { + static absl::StatusOr Create(std::string path, + std::string_view mode) { FILE *file = std::fopen(path.c_str(), mode.data()); if (file == nullptr) { - LOG(ERROR) << "Failed to open file: " << VV(path); - return nullptr; + return absl::UnknownError(absl::StrCat( + "fopen() failed, path: ", path, ", errno: ", std::strerror(errno))); } return new LocalRemoteFile{std::move(path), file}; } @@ -67,41 +73,77 @@ class LocalRemoteFile : public RemoteFile { LocalRemoteFile(LocalRemoteFile &&) = default; LocalRemoteFile &operator=(LocalRemoteFile &&) = default; - void SetWriteBufSize(size_t size) { - CHECK(write_buf_ == nullptr) << "SetWriteBufCapacity called twice"; + absl::Status SetWriteBufSize(size_t size) { + if (write_buf_ != nullptr) { + return absl::FailedPreconditionError("SetWriteBufCapacity called twice"); + } write_buf_ = std::make_unique(size); - CHECK_EQ(std::setvbuf(file_, write_buf_.get(), _IOFBF, size), 0) - << VV(path_); + if (std::setvbuf(file_, write_buf_.get(), _IOFBF, size) != 0) { + return absl::UnknownError( + absl::StrCat("std::setvbuf failed, path: ", path_, + ", errno: ", std::strerror(errno))); + } + return absl::OkStatus(); } - void Write(const ByteArray &ba) { + absl::Status Write(const ByteArray &ba) { static constexpr auto elt_size = sizeof(ba[0]); const auto elts_to_write = ba.size(); const auto elts_written = std::fwrite(ba.data(), elt_size, elts_to_write, file_); - CHECK_EQ(elts_written, elts_to_write) << VV(path_); + if (elts_written != elts_to_write) { + return absl::UnknownError(absl::StrCat( + "fwrite() wrote less elements that expected, wrote: ", elts_written, + ", expected: ", elts_to_write, ", path: ", path_)); + } + return absl::OkStatus(); } - void Flush() { std::fflush(file_); } + absl::Status Flush() { + if (std::fflush(file_) != 0) { + return absl::UnknownError("fflush() failed"); + } + return absl::OkStatus(); + } - void Read(ByteArray &ba) { + absl::Status Read(ByteArray &ba) { // Compute the file size as a difference between the end and start offsets. - CHECK_EQ(std::fseek(file_, 0, SEEK_END), 0) << VV(path_); + if (std::fseek(file_, 0, SEEK_END), 0 != 0) { + return absl::UnknownError(absl::StrCat("fseek() failed on path: ", path_, + ": ", std::strerror(errno))); + } const auto file_size = std::ftell(file_); - CHECK_EQ(std::fseek(file_, 0, SEEK_SET), 0) << VV(path_); + if (std::fseek(file_, 0, SEEK_SET), 0) { + return absl::UnknownError(absl::StrCat("fseek() failed on path: ", path_, + ": ", std::strerror(errno))); + } static constexpr auto elt_size = sizeof(ba[0]); CHECK_EQ(file_size % elt_size, 0) << VV(file_size) << VV(elt_size) << VV(path_); + if (file_size % elt_size != 0) { + return absl::FailedPreconditionError( + absl::StrCat("Attempting to read a file with inconsistent element (", + elt_size, ") and file size (", file_size, "): ", path_)); + } const auto elts_to_read = file_size / elt_size; ba.resize(elts_to_read); const auto elts_read = std::fread(ba.data(), elt_size, elts_to_read, file_); - CHECK_EQ(elts_read, elts_to_read) << VV(path_); + if (elts_read != elts_to_read) { + return absl::UnknownError(absl::StrCat( + "fread() read less elements that expected, wrote: ", elts_read, + ", expected: ", elts_to_read, ", path: ", path_)); + } + return absl::OkStatus(); } - void Close() { - CHECK_EQ(std::fclose(file_), 0) << VV(path_); + absl::Status Close() { + if (std::fclose(file_) != 0) { + return absl::UnknownError(absl::StrCat("fclose() failed on path: ", path_, + ": ", std::strerror(errno))); + } file_ = nullptr; write_buf_ = nullptr; + return absl::OkStatus(); } private: @@ -117,38 +159,45 @@ class LocalRemoteFile : public RemoteFile { #if defined(FUZZTEST_STUB_STD_FILESYSTEM) -void RemoteMkdir(std::string_view path) { +absl::Status RemoteMkdir(std::string_view path) { LOG(FATAL) << "Filesystem API not supported in iOS/MacOS"; } -bool RemotePathExists(std::string_view path) { +absl::Status RemotePathExists(std::string_view path) { LOG(FATAL) << "Filesystem API not supported in iOS/MacOS"; } -bool RemotePathIsDirectory(std::string_view path) { +absl::Status RemotePathIsDirectory(std::string_view path) { LOG(FATAL) << "Filesystem API not supported in iOS/MacOS"; } -std::vector RemoteListFiles(std::string_view path, - bool recursively) { +absl::StatusOr> RemoteListFiles(std::string_view path, + bool recursively) { LOG(FATAL) << "Filesystem API not supported in iOS/MacOS"; } -void RemotePathRename(std::string_view from, std::string_view to) { +absl::Status RemotePathRename(std::string_view from, std::string_view to) { LOG(FATAL) << "Filesystem API not supported in iOS/MacOS"; } -void RemotePathDelete(std::string_view path, bool recursively) { +absl::Status RemotePathDelete(std::string_view path, bool recursively) { LOG(FATAL) << "Filesystem API not supported in iOS/MacOS"; } #else -void RemoteMkdir(std::string_view path) { - CHECK(!path.empty()); +absl::Status RemoteMkdir(std::string_view path) { + if (path.empty()) { + return absl::InvalidArgumentError("Unable to RemoteMkdir() an empty path"); + } std::error_code error; std::filesystem::create_directories(path, error); - CHECK(!error) << VV(path) << VV(error); + if (error) { + return absl::UnknownError( + absl::StrCat("create_directories() failed, path: ", path, + ", error: ", error.message())); + } + return absl::OkStatus(); } bool RemotePathExists(std::string_view path) { @@ -159,9 +208,9 @@ bool RemotePathIsDirectory(std::string_view path) { return std::filesystem::is_directory(path); } -std::vector RemoteListFiles(std::string_view path, - bool recursively) { - if (!std::filesystem::exists(path)) return {}; +absl::StatusOr> RemoteListFiles(std::string_view path, + bool recursively) { + if (!std::filesystem::exists(path)) return std::vector(); auto list_files = [](auto dir_iter) { std::vector ret; for (const auto &entry : dir_iter) { @@ -177,20 +226,30 @@ std::vector RemoteListFiles(std::string_view path, : list_files(std::filesystem::directory_iterator(path)); } -void RemotePathRename(std::string_view from, std::string_view to) { +absl::Status RemotePathRename(std::string_view from, std::string_view to) { std::error_code error; std::filesystem::rename(from, to, error); - CHECK(!error) << VV(from) << VV(to) << VV(error); + if (error) { + return absl::UnknownError( + absl::StrCat("filesystem::rename() failed, from: ", from, ", to: ", to, + ", error: ", error.message())); + } + return absl::OkStatus(); } -void RemotePathDelete(std::string_view path, bool recursively) { +absl::Status RemotePathDelete(std::string_view path, bool recursively) { std::error_code error; if (recursively) { std::filesystem::remove_all(path, error); } else { std::filesystem::remove(path, error); } - CHECK(!error) << VV(path) << VV(error); + if (error) { + return absl::UnknownError(absl::StrCat( + "filesystem::remove() or remove_all() failed, path: ", path, + ", error: ", error.message())); + } + return absl::OkStatus(); } #endif // defined(FUZZTEST_STUB_STD_FILESYSTEM) @@ -198,38 +257,51 @@ void RemotePathDelete(std::string_view path, bool recursively) { // TODO(ussuri): For now, simulate the old behavior, where a failure to open // a file returned nullptr. Adjust the clients to expect non-null and use a // normal ctor with a CHECK instead of `Create()` here instead. -absl::Nullable RemoteFileOpen(std::string_view path, +absl::StatusOr RemoteFileOpen(std::string_view path, const char *mode) { return LocalRemoteFile::Create(std::string(path), mode); } -void RemoteFileClose(absl::Nonnull f) { +absl::Status RemoteFileClose(absl::Nonnull f) { auto *file = static_cast(f); - file->Close(); + RETURN_IF_NOT_OK(file->Close()); delete file; + return absl::OkStatus(); } -void RemoteFileSetWriteBufferSize(absl::Nonnull f, size_t size) { - static_cast(f)->SetWriteBufSize(size); +absl::Status RemoteFileSetWriteBufferSize(absl::Nonnull f, + size_t size) { + return static_cast(f)->SetWriteBufSize(size); } -void RemoteFileAppend(absl::Nonnull f, const ByteArray &ba) { - static_cast(f)->Write(ba); +absl::Status RemoteFileAppend(absl::Nonnull f, + const ByteArray &ba) { + return static_cast(f)->Write(ba); } -void RemoteFileFlush(absl::Nonnull f) { - static_cast(f)->Flush(); +absl::Status RemoteFileFlush(absl::Nonnull f) { + return static_cast(f)->Flush(); } -void RemoteFileRead(absl::Nonnull f, ByteArray &ba) { - static_cast(f)->Read(ba); +absl::Status RemoteFileRead(absl::Nonnull f, ByteArray &ba) { + return static_cast(f)->Read(ba); } -int64_t RemoteFileGetSize(std::string_view path) { +absl::StatusOr RemoteFileGetSize(std::string_view path) { FILE *f = std::fopen(path.data(), "r"); - CHECK(f != nullptr) << VV(path); - std::fseek(f, 0, SEEK_END); + if (f == nullptr) { + return absl::UnknownError(absl::StrCat("fopen() failed, path: ", path, + ", errno: ", std::strerror(errno))); + } + if (std::fseek(f, 0, SEEK_END) != 0) { + return absl::UnknownError(absl::StrCat("fseek() failed, path: ", path, + ", errno: ", std::strerror(errno))); + } const auto sz = std::ftell(f); + if (sz == -1L) { + return absl::UnknownError(absl::StrCat("ftell() failed, path: ", path, + ", errno: ", std::strerror(errno))); + } std::fclose(f); return sz; } @@ -245,33 +317,41 @@ int HandleGlobError(const char *epath, int eerrno) { } // namespace -void RemoteGlobMatch(std::string_view glob, std::vector &matches) { +absl::Status RemoteGlobMatch(std::string_view glob, + std::vector &matches) { #if defined(FUZZTEST_HAS_OSS_GLOB) // See `man glob.3`. ::glob_t glob_ret = {}; - CHECK_EQ( - ::glob(std::string{glob}.c_str(), GLOB_TILDE, HandleGlobError, &glob_ret), - 0) - << "Error while globbing glob: " << VV(glob); + if (int ret = ::glob(std::string{glob}.c_str(), GLOB_TILDE, HandleGlobError, + &glob_ret); + ret != 0) { + return absl::UnknownError( + absl::StrCat("glob() failed, pattern: ", glob, ", returned: ", ret)); + } for (int i = 0; i < glob_ret.gl_pathc; ++i) { matches.emplace_back(glob_ret.gl_pathv[i]); } ::globfree(&glob_ret); + return absl::OkStatus(); #else LOG(FATAL) << __func__ << "() is not supported on this platform."; #endif // defined(FUZZTEST_HAS_OSS_GLOB) } #ifndef CENTIPEDE_DISABLE_RIEGELI -std::unique_ptr CreateRiegeliFileReader( +absl::StatusOr> CreateRiegeliFileReader( std::string_view file_path) { - return std::make_unique>(file_path); + auto ret = std::make_unique>(file_path); + RETURN_IF_NOT_OK(ret->status()); + return ret; } -std::unique_ptr CreateRiegeliFileWriter( +absl::StatusOr> CreateRiegeliFileWriter( std::string_view file_path, bool append) { - return std::make_unique>( + auto ret = std::make_unique>( file_path, riegeli::FdWriterBase::Options().set_append(append)); + RETURN_IF_NOT_OK(ret->status()); + return ret; } #endif // CENTIPEDE_DISABLE_RIEGELI diff --git a/common/remote_file_test.cc b/common/remote_file_test.cc index f6334d72..51ea601e 100644 --- a/common/remote_file_test.cc +++ b/common/remote_file_test.cc @@ -32,6 +32,8 @@ namespace fs = std::filesystem; using ::testing::IsEmpty; using ::testing::UnorderedElementsAre; +using ::testing::status::IsOk; +using ::testing::status::IsOkAndHolds; void CreateFileOrDie(std::string_view path, std::string_view contents = "") { std::ofstream f{std::string(path)}; @@ -46,12 +48,14 @@ TEST(RemoteFile, GetSize) { { const std::string file_contents1 = "abcd1234"; CreateFileOrDie(file_path, file_contents1); - EXPECT_EQ(RemoteFileGetSize(file_path), file_contents1.size()); + EXPECT_THAT(RemoteFileGetSize(file_path), + IsOkAndHolds(file_contents1.size())); } { const std::string file_contents2 = "efg567"; - RemoteFileSetContents(file_path, file_contents2); - EXPECT_EQ(RemoteFileGetSize(file_path), file_contents2.size()); + ASSERT_THAT(RemoteFileSetContents(file_path, file_contents2), IsOk()); + EXPECT_THAT(RemoteFileGetSize(file_path), + IsOkAndHolds(file_contents2.size())); } } @@ -59,7 +63,7 @@ TEST(RemoteMkdir, CreatesMissingParentDirectories) { const fs::path temp_dir = GetTestTempDir(test_info_->name()); const std::string dir_path = temp_dir / "a" / "b" / "c"; - RemoteMkdir(dir_path); + ASSERT_THAT(RemoteMkdir(dir_path), IsOk()); EXPECT_TRUE(fs::exists(dir_path)); } @@ -74,7 +78,7 @@ TEST(RemoteListFiles, DoesNotRecurseIntoSubdirectories) { CreateFileOrDie(file2_path); EXPECT_THAT(RemoteListFiles(temp_dir.string(), /*recursively=*/false), - UnorderedElementsAre(file1_path)); + IsOkAndHolds(UnorderedElementsAre(file1_path))); } TEST(RemoteListFiles, ListsFilesInRecursiveDirectories) { @@ -90,14 +94,15 @@ TEST(RemoteListFiles, ListsFilesInRecursiveDirectories) { const std::string file3_path = dir1_path / "file_03"; CreateFileOrDie(file3_path); - EXPECT_THAT(RemoteListFiles(temp_dir.string(), /*recursively=*/true), - UnorderedElementsAre(file1_path, file2_path, file3_path)); + EXPECT_THAT( + RemoteListFiles(temp_dir.string(), /*recursively=*/true), + IsOkAndHolds(UnorderedElementsAre(file1_path, file2_path, file3_path))); } TEST(RemoteListFiles, ReturnsAnEmptyResultWhenNoFilesAreFound) { const fs::path temp_dir = GetTestTempDir(test_info_->name()); EXPECT_THAT(RemoteListFiles(temp_dir.string(), /*recursively=*/false), - IsEmpty()); + IsOkAndHolds(IsEmpty())); } TEST(RemoteListFiles, ReturnsASingleFileWhenListingAFile) { @@ -107,13 +112,13 @@ TEST(RemoteListFiles, ReturnsASingleFileWhenListingAFile) { CreateFileOrDie(file1_path); EXPECT_THAT(RemoteListFiles(temp_dir.string(), /*recursively=*/false), - UnorderedElementsAre(file1_path)); + IsOkAndHolds(UnorderedElementsAre(file1_path))); } TEST(RemoteListFiles, ReturnsAnEmptyVectorWhenPathDoesNotExist) { EXPECT_THAT( RemoteListFiles("/this/file/path/does/not/exist", /*recursively=*/false), - IsEmpty()); + IsOkAndHolds(IsEmpty())); } TEST(RemotePathDelete, RecursivelyDeletesAllFilesAndSubdirectories) { @@ -123,7 +128,8 @@ TEST(RemotePathDelete, RecursivelyDeletesAllFilesAndSubdirectories) { const std::string file_path = a_b_c / "file"; CreateFileOrDie(file_path); - RemotePathDelete(temp_dir.string(), /*recursively=*/true); + ASSERT_THAT(RemotePathDelete(temp_dir.string(), /*recursively=*/true), + IsOk()); EXPECT_FALSE(fs::exists(a_b_c)); } diff --git a/common/status_macros.h b/common/status_macros.h new file mode 100644 index 00000000..48c9dd2e --- /dev/null +++ b/common/status_macros.h @@ -0,0 +1,64 @@ +// Copyright 2024 The Centipede Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Convenience macros for dealing with absl::Status and friends. + +#ifndef FUZZTEST_COMMON_STATUS_MACROS_H_ +#define FUZZTEST_COMMON_STATUS_MACROS_H_ + +#include "absl/base/attributes.h" +#include "absl/base/optimization.h" +#include "absl/log/log.h" +#include "absl/types/source_location.h" + +// If `status_expr` (an expression of type `absl::Status`) is not OK then return +// it from the current function. Otherwise, do nothing. +#define RETURN_IF_NOT_OK(status_expr) \ + do { \ + const absl::Status status_value = (status_expr); \ + if (ABSL_PREDICT_FALSE(!status_value.ok())) { \ + return status_value; \ + } \ + } while (false) + +// Assigns `dest` to the value contained within the `absl::StatusOr src` if +// `src.ok()`, otherwise, returns `src.status()` from the current function. +#define ASSIGN_OR_RETURN_IF_NOT_OK(dest, src) \ + ASSIGN_OR_RETURN_IF_NOT_OK_IMPL_( \ + CHECKS_INTERNAL_CONCAT_(value_or_, __LINE__), dest, src) +#define ASSIGN_OR_RETURN_IF_NOT_OK_IMPL_(value_or, dest, src) \ + auto value_or = (src); \ + if (ABSL_PREDICT_FALSE(!value_or.ok())) { \ + return std::move(value_or).status(); \ + } \ + dest = std::move(value_or).value() + +// Internal helper for concatenating macro values. +#define CHECKS_INTERNAL_CONCAT_IMPL_(x, y) x##y +#define CHECKS_INTERNAL_CONCAT_(x, y) CHECKS_INTERNAL_CONCAT_IMPL_(x, y) + +namespace centipede { +template +decltype(auto) ValueOrDie( + T&& value ABSL_ATTRIBUTE_LIFETIME_BOUND, + absl::SourceLocation loc = absl::SourceLocation::current()) { + if (ABSL_PREDICT_FALSE(!value.ok())) { + LOG(FATAL).AtLocation(loc) + << "ValueOrDie on non-OK status: " << value.status(); + } + return *std::forward(value); +} +} // namespace centipede + +#endif // FUZZTEST_COMMON_STATUS_MACROS_H_ diff --git a/fuzztest/internal/io.cc b/fuzztest/internal/io.cc index e351712b..eca94f96 100644 --- a/fuzztest/internal/io.cc +++ b/fuzztest/internal/io.cc @@ -253,7 +253,11 @@ void ForEachSerializedInput(absl::Span file_paths, // file and a file that is not a blob file. Once we can, we should not fall // back to reading the file directly if it is an empty blob file. std::string contents; - centipede::RemoteFileGetContents(file_path, contents); + const absl::Status get_contents_status = + centipede::RemoteFileGetContents(file_path, contents); + FUZZTEST_INTERNAL_CHECK_PRECONDITION( + get_contents_status.ok(), "RemoteFileGetContents failed on ", file_path, + ", status: ", get_contents_status.message()); absl::Status result = consume(file_path, std::nullopt, std::move(contents)); if (result.ok()) { ++total_loaded_inputs;