fuzz: Print error message when FUZZ is missing #27672

pull maflcko wants to merge 2 commits into bitcoin:master from maflcko:2305-fuzz-log-FUZZ- changing 3 files +27 −7

maflcko commented at 11:02 AM on May 16, 2023: member
Some trivial UX improvements.
- Change the exit code for PRINT_ALL_FUZZ_TARGETS_AND_ABORT and WRITE_ALL_FUZZ_TARGETS_AND_ABORT to EXIT_SUCCESS instead of Aborted (core dumped).
- Print readable error message when FUZZ is missing instead of Aborted (core dumped).
- Clarify that a fuzz target needs to be compiled into the executable.
DrahtBot commented at 11:02 AM on May 16, 2023: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK dergoegge

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Tests on May 16, 2023
maflcko commented at 11:03 AM on May 16, 2023: member

(Also contains some no-op code to allow for easier hacking)
fanquake requested review from dergoegge on May 16, 2023
maflcko force-pushed on May 17, 2023
maflcko force-pushed on May 17, 2023
DrahtBot added the label CI failed on May 17, 2023
DrahtBot removed the label CI failed on May 17, 2023

Type	Reviewers
ACK	dergoegge

in src/test/fuzz/fuzz.cpp:110 in fac80580a3 outdated

 106 | +    if (const auto* env_fuzz{std::getenv("FUZZ")}) {
 107 | +        // To allow for easier fuzz executable binary modification,
 108 | +        static std::string g_copy{env_fuzz}; // create copy to avoid compiler optimizations, and
 109 | +        g_fuzz_target = g_copy.c_str();      // strip string after the first null-char.
 110 | +    } else {
 111 | +        std::cerr << "Must select fuzz target with the FUZZ env var." << std::endl;

dergoegge commented at 8:23 AM on May 18, 2023:

nit: Would it be good to hint at PRINT_ALL_FUZZ_TARGETS_AND_ABORT=1 in the message?

maflcko commented at 10:19 AM on May 18, 2023:

Thx, done

dergoegge commented at 8:24 AM on May 18, 2023: member

Concept ACK
maflcko force-pushed on May 18, 2023
fuzz: Print error message when FUZZ is missing
Also, add missing includes.
fa22966f33

ci: Log qa-assets repo last commit

This documents the state in the CI output and may help debugging in case
of failure.

fa1b3abc83

maflcko force-pushed on May 22, 2023
DrahtBot added the label CI failed on May 22, 2023
DrahtBot removed the label CI failed on May 22, 2023
maflcko commented at 9:20 AM on May 22, 2023: member

Added a commit to also improve fuzz logging in the CI system: https://cirrus-ci.com/task/4987695903014912?logs=ci#L222
dergoegge commented at 10:38 AM on May 22, 2023: member

ACK fa1b3abc834a90fb1cfbd5ac63deb28f3990c1fb
fanquake merged this on May 22, 2023
fanquake closed this on May 22, 2023
maflcko deleted the branch on May 22, 2023
sidhujag referenced this in commit c434df6391 on May 23, 2023
bitcoin locked this on May 21, 2024

Contributors

Labels

fuzz: Print error message when FUZZ is missing #27672

Reviews