From: Josh Steadmon <steadmon@google.com>
To: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH 2/2] fuzz: link fuzz programs with `make all` on Linux
Date: Wed, 10 Apr 2024 13:49:39 -0700 [thread overview]
Message-ID: <Zhb7YwMdtNKzpCSw@google.com> (raw)
In-Reply-To: <ZhW6BM9V-Rto_CW4@google.com>
On 2024.04.09 14:58, Josh Steadmon wrote:
> On 2024.03.05 13:44, Junio C Hamano wrote:
> > Josh Steadmon <steadmon@google.com> writes:
> >
> > > Since 5e47215080 (fuzz: add basic fuzz testing target., 2018-10-12), we
> > > have compiled object files for the fuzz tests as part of the default
> > > 'make all' target. This helps prevent bit-rot in lesser-used parts of
> > > the codebase, by making sure that incompatible changes are caught at
> > > build time.
> > >
> > > However, since we never linked the fuzzer executables, this did not
> > > protect us from link-time errors. As of 8b9a42bf48 (fuzz: fix fuzz test
> > > build rules, 2024-01-19), it's now possible to link the fuzzer
> > > executables without using a fuzzing engine and a variety of
> > > compiler-specific (and compiler-version-specific) flags, at least on
> > > Linux. So let's add a platform-specific option in config.mak.uname to
> > > link the executables as part of the default `make all` target.
> > >
> > > Suggested-by: Junio C Hamano <gitster@pobox.com>
> > > Signed-off-by: Josh Steadmon <steadmon@google.com>
> > > ---
> > > Makefile | 14 +++++++++++---
> > > config.mak.uname | 1 +
> > > 2 files changed, 12 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 4e255c81f2..f74e96d7c2 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -409,6 +409,9 @@ include shared.mak
> > > # to the "<name>" of the corresponding `compat/fsmonitor/fsm-settings-<name>.c`
> > > # that implements the `fsm_os_settings__*()` routines.
> > > #
> > > +# Define LINK_FUZZ_PROGRAMS if you want `make all` to also build the fuzz test
> > > +# programs in oss-fuzz/.
> > > +#
> > > # === Optional library: libintl ===
> > > #
> > > # Define NO_GETTEXT if you don't want Git output to be translated.
> > > @@ -763,9 +766,6 @@ FUZZ_OBJS += oss-fuzz/fuzz-pack-idx.o
> > > .PHONY: fuzz-objs
> > > fuzz-objs: $(FUZZ_OBJS)
> > >
> > > -# Always build fuzz objects even if not testing, to prevent bit-rot.
> > > -all:: $(FUZZ_OBJS)
> > > -
> > > FUZZ_PROGRAMS += $(patsubst %.o,%,$(filter-out %dummy-cmd-main.o,$(FUZZ_OBJS)))
> > >
> > > # Empty...
> > > @@ -2368,6 +2368,14 @@ ifndef NO_TCLTK
> > > endif
> > > $(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) SHELL_PATH='$(SHELL_PATH_SQ)' PERL_PATH='$(PERL_PATH_SQ)'
> > >
> > > +# Build fuzz programs if possible, or at least compile the object files; even
> > > +# without the necessary fuzzing support, this prevents bit-rot.
> > > +ifdef LINK_FUZZ_PROGRAMS
> > > +all:: $(FUZZ_PROGRAMS)
> > > +else
> > > +all:: $(FUZZ_OBJS)
> > > +endif
> >
> > It would have been easier on the eyes if we had the fuzz things
> > together, perhaps like this simplified version? We build FUZZ_OBJS
> > either way, and when the LINK_FUZZ_PROGRAMS is requested, we follow
> > the fuzz-all recipe, too.
>
> We need the LINK_FUZZ_PROGRAMS conditional to happen after we import
> config.mak.uname (line 1434 in my V1). We also need to define FUZZ_OBJS
> prior to adding it to OBJECTS (line 2698 in V1). I can move all of the
> fuzz-definition within that range, keeping everything in one place at
> the cost of a larger diff. I'll do that for V2, but if you prefer
> otherwise please let me know.
>
> Although I'm not 100% sure that we even need to add FUZZ_OBJS to
> OBJECTS, so let me check that tomorrow. If not, then I can move
> everything to the bottom of the Makefile where we also define fuzz-all
> and the build rules for FUZZ_PROGRAMS.
It turns out we do need FUZZ_OBJS in OBJECTS so that we define a build
rule, otherwise the Makefile doesn't know how to compile the fuzzer
objects. So V2 will have most of the fuzzer rules in the line
(1434,2698) range.
next prev parent reply other threads:[~2024-04-10 20:49 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-05 21:11 [PATCH 0/2] fuzz: build fuzzers by default on Linux Josh Steadmon
2024-03-05 21:11 ` [PATCH 1/2] ci: also define CXX environment variable Josh Steadmon
2024-03-05 21:37 ` Junio C Hamano
2024-04-09 21:32 ` Josh Steadmon
2024-03-06 0:50 ` Jeff King
2024-03-06 1:00 ` Jeff King
2024-04-10 20:58 ` Josh Steadmon
2024-03-05 21:12 ` [PATCH 2/2] fuzz: link fuzz programs with `make all` on Linux Josh Steadmon
2024-03-05 21:44 ` Junio C Hamano
2024-04-09 21:58 ` Josh Steadmon
2024-04-10 20:49 ` Josh Steadmon [this message]
2024-04-10 20:57 ` Junio C Hamano
2024-04-10 21:11 ` Jeff King
2024-03-26 21:51 ` [PATCH 0/2] fuzz: build fuzzers by default " Junio C Hamano
2024-04-09 21:34 ` Josh Steadmon
2024-04-11 18:14 ` [PATCH v2 " Josh Steadmon
2024-04-11 18:14 ` [PATCH v2 1/2] ci: also define CXX environment variable Josh Steadmon
2024-04-12 4:22 ` Jeff King
2024-04-24 18:15 ` Josh Steadmon
2024-04-11 18:14 ` [PATCH v2 2/2] fuzz: link fuzz programs with `make all` on Linux Josh Steadmon
2024-04-11 21:39 ` Junio C Hamano
2024-04-24 18:14 ` [PATCH v3] " Josh Steadmon
2024-04-24 19:07 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zhb7YwMdtNKzpCSw@google.com \
--to=steadmon@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).