Re: [PATCH 1/2] ci: also define CXX environment variable

From: Jeff King <peff@peff.net>
Date: Mar 6, 2024, 01:00 UTC
Message-ID: <20240306010016.GA3811328@coredump.intra.peff.net>
In-Reply-To: <20240306005057.GC3797463@coredump.intra.peff.net>

On Tue, Mar 05, 2024 at 07:50:58PM -0500, Jeff King wrote:

Show 19 quoted lines

> 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