Re: [PATCH v5 3/3] Makefile: correct example fuzz build

[Josh Steadmon <steadmon@google.com>]

On 2019.01.15 12:39, Junio C Hamano wrote:
> Josh Steadmon <steadmon@google.com> writes:
> 
> > Signed-off-by: Josh Steadmon <steadmon@google.com>
> > ---
> >  Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 6b72f37c29..bbcfc2bc9f 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -3104,7 +3104,7 @@ cover_db_html: cover_db
> >  # An example command to build against libFuzzer from LLVM 4.0.0:
> >  #
> >  # make CC=clang CXX=clang++ \
> > -#      FUZZ_CXXFLAGS="-fsanitize-coverage=trace-pc-guard -fsanitize=address" \
> > +#      CFLAGS="-fsanitize-coverage=trace-pc-guard -fsanitize=address" \
> >  #      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \
> >  #      fuzz-all
> >  #
> 
> I know this appeared in v2 of the series, but I cannot quite read
> the reasoning/justification behind this change.  After this hunk
> there is
> 
>     FUZZ_CXXFLAGS ?= $(CFLAGS)
> 
> so if you do not give CFLAGS but give FUZZ_CXXFLAGS, like the
> sample, shouldn't it have worked just fine?  IOW "correct" in the
> title is a bit too terse as an explanation for this change.

Sorry for being too vague. The problem with only including FUZZ_CXXFLAGS
is that all the .c files need to be compiled with coverage tracking
enabled, not just the fuzzer itself.

The motivation for splitting CFLAGS and FUZZ_CXXFLAGS in the first place
was to enable OSS-Fuzz (and others) to include C++-specific flags
without causing a ton of compiler warnings when those are applied to the
.c files. So OSS-Fuzz sets both CFLAGS and FUZZ_CXXFLAGS when building.

We could fix the comment by setting both CFLAGS and FUZZ_CXXFLAGS, but
since we're not including any C++-specific flags there's no reason to
set both.

I'll fix the commit message in v6.