Re: [PATCH v5 3/3] Makefile: correct example fuzz build
- From
Josh Steadmon <steadmon@google.com>
- Date
- Jan 15, 2019, 21:59 UTC
- Message-ID
- <20190115215945.GB35794@google.com>
- In-Reply-To
- <xmqqimypr6yx.fsf@gitster-ct.c.googlers.com>
On 2019.01.15 12:39, Junio C Hamano wrote:
Show 30 quoted lines
> 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.