From c35abd2386c48daa0ca5f99313ba053253f2fd38 Mon Sep 17 00:00:00 2001 From: Filip Niksic Date: Mon, 23 Oct 2023 08:51:01 -0700 Subject: [PATCH] Turn `BasicTestInfo` into a proper value type. We do this by storing the strings as `std::string` instead of `const char*`. This is a preparatory step to allow dynamically generated fuzz test names, but it has value in itself as it reduces a chance of dangling pointers. PiperOrigin-RevId: 575831577 --- fuzztest/internal/googletest_adaptor.cc | 12 ++++++------ fuzztest/internal/registration.h | 21 +++++++++++---------- fuzztest/internal/registry.cc | 2 +- fuzztest/internal/registry.h | 7 ++++--- fuzztest/internal/runtime.h | 11 ++++++----- 5 files changed, 28 insertions(+), 25 deletions(-) diff --git a/fuzztest/internal/googletest_adaptor.cc b/fuzztest/internal/googletest_adaptor.cc index e2c2952f..407befe4 100644 --- a/fuzztest/internal/googletest_adaptor.cc +++ b/fuzztest/internal/googletest_adaptor.cc @@ -15,13 +15,13 @@ void RegisterFuzzTestsAsGoogleTests(int* argc, char*** argv) { return new ::fuzztest::internal::GTest_TestAdaptor(test, argc, argv); }; if (test.uses_fixture()) { - ::testing::RegisterTest(test.suite_name(), test.test_name(), nullptr, - nullptr, test.file(), test.line(), - std::move(fixture_factory)); + ::testing::RegisterTest( + test.suite_name().c_str(), test.test_name().c_str(), nullptr, nullptr, + test.file().c_str(), test.line(), std::move(fixture_factory)); } else { - ::testing::RegisterTest(test.suite_name(), test.test_name(), nullptr, - nullptr, test.file(), test.line(), - std::move(test_factory)); + ::testing::RegisterTest( + test.suite_name().c_str(), test.test_name().c_str(), nullptr, nullptr, + test.file().c_str(), test.line(), std::move(test_factory)); } }); diff --git a/fuzztest/internal/registration.h b/fuzztest/internal/registration.h index c9e032c9..3f36b968 100644 --- a/fuzztest/internal/registration.h +++ b/fuzztest/internal/registration.h @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -36,9 +37,9 @@ namespace fuzztest::internal { struct BasicTestInfo { - const char* suite_name = nullptr; - const char* test_name = nullptr; - const char* file = nullptr; + std::string suite_name; + std::string test_name; + std::string file; int line = 0; bool uses_fixture = false; }; @@ -127,7 +128,7 @@ class Registration : private Base { public: explicit Registration(BasicTestInfo info, TargetFunction target_function) - : test_info_(info), target_function_(target_function) {} + : test_info_(std::move(info)), target_function_(target_function) {} // Registers domains for a property function as a `TupleOf` individual // domains. This is useful when the domains are specified indirectly, e.g., @@ -155,7 +156,7 @@ class Registration : private Base { "the number of function parameters."); using NewBase = RegistrationWithDomainsBase...>; return Registration( - test_info_, target_function_, NewBase{std::move(domain)}); + std::move(test_info_), target_function_, NewBase{std::move(domain)}); } // Registers a domain for each parameter of the property function. This is the @@ -194,7 +195,7 @@ class Registration : private Base { if constexpr (!Registration::kHasSeeds) { return Registration, SeedProvider>( - test_info_, target_function_, + std::move(test_info_), target_function_, RegistrationWithSeedsBase(std::move(*this))) .WithSeeds(seeds); } else { @@ -211,7 +212,7 @@ class Registration : private Base { if (!status.ok()) { ReportBadSeed(seed, status); continue; - }; + } this->seeds_.push_back(*std::move(corpus_value)); } return std::move(*this); @@ -231,7 +232,7 @@ class Registration : private Base { Fixture, TargetFunction, RegistrationWithSeedProviderBase, decltype(seed_provider)>( - test_info_, target_function_, + std::move(test_info_), target_function_, RegistrationWithSeedProviderBase( std::move(*this), std::move(seed_provider))); } @@ -259,7 +260,7 @@ class Registration : private Base { absl::AnyInvocable(BaseFixture*) const>; using NewBase = RegistrationWithSeedProviderBase; return Registration( - test_info_, target_function_, + std::move(test_info_), target_function_, NewBase(std::move(*this), std::mem_fn(seed_provider))); } @@ -287,7 +288,7 @@ class Registration : private Base { explicit Registration(BasicTestInfo info, TargetFunction target_function, Base base) : Base(std::move(base)), - test_info_(info), + test_info_(std::move(info)), target_function_(target_function) {} BasicTestInfo test_info_; diff --git a/fuzztest/internal/registry.cc b/fuzztest/internal/registry.cc index cafa3775..ac695f00 100644 --- a/fuzztest/internal/registry.cc +++ b/fuzztest/internal/registry.cc @@ -58,7 +58,7 @@ void ForEachTest(absl::FunctionRef func) { } void RegisterImpl(BasicTestInfo test_info, FuzzTestFuzzerFactory factory) { - Regs().emplace_back(test_info, std::move(factory)); + Regs().emplace_back(std::move(test_info), std::move(factory)); } void RegisterSetUpTearDownTestSuiteFunctions( diff --git a/fuzztest/internal/registry.h b/fuzztest/internal/registry.h index e9b9fcdc..541c08fd 100644 --- a/fuzztest/internal/registry.h +++ b/fuzztest/internal/registry.h @@ -54,13 +54,14 @@ struct RegistrationToken { typename SeedProvider> RegistrationToken& operator=( Registration&& reg) { - const BasicTestInfo test_info = reg.test_info_; - RegisterImpl(test_info, GetFuzzTestFuzzerFactory(std::move(reg))); if constexpr (std::is_base_of_v) { - RegisterSetUpTearDownTestSuiteFunctions(test_info.suite_name, + RegisterSetUpTearDownTestSuiteFunctions(reg.test_info_.suite_name, &Fixture::SetUpTestSuite, &Fixture::TearDownTestSuite); } + BasicTestInfo test_info = reg.test_info_; + RegisterImpl(std::move(test_info), + GetFuzzTestFuzzerFactory(std::move(reg))); return *this; } diff --git a/fuzztest/internal/runtime.h b/fuzztest/internal/runtime.h index 33bcbce0..28e4f673 100644 --- a/fuzztest/internal/runtime.h +++ b/fuzztest/internal/runtime.h @@ -32,6 +32,7 @@ #include "absl/functional/function_ref.h" #include "absl/random/bit_gen_ref.h" #include "absl/random/discrete_distribution.h" +#include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" #include "absl/time/time.h" @@ -77,14 +78,14 @@ using FuzzTestFuzzerFactory = class FuzzTest { public: FuzzTest(BasicTestInfo test_info, FuzzTestFuzzerFactory factory) - : test_info_(test_info), make_(std::move(factory)) {} + : test_info_(std::move(test_info)), make_(std::move(factory)) {} - const char* suite_name() const { return test_info_.suite_name; } - const char* test_name() const { return test_info_.test_name; } + const std::string& suite_name() const { return test_info_.suite_name; } + const std::string& test_name() const { return test_info_.test_name; } std::string full_name() const { - return suite_name() + std::string(".") + test_name(); + return absl::StrCat(test_info_.suite_name, ".", test_info_.test_name); } - const char* file() const { return test_info_.file; } + const std::string& file() const { return test_info_.file; } int line() const { return test_info_.line; } bool uses_fixture() const { return test_info_.uses_fixture; } auto make() const { return make_(*this); }