Let runner/sancov use the shared EngineFlagHelper in runner_utils. ALso change the flags-getting functions to be idempotent, which makes more sense. PiperOrigin-RevId: 974577600
diff --git a/centipede/BUILD b/centipede/BUILD index 2062db8..108e5e0 100644 --- a/centipede/BUILD +++ b/centipede/BUILD
@@ -1074,7 +1074,6 @@ RUNNER_DEPS = [ ":byte_array_mutator", ":callstack", - ":dispatcher_flag_helper", ":execution_metadata", ":feature", ":foreach_nonzero", @@ -1209,15 +1208,6 @@ ) cc_library( - name = "dispatcher_flag_helper", - hdrs = ["dispatcher_flag_helper.h"], - copts = DISABLE_SANCOV_COPTS, - deps = [ - "@abseil-cpp//absl/base:nullability", - ], -) - -cc_library( name = "sancov_runtime", srcs = [ "pc_info.h", @@ -1238,7 +1228,6 @@ copts = DISABLE_SANCOV_COPTS, deps = [ ":callstack", - ":dispatcher_flag_helper", ":engine_abi", ":execution_metadata", ":feature",
diff --git a/centipede/dispatcher_flag_helper.h b/centipede/dispatcher_flag_helper.h deleted file mode 100644 index d143831..0000000 --- a/centipede/dispatcher_flag_helper.h +++ /dev/null
@@ -1,76 +0,0 @@ -// Copyright 2022 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. - -#ifndef FUZZTEST_CENTIPEDE_DISPATCHER_FLAG_HELPER_H_ -#define FUZZTEST_CENTIPEDE_DISPATCHER_FLAG_HELPER_H_ - -#include <stdlib.h> - -#include <cstdint> -#include <cstring> - -#include "absl/base/nullability.h" - -namespace fuzztest::internal { - -struct DispatcherFlagHelper { - // We don't use flags passed via argv so that argv flags can be passed - // directly to LLVMFuzzerInitialize, w/o filtering. The flags are separated - // with ':' on both sides, i.e. like this: ":flag1:flag2:flag3=value3". - // We do it this way to make the flag parsing code extremely simple. The - // interface is private between Centipede and the runner and may change. - DispatcherFlagHelper(const char *absl_nullable flags_) : flags(flags_) {} - - const char *absl_nullable flags; - - // Returns true iff `flag` is present. - // Typical usage: pass ":some_flag:", i.e. the flag name surrounded with ':'. - // TODO(ussuri): Refactor `char *` into a `string_view`. - bool HasFlag(const char *absl_nonnull flag) const { - if (!flags) return false; - return strstr(flags, flag) != nullptr; - } - - // If a flag=value pair is present, returns value, - // otherwise returns `default_value`. - // Typical usage: pass ":some_flag=". - // TODO(ussuri): Refactor `char *` into a `string_view`. - uint64_t HasIntFlag(const char *absl_nonnull flag, - uint64_t default_value) const { - if (!flags) return default_value; - const char *beg = strstr(flags, flag); - if (!beg) return default_value; - return atoll(beg + strlen(flag)); // NOLINT: can't use strto64, etc. - } - - // If a :flag=value: pair is present returns value, otherwise returns nullptr. - // The result is obtained by calling strndup, so make sure to save - // it in `this` to avoid a leak. - // Typical usage: pass ":some_flag=". - // TODO(ussuri): Refactor `char *` into a `string_view`. - const char *absl_nullable GetStringFlag(const char *absl_nonnull flag) const { - if (!flags) return nullptr; - // Extract "value" from ":flag=value:" inside centipede_runner_flags. - const char *beg = strstr(flags, flag); - if (!beg) return nullptr; - const char *value_beg = beg + strlen(flag); - const char *end = strstr(value_beg, ":"); - if (!end) return nullptr; - return strndup(value_beg, end - value_beg); - } -}; - -} // namespace fuzztest::internal - -#endif // FUZZTEST_CENTIPEDE_DISPATCHER_FLAG_HELPER_H_
diff --git a/centipede/engine_worker.cc b/centipede/engine_worker.cc index 80281ef..e0f6785 100644 --- a/centipede/engine_worker.cc +++ b/centipede/engine_worker.cc
@@ -826,12 +826,12 @@ // to happen when the stdout/stderr are not redirected to a file. (void)ftruncate(fd, 0); } - WorkerLog("FuzzTest engine worker (", - req == PersistentModeRequest::kExit ? "exiting persistent mode" - : "persistent mode batch", - "); flags: ", - GetWorkerFlagsEnv() != nullptr ? GetWorkerFlagsEnv() : "", - LogLnSync{}); + WorkerLog( + "FuzzTest engine worker (", + req == PersistentModeRequest::kExit ? "exiting persistent mode" + : "persistent mode batch", + "); flags: ", GetWorkerFlagsEnv() ? GetWorkerFlagsEnv() : "(unset)", + LogLnSync{}); } if (req == PersistentModeRequest::kExit) break; WorkerCheck(req == PersistentModeRequest::kRunBatch,
diff --git a/centipede/runner.cc b/centipede/runner.cc index 40e4f39..88a4ce9 100644 --- a/centipede/runner.cc +++ b/centipede/runner.cc
@@ -51,7 +51,6 @@ #include "absl/base/optimization.h" #include "absl/types/span.h" #include "./centipede/byte_array_mutator.h" -#include "./centipede/dispatcher_flag_helper.h" #include "./centipede/execution_metadata.h" #include "./centipede/feature.h" #include "./centipede/mutation_data.h" @@ -810,7 +809,7 @@ // No-op under ASAN/TSAN/MSAN - those may still rely on rss_limit_mb. if (vm_size_in_bytes < one_tb) { size_t address_space_limit_mb = - state->flag_helper.HasIntFlag(":address_space_limit_mb=", 0); + state->flag_helper.GetIntFlag("address_space_limit_mb=", 0); if (address_space_limit_mb > 0) { size_t limit_in_bytes = address_space_limit_mb << 20; struct rlimit rlimit_as = {limit_in_bytes, limit_in_bytes}; @@ -902,7 +901,7 @@ // This means, the binary is standalone with its own main(), and we need to // report the coverage now. if (!state->centipede_runner_main_executed && - flag_helper.HasFlag(":shmem:")) { + flag_helper.HasSwitchFlag("shmem")) { PostProcessSancov(); // TODO(xinhaoyuan): do we know our exit status? SharedMemoryBlobSequence outputs_blobseq(sancov_state->arg2); StartSendingOutputsToEngine(outputs_blobseq); @@ -966,10 +965,11 @@ // to happen when the stdout/stderr are not redirected to a file. (void)ftruncate(fd, 0); } - fprintf(stderr, "Centipede fuzz target runner (%s); flags: %s\n", - req == PersistentModeRequest::kExit ? "exiting persistent mode" - : "persistent mode batch", - state->flag_helper.flags); + fprintf( + stderr, "Centipede fuzz target runner (%s); flags: %s\n", + req == PersistentModeRequest::kExit ? "exiting persistent mode" + : "persistent mode batch", + CentipedeGetRunnerFlags() ? CentipedeGetRunnerFlags() : "(unset)"); } if (req == PersistentModeRequest::kExit) break; RunnerCheck(req == PersistentModeRequest::kRunBatch, @@ -987,7 +987,7 @@ return EXIT_SUCCESS; } -// If HasFlag(:shmem:), state->arg1 and state->arg2 are the names +// If HasSwitchFlag(:shmem:), state->arg1 and state->arg2 are the names // of in/out shared memory locations. // Read inputs and write outputs via shared memory. // @@ -998,22 +998,23 @@ state->centipede_runner_main_executed = true; fprintf(stderr, "Centipede fuzz target runner; argv[0]: %s flags: %s\n", - argv[0], state->flag_helper.flags); + argv[0], + CentipedeGetRunnerFlags() ? CentipedeGetRunnerFlags() : "(unset)"); - if (state->flag_helper.HasFlag(":dump_configuration:")) { + if (state->flag_helper.HasSwitchFlag("dump_configuration")) { DumpSerializedTargetConfigToFile(callbacks, /*output_file_path=*/sancov_state->arg1); return EXIT_SUCCESS; } - if (state->flag_helper.HasFlag(":dump_seed_inputs:")) { + if (state->flag_helper.HasSwitchFlag("dump_seed_inputs")) { // Seed request. DumpSeedsToDir(callbacks, /*output_dir=*/sancov_state->arg1); return EXIT_SUCCESS; } // Inputs / outputs from shmem. - if (state->flag_helper.HasFlag(":shmem:")) { + if (state->flag_helper.HasSwitchFlag("shmem")) { if (!sancov_state->arg1 || !sancov_state->arg2) return EXIT_FAILURE; SharedMemoryBlobSequence inputs_blobseq(sancov_state->arg1); SharedMemoryBlobSequence outputs_blobseq(sancov_state->arg2); @@ -1067,9 +1068,13 @@ extern "C" __attribute__((weak)) const char* absl_nullable CentipedeGetRunnerFlags() { - if (const char* runner_flags_env = getenv("CENTIPEDE_RUNNER_FLAGS")) - return strdup(runner_flags_env); - return nullptr; + static const char* flags = []() -> const char* { + if (const char* runner_flags_env = getenv("CENTIPEDE_RUNNER_FLAGS")) { + return strdup(runner_flags_env); + } + return nullptr; + }(); + return flags; } // TODO: xinhaoyuan - write test for this.
diff --git a/centipede/runner.h b/centipede/runner.h index 5ddc3b3..7cdbe5a 100644 --- a/centipede/runner.h +++ b/centipede/runner.h
@@ -22,7 +22,6 @@ #include <cstdint> #include "./centipede/byte_array_mutator.h" -#include "./centipede/dispatcher_flag_helper.h" #include "./centipede/knobs.h" #include "./centipede/runner_interface.h" #include "./centipede/runner_result.h" @@ -59,29 +58,28 @@ // Performs necessary cleanup on process termination. void OnTermination(); - DispatcherFlagHelper flag_helper = - DispatcherFlagHelper(CentipedeGetRunnerFlags()); + EngineFlagHelper flag_helper = EngineFlagHelper(CentipedeGetRunnerFlags()); // Note that this field reflects the initial runner flags. But some // flags can change later (if wrapped with std::atomic). RunTimeFlags run_time_flags = { - /*timeout_per_input=*/flag_helper.HasIntFlag(":timeout_per_input=", 0), - /*rss_limit_mb=*/flag_helper.HasIntFlag(":rss_limit_mb=", 0), - /*crossover_level=*/flag_helper.HasIntFlag(":crossover_level=", 50), + /*timeout_per_input=*/flag_helper.GetIntFlag("timeout_per_input=", 0), + /*rss_limit_mb=*/flag_helper.GetIntFlag("rss_limit_mb=", 0), + /*crossover_level=*/flag_helper.GetIntFlag("crossover_level=", 50), /*ignore_timeout_reports=*/ - flag_helper.HasFlag(":ignore_timeout_reports:"), - /*max_len=*/flag_helper.HasIntFlag(":max_len=", 4000), - /*stack_limit_kb=*/flag_helper.HasIntFlag(":stack_limit_kb=", 0), + flag_helper.HasSwitchFlag("ignore_timeout_reports"), + /*max_len=*/flag_helper.GetIntFlag("max_len=", 4000), + /*stack_limit_kb=*/flag_helper.GetIntFlag("stack_limit_kb=", 0), }; // The path to a file where the runner may write the description of failure. - const char *failure_description_path = - flag_helper.GetStringFlag(":failure_description_path="); + const char* failure_description_path = + flag_helper.GetStringFlag("failure_description_path="); std::atomic<bool> has_failure_description; const char* persistent_mode_socket_path = - flag_helper.GetStringFlag(":persistent_mode_socket="); + flag_helper.GetStringFlag("persistent_mode_socket="); int persistent_mode_socket = 0; pthread_mutex_t execution_result_override_mu = PTHREAD_MUTEX_INITIALIZER;
diff --git a/centipede/runner_interface.h b/centipede/runner_interface.h index 9b560bd..7a2208b 100644 --- a/centipede/runner_interface.h +++ b/centipede/runner_interface.h
@@ -80,7 +80,8 @@ // gets the flags from CENTIPEDE_RUNNER_FLAGS env var. // // It should return either a nullptr or a constant string that is valid -// throughout the entire process life-time. +// throughout the entire process life-time. Multiple calls should always return +// the same value. extern "C" const char* absl_nullable CentipedeGetRunnerFlags(); // An overridable function to override `LLVMFuzzerMutate` behavior.
diff --git a/centipede/sancov_callbacks.cc b/centipede/sancov_callbacks.cc index 3e5d0c5..b1c180c 100644 --- a/centipede/sancov_callbacks.cc +++ b/centipede/sancov_callbacks.cc
@@ -23,7 +23,6 @@ #include "absl/base/nullability.h" #include "absl/base/optimization.h" -#include "./centipede/dispatcher_flag_helper.h" #include "./centipede/feature.h" #include "./centipede/int_utils.h" #include "./centipede/pc_info.h" @@ -356,7 +355,7 @@ static pthread_once_t main_object_lazy_init_once = PTHREAD_ONCE_INIT; static void MainObjectLazyInitOnceCallback() { sancov_state->main_object = fuzztest::internal::GetDlInfo( - sancov_state->flag_helper.GetStringFlag(":dl_path_suffix=")); + sancov_state->flag_helper.GetStringFlag("dl_path_suffix=")); fprintf(stderr, "MainObjectLazyInitOnceCallback %zx\n", sancov_state->main_object.start_address); UpdatePcCounterSetSizeAligned(sancov_state->reverse_pc_table.NumPcs());
diff --git a/centipede/sancov_state.cc b/centipede/sancov_state.cc index c5bbaf3..8adedee 100644 --- a/centipede/sancov_state.cc +++ b/centipede/sancov_state.cc
@@ -25,7 +25,6 @@ #include <vector> #include "absl/base/nullability.h" -#include "./centipede/dispatcher_flag_helper.h" #include "./centipede/engine_abi.h" #include "./centipede/execution_metadata.h" #include "./centipede/feature.h" @@ -197,7 +196,7 @@ static void MaybePopulateReversePcTable() { const char* pcs_file_path = - sancov_state->flag_helper.GetStringFlag(":pcs_file_path="); + sancov_state->flag_helper.GetStringFlag("pcs_file_path="); if (!pcs_file_path) return; const auto pc_table = ReadBytesFromFilePath<PCInfo>(pcs_file_path); sancov_state->reverse_pc_table.SetFromPCs(pc_table); @@ -253,8 +252,8 @@ SancovState::SancovState() { tls.OnThreadStart(); // Compute main_object. - main_object = GetDlInfo(flag_helper.GetStringFlag(":dl_path_suffix=")); - if (!sancov_state->main_object.IsSet()) { + main_object = GetDlInfo(flag_helper.GetStringFlag("dl_path_suffix=")); + if (!main_object.IsSet()) { fprintf( stderr, "Failed to compute main_object. This may happen" @@ -262,7 +261,7 @@ } // Dump the binary info tables. - if (flag_helper.HasFlag(":dump_binary_info:")) { + if (flag_helper.HasSwitchFlag("dump_binary_info")) { RunnerCheck(arg1 && arg2 && arg3, "dump_binary_info requires 3 arguments"); if (!arg1 || !arg2 || !arg3) _exit(EXIT_FAILURE); DumpPcTable(arg1); @@ -561,10 +560,14 @@ } // namespace fuzztest::internal // Can be overridden to not depend explicitly on CENTIPEDE_RUNNER_FLAGS. -extern "C" __attribute__((weak)) const char *absl_nullable GetSancovFlags() { - if (const char *sancov_flags_env = getenv("CENTIPEDE_RUNNER_FLAGS")) - return strdup(sancov_flags_env); - return nullptr; +extern "C" __attribute__((weak)) const char* absl_nullable GetSancovFlags() { + static const char* flags = []() -> const char* { + if (const char* sancov_flags_env = getenv("CENTIPEDE_RUNNER_FLAGS")) { + return strdup(sancov_flags_env); + } + return nullptr; + }(); + return flags; } void SanCovRuntimeClearCoverage(bool full_clear) {
diff --git a/centipede/sancov_state.h b/centipede/sancov_state.h index bde30e1..5e105e0 100644 --- a/centipede/sancov_state.h +++ b/centipede/sancov_state.h
@@ -31,7 +31,6 @@ #include "./centipede/callstack.h" #include "./centipede/concurrent_bitset.h" #include "./centipede/concurrent_byteset.h" -#include "./centipede/dispatcher_flag_helper.h" #include "./centipede/execution_metadata.h" #include "./centipede/feature.h" #include "./centipede/hashed_ring_buffer.h" @@ -42,7 +41,7 @@ #include "./centipede/sancov_object_array.h" #include "./centipede/sancov_runtime.h" -extern "C" const char *absl_nullable GetSancovFlags(); +extern "C" const char* absl_nullable GetSancovFlags(); namespace fuzztest::internal { @@ -150,27 +149,27 @@ SancovState(); ~SancovState(); - DispatcherFlagHelper flag_helper = DispatcherFlagHelper(GetSancovFlags()); + EngineFlagHelper flag_helper = EngineFlagHelper(GetSancovFlags()); // TODO(xinhaoyuan): Change to use meaningful flag names instead of the // generic names arg1/2/3. - const char *arg1 = flag_helper.GetStringFlag(":arg1="); - const char *arg2 = flag_helper.GetStringFlag(":arg2="); - const char *arg3 = flag_helper.GetStringFlag(":arg3="); + const char* arg1 = flag_helper.GetStringFlag("arg1="); + const char* arg2 = flag_helper.GetStringFlag("arg2="); + const char* arg3 = flag_helper.GetStringFlag("arg3="); SancovFlags flags = { /*path_level=*/std::min(ThreadLocalSancovState::kBoundedPathLength, - flag_helper.HasIntFlag(":path_level=", 0)), - /*use_pc_features=*/flag_helper.HasFlag(":use_pc_features:"), + flag_helper.GetIntFlag("path_level=", 0)), + /*use_pc_features=*/flag_helper.HasSwitchFlag("use_pc_features"), /*use_dataflow_features=*/ - flag_helper.HasFlag(":use_dataflow_features:"), - /*use_cmp_features=*/flag_helper.HasFlag(":use_cmp_features:"), - /*callstack_level=*/flag_helper.HasIntFlag(":callstack_level=", 0), + flag_helper.HasSwitchFlag("use_dataflow_features"), + /*use_cmp_features=*/flag_helper.HasSwitchFlag("use_cmp_features"), + /*callstack_level=*/flag_helper.GetIntFlag("callstack_level=", 0), /*use_counter_features=*/ - flag_helper.HasFlag(":use_counter_features:"), + flag_helper.HasSwitchFlag("use_counter_features"), /*use_auto_dictionary=*/ - flag_helper.HasFlag(":use_auto_dictionary:"), - /*skip_seen_features=*/flag_helper.HasFlag(":skip_seen_features:"), + flag_helper.HasSwitchFlag("use_auto_dictionary"), + /*skip_seen_features=*/flag_helper.HasSwitchFlag("skip_seen_features"), }; // Computed by DlInfo().
diff --git a/fuzztest/internal/centipede_adaptor.cc b/fuzztest/internal/centipede_adaptor.cc index 73f5e3a..5885eaa 100644 --- a/fuzztest/internal/centipede_adaptor.cc +++ b/fuzztest/internal/centipede_adaptor.cc
@@ -1124,16 +1124,19 @@ } // namespace extern "C" const char* CentipedeGetRunnerFlags() { - if (const char* runner_flags_env = std::getenv("CENTIPEDE_RUNNER_FLAGS")) { - // Runner mode. Use the existing flags. - return strdup(runner_flags_env); - } + static const char* flags = []() -> const char* { + if (const char* runner_flags_env = std::getenv("CENTIPEDE_RUNNER_FLAGS")) { + // Runner mode. Use the existing flags. + return strdup(runner_flags_env); + } - // Set the runner flags according to the FuzzTest default environment. - const auto env = fuzztest::internal::CreateDefaultCentipedeEnvironment(); - CentipedeCallbacksForRunnerFlagsExtraction callbacks( - env, fuzztest::internal::global_stop_condition); - const std::string runner_flags = callbacks.GetRunnerFlagsContent(); - ABSL_VLOG(1) << "[.] Centipede runner flags: " << runner_flags; - return strdup(runner_flags.c_str()); + // Set the runner flags according to the FuzzTest default environment. + const auto env = fuzztest::internal::CreateDefaultCentipedeEnvironment(); + CentipedeCallbacksForRunnerFlagsExtraction callbacks( + env, fuzztest::internal::global_stop_condition); + const char* flags = strdup(callbacks.GetRunnerFlagsContent().c_str()); + FUZZTEST_VLOG(1) << "[.] Centipede runner flags: " << flags; + return flags; + }(); + return flags; }