Re: [PATCH] fuzz: reorganise the path for existing oss-fuzz fuzzers

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 16, 2022, 21:55 UTC
Message-ID: <xmqqo7vff0gi.fsf@gitster.g>
In-Reply-To: <pull.1353.git.1663355009333.gitgitgadget@gmail.com>

"Arthur Chan via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 8 quoted lines

> From: Arthur Chan <arthur.chan@adalogics.com>
>
> This patch is aimed to provide a better organisation for oss-fuzz
> fuzzers, allowing more fuzzers for the git project to be added
> in a later development.
>
> A new folder oss-fuzz has been created and existing fuzzers are
> moved into the new folders. Makefile has been fixed accordingly.

"folder" -> "directory" everywhere.

Show 8 quoted lines

>  Makefile                                            | 6 +++---
>  fuzz-commit-graph.c => oss-fuzz/fuzz-commit-graph.c | 0
>  fuzz-pack-headers.c => oss-fuzz/fuzz-pack-headers.c | 0
>  fuzz-pack-idx.c => oss-fuzz/fuzz-pack-idx.c         | 0
>  4 files changed, 3 insertions(+), 3 deletions(-)
>  rename fuzz-commit-graph.c => oss-fuzz/fuzz-commit-graph.c (100%)
>  rename fuzz-pack-headers.c => oss-fuzz/fuzz-pack-headers.c (100%)
>  rename fuzz-pack-idx.c => oss-fuzz/fuzz-pack-idx.c (100%)

It is curious that we do not have any changes to .gitignore patterns.

    $ git grep fuzz .gitignore Makefile
    .gitignore:/fuzz-commit-graph
    .gitignore:/fuzz_corpora
    .gitignore:/fuzz-pack-headers
    .gitignore:/fuzz-pack-idx
    Makefile:FUZZ_OBJS += fuzz-commit-graph.o
    Makefile:FUZZ_OBJS += fuzz-pack-headers.o
    Makefile:FUZZ_OBJS += fuzz-pack-idx.o
    Makefile:.PHONY: fuzz-objs
    Makefile:fuzz-objs: $(FUZZ_OBJS)
    Makefile:# Always build fuzz objects even if not testing, to prevent bit-rot.
    Makefile:# Building fuzz targets generally requires a special set of compiler flags that
    Makefile:#      CFLAGS="-fsanitize=fuzzer-no-link,address" \
    Makefile:#      LIB_FUZZING_ENGINE="-fsanitize=fuzzer" \
    Makefile:#      fuzz-all
    Makefile:.PHONY: fuzz-all
    Makefile:fuzz-all: $(FUZZ_PROGRAMS)

I do not know what "fuzz_corpora" is, which step in build creates it, and why we do not have to bother removing it in "make clean", the last of which is not the fault of this patch, but I suspect that at least other three existing entries that name $(FUZZ_PROGRAMS) need to be updated, because ...

Show 14 quoted lines

> diff --git a/Makefile b/Makefile
> index d9247ead45b..2d56aae7a1d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -686,9 +686,9 @@ SCRIPTS = $(SCRIPT_SH_GEN) \
>  
>  ETAGS_TARGET = TAGS
>  
> -FUZZ_OBJS += fuzz-commit-graph.o
> -FUZZ_OBJS += fuzz-pack-headers.o
> -FUZZ_OBJS += fuzz-pack-idx.o
> +FUZZ_OBJS += oss-fuzz/fuzz-commit-graph.o
> +FUZZ_OBJS += oss-fuzz/fuzz-pack-headers.o
> +FUZZ_OBJS += oss-fuzz/fuzz-pack-idx.o

... FUZZ_OBJS now live in the oss-fuzz/ directory, and Makefile has

    FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS))

    $(FUZZ_PROGRAMS): all
	$(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \
		$(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@

neither of which has been touched by the patch, so presumably the executables are now created in the oss-fuzz/ directory as well, and they are what .gitignore should be listing, right?

Also, compiling the exectuable files would not be the end of the story, right? Do folks (like test script, makefile targets and CI recipes) who used to run ./fuzz-commit-graph need to be told that they now need to run oss-fuzz/fuzz-commit-graph instead? They may not be inside my tree, but what's the best way to inform them? Add entries to release notes (not asking you to add one immediately --- asking you to help formulating the plans).

Thanks.