Re: [PATCH 2/2] fuzz: link fuzz programs with `make all` on Linux
- From
Josh Steadmon <steadmon@google.com>
- Date
- Apr 9, 2024, 21:58 UTC
- Message-ID
- <ZhW6BM9V-Rto_CW4@google.com>
- In-Reply-To
- <xmqqplw8z73y.fsf@gitster.g>
On 2024.03.05 13:44, Junio C Hamano wrote:
Show 63 quoted lines
> 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.
Show 27 quoted lines
> diff --git c/Makefile w/Makefile > index 4e255c81f2..46e457a7a8 100644 > --- c/Makefile > +++ w/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. > @@ -766,6 +769,12 @@ fuzz-objs: $(FUZZ_OBJS) > # Always build fuzz objects even if not testing, to prevent bit-rot. > all:: $(FUZZ_OBJS) > > +# Build fuzz programs, even without the necessary fuzzing support, > +# this prevents bit-rot. > +ifdef LINK_FUZZ_PROGRAMS > +all:: fuzz-all > +endif > + > FUZZ_PROGRAMS += $(patsubst %.o,%,$(filter-out %dummy-cmd-main.o,$(FUZZ_OBJS))) > > # Empty...