Skip to content

Commit

Permalink
Add the ability to specify environment variables to unset in the chil…
Browse files Browse the repository at this point in the history 
…d process.

Not to clutter the interface of `Command`, I combined the constructor's
parameters into an options struct.

#Centipede

PiperOrigin-RevId: 702073162
  • Loading branch information
@fniksic @copybara-github
fniksic authored and copybara-github committed Dec 2, 2024
1 parent 55a088d commit c02bb6a
Show file tree
Hide file tree
Showing 9 changed files with 153 additions and 126 deletions.
7 changes: 6 additions & 1 deletion centipede/batch_fuzz_example/customized_centipede.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <cstring>
#include <filesystem> // NOLINT
#include <string>
#include <utility>
#include <vector>

#include "absl/log/check.h"
Expand Down Expand Up @@ -148,7 +149,11 @@ bool CustomizedCallbacks::Execute(std::string_view binary,
}

// Execute.
Command cmd{env_.binary, args, env, tmp_log_filepath, tmp_log_filepath};
Command cmd{env_.binary,
{.args = std::move(args),
.env_add = std::move(env),
.stdout_file = tmp_log_filepath,
.stderr_file = tmp_log_filepath}};
const int retval = cmd.Execute();

std::string tmp_log;
Expand Down
12 changes: 6 additions & 6 deletions centipede/binary_info.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ void BinaryInfo::InitializeFromSanCovBinary(
ScopedFile log_path(tmp_dir_path, "binary_info_log_tmp");
LOG(INFO) << __func__ << ": tmp_dir: " << tmp_dir;

Command cmd(
binary_path_with_args, {},
{absl::StrCat("CENTIPEDE_RUNNER_FLAGS=:dump_binary_info:arg1=",
pc_table_path.path(), ":arg2=", cf_table_path.path(),
":arg3=", dso_table_path.path(), ":")},
log_path.path());
Command cmd(binary_path_with_args,
{.env_add = {absl::StrCat(
"CENTIPEDE_RUNNER_FLAGS=:dump_binary_info:arg1=",
pc_table_path.path(), ":arg2=", cf_table_path.path(),
":arg3=", dso_table_path.path(), ":")},
.stdout_file = std::string(log_path.path())});
int exit_code = cmd.Execute();
if (exit_code != EXIT_SUCCESS) {
LOG(INFO) << __func__ << ": exit_code: " << exit_code;
Expand Down
14 changes: 8 additions & 6 deletions centipede/centipede.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,9 @@ Centipede::Centipede(const Environment &env, CentipedeCallbacks &user_callbacks,
stats_(stats),
input_filter_path_(std::filesystem::path(TemporaryLocalDirPath())
.append("filter-input")),
input_filter_cmd_(env_.input_filter, {input_filter_path_}, {/*env*/},
"/dev/null", "/dev/null"),
input_filter_cmd_(env_.input_filter, {.args = {input_filter_path_},
.stdout_file = "/dev/null",
.stderr_file = "/dev/null"}),
rusage_profiler_(
/*scope=*/perf::RUsageScope::ThisProcess(),
/*metrics=*/env.DumpRUsageTelemetryInThisShard()
Expand Down Expand Up @@ -554,17 +555,18 @@ void Centipede::GenerateSourceBasedCoverageReport(
merge_arguments.push_back(raw_profile);
}

Command merge_command("llvm-profdata", merge_arguments);
Command merge_command("llvm-profdata", {.args = std::move(merge_arguments)});
if (merge_command.Execute() != EXIT_SUCCESS) {
LOG(ERROR) << "Failed to run command " << merge_command.ToString();
return;
}

Command generate_report_command(
"llvm-cov",
{"show", "-format=html", absl::StrCat("-output-dir=", report_path),
absl::StrCat("-instr-profile=", indexed_profile_path),
env_.clang_coverage_binary});
{.args = {"show", "-format=html",
absl::StrCat("-output-dir=", report_path),
absl::StrCat("-instr-profile=", indexed_profile_path),
env_.clang_coverage_binary}});
if (generate_report_command.Execute() != EXIT_SUCCESS) {
LOG(ERROR) << "Failed to run command "
<< generate_report_command.ToString();
Expand Down
41 changes: 20 additions & 21 deletions centipede/centipede_callbacks.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <string>
#include <string_view>
#include <system_error> // NOLINT
#include <utility>
#include <vector>

#include "absl/log/check.h"
Expand Down Expand Up @@ -137,13 +138,13 @@ Command &CentipedeCallbacks::GetOrCreateCommandForBinary(
env_.timeout_per_batch == 0
? absl::InfiniteDuration()
: absl::Seconds(env_.timeout_per_batch) + absl::Seconds(5);
Command &cmd = commands_.emplace_back(Command(
/*path=*/binary, /*args=*/{},
/*env=*/env,
/*out=*/execute_log_path_,
/*err=*/execute_log_path_,
/*timeout=*/amortized_timeout,
/*temp_file_path=*/temp_input_file_path_));
Command &cmd = commands_.emplace_back(
Command{binary,
{.env_add = std::move(env),
.stdout_file = execute_log_path_,
.stderr_file = execute_log_path_,
.timeout = amortized_timeout,
.temp_file_path = temp_input_file_path_}});
if (env_.fork_server) cmd.StartForkServer(temp_dir_, Hash(binary));

return cmd;
Expand Down Expand Up @@ -225,13 +226,12 @@ bool CentipedeCallbacks::GetSeedsViaExternalBinary(
CHECK(!error);

Command cmd{binary,
{},
{absl::StrCat("CENTIPEDE_RUNNER_FLAGS=:dump_seed_inputs:arg1=",
output_dir.string(), ":")},
/*out=*/execute_log_path_,
/*err=*/execute_log_path_,
/*timeout=*/absl::InfiniteDuration(),
/*temp_file_path=*/temp_input_file_path_};
{.env_add = {absl::StrCat(
"CENTIPEDE_RUNNER_FLAGS=:dump_seed_inputs:arg1=",
output_dir.string(), ":")},
.stdout_file = execute_log_path_,
.stderr_file = execute_log_path_,
.temp_file_path = temp_input_file_path_}};
const int retval = cmd.Execute();

std::vector<std::string> seed_input_filenames;
Expand Down Expand Up @@ -261,13 +261,12 @@ bool CentipedeCallbacks::GetSerializedTargetConfigViaExternalBinary(
const auto config_file_path =
std::filesystem::path{temp_dir_} / "configuration";
Command cmd{binary,
{},
{absl::StrCat("CENTIPEDE_RUNNER_FLAGS=:dump_configuration:arg1=",
config_file_path.string(), ":")},
/*out=*/execute_log_path_,
/*err=*/execute_log_path_,
/*timeout=*/absl::InfiniteDuration(),
/*temp_file_path=*/temp_input_file_path_};
{.env_add = {absl::StrCat(
"CENTIPEDE_RUNNER_FLAGS=:dump_configuration:arg1=",
config_file_path.string(), ":")},
.stdout_file = execute_log_path_,
.stderr_file = execute_log_path_,
.temp_file_path = temp_input_file_path_}};
const bool is_success = cmd.Execute() == 0;

if (is_success) {
Expand Down
68 changes: 35 additions & 33 deletions centipede/command.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,24 +144,23 @@ struct Command::ForkServerProps {
Command::Command(Command &&other) noexcept = default;
Command::~Command() = default;

Command::Command(std::string_view path, std::vector<std::string> args,
std::vector<std::string> env, std::string_view out,
std::string_view err, absl::Duration timeout,
std::string_view temp_file_path)
: path_(path),
args_(std::move(args)),
env_(std::move(env)),
out_(out),
err_(err),
timeout_(timeout),
temp_file_path_(temp_file_path) {}
Command::Command(std::string_view path, Options options)
: path_(path), options_(std::move(options)) {}

Command::Command(std::string_view path) : Command(path, {}) {}

std::string Command::ToString() const {
std::vector<std::string> ss;
ss.reserve(/*env*/ 1 + options_.env_add.size() + options_.env_remove.size() +
/*path*/ 1 + /*args*/ options_.args.size() + /*out/err*/ 2);
// env.
ss.reserve(env_.size());
for (const auto &env : env_) {
ss.emplace_back(env);
ss.push_back("env");
// Arguments that unset environment variables must appear first.
for (const auto &var : options_.env_remove) {
ss.push_back(absl::StrCat("-u ", var));
}
for (const auto &var : options_.env_add) {
ss.push_back(var);
}
// path.
std::string path = path_;
Expand All @@ -172,23 +171,24 @@ std::string Command::ToString() const {
// Replace @@ with temp_file_path_.
constexpr std::string_view kTempFileWildCard = "@@";
if (absl::StrContains(path, kTempFileWildCard)) {
CHECK(!temp_file_path_.empty());
path = absl::StrReplaceAll(path, {{kTempFileWildCard, temp_file_path_}});
CHECK(!options_.temp_file_path.empty());
path = absl::StrReplaceAll(path,
{{kTempFileWildCard, options_.temp_file_path}});
}
ss.emplace_back(path);
ss.push_back(std::move(path));
// args.
for (const auto &arg : args_) {
ss.emplace_back(arg);
for (const auto &arg : options_.args) {
ss.push_back(arg);
}
// out/err.
if (!out_.empty()) {
ss.emplace_back(absl::StrCat("> ", out_));
if (!options_.stdout_file.empty()) {
ss.push_back(absl::StrCat("> ", options_.stdout_file));
}
if (!err_.empty()) {
if (out_ != err_) {
ss.emplace_back(absl::StrCat("2> ", err_));
if (!options_.stderr_file.empty()) {
if (options_.stdout_file != options_.stderr_file) {
ss.push_back(absl::StrCat("2> ", options_.stderr_file));
} else {
ss.emplace_back("2>&1");
ss.push_back("2>&1");
}
}
// Trim trailing space and return.
Expand Down Expand Up @@ -314,7 +314,8 @@ int Command::Execute() {
int exit_code = EXIT_SUCCESS;

if (fork_server_ != nullptr) {
VLOG(1) << "Sending execution request to fork server: " << VV(timeout_);
VLOG(1) << "Sending execution request to fork server: "
<< VV(options_.timeout);

if (const auto status = VerifyForkServerIsHealthy(); !status.ok()) {
LogProblemInfo(absl::StrCat("Fork server should be running, but isn't: ",
Expand All @@ -331,7 +332,7 @@ int Command::Execute() {
// execution result to it).
struct pollfd poll_fd = {};
int poll_ret = -1;
auto poll_deadline = absl::Now() + timeout_;
auto poll_deadline = absl::Now() + options_.timeout;
// The `poll()` syscall can get interrupted: it sets errno==EINTR in that
// case. We should tolerate that.
do {
Expand All @@ -351,7 +352,7 @@ int Command::Execute() {
if (poll_ret == 0) {
LogProblemInfo(
absl::StrCat("Timeout while waiting for fork server: timeout is ",
absl::FormatDuration(timeout_)));
absl::FormatDuration(options_.timeout)));
} else {
LogProblemInfo(absl::StrCat(
"Error while waiting for fork server: poll() returned ", poll_ret));
Expand Down Expand Up @@ -423,20 +424,21 @@ int Command::Execute() {

std::string Command::ReadRedirectedStdout() const {
std::string ret;
if (!out_.empty()) {
ReadFromLocalFile(out_, ret);
if (!options_.stdout_file.empty()) {
ReadFromLocalFile(options_.stdout_file, ret);
if (ret.empty()) ret = "<EMPTY>";
}
return ret;
}

std::string Command::ReadRedirectedStderr() const {
std::string ret;
if (!err_.empty()) {
if (err_ == "2>&1" || err_ == out_) {
if (!options_.stderr_file.empty()) {
if (options_.stderr_file == "2>&1" ||
options_.stderr_file == options_.stdout_file) {
ret = "<DUPED TO STDOUT>";
} else {
ReadFromLocalFile(err_, ret);
ReadFromLocalFile(options_.stderr_file, ret);
if (ret.empty()) ret = "<EMPTY>";
}
}
Expand Down
60 changes: 34 additions & 26 deletions centipede/command.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,38 +27,51 @@ namespace centipede {

class Command final {
public:
struct Options {
// Arguments to pass to the executed command. The command is executed by the
// shell, so the arguments need to be shell-escaped.
// TODO(b/381910257): Escape the arguments for passing to the shell.
std::vector<std::string> 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<std::string> env_add;
// Environment variables to unset in the subprocess executing the command.
std::vector<std::string> env_remove;
// Redirect stdout to this file. If empty, use parent's STDOUT.
std::string stdout_file;
// Redirect stderr to this file. If empty, use parent's STDERR. If `out` ==
// `err` and both are non-empty, stdout/stderr are combined.
std::string stderr_file;
// Terminate a fork server execution attempt after this duration.
absl::Duration timeout = absl::InfiniteDuration();
// "@@" in the command will be replaced with `temp_file_path`.
std::string temp_file_path;
};

// Constructs a command to run the binary at `path` with the given `options`.
// The path can contain "@@" which will be replaced with
// `options.temp_file_path`.
explicit Command(std::string_view path, Options options);

// Constructs a command to run the binary at `path` with default options.
explicit Command(std::string_view path);

// Move-constructible only.
Command(const Command& other) = delete;
Command& operator=(const Command& other) = delete;
Command(Command&& other) noexcept;
Command& operator=(Command&& other) noexcept = delete;

// Constructs a command:
// `path`: path to the binary.
// `args`: arguments.
// `env`: environment variables/values (in the form "KEY=VALUE").
// `out`: stdout redirect path (empty means use parent's STDOUT).
// `err`: stderr redirect path (empty means use parent's STDERR).
// `timeout`: terminate a fork server execution attempt after this duration.
// `temp_file_path`: "@@" in `path` will be replaced with `temp_file_path`.
// If `out` == `err` and both are non-empty, stdout/stderr are combined.
// TODO(ussuri): The number of parameters became untenable and error-prone.
// Use the Options or Builder pattern instead.
explicit Command(std::string_view path, std::vector<std::string> args = {},
std::vector<std::string> env = {}, std::string_view out = "",
std::string_view err = "",
absl::Duration timeout = absl::InfiniteDuration(),
std::string_view temp_file_path = "");

// Cleans up the fork server, if that was created.
~Command();

// Returns a string representing the command, e.g. like this
// "ENV1=VAL1 path arg1 arg2 > out 2>& err"
// "env -u ENV1 ENV2=VAL2 path arg1 arg2 > out 2>& err"
std::string ToString() const;
// Executes the command, returns the exit status.
// Can be called more than once.
// If interrupted, may call RequestEarlyExit().
// If interrupted, may call `RequestEarlyStop()` (see stop.h).
int Execute();

// Attempts to start a fork server, returns true on success.
Expand Down Expand Up @@ -86,19 +99,14 @@ class Command final {
std::string ReadRedirectedStderr() const;
// Possibly logs information about a crash, starting with `message`, followed
// by the the command line, followed by the redirected stdout and stderr read
// from `out_` and `err_` files, if any.
// from `options_.out` and `options_.err` files, if any.
void LogProblemInfo(std::string_view message) const;
// Just as `LogCrashInfo()`, but logging occurs only when the VLOG level (set
// via `--v` or its equivalents) is >= `min_vlog`.
void VlogProblemInfo(std::string_view message, int vlog_level) const;

const std::string path_;
const std::vector<std::string> args_;
const std::vector<std::string> env_;
const std::string out_;
const std::string err_;
const absl::Duration timeout_;
const std::string temp_file_path_;
const Options options_;
const std::string command_line_ = ToString();

std::unique_ptr<ForkServerProps> fork_server_;
Expand Down
Loading

0 comments on commit c02bb6a

Please sign in to comment.