{"thread":{"id":"49813","subject":"[PATCH] Makefile: use CXXFLAGS for linking fuzzers","startedAt":"2018-11-12T22:03:01Z","lastAt":"2018-11-14T19:41:52Z","messageCount":6,"participants":["steadmon@google.com","Stefan Beller","Junio C Hamano","Josh Steadmon"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"363088","messageId":"1630a93f8270ca090459be8cc7213221cc6250cf.1542060094.git.steadmon@google.com","threadId":"49813","inReplyTo":null,"subject":"[PATCH] Makefile: use CXXFLAGS for linking fuzzers","fromName":"","fromEmail":"steadmon@google.com","sentAt":"2018-11-12T22:02:56Z","receivedAt":"2018-11-12T22:03:01Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"OSS-Fuzz requires C++-specific flags to link fuzzers. Passing these in\nCFLAGS causes lots of build warnings. Using separate CXXFLAGS avoids\nthis.\n\nSigned-off-by: Josh Steadmon <steadmon@google.com>\n---\n Makefile | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex bbfbb4292d..5462bc4b6b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -497,6 +497,7 @@ GIT-VERSION-FILE: FORCE\n # CFLAGS and LDFLAGS are for the users to override from the command line.\n \n CFLAGS = -g -O2 -Wall\n+CXXFLAGS ?= $(CFLAGS)\n LDFLAGS =\n ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n ALL_LDFLAGS = $(LDFLAGS)\n@@ -3098,14 +3099,14 @@ cover_db_html: cover_db\n # An example command to build against libFuzzer from LLVM 4.0.0:\n #\n # make CC=clang CXX=clang++ \\\n-#      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n+#      CXXFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n #      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \\\n #      fuzz-all\n #\n .PHONY: fuzz-all\n \n $(FUZZ_PROGRAMS): all\n-\t$(QUIET_LINK)$(CXX) $(CFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n+\t$(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n \t\t$(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@\n \n fuzz-all: $(FUZZ_PROGRAMS)\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"363094","messageId":"CAGZ79kb68+vxHmwbttvXgaxvPdqAh3vJjZ9+U+BjY7ZeZzOxUA@mail.gmail.com","threadId":"49813","inReplyTo":"1630a93f8270ca090459be8cc7213221cc6250cf.1542060094.git.steadmon@google.com","subject":"Re: [PATCH] Makefile: use CXXFLAGS for linking fuzzers","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-11-12T22:47:37Z","receivedAt":"2018-11-12T22:47:51Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Nov 12, 2018 at 2:03 PM <steadmon@google.com> wrote:\n>\n> OSS-Fuzz requires C++-specific flags to link fuzzers. Passing these in\n> CFLAGS causes lots of build warnings. Using separate CXXFLAGS avoids\n> this.\n>\n\nThat makes sense in this context, ....\n\n>  CFLAGS = -g -O2 -Wall\n> +CXXFLAGS ?= $(CFLAGS)\n\n... but out of context, just by reading the relevant part of the Makefile,\na user might mistakenly assume we do some C++ trickery for standard\ncompilation of Git. (Is that bad or do we just not care?)\n\nI wonder if setting the CXXFLAGS near or in the fuzz target\nwould be better.\n\n>  LDFLAGS =\n>  ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n>  ALL_LDFLAGS = $(LDFLAGS)\n> @@ -3098,14 +3099,14 @@ cover_db_html: cover_db\n>  # An example command to build against libFuzzer from LLVM 4.0.0:\n>  #\n>  # make CC=clang CXX=clang++ \\\n> -#      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n> +#      CXXFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n>  #      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \\\n>  #      fuzz-all\n>  #\n>  .PHONY: fuzz-all\n\nMaybe here?\n\n>\n>  $(FUZZ_PROGRAMS): all\n> -       $(QUIET_LINK)$(CXX) $(CFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n> +       $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n>                 $(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@\n\nThanks,\nStefan\n"},{"id":"363127","messageId":"xmqq36s5y8l2.fsf@gitster-ct.c.googlers.com","threadId":"49813","inReplyTo":"1630a93f8270ca090459be8cc7213221cc6250cf.1542060094.git.steadmon@google.com","subject":"Re: [PATCH] Makefile: use CXXFLAGS for linking fuzzers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-13T03:12:09Z","receivedAt":"2018-11-13T03:12:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"steadmon@google.com writes:\n\n> OSS-Fuzz requires C++-specific flags to link fuzzers. Passing these in\n> CFLAGS causes lots of build warnings. Using separate CXXFLAGS avoids\n> this.\n\nWe are not a C++ shop, so allow me to show ignorance about how\nprojects that are OSS-Fuzz-enabled work.  Do they use one set of\nCXXFLAGS when compiling the \"real thing\" and a separate set (perhaps\none is subset of the other, or perhaps these two just have overlap)\nof CXXFLAGS when building to link with the fuzzer?\n\nWhat I am trying to get at is if this should be CXXFLAGS or\nCXX_FUZZER_FLAGS.  If the OSS-Fuzz-enabled C++ projects use one set\nof flags for the \"main\" part of the project (to produce binaries to\nbe run by the end users) and then link with extra flags to work with\nfuzzers, I would imagine that they won't call the latter CXXFLAGS\nbut call it something else, and we probably should follow suit if\nthat is the case.\n\nNot that we plan to (re)write the maint part of Git in C++ ever, so\nI am personally OK with sacrificing the most generic CXXFLAGS macro\nfor the sole use of OSS-Fuzz linkage, but I'd prefer to leave the\ndoor open so that other things like OSS-Fuzz that require C++ can be\nadded like your work does to the project.\n\nThanks.\n\n\n> Signed-off-by: Josh Steadmon <steadmon@google.com>\n> ---\n>  Makefile | 5 +++--\n>  1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index bbfbb4292d..5462bc4b6b 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -497,6 +497,7 @@ GIT-VERSION-FILE: FORCE\n>  # CFLAGS and LDFLAGS are for the users to override from the command line.\n>  \n>  CFLAGS = -g -O2 -Wall\n> +CXXFLAGS ?= $(CFLAGS)\n>  LDFLAGS =\n>  ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n>  ALL_LDFLAGS = $(LDFLAGS)\n> @@ -3098,14 +3099,14 @@ cover_db_html: cover_db\n>  # An example command to build against libFuzzer from LLVM 4.0.0:\n>  #\n>  # make CC=clang CXX=clang++ \\\n> -#      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n> +#      CXXFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n>  #      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \\\n>  #      fuzz-all\n>  #\n>  .PHONY: fuzz-all\n>  \n>  $(FUZZ_PROGRAMS): all\n> -\t$(QUIET_LINK)$(CXX) $(CFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n> +\t$(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n>  \t\t$(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@\n>  \n>  fuzz-all: $(FUZZ_PROGRAMS)\n"},{"id":"363191","messageId":"20181113185032.GC126896@google.com","threadId":"49813","inReplyTo":"xmqq36s5y8l2.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] Makefile: use CXXFLAGS for linking fuzzers","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2018-11-13T18:50:32Z","receivedAt":"2018-11-13T18:50:42Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2018.11.13 12:12, Junio C Hamano wrote:\n> steadmon@google.com writes:\n> \n> > OSS-Fuzz requires C++-specific flags to link fuzzers. Passing these in\n> > CFLAGS causes lots of build warnings. Using separate CXXFLAGS avoids\n> > this.\n> \n> We are not a C++ shop, so allow me to show ignorance about how\n> projects that are OSS-Fuzz-enabled work.  Do they use one set of\n> CXXFLAGS when compiling the \"real thing\" and a separate set (perhaps\n> one is subset of the other, or perhaps these two just have overlap)\n> of CXXFLAGS when building to link with the fuzzer?\n> \n> What I am trying to get at is if this should be CXXFLAGS or\n> CXX_FUZZER_FLAGS.  If the OSS-Fuzz-enabled C++ projects use one set\n> of flags for the \"main\" part of the project (to produce binaries to\n> be run by the end users) and then link with extra flags to work with\n> fuzzers, I would imagine that they won't call the latter CXXFLAGS\n> but call it something else, and we probably should follow suit if\n> that is the case.\n> \n> Not that we plan to (re)write the maint part of Git in C++ ever, so\n> I am personally OK with sacrificing the most generic CXXFLAGS macro\n> for the sole use of OSS-Fuzz linkage, but I'd prefer to leave the\n> door open so that other things like OSS-Fuzz that require C++ can be\n> added like your work does to the project.\n> \n> Thanks.\n\nOSS-Fuzz only provides one set of CXXFLAGS for use on both compiling\nproject C++ project files as well linking the fuzzers themselves. So in\nthe event that Git ever added any C++ sources, they would need to use\nthe same set of CXXFLAGS.\n\nGiven that, do you agree with Stefan that it is more intuitive to define\nCXXFLAGS next to the fuzzer build rules, since that's the only place\nit's used for now?\n"},{"id":"363288","messageId":"xmqqsh04qsd6.fsf@gitster-ct.c.googlers.com","threadId":"49813","inReplyTo":"20181113185032.GC126896@google.com","subject":"Re: [PATCH] Makefile: use CXXFLAGS for linking fuzzers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T02:56:37Z","receivedAt":"2018-11-14T02:56:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Steadmon <steadmon@google.com> writes:\n\n> OSS-Fuzz only provides one set of CXXFLAGS for use on both compiling\n> project C++ project files as well linking the fuzzers themselves. So in\n> the event that Git ever added any C++ sources, they would need to use\n> the same set of CXXFLAGS.\n\nOK.\n\n> Given that, do you agree with Stefan that it is more intuitive to define\n> CXXFLAGS next to the fuzzer build rules, since that's the only place\n> it's used for now?\n\nI am not sure.  Until we gain other C++ targets (in other words,\nwhile linking with fuzzer is the only consumer of CXXFLAGS), I'd\nconsider it similar to SPARSE_FLAGS and SPATCH_FLAGS, i.e. settings\nspecific to an auxiliary tool that supports our development process,\nand it would make more sense to define it near them higher in the\nMakefile.\n\nI'd probably feel differently if this were called FUZZ_CXXFLAGS or\nsomething like that, which would make its natural home next to\nthe rule to build $(FUZZ_PROGRAMS), though.\n"},{"id":"363392","messageId":"6b3d6dd7f0ab9324fcdf1b9322ef7e35d8355964.1542224326.git.steadmon@google.com","threadId":"49813","inReplyTo":"1630a93f8270ca090459be8cc7213221cc6250cf.1542060094.git.steadmon@google.com","subject":"[PATCH v2] Makefile: use FUZZ_CXXFLAGS for linking fuzzers","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2018-11-14T19:41:47Z","receivedAt":"2018-11-14T19:41:52Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"OSS-Fuzz requires C++-specific flags to link fuzzers. Passing these in\nCFLAGS causes lots of build warnings. Using separate FUZZ_CXXFLAGS\navoids this.\n\nSigned-off-by: Josh Steadmon <steadmon@google.com>\n---\nSince there's nothing else using CXXFLAGS, let's just make it explicit\nthat these apply to the fuzzers.\n\n\nRange-diff against v1:\n1:  1630a93f82 < -:  ---------- Makefile: use CXXFLAGS for linking fuzzers\n-:  ---------- > 1:  6b3d6dd7f0 Makefile: use FUZZ_CXXFLAGS for linking fuzzers\n\n Makefile | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex bbfbb4292d..0c05896797 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3098,14 +3098,16 @@ cover_db_html: cover_db\n # An example command to build against libFuzzer from LLVM 4.0.0:\n #\n # make CC=clang CXX=clang++ \\\n-#      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n+#      FUZZ_CXXFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n #      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \\\n #      fuzz-all\n #\n+FUZZ_CXXFLAGS ?= $(CFLAGS)\n+\n .PHONY: fuzz-all\n \n $(FUZZ_PROGRAMS): all\n-\t$(QUIET_LINK)$(CXX) $(CFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n+\t$(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n \t\t$(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@\n \n fuzz-all: $(FUZZ_PROGRAMS)\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"}]}