On 2024.03.05 20:00, Jeff King wrote: > On Tue, Mar 05, 2024 at 07:50:58PM -0500, Jeff King wrote: > > > On Tue, Mar 05, 2024 at 01:11:59PM -0800, Josh Steadmon wrote: > > > > > In a future commit, we will build the fuzzer executables as part of the > > > default 'make all' target, which requires a C++ compiler. If we do not > > > explicitly set CXX, it defaults to g++ on GitHub CI. However, this can > > > lead to incorrect feature detection when CC=clang, since the > > > 'detect-compiler' script only looks at CC. Fix the issue by always > > > setting CXX to match CC in our CI config. > > > > > > We only plan on building fuzzers on Linux, so none of the other CI > > > configs need a similar adjustment. > > > > Does this mean that after your patch 2, running: > > > > make CC=clang > > > > may have problems on Linux, because it will now try to link fuzzers > > using g++, even though everything else is built with clang (and ditto > > the detect-compiler used it)? > > Also, if the answer is "yes": do we really need a c++ linker here? My > understanding from reading "git log -SCXX Makefile" is that when using > oss-fuzz, you'd sometimes want to pass c++ specific things in > FUZZ_CXXFLAGS. But we're not using that here, and are just making sure > that things can be linked. Can we just use $(CC) by default here, then? > > Something like: > > diff --git a/Makefile b/Makefile > index f74e96d7c2..3f09d75f46 100644 > --- a/Makefile > +++ b/Makefile > @@ -3861,17 +3861,18 @@ cover_db_html: cover_db > # > # An example command to build against libFuzzer from LLVM 11.0.0: > # > -# make CC=clang CXX=clang++ \ > +# make CC=clang FUZZ_CXX=clang++ \ > # CFLAGS="-fsanitize=fuzzer-no-link,address" \ > # LIB_FUZZING_ENGINE="-fsanitize=fuzzer,address" \ > # fuzz-all > # > +FUZZ_CXX ?= $(CC) > FUZZ_CXXFLAGS ?= $(ALL_CFLAGS) > > .PHONY: fuzz-all > > $(FUZZ_PROGRAMS): %: %.o oss-fuzz/dummy-cmd-main.o $(GITLIBS) GIT-LDFLAGS > - $(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) -o $@ $(ALL_LDFLAGS) \ > + $(QUIET_LINK)$(FUZZ_CXX) $(FUZZ_CXXFLAGS) -o $@ $(ALL_LDFLAGS) \ > -Wl,--allow-multiple-definition \ > $(filter %.o,$^) $(filter %.a,$^) $(LIBS) $(LIB_FUZZING_ENGINE) > > > -Peff Indeed, it does break, and this is a good fix. Thanks for the catch!
