| Message ID | 75f98cbf98005b0a069977096ec5501f2f7830fe.1709673020.git.steadmon@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fuzz: build fuzzers by default on Linux | expand |
Josh Steadmon <steadmon@google.com> writes: > 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. Sounds fair. It's not like we as the project decides to never build fuzzers on macOS and will forbid others from doing so. Those who are not part of "we" are welcome to add support to build fuzzers on other platforms. So perhaps We only plan on building fuzzers on Linux with the next patch, so for now, only adjust configuration for the Linux CI jobs. may convey our intention better to our future selves. Thanks.
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)? -Peff
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
On 2024.03.05 13:37, Junio C Hamano wrote: > Josh Steadmon <steadmon@google.com> writes: > > > 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. > > Sounds fair. It's not like we as the project decides to never build > fuzzers on macOS and will forbid others from doing so. Those who > are not part of "we" are welcome to add support to build fuzzers on > other platforms. So perhaps > > We only plan on building fuzzers on Linux with the next patch, > so for now, only adjust configuration for the Linux CI jobs. > > may convey our intention better to our future selves. > > Thanks. Reworded as suggested. Sorry for letting this series sit without attention for so long. Will send V2 soon.
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!
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 683a2d633e..83945a3235 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -265,42 +265,54 @@ jobs: vector: - jobname: linux-sha256 cc: clang + cxx: clang++ pool: ubuntu-latest - jobname: linux-reftable cc: clang + cxx: clang++ pool: ubuntu-latest - jobname: linux-gcc cc: gcc + cxx: g++ cc_package: gcc-8 pool: ubuntu-20.04 - jobname: linux-TEST-vars cc: gcc + cxx: g++ cc_package: gcc-8 pool: ubuntu-20.04 - jobname: osx-clang cc: clang + cxx: clang++ pool: macos-13 - jobname: osx-reftable cc: clang + cxx: clang++ pool: macos-13 - jobname: osx-gcc cc: gcc + cxx: g++ cc_package: gcc-13 pool: macos-13 - jobname: linux-gcc-default cc: gcc + cxx: g++ pool: ubuntu-latest - jobname: linux-leaks cc: gcc + cxx: g++ pool: ubuntu-latest - jobname: linux-reftable-leaks cc: gcc + cxx: g++ pool: ubuntu-latest - jobname: linux-asan-ubsan cc: clang + cxx: clang++ pool: ubuntu-latest env: CC: ${{matrix.vector.cc}} + CXX: ${{matrix.vector.cxx}} CC_PACKAGE: ${{matrix.vector.cc_package}} jobname: ${{matrix.vector.jobname}} runs_on_pool: ${{matrix.vector.pool}}
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. Signed-off-by: Josh Steadmon <steadmon@google.com> --- .github/workflows/main.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+)