| Message ID | 350ea5f7c97aa4166eea56aa57b66ddc9c53de4a.1547582104.git.steadmon@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add commit-graph fuzzer and fix buffer overflow | expand |
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.
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.
Josh Steadmon <steadmon@google.com> writes: > 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. OK. > 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. Yes.
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 #
Signed-off-by: Josh Steadmon <steadmon@google.com> --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)