Seed value validation. By calling ValidateCorpusValue after FromValue(), and not validating inside FromValue(). Only warns on invalid seeds, does not check-fail. Also simplify validation in Protobuf Domain. PiperOrigin-RevId: 552597970
diff --git a/domain_tests/aggregate_combinators_test.cc b/domain_tests/aggregate_combinators_test.cc index 49d80ce..6a38106 100644 --- a/domain_tests/aggregate_combinators_test.cc +++ b/domain_tests/aggregate_combinators_test.cc
@@ -347,9 +347,11 @@ } } -TEST(OptionalOfDeathTest, FromValueOnNulloptDiesWhenPolicySetToAlwaysSet) { +TEST(OptionalOf, ValidationRejectsNullValueWhenPolicySetToAlwaysSet) { auto domain = NonNull(OptionalOf(Arbitrary<int>())); - EXPECT_DEATH(domain.FromValue(std::nullopt), "cannot be null"); + auto corpus_value = domain.FromValue(std::nullopt); + EXPECT_THAT(domain.ValidateCorpusValue(*corpus_value), + IsInvalid("Optional value must be set")); } TEST(OptionalOf, ValidationRejectsInvalidNullness) {
diff --git a/e2e_tests/functional_test.cc b/e2e_tests/functional_test.cc index d7c5b6a..070451e 100644 --- a/e2e_tests/functional_test.cc +++ b/e2e_tests/functional_test.cc
@@ -164,12 +164,18 @@ EXPECT_THAT(status, Eq(ExitCode(0))); } -TEST(UnitTestModeTest, InvalidSeedsCauseErrorMessageAndExit) { +TEST(UnitTestModeTest, InvalidSeedsAreSkippedAndReported) { auto [status, std_out, std_err] = RunBinaryWith(BinaryPath("testdata/fuzz_tests_with_invalid_seeds"), ""); - EXPECT_THAT(std_err, HasSubstr("[!] Error using `WithSeeds()` in")); - EXPECT_THAT(std_err, HasSubstr("Invalid seed value:\n\n{17}\n")); - EXPECT_THAT(status, Ne(ExitCode(0))); + EXPECT_THAT(std_err, HasSubstr("[!] Skipping WithSeeds() value in")); + EXPECT_THAT(std_err, + HasSubstr("Could not turn value into corpus type:\n{17}")); + EXPECT_THAT(std_err, HasSubstr("The value 17 is not InRange(0, 10):\n{17}")); + // Valid seeds are not reported. + EXPECT_THAT(std_err, Not(HasSubstr("{6}"))); + // Tests should still run. + EXPECT_THAT(std_out, HasSubstr("[ PASSED ] 3 tests.")); + EXPECT_THAT(status, ExitCode(0)); } TEST(UnitTestModeTest, CorpusIsMutatedInUnitTestMode) {
diff --git a/e2e_tests/testdata/fuzz_tests_with_invalid_seeds.cc b/e2e_tests/testdata/fuzz_tests_with_invalid_seeds.cc index 80de85c..2596ea1 100644 --- a/e2e_tests/testdata/fuzz_tests_with_invalid_seeds.cc +++ b/e2e_tests/testdata/fuzz_tests_with_invalid_seeds.cc
@@ -20,11 +20,24 @@ // TODO(b/203465367): Investigate if we can delay execution of seed parsing // until main(). -void InvalidSeed(int) {} -FUZZ_TEST(MySuite, InvalidSeed) +void InvalidSeedDueToUserValueNotConvertibleToCorpusValue(int) {} +FUZZ_TEST(MySuite, InvalidSeedDueToUserValueNotConvertibleToCorpusValue) // Map does not support seeds. .WithDomains(fuzztest::Map([](int) { return 0; }, fuzztest::Arbitrary<int>())) .WithSeeds({{17}}); +void InvalidSeedDueToCorpusValueOutOfDomain(int) {} +FUZZ_TEST(MySuite, InvalidSeedDueToCorpusValueOutOfDomain) + .WithDomains(fuzztest::InRange(0, 10)) + .WithSeeds({{2}, {17}, {6}}); + +struct MyTest { + void ShouldNotCrash(int) {} + std::vector<std::tuple<int>> GetSeeds() { return {{2}, {17}, {6}}; } +}; +FUZZ_TEST_F(MyTest, ShouldNotCrash) + .WithDomains(fuzztest::InRange(0, 10)) + .WithSeeds(&MyTest::GetSeeds); + } // namespace
diff --git a/fuzztest/internal/domains/optional_of_impl.h b/fuzztest/internal/domains/optional_of_impl.h index b592588..e7a67bd 100644 --- a/fuzztest/internal/domains/optional_of_impl.h +++ b/fuzztest/internal/domains/optional_of_impl.h
@@ -99,12 +99,8 @@ std::optional<corpus_type> FromValue(const value_type& v) const { if (!v) { - FUZZTEST_INTERNAL_CHECK(policy_ != OptionalPolicy::kWithoutNull, - "Value cannot be null!"); return corpus_type(std::in_place_index<0>); } - FUZZTEST_INTERNAL_CHECK(policy_ != OptionalPolicy::kAlwaysNull, - "Value cannot be non-null!"); if (auto inner_value = inner_.FromValue(*v)) { return corpus_type(std::in_place_index<1>, *std::move(inner_value)); } else {
diff --git a/fuzztest/internal/domains/protobuf_domain_impl.h b/fuzztest/internal/domains/protobuf_domain_impl.h index a4cb422..785a9db 100644 --- a/fuzztest/internal/domains/protobuf_domain_impl.h +++ b/fuzztest/internal/domains/protobuf_domain_impl.h
@@ -893,7 +893,8 @@ const GenericDomainCorpusType value = corpus_value.has_value() ? *corpus_value - : GetUnsetCorpusValue<T, /*is_repeated=*/false>(field); + : *self.GetSubDomain<T, /*is_repeated=*/false>(field).FromValue( + std::nullopt); absl::Status s = self.GetSubDomain<T, /*is_repeated=*/false>(field) .ValidateCorpusValue(value); out = Prefix(s, absl::StrCat("Invalid value for field ", field->name())); @@ -904,28 +905,13 @@ const GenericDomainCorpusType value = corpus_value.has_value() ? *corpus_value - : GetUnsetCorpusValue<T, /*is_repeated=*/true>(field); + : *self.GetSubDomain<T, /*is_repeated=*/true>(field).FromValue( + {}); absl::Status s = self.GetSubDomain<T, /*is_repeated=*/true>(field).ValidateCorpusValue( value); out = Prefix(s, absl::StrCat("Invalid value for field ", field->name())); } - - private: - template <typename T, bool is_repeated> - GenericDomainCorpusType GetUnsetCorpusValue(const FieldDescriptor* field) { - IRObject unset_value; - if constexpr (is_repeated) { - unset_value = IRObject(std::vector<IRObject>{}); - } else { - unset_value = IRObject(std::vector<IRObject>{IRObject(0)}); - } - std::optional<GenericDomainCorpusType> result = - self.GetSubDomain<T, is_repeated>(field).ParseCorpus(unset_value); - FUZZTEST_INTERNAL_CHECK(result.has_value(), - "Invalid unset value for field."); - return *result; - } }; absl::Status ValidateCorpusValue(const corpus_type& corpus_value) const {
diff --git a/fuzztest/internal/fixture_driver.h b/fuzztest/internal/fixture_driver.h index a00905d..60fd108 100644 --- a/fuzztest/internal/fixture_driver.h +++ b/fuzztest/internal/fixture_driver.h
@@ -123,21 +123,36 @@ std::vector<GenericDomainCorpusType> seeds; seeds.reserve(values.size()); for (const ValueType& val : values) { - std::optional<GenericDomainCorpusType> seed = + std::optional<GenericDomainCorpusType> corpus_value = domain_->TypedFromValue(val); - if (!seed.has_value()) { - absl::FPrintF(GetStderr(), "[!] Invalid seed value:\n\n{"); - AutodetectTypePrinter<ValueType>().PrintUserValue( - val, &std::cerr, PrintMode::kHumanReadable); - absl::FPrintF(GetStderr(), "}\n"); - std::exit(1); + if (!corpus_value.has_value()) { + const absl::Status status = + absl::InvalidArgumentError("Could not turn value into corpus type"); + ReportBadSeed(val, status); + continue; } - seeds.push_back(*std::move(seed)); + + const absl::Status status = + domain_->UntypedValidateCorpusValue(*corpus_value); + if (!status.ok()) { + ReportBadSeed(val, status); + continue; + } + + seeds.push_back(*std::move(corpus_value)); } return seeds; } private: + void ReportBadSeed(const ValueType& seed, const absl::Status& status) const { + absl::FPrintF(GetStderr(), "\n[!] Skipping WithSeeds() value: %s:\n{", + status.ToString()); + AutodetectTypePrinter<ValueType>().PrintUserValue( + seed, &std::cerr, PrintMode::kHumanReadable); + absl::FPrintF(GetStderr(), "}\n\n"); + } + virtual std::vector<GenericDomainCorpusType> GetSeedsFromSeedProvider() const = 0;
diff --git a/fuzztest/internal/fixture_driver_test.cc b/fuzztest/internal/fixture_driver_test.cc index f36e3b8..d1e2760 100644 --- a/fuzztest/internal/fixture_driver_test.cc +++ b/fuzztest/internal/fixture_driver_test.cc
@@ -290,7 +290,7 @@ UnorderedElementsAre(std::tuple{7}, std::tuple{42})); } -TEST(FixtureDriverTest, DiesIfConversionOfSeedsFromSeedProviderFails) { +TEST(FixtureDriverTest, InvalidSeedsFromSeedProviderAreSkipped) { FixtureDriverImpl<Domain<std::tuple<int>>, NoFixture, decltype(&TakesInt), decltype(&GetSeeds)> fixture_driver( @@ -299,7 +299,8 @@ Arbitrary<std::tuple<int>>()), {}, &GetSeeds); - EXPECT_DEATH_IF_SUPPORTED(fixture_driver.GetSeeds(), "Invalid seed value"); + EXPECT_THAT(UnpackGenericValues<std::tuple<int>>(fixture_driver.GetSeeds()), + UnorderedElementsAre(std::tuple{42})); } } // namespace
diff --git a/fuzztest/internal/registration.h b/fuzztest/internal/registration.h index 2d561a9..c9e032c 100644 --- a/fuzztest/internal/registration.h +++ b/fuzztest/internal/registration.h
@@ -170,6 +170,26 @@ TupleOf(std::forward<NewDomains>(domains)...)); } + void ReportBadSeed(const SeedT& seed, const absl::Status& status) { + absl::FPrintF(stderr, + "\n[!] Skipping WithSeeds() value in %s.%s:\n%s:%d: %s:\n{", + test_info_.suite_name, test_info_.test_name, test_info_.file, + test_info_.line, status.ToString()); + + // We use a direct call to PrintUserValue because we don't have a + // corpus_type object to pass to PrintValue. + bool first = true; + const auto print_one_arg = [&](auto I) { + using value_type = std::decay_t<std::tuple_element_t<I, SeedT>>; + AutodetectTypePrinter<value_type>().PrintUserValue( + std::get<I>(seed), &std::cerr, PrintMode::kHumanReadable); + if (!first) absl::FPrintF(stderr, ", "); + first = false; + }; + ApplyIndex<Base::kNumArgs>([&](auto... I) { (print_one_arg(I), ...); }); + absl::FPrintF(stderr, "}\n\n"); + } + auto WithSeeds(absl::Span<const SeedT> seeds) && { if constexpr (!Registration::kHasSeeds) { return Registration<Fixture, TargetFunction, @@ -179,38 +199,21 @@ .WithSeeds(seeds); } else { const auto& domains = this->GetDomains(); - bool found_error = false; for (const auto& seed : seeds) { - if (auto from_value = domains.FromValue(seed)) { - this->seeds_.push_back(*std::move(from_value)); - } else { - // TODO(sbenzaquen): Should we abort the process or make the test fail - // when seeds are ignored? - absl::FPrintF( - stderr, - "[!] Error using `WithSeeds()` in %s.%s:\n\n%s:%d: Invalid seed " - "value:\n\n{", - test_info_.suite_name, test_info_.test_name, test_info_.file, - test_info_.line); - - // We use a direct call to PrintUserValue because we don't have a - // corpus_type object to pass to PrintValue. - bool first = true; - const auto print_one_arg = [&](auto I) { - if (!first) absl::FPrintF(stderr, ", "); - using value_type = std::decay_t<std::tuple_element_t<I, SeedT>>; - AutodetectTypePrinter<value_type>().PrintUserValue( - std::get<I>(seed), &std::cerr, PrintMode::kHumanReadable); - first = false; - }; - ApplyIndex<Base::kNumArgs>( - [&](auto... I) { (print_one_arg(I), ...); }); - - absl::FPrintF(stderr, "}\n"); - found_error = true; + auto corpus_value = domains.FromValue(seed); + if (!corpus_value) { + const absl::Status status = absl::InvalidArgumentError( + "Could not turn value into corpus type"); + ReportBadSeed(seed, status); + continue; } + const absl::Status status = domains.ValidateCorpusValue(*corpus_value); + if (!status.ok()) { + ReportBadSeed(seed, status); + continue; + }; + this->seeds_.push_back(*std::move(corpus_value)); } - if (found_error) exit(1); return std::move(*this); } }