The FUZZ_TARGET macros have many issues:
Fix all issues by using only a single macro.
This refactor does not change behavior.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
11@@ -12,7 +12,7 @@
12 #include <string>
13 #include <vector>
14
15-FUZZ_TARGET(blockfilter)
16+FUZZ_TARGET(blockfilter, {})
Yes, it is possible. For example:
0diff --git a/src/test/fuzz/blockfilter.cpp b/src/test/fuzz/blockfilter.cpp
1index aa06af549a..3adc114515 100644
2--- a/src/test/fuzz/blockfilter.cpp
3+++ b/src/test/fuzz/blockfilter.cpp
4@@ -12,7 +12,7 @@
5 #include <string>
6 #include <vector>
7
8-FUZZ_TARGET(blockfilter, {})
9+FUZZ_TARGET(blockfilter)
10 {
11 FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
12 const std::optional<BlockFilter> block_filter = ConsumeDeserializable<BlockFilter>(fuzzed_data_provider);
13diff --git a/src/test/fuzz/fuzz.h b/src/test/fuzz/fuzz.h
14index 7c6ac0ccaa..409c721730 100644
15--- a/src/test/fuzz/fuzz.h
16+++ b/src/test/fuzz/fuzz.h
17@@ -38,7 +38,7 @@ void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target,
18 struct name##_Before_Main { \
19 name##_Before_Main() \
20 { \
21- FuzzFrameworkRegisterTarget(#name, name##_fuzz_target, __VA_ARGS__); \
22+ FuzzFrameworkRegisterTarget(#name, name##_fuzz_target, {__VA_ARGS__}); \
23 } \
24 } const static g_##name##_before_main; \
25 void name##_fuzz_target(FuzzBufferType buffer)
26diff --git a/src/test/fuzz/script_assets_test_minimizer.cpp b/src/test/fuzz/script_assets_test_minimizer.cpp
27index 29d2595266..7862be2f21 100644
28--- a/src/test/fuzz/script_assets_test_minimizer.cpp
29+++ b/src/test/fuzz/script_assets_test_minimizer.cpp
30@@ -186,7 +186,7 @@ void Test(const std::string& str)
31
32 void test_init() {}
33
34-FUZZ_TARGET(script_assets_test_minimizer, {.init = test_init, .hidden = true})
35+FUZZ_TARGET(script_assets_test_minimizer, .init = test_init, .hidden = true)
36 {
37 if (buffer.size() < 2 || buffer.back() != '\n' || buffer[buffer.size() - 2] != ',') return;
38 const std::string str((const char*)buffer.data(), buffer.size() - 2);
39diff --git a/src/wallet/test/fuzz/coincontrol.cpp b/src/wallet/test/fuzz/coincontrol.cpp
40index 36b2bd089b..0f71f28df2 100644
41--- a/src/wallet/test/fuzz/coincontrol.cpp
42+++ b/src/wallet/test/fuzz/coincontrol.cpp
43@@ -20,7 +20,7 @@ void initialize_coincontrol()
44 g_setup = testing_setup.get();
45 }
46
47-FUZZ_TARGET(coincontrol, {.init = initialize_coincontrol})
48+FUZZ_TARGET(coincontrol, .init = initialize_coincontrol)
49 {
50 FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
51 const auto& node = g_setup->m_node;
Do you want me to push that?
Concept ACK
Adding a new option requires doubling the number of existing macros in the worst case.
I’ve run into this before and it is indeed pretty annoying.
38-#define FUZZ_TARGET_INIT(name, init_fun) \
39- FUZZ_TARGET_INIT_HIDDEN(name, init_fun, false)
40+void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target, FuzzTargetOptions opts);
41
42-#define FUZZ_TARGET_INIT_HIDDEN(name, init_fun, hidden) \
43+#define FUZZ_TARGET(name, ...) \
59@@ -60,9 +60,9 @@ std::map<std::string_view, std::tuple<TypeTestOneInput, TypeInitialize, TypeHidd
60 return g_fuzz_targets;
61 }
62
63-void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target, TypeInitialize init, TypeHidden hidden)
64+void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target, FuzzTargetOptions opts)
65 {
66- const auto it_ins = FuzzTargets().try_emplace(name, std::move(target), std::move(init), hidden);
67+ const auto it_ins = FuzzTargets().try_emplace(name, std::move(target), std::move(opts.init), opts.hidden);
g_fuzz_targets store the opts directly? Would mean less churn when adding an option.
* This allows to reduce the number of total macros.
* Also, adding a new option no longer requires doubling the number of
macros in the worst case.
-BEGIN VERIFY SCRIPT-
ren() { sed --regexp-extended -i "s|$1|$2|g" $(git grep -l --extended-regexp "$1"); }
# Replace FUZZ_TARGET_INIT
ren 'FUZZ_TARGET_INIT\((.+), (.+)\)' 'FUZZ_TARGET(\1, .init = \2)'
# Delete unused FUZZ_TARGET_INIT
sed -i -e '37,39d' src/test/fuzz/fuzz.h
-END VERIFY SCRIPT-69
70-void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target, TypeInitialize init, TypeHidden hidden)
71+void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target, FuzzTargetOptions opts)
72 {
73- const auto it_ins = FuzzTargets().try_emplace(name, std::move(target), std::move(init), hidden);
74+ const auto it_ins{FuzzTargets().try_emplace(name, FuzzTarget /* temporary can be dropped in C++20 */ {std::move(target), std::move(opts)})};
0 const auto it_ins{FuzzTargets().try_emplace(name, std::move(target), std::move(opts))};
Haven’t tracked down which C++20 LWG paper this was. Let me know if someone finds something.