From 1741b5e37f6663d2f87fe4ff577c8109b71353a7 Mon Sep 17 00:00:00 2001 From: Filip Niksic Date: Fri, 10 Apr 2026 15:11:51 -0700 Subject: [PATCH] No public description PiperOrigin-RevId: 897910361 --- centipede/analyze_corpora_test.cc | 7 ++-- centipede/binary_info.cc | 12 +++--- centipede/binary_info.h | 6 +++ centipede/centipede_callbacks.cc | 53 +++++++------------------- centipede/centipede_flags.inc | 7 ++++ centipede/command.cc | 18 ++++++--- centipede/command.h | 10 ++--- centipede/command_test.cc | 3 +- centipede/control_flow_test.cc | 5 ++- centipede/coverage_test.cc | 8 ++-- fuzztest/internal/centipede_adaptor.cc | 25 ++++++++++++ 11 files changed, 86 insertions(+), 68 deletions(-) diff --git a/centipede/analyze_corpora_test.cc b/centipede/analyze_corpora_test.cc index cf00c5478..c349ec308 100644 --- a/centipede/analyze_corpora_test.cc +++ b/centipede/analyze_corpora_test.cc @@ -57,8 +57,8 @@ TEST(GetCoverage, SimpleCoverageResults) { bool uses_legacy_trace_pc_instrumentation = {}; BinaryInfo binary_info; binary_info.InitializeFromSanCovBinary( - GetTargetPath(), GetObjDumpPath(), GetLLVMSymbolizerPath(), - GetTestTempDir(test_info_->name()).string()); + GetTargetPath(), /*env_diff=*/{}, GetObjDumpPath(), + GetLLVMSymbolizerPath(), GetTestTempDir(test_info_->name()).string()); const auto &pc_table = binary_info.pc_table; EXPECT_FALSE(uses_legacy_trace_pc_instrumentation); const SymbolTable &symbols = binary_info.symbols; @@ -93,7 +93,8 @@ TEST(DumpCoverageReport, SimpleCoverageResults) { const std::string test_tmpdir = GetTestTempDir(test_info_->name()); BinaryInfo binary_info; - binary_info.InitializeFromSanCovBinary(GetTargetPath(), GetObjDumpPath(), + binary_info.InitializeFromSanCovBinary(GetTargetPath(), /*env_diff=*/{}, + GetObjDumpPath(), GetLLVMSymbolizerPath(), test_tmpdir); CoverageResults coverage_results = GetCoverage(corpus_records, std::move(binary_info)); diff --git a/centipede/binary_info.cc b/centipede/binary_info.cc index 4517c9834..0d02c53eb 100644 --- a/centipede/binary_info.cc +++ b/centipede/binary_info.cc @@ -40,8 +40,9 @@ constexpr std::string_view kCfTableFileName = "cf-table"; } // namespace void BinaryInfo::InitializeFromSanCovBinary( - std::string_view binary_path_with_args, std::string_view objdump_path, - std::string_view symbolizer_path, std::string_view tmp_dir_path) { + std::string_view binary_path_with_args, std::vector env_diff, + std::string_view objdump_path, std::string_view symbolizer_path, + std::string_view tmp_dir_path) { if (binary_path_with_args.empty()) { // This usually happens in tests. FUZZTEST_LOG(INFO) << __func__ << ": binary_path_with_args is empty"; @@ -58,10 +59,11 @@ void BinaryInfo::InitializeFromSanCovBinary( std::filesystem::path{tmp_dir_path} / "binary_info_log_tmp"; FUZZTEST_LOG(INFO) << __func__ << ": tmp_dir: " << tmp_dir; - Command::Options cmd_options; - cmd_options.env_add = {absl::StrCat( + env_diff.push_back(absl::StrCat( "CENTIPEDE_RUNNER_FLAGS=:dump_binary_info:arg1=", pc_table_path.path(), - ":arg2=", cf_table_path.path(), ":arg3=", dso_table_path.path(), ":")}; + ":arg2=", cf_table_path.path(), ":arg3=", dso_table_path.path(), ":")); + Command::Options cmd_options; + cmd_options.env_diff = std::move(env_diff); cmd_options.stdout_file_prefix = log_prefix; Command cmd{binary_path_with_args, std::move(cmd_options)}; int exit_code = cmd.Execute(); diff --git a/centipede/binary_info.h b/centipede/binary_info.h index 9a1e53d5b..f082fa433 100644 --- a/centipede/binary_info.h +++ b/centipede/binary_info.h @@ -15,7 +15,9 @@ #ifndef THIRD_PARTY_CENTIPEDE_BINARY_INFO_H_ #define THIRD_PARTY_CENTIPEDE_BINARY_INFO_H_ +#include #include +#include #include "./centipede/call_graph.h" #include "./centipede/control_flow.h" @@ -39,9 +41,13 @@ struct BinaryInfo { // `uses_legacy_trace_pc_instrumentation` based on `binary_path_with_args`. // * `binary_path_with_args` is the path to the instrumented binary, // possibly with space-separated arguments. + // * `env_diff` is a list of environment variables to set (given as `VAR=val`) + // or unset (given as `-VAR`) when running the binary, relative to the + // parent process. // * `objdump_path` and `symbolizer_path` are paths to respective tools. // * `tmp_dir_path` is a path to a temp dir, that must exist. void InitializeFromSanCovBinary(std::string_view binary_path_with_args, + std::vector env_diff, std::string_view objdump_path, std::string_view symbolizer_path, std::string_view tmp_dir_path); diff --git a/centipede/centipede_callbacks.cc b/centipede/centipede_callbacks.cc index 653721210..af247e1fe 100644 --- a/centipede/centipede_callbacks.cc +++ b/centipede/centipede_callbacks.cc @@ -290,38 +290,10 @@ class CentipedeCallbacks::PersistentModeServer { int conn_socket_ = -1; }; -namespace { - -// When running a test binary in a subprocess, we don't want these environment -// variables to be inherited and affect the execution of the tests. -// -// See list of environment variables here: -// https://bazel.build/reference/test-encyclopedia#initial-conditions -// -// TODO(fniksic): Add end-to-end tests that make sure we don't observe the -// effects of these variables in the test binary. -std::vector EnvironmentVariablesToUnset() { - return {"TEST_DIAGNOSTICS_OUTPUT_DIR", // - "TEST_INFRASTRUCTURE_FAILURE_FILE", // - "TEST_LOGSPLITTER_OUTPUT_FILE", // - "TEST_PREMATURE_EXIT_FILE", // - "TEST_RANDOM_SEED", // - "TEST_RUN_NUMBER", // - "TEST_SHARD_INDEX", // - "TEST_SHARD_STATUS_FILE", // - "TEST_TOTAL_SHARDS", // - "TEST_UNDECLARED_OUTPUTS_ANNOTATIONS_DIR", // - "TEST_UNDECLARED_OUTPUTS_DIR", // - "TEST_WARNINGS_OUTPUT_FILE", // - "GTEST_OUTPUT", // - "XML_OUTPUT_FILE"}; -} - -} // namespace - void CentipedeCallbacks::PopulateBinaryInfo(BinaryInfo& binary_info) { binary_info.InitializeFromSanCovBinary( - env_.coverage_binary, env_.objdump_path, env_.symbolizer_path, temp_dir_); + env_.coverage_binary, env_.env_diff_for_binaries, env_.objdump_path, + env_.symbolizer_path, temp_dir_); // Check the PC table. if (binary_info.pc_table.empty()) { if (env_.require_pc_table) { @@ -415,7 +387,8 @@ CentipedeCallbacks::GetOrCreateCommandContextForBinary( std::make_unique( std::move(server_path)); } - std::vector env = {ConstructRunnerFlags( + std::vector env_diff = env_.env_diff_for_binaries; + env_diff.push_back(ConstructRunnerFlags( absl::StrCat(":shmem:test=", env_.test_name, ":arg1=", inputs_blobseq_.path(), ":arg2=", outputs_blobseq_.path(), ":failure_description_path=", failure_description_path_, @@ -425,16 +398,16 @@ CentipedeCallbacks::GetOrCreateCommandContextForBinary( : absl::StrCat(":persistent_mode_socket=", persistent_mode_server->server_path()), ":"), - disable_coverage)}; + disable_coverage)); - if (env_.clang_coverage_binary == binary) - env.emplace_back( + if (env_.clang_coverage_binary == binary) { + env_diff.push_back( absl::StrCat("LLVM_PROFILE_FILE=", WorkDir{env_}.SourceBasedCoverageRawProfilePath())); + } Command::Options cmd_options; - cmd_options.env_add = std::move(env); - cmd_options.env_remove = EnvironmentVariablesToUnset(); + cmd_options.env_diff = std::move(env_diff); cmd_options.stdout_file_prefix = execute_log_prefix_; cmd_options.stderr_file_prefix = execute_log_prefix_; cmd_options.temp_file_path = temp_input_file_path_; @@ -642,8 +615,8 @@ bool CentipedeCallbacks::GetSeedsViaExternalBinary( "dl_path_suffix=", env_.runner_dl_path_suffix, ":"); } Command::Options cmd_options; - cmd_options.env_add = {std::move(centipede_runner_flags)}; - cmd_options.env_remove = EnvironmentVariablesToUnset(); + cmd_options.env_diff = env_.env_diff_for_binaries; + cmd_options.env_diff.push_back(std::move(centipede_runner_flags)); cmd_options.stdout_file_prefix = execute_log_prefix_; cmd_options.stderr_file_prefix = execute_log_prefix_; cmd_options.temp_file_path = temp_input_file_path_; @@ -707,8 +680,8 @@ bool CentipedeCallbacks::GetSerializedTargetConfigViaExternalBinary( "dl_path_suffix=", env_.runner_dl_path_suffix, ":"); } Command::Options cmd_options; - cmd_options.env_add = {std::move(centipede_runner_flags)}; - cmd_options.env_remove = EnvironmentVariablesToUnset(); + cmd_options.env_diff = env_.env_diff_for_binaries; + cmd_options.env_diff.push_back(std::move(centipede_runner_flags)); cmd_options.stdout_file_prefix = execute_log_prefix_; cmd_options.stderr_file_prefix = execute_log_prefix_; cmd_options.temp_file_path = temp_input_file_path_; diff --git a/centipede/centipede_flags.inc b/centipede/centipede_flags.inc index 1a171d769..ca5e47ac6 100644 --- a/centipede/centipede_flags.inc +++ b/centipede/centipede_flags.inc @@ -484,3 +484,10 @@ CENTIPEDE_FLAG( bool, report_crash_summary, true, "If set, reports a summary of crashes found during fuzzing or replay." ) +CENTIPEDE_FLAG( + std::vector, env_diff_for_binaries, {}, + "A comma-separated list of environment variables to set or unset when " + "running the target binary, coverage binary, and extra binaries, relative " + "to the parent process's environment. Variables in the form 'VAR=VAL' are " + "set, and variables in the form '-VAR' are unset." +) diff --git a/centipede/command.cc b/centipede/command.cc index 307142823..4c765985d 100644 --- a/centipede/command.cc +++ b/centipede/command.cc @@ -204,16 +204,22 @@ Command::Command(std::string_view path) : Command{path, {}} {} std::string Command::ToString() const { std::vector ss; - ss.reserve(/*env*/ 1 + options_.env_add.size() + options_.env_remove.size() + - /*path*/ 1 + /*args*/ options_.args.size() + /*out/err*/ 2); + ss.reserve(/*env*/ 1 + options_.env_diff.size() + /*path*/ 1 + + /*args*/ options_.args.size() + /*out/err*/ 2); // env. ss.push_back("env"); + std::vector env_to_set; + env_to_set.reserve(options_.env_diff.size()); // Arguments that unset environment variables must appear first. - for (const auto& var : options_.env_remove) { - ss.push_back(absl::StrCat("-u ", var)); + for (std::string_view var : options_.env_diff) { + if (absl::StartsWith(var, "-")) { + ss.push_back(absl::StrCat("-u ", var.substr(1))); + } else { + env_to_set.emplace_back(var); + } } - for (const auto& var : options_.env_add) { - ss.push_back(var); + for (auto& var : env_to_set) { + ss.push_back(std::move(var)); } // path. std::string path = path_; diff --git a/centipede/command.h b/centipede/command.h index d9edf8d52..f1272483b 100644 --- a/centipede/command.h +++ b/centipede/command.h @@ -34,12 +34,10 @@ class Command final { // shell, so the arguments need to be shell-escaped. // TODO(b/381910257): Escape the arguments for passing to the shell. std::vector args; - // Environment variables/values in the form "KEY=VALUE" to set in the - // subprocess executing the command. These are added to the environment - // variables inherited from the parent process. - std::vector env_add; - // Environment variables to unset in the subprocess executing the command. - std::vector env_remove; + // Environment variables to set or unset in the subprocess, relative to the + // environment of the parent process. The variables in the form "KEY=VALUE" + // are set, and the variables in the form "-KEY" are unset. + std::vector env_diff; // Redirect stdout to this file path with an opaque suffix. If empty, use // parent's STDOUT. // diff --git a/centipede/command_test.cc b/centipede/command_test.cc index dc163c923..d7f2f583e 100644 --- a/centipede/command_test.cc +++ b/centipede/command_test.cc @@ -48,8 +48,7 @@ TEST(CommandTest, ToString) { } { Command::Options cmd_options; - cmd_options.env_add = {"K1=V1", "K2=V2"}; - cmd_options.env_remove = {"K3"}; + 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"); } diff --git a/centipede/control_flow_test.cc b/centipede/control_flow_test.cc index 5327b8567..3fe228231 100644 --- a/centipede/control_flow_test.cc +++ b/centipede/control_flow_test.cc @@ -200,7 +200,7 @@ TEST(CFTable, GetCfTable) { // Load the cf table. BinaryInfo binary_info; binary_info.InitializeFromSanCovBinary( - target_path, GetObjDumpPath(), GetLLVMSymbolizerPath(), + target_path, /*env_diff=*/{}, GetObjDumpPath(), GetLLVMSymbolizerPath(), GetTestTempDir(test_info_->name()).string()); const auto &cf_table = binary_info.cf_table; FUZZTEST_LOG(INFO) << VV(target_path) << VV(tmp_path1) << VV(cf_table.size()); @@ -278,7 +278,8 @@ TEST(CFTable, GetCfTable) { static void SymbolizeBinary(std::string_view test_dir, std::string_view target_path, bool use_trace_pc) { BinaryInfo binary_info; - binary_info.InitializeFromSanCovBinary(target_path, GetObjDumpPath(), + binary_info.InitializeFromSanCovBinary(target_path, /*env_diff=*/{}, + GetObjDumpPath(), GetLLVMSymbolizerPath(), test_dir); // Load the pc table. const auto &pc_table = binary_info.pc_table; diff --git a/centipede/coverage_test.cc b/centipede/coverage_test.cc index 12cbf539a..4f3a830b0 100644 --- a/centipede/coverage_test.cc +++ b/centipede/coverage_test.cc @@ -196,8 +196,8 @@ TEST(Coverage, CoverageFeatures) { bool uses_legacy_trace_pc_instrumentation = {}; BinaryInfo binary_info; binary_info.InitializeFromSanCovBinary( - GetTargetPath(), GetObjDumpPath(), GetLLVMSymbolizerPath(), - GetTestTempDir(test_info_->name()).string()); + GetTargetPath(), /*env_diff=*/{}, GetObjDumpPath(), + GetLLVMSymbolizerPath(), GetTestTempDir(test_info_->name()).string()); const auto &pc_table = binary_info.pc_table; EXPECT_FALSE(uses_legacy_trace_pc_instrumentation); const SymbolTable &symbols = binary_info.symbols; @@ -422,8 +422,8 @@ TEST(Coverage, FunctionFilter) { // Initialize coverage data. BinaryInfo binary_info; binary_info.InitializeFromSanCovBinary( - GetTargetPath(), GetObjDumpPath(), GetLLVMSymbolizerPath(), - GetTestTempDir(test_info_->name()).string()); + GetTargetPath(), /*env_diff=*/{}, GetObjDumpPath(), + GetLLVMSymbolizerPath(), GetTestTempDir(test_info_->name()).string()); const PCTable &pc_table = binary_info.pc_table; EXPECT_FALSE(binary_info.uses_legacy_trace_pc_instrumentation); diff --git a/fuzztest/internal/centipede_adaptor.cc b/fuzztest/internal/centipede_adaptor.cc index c4fc98991..e97afd988 100644 --- a/fuzztest/internal/centipede_adaptor.cc +++ b/fuzztest/internal/centipede_adaptor.cc @@ -192,6 +192,30 @@ fuzztest::internal::Environment CreateDefaultCentipedeEnvironment() { return env; } +std::vector GetEnvDiffForBinaries() { + // When running a test binary in a subprocess, we don't want these environment + // variables to be inherited and affect the execution of the tests. + // + // See list of environment variables here: + // https://bazel.build/reference/test-encyclopedia#initial-conditions + std::vector env_diff = { + "-TEST_DIAGNOSTICS_OUTPUT_DIR", // + "-TEST_INFRASTRUCTURE_FAILURE_FILE", // + "-TEST_LOGSPLITTER_OUTPUT_FILE", // + "-TEST_PREMATURE_EXIT_FILE", // + "-TEST_RANDOM_SEED", // + "-TEST_RUN_NUMBER", // + "-TEST_SHARD_INDEX", // + "-TEST_SHARD_STATUS_FILE", // + "-TEST_TOTAL_SHARDS", // + "-TEST_UNDECLARED_OUTPUTS_ANNOTATIONS_DIR", // + "-TEST_UNDECLARED_OUTPUTS_DIR", // + "-TEST_WARNINGS_OUTPUT_FILE", // + "-GTEST_OUTPUT", // + "-XML_OUTPUT_FILE"}; + return env_diff; +} + fuzztest::internal::Environment CreateCentipedeEnvironmentFromConfiguration( const Configuration& configuration, absl::string_view workdir, absl::string_view test_name, RunMode run_mode) { @@ -265,6 +289,7 @@ fuzztest::internal::Environment CreateCentipedeEnvironmentFromConfiguration( env.coverage_binary = (*args)[0]; env.binary_name = std::filesystem::path{(*args)[0]}.filename(); env.binary_hash = "DUMMY_HASH"; + env.env_diff_for_binaries = GetEnvDiffForBinaries(); env.exit_on_crash = !configuration.continue_after_crash || // Always fail on crash for reproducer tests. configuration.crashing_input_to_reproduce.has_value();