{"thread":{"id":"55232","subject":"[PATCH] Update 'make fuzz-all' docs to reflect modern clang","startedAt":"2021-02-28T12:23:40Z","lastAt":"2021-03-10T18:53:34Z","messageCount":10,"participants":["Andrzej Hunt via GitGitGadget","Josh Steadmon","Andrzej Hunt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"417972","messageId":"pull.889.git.1614514959347.gitgitgadget@gmail.com","threadId":"55232","inReplyTo":null,"subject":"[PATCH] Update 'make fuzz-all' docs to reflect modern clang","fromName":"Andrzej Hunt via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-28T12:22:38Z","receivedAt":"2021-02-28T12:23:40Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"From: Andrzej Hunt <ajrhunt@google.com>\n\nClang no longer produces a libFuzzer.a, instead you can include\nlibFuzzer by using -fsanitize=fuzzer. Therefore we should use\nthat in the example command for building fuzzers.\n\nI happen to have tested with LLVM 11 - however -fsanitize=fuzzer appears to\nwork in a wide range of reasonably modern clangs.\n\n(On my system what used to be libFuzzer.a now lives under the following path,\n which is tricky albeit not impossible for a novice such as myself to find:\n/usr/lib64/clang/11.0.0/lib/linux/libclang_rt.fuzzer-x86_64.a )\n\nSigned-off-by: Andrzej Hunt <ajrhunt@google.com>\n---\n    Update 'make fuzz-all' docs to reflect modern clang\n    \n    I would like to update the examples for 'make fuzz-all' to make it\n    easier to build fuzzers locally.\n    \n    This change should make it easier for the uninitiated to build fuzzers\n    locally without first having to figure out what LIB_FUZZING_ENGINE is\n    for.\n    \n    ATB, Andrzej\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-889%2Fahunt%2Ffuzz-docs-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-889/ahunt/fuzz-docs-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/889\n\n Makefile | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 9b1bde2e0e64..9f8f459f87b4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3291,11 +3291,11 @@ cover_db_html: cover_db\n # are not necessarily appropriate for general builds, and that vary greatly\n # depending on the compiler version used.\n #\n-# An example command to build against libFuzzer from LLVM 4.0.0:\n+# An example command to build against libFuzzer from LLVM 11.0.0:\n #\n # make CC=clang CXX=clang++ \\\n #      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n-#      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \\\n+#      LIB_FUZZING_ENGINE=-fsanitize=fuzzer \\\n #      fuzz-all\n #\n FUZZ_CXXFLAGS ?= $(CFLAGS)\n\nbase-commit: 225365fb5195e804274ab569ac3cc4919451dc7f\n-- \ngitgitgadget\n"},{"id":"418088","messageId":"YD1tJlY/mqZOmTNm@google.com","threadId":"55232","inReplyTo":"pull.889.git.1614514959347.gitgitgadget@gmail.com","subject":"Re: [PATCH] Update 'make fuzz-all' docs to reflect modern clang","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2021-03-01T22:39:34Z","receivedAt":"2021-03-01T23:35:04Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2021.02.28 12:22, Andrzej Hunt via GitGitGadget wrote:\n> From: Andrzej Hunt <ajrhunt@google.com>\n> \n> Clang no longer produces a libFuzzer.a, instead you can include\n> libFuzzer by using -fsanitize=fuzzer. Therefore we should use\n> that in the example command for building fuzzers.\n> \n> I happen to have tested with LLVM 11 - however -fsanitize=fuzzer appears to\n> work in a wide range of reasonably modern clangs.\n> \n> (On my system what used to be libFuzzer.a now lives under the following path,\n>  which is tricky albeit not impossible for a novice such as myself to find:\n> /usr/lib64/clang/11.0.0/lib/linux/libclang_rt.fuzzer-x86_64.a )\n> \n> Signed-off-by: Andrzej Hunt <ajrhunt@google.com>\n> ---\n>     Update 'make fuzz-all' docs to reflect modern clang\n>     \n>     I would like to update the examples for 'make fuzz-all' to make it\n>     easier to build fuzzers locally.\n>     \n>     This change should make it easier for the uninitiated to build fuzzers\n>     locally without first having to figure out what LIB_FUZZING_ENGINE is\n>     for.\n>     \n>     ATB, Andrzej\n\nThanks for taking a look at this! This looked correct to me, but when I\ntried to run the fuzzers I got an error about\n\"-fsanitize-coverage=trace-pc-guard\" not being supported any longer.\nLooking at the LLVM 11.0.0 docs [1], I see that it recommends using\n\"-fsanitize=fuzzer-no-link\" instead (the \"-no-link\" is because we're\nalso building executables that have their own main()).\n\nSo we'd also want to change CFLAGS to\n\"-fsanitize=fuzzer-no-link,address\".\n\n[1]: https://releases.llvm.org/11.0.0/docs/LibFuzzer.html#fuzzer-usage\n"},{"id":"418238","messageId":"094d75bf-707a-5b0d-fead-5c9ad08e0a18@ahunt.org","threadId":"55232","inReplyTo":"YD1tJlY/mqZOmTNm@google.com","subject":"Re: [PATCH] Update 'make fuzz-all' docs to reflect modern clang","fromName":"Andrzej Hunt","fromEmail":"andrzej@ahunt.org","sentAt":"2021-03-04T15:26:21Z","receivedAt":"2021-03-04T15:29:39Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"On 01/03/2021 23:39, Josh Steadmon wrote:\n> Thanks for taking a look at this! This looked correct to me, but when I\n> tried to run the fuzzers I got an error about\n> \"-fsanitize-coverage=trace-pc-guard\" not being supported any longer.\n\nOops, I realised I was accidentally using clang 7 (instead of 11) \nlocally. I can reproduce the same error with my copy of clang-11. Thanks \nfor catching this!\n\n> Looking at the LLVM 11.0.0 docs [1], I see that it recommends using\n> \"-fsanitize=fuzzer-no-link\" instead (the \"-no-link\" is because we're\n> also building executables that have their own main()).\n> \n> So we'd also want to change CFLAGS to\n> \"-fsanitize=fuzzer-no-link,address\".\n\nI will fix this too!\n\nI suspect that when I built without fuzzer-no-link, the fuzzer binaries \nincluded libFuzzer, but were missing whatever fuzzing-related\ninstrumentation clang should have added. (Fortunately oss-fuzz seems to\nbe adding this to the CFLAGS automatically [1].)\n\n[1] \nhttps://oss-fuzz-build-logs.storage.googleapis.com/log-74f40f33-f384-475b-b141-0e44afb272f5.txt\n"},{"id":"418239","messageId":"pull.889.v2.git.1614871707845.gitgitgadget@gmail.com","threadId":"55232","inReplyTo":"pull.889.git.1614514959347.gitgitgadget@gmail.com","subject":"[PATCH v2] Update 'make fuzz-all' docs to reflect modern clang","fromName":"Andrzej Hunt via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-04T15:28:27Z","receivedAt":"2021-03-04T15:29:56Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"From: Andrzej Hunt <ajrhunt@google.com>\n\nClang no longer produces a libFuzzer.a, instead you can include\nlibFuzzer by using -fsanitize=fuzzer. Therefore we should use\nthat in the example command for building fuzzers.\n\nWe also add -fsanitize=fuzzer-no-link to ensure that all the required\ninstrumentation is added when compiling git [1], and remove\n -fsanitize-coverage=trace-pc-guard as it is deprecated.\n\nI happen to have tested with LLVM 11 - however -fsanitize=fuzzer appears to\nwork in a wide range of reasonably modern clangs.\n\n(On my system: what used to be libFuzzer.a now lives under the following path,\n which is tricky albeit not impossible for a novice such as myself to find:\n/usr/lib64/clang/11.0.0/lib/linux/libclang_rt.fuzzer-x86_64.a )\n\n[1] https://releases.llvm.org/11.0.0/docs/LibFuzzer.html#fuzzer-usage\n\nSigned-off-by: Andrzej Hunt <ajrhunt@google.com>\n---\n    Update 'make fuzz-all' docs to reflect modern clang\n    \n    I have updated my patch to:\n    \n     * Remove -fsanitize-coverage=trace-pc-guard as it is deprecated.\n     * Add -fsanitize=fuzzer-no-link as per Josh's suggestion.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-889%2Fahunt%2Ffuzz-docs-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-889/ahunt/fuzz-docs-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/889\n\nRange-diff vs v1:\n\n 1:  d804b24907fd ! 1:  f5b5a11966ca Update 'make fuzz-all' docs to reflect modern clang\n     @@ Commit message\n          libFuzzer by using -fsanitize=fuzzer. Therefore we should use\n          that in the example command for building fuzzers.\n      \n     +    We also add -fsanitize=fuzzer-no-link to ensure that all the required\n     +    instrumentation is added when compiling git [1], and remove\n     +     -fsanitize-coverage=trace-pc-guard as it is deprecated.\n     +\n          I happen to have tested with LLVM 11 - however -fsanitize=fuzzer appears to\n          work in a wide range of reasonably modern clangs.\n      \n     -    (On my system what used to be libFuzzer.a now lives under the following path,\n     +    (On my system: what used to be libFuzzer.a now lives under the following path,\n           which is tricky albeit not impossible for a novice such as myself to find:\n          /usr/lib64/clang/11.0.0/lib/linux/libclang_rt.fuzzer-x86_64.a )\n      \n     +    [1] https://releases.llvm.org/11.0.0/docs/LibFuzzer.html#fuzzer-usage\n     +\n          Signed-off-by: Andrzej Hunt <ajrhunt@google.com>\n      \n       ## Makefile ##\n     @@ Makefile: cover_db_html: cover_db\n      +# An example command to build against libFuzzer from LLVM 11.0.0:\n       #\n       # make CC=clang CXX=clang++ \\\n     - #      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n     +-#      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n      -#      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \\\n     -+#      LIB_FUZZING_ENGINE=-fsanitize=fuzzer \\\n     ++#      CFLAGS=\"-fsanitize=fuzzer-no-link,address\" \\\n     ++#      LIB_FUZZING_ENGINE=\"-fsanitize=fuzzer\" \\\n       #      fuzz-all\n       #\n       FUZZ_CXXFLAGS ?= $(CFLAGS)\n\n\n Makefile | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex dd08b4ced01c..c7248ac6057b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3292,11 +3292,11 @@ cover_db_html: cover_db\n # are not necessarily appropriate for general builds, and that vary greatly\n # depending on the compiler version used.\n #\n-# An example command to build against libFuzzer from LLVM 4.0.0:\n+# An example command to build against libFuzzer from LLVM 11.0.0:\n #\n # make CC=clang CXX=clang++ \\\n-#      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n-#      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \\\n+#      CFLAGS=\"-fsanitize=fuzzer-no-link,address\" \\\n+#      LIB_FUZZING_ENGINE=\"-fsanitize=fuzzer\" \\\n #      fuzz-all\n #\n FUZZ_CXXFLAGS ?= $(CFLAGS)\n\nbase-commit: f01623b2c9d14207e497b21ebc6b3ec4afaf4b46\n-- \ngitgitgadget\n"},{"id":"418267","messageId":"xmqqlfb2cz8c.fsf@gitster.c.googlers.com","threadId":"55232","inReplyTo":"pull.889.v2.git.1614871707845.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] Update 'make fuzz-all' docs to reflect modern clang","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-04T22:48:03Z","receivedAt":"2021-03-04T22:48:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Andrzej Hunt via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Andrzej Hunt <ajrhunt@google.com>\n\n> Subject: Re: [PATCH v2] Update 'make fuzz-all' docs to reflect modern clang\n\nI'd retitte it to\n\n    Makefile: update 'make fuzz-all' docs to reflect modern clang\n\n> Clang no longer produces a libFuzzer.a, instead you can include\n> libFuzzer by using -fsanitize=fuzzer.\n\nDo we see two sentences here?  IOW, s/, instead/. Instead/ is needed?\n\n> Therefore we should use\n> that in the example command for building fuzzers.\n>\n> We also add -fsanitize=fuzzer-no-link to ensure that all the required\n> instrumentation is added when compiling git [1], and remove\n>  -fsanitize-coverage=trace-pc-guard as it is deprecated.\n\nWithout something like s/add/add to CFLAGS/, I found this a bit\ncryptic and failed to read what it wanted to do without looking at\nthe patch text itself.\n\n> I happen to have tested with LLVM 11 - however -fsanitize=fuzzer appears to\n> work in a wide range of reasonably modern clangs.\n>\n> (On my system: what used to be libFuzzer.a now lives under the following path,\n>  which is tricky albeit not impossible for a novice such as myself to find:\n> /usr/lib64/clang/11.0.0/lib/linux/libclang_rt.fuzzer-x86_64.a )\n\nAll nice things to have in the log message.\n\n>  Makefile | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index dd08b4ced01c..c7248ac6057b 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -3292,11 +3292,11 @@ cover_db_html: cover_db\n>  # are not necessarily appropriate for general builds, and that vary greatly\n>  # depending on the compiler version used.\n>  #\n> -# An example command to build against libFuzzer from LLVM 4.0.0:\n> +# An example command to build against libFuzzer from LLVM 11.0.0:\n>  #\n>  # make CC=clang CXX=clang++ \\\n> -#      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n> -#      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \\\n> +#      CFLAGS=\"-fsanitize=fuzzer-no-link,address\" \\\n> +#      LIB_FUZZING_ENGINE=\"-fsanitize=fuzzer\" \\\n>  #      fuzz-all\n>  #\n>  FUZZ_CXXFLAGS ?= $(CFLAGS)\n\nLIB_FUZZING_ENGINE is used this way in the Makefile:\n\n    $(FUZZ_PROGRAMS): all\n            $(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n                    $(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@\n\nand it is somewhat annoying to see a compiler/linker option that\nlate on the command line, where readers would expect an object file\nor a library archive would appear.  It makes me wonder if we should\ninstead be doing something along the following line:\n\n - empty LIB_FUZZING_ENGINE by default\n - add -fsanitize=fuzzer names to FUZZ_CXXFLAGS\n\ni.e.\n\ndiff --git c/Makefile w/Makefile\nindex 4128b457e1..b5df76b33b 100644\n--- c/Makefile\n+++ w/Makefile\n@@ -3306,14 +3306,15 @@ cover_db_html: cover_db\n # are not necessarily appropriate for general builds, and that vary greatly\n # depending on the compiler version used.\n #\n-# An example command to build against libFuzzer from LLVM 4.0.0:\n+# An example command to build against libFuzzer from LLVM 11.0.0:\n #\n # make CC=clang CXX=clang++ \\\n-#      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n-#      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \\\n+#      CFLAGS=\"-fsanitize=fuzzer-no-link,address\" \\\n #      fuzz-all\n #\n FUZZ_CXXFLAGS ?= $(CFLAGS)\n+FUZZ_CXXFLAGS += -fsanitize=fuzzer\n+LIB_FUZZING_ENGINE =\n \n .PHONY: fuzz-all\n \n\nIn the meantime, I'll queue the version you sent as-is (modulo the\nretitling).\n\nThanks.\n\n\n"},{"id":"418481","messageId":"defff7a3-2104-4fa1-7750-0b13ca5cdf59@ahunt.org","threadId":"55232","inReplyTo":"xmqqlfb2cz8c.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] Update 'make fuzz-all' docs to reflect modern clang","fromName":"Andrzej Hunt","fromEmail":"andrzej@ahunt.org","sentAt":"2021-03-08T17:05:08Z","receivedAt":"2021-03-08T17:06:12Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"On 04/03/2021 23:48, Junio C Hamano wrote:>\n> LIB_FUZZING_ENGINE is used this way in the Makefile:\n> \n>      $(FUZZ_PROGRAMS): all\n>              $(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n>                      $(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@\n> \n> and it is somewhat annoying to see a compiler/linker option that\n> late on the command line, where readers would expect an object file\n> or a library archive would appear.  It makes me wonder if we should\n> instead be doing something along the following line:\n> \n>   - empty LIB_FUZZING_ENGINE by default\n>   - add -fsanitize=fuzzer names to FUZZ_CXXFLAGS\n\nThis sounds sensible to me, and will certainly simplify the use of\n\"make fuzz-all\" by beginners - although I'm not sure just how useful the \nchange would be since my understanding is that this target is almost \nexclusively used by oss-fuzz.\n\nHowever I would prefer to wait for Josh's feedback before making such a \nchange, as he is the owner of oss-fuzz's git integration [1], and as \nsuch is most likely to be affected by any changes to this target.\n\nIn the meantime I'll prepare an updated patch with a fixed commit message!\n\n[1] \nhttps://github.com/google/oss-fuzz/blob/c41e46ffc8bc409bdfde0c0d2c97e1305f0c4106/projects/git/project.yaml#L3\n"},{"id":"418482","messageId":"pull.889.v3.git.1615223682911.gitgitgadget@gmail.com","threadId":"55232","inReplyTo":"pull.889.v2.git.1614871707845.gitgitgadget@gmail.com","subject":"[PATCH v3] Makefile: update 'make fuzz-all' docs to reflect modern clang","fromName":"Andrzej Hunt via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-08T17:14:42Z","receivedAt":"2021-03-08T17:15:20Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"From: Andrzej Hunt <ajrhunt@google.com>\n\nClang no longer produces a libFuzzer.a. Instead, you can include\nlibFuzzer by using -fsanitize=fuzzer. Therefore we should use that in\nthe example command for building fuzzers.\n\nWe also add -fsanitize=fuzzer-no-link to the CFLAGS to ensure that all\nthe required instrumentation is added when compiling git [1], and remove\n -fsanitize-coverage=trace-pc-guard as it is deprecated.\n\nI happen to have tested with LLVM 11 - however -fsanitize=fuzzer appears\nto work in a wide range of reasonably modern clangs.\n\n(On my system: what used to be libFuzzer.a now lives under the following\n path, which is tricky albeit not impossible for a novice such as myself\n to find:\n/usr/lib64/clang/11.0.0/lib/linux/libclang_rt.fuzzer-x86_64.a )\n\n[1] https://releases.llvm.org/11.0.0/docs/LibFuzzer.html#fuzzer-usage\n\nSigned-off-by: Andrzej Hunt <ajrhunt@google.com>\n---\n    Update 'make fuzz-all' docs to reflect modern clang\n    \n    This version of the patch fixes the commit message as per Junio's\n    feedback. Thank you!\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-889%2Fahunt%2Ffuzz-docs-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-889/ahunt/fuzz-docs-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/889\n\nRange-diff vs v2:\n\n 1:  f5b5a11966ca ! 1:  bc0d8b615410 Update 'make fuzz-all' docs to reflect modern clang\n     @@ Metadata\n      Author: Andrzej Hunt <ajrhunt@google.com>\n      \n       ## Commit message ##\n     -    Update 'make fuzz-all' docs to reflect modern clang\n     +    Makefile: update 'make fuzz-all' docs to reflect modern clang\n      \n     -    Clang no longer produces a libFuzzer.a, instead you can include\n     -    libFuzzer by using -fsanitize=fuzzer. Therefore we should use\n     -    that in the example command for building fuzzers.\n     +    Clang no longer produces a libFuzzer.a. Instead, you can include\n     +    libFuzzer by using -fsanitize=fuzzer. Therefore we should use that in\n     +    the example command for building fuzzers.\n      \n     -    We also add -fsanitize=fuzzer-no-link to ensure that all the required\n     -    instrumentation is added when compiling git [1], and remove\n     +    We also add -fsanitize=fuzzer-no-link to the CFLAGS to ensure that all\n     +    the required instrumentation is added when compiling git [1], and remove\n           -fsanitize-coverage=trace-pc-guard as it is deprecated.\n      \n     -    I happen to have tested with LLVM 11 - however -fsanitize=fuzzer appears to\n     -    work in a wide range of reasonably modern clangs.\n     +    I happen to have tested with LLVM 11 - however -fsanitize=fuzzer appears\n     +    to work in a wide range of reasonably modern clangs.\n      \n     -    (On my system: what used to be libFuzzer.a now lives under the following path,\n     -     which is tricky albeit not impossible for a novice such as myself to find:\n     +    (On my system: what used to be libFuzzer.a now lives under the following\n     +     path, which is tricky albeit not impossible for a novice such as myself\n     +     to find:\n          /usr/lib64/clang/11.0.0/lib/linux/libclang_rt.fuzzer-x86_64.a )\n      \n          [1] https://releases.llvm.org/11.0.0/docs/LibFuzzer.html#fuzzer-usage\n\n\n Makefile | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex dfb0f1000fa3..f3dc2178324e 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3299,11 +3299,11 @@ cover_db_html: cover_db\n # are not necessarily appropriate for general builds, and that vary greatly\n # depending on the compiler version used.\n #\n-# An example command to build against libFuzzer from LLVM 4.0.0:\n+# An example command to build against libFuzzer from LLVM 11.0.0:\n #\n # make CC=clang CXX=clang++ \\\n-#      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n-#      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \\\n+#      CFLAGS=\"-fsanitize=fuzzer-no-link,address\" \\\n+#      LIB_FUZZING_ENGINE=\"-fsanitize=fuzzer\" \\\n #      fuzz-all\n #\n FUZZ_CXXFLAGS ?= $(CFLAGS)\n\nbase-commit: be7935ed8bff19f481b033d0d242c5d5f239ed50\n-- \ngitgitgadget\n"},{"id":"418491","messageId":"xmqqsg555wlq.fsf@gitster.c.googlers.com","threadId":"55232","inReplyTo":"defff7a3-2104-4fa1-7750-0b13ca5cdf59@ahunt.org","subject":"Re: [PATCH v2] Update 'make fuzz-all' docs to reflect modern clang","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-08T18:28:01Z","receivedAt":"2021-03-08T18:28:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrzej Hunt <andrzej@ahunt.org> writes:\n\n> However I would prefer to wait for Josh's feedback before making such\n> a change, as he is the owner of oss-fuzz's git integration [1], and as \n> such is most likely to be affected by any changes to this target.\n>\n>\n> In the meantime I'll prepare an updated patch with a fixed commit message!\n\nMakes sense.  Let's wait and see what Josh says before going forward.\n"},{"id":"418700","messageId":"YEkVCRtVimEct0D8@google.com","threadId":"55232","inReplyTo":"xmqqlfb2cz8c.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] Update 'make fuzz-all' docs to reflect modern clang","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2021-03-10T18:50:49Z","receivedAt":"2021-03-10T18:51:57Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2021.03.04 14:48, Junio C Hamano wrote:\n> \"Andrzej Hunt via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n> > From: Andrzej Hunt <ajrhunt@google.com>\n> \n> > Subject: Re: [PATCH v2] Update 'make fuzz-all' docs to reflect modern clang\n> \n> I'd retitte it to\n> \n>     Makefile: update 'make fuzz-all' docs to reflect modern clang\n> \n> > Clang no longer produces a libFuzzer.a, instead you can include\n> > libFuzzer by using -fsanitize=fuzzer.\n> \n> Do we see two sentences here?  IOW, s/, instead/. Instead/ is needed?\n> \n> > Therefore we should use\n> > that in the example command for building fuzzers.\n> >\n> > We also add -fsanitize=fuzzer-no-link to ensure that all the required\n> > instrumentation is added when compiling git [1], and remove\n> >  -fsanitize-coverage=trace-pc-guard as it is deprecated.\n> \n> Without something like s/add/add to CFLAGS/, I found this a bit\n> cryptic and failed to read what it wanted to do without looking at\n> the patch text itself.\n> \n> > I happen to have tested with LLVM 11 - however -fsanitize=fuzzer appears to\n> > work in a wide range of reasonably modern clangs.\n> >\n> > (On my system: what used to be libFuzzer.a now lives under the following path,\n> >  which is tricky albeit not impossible for a novice such as myself to find:\n> > /usr/lib64/clang/11.0.0/lib/linux/libclang_rt.fuzzer-x86_64.a )\n> \n> All nice things to have in the log message.\n> \n> >  Makefile | 6 +++---\n> >  1 file changed, 3 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/Makefile b/Makefile\n> > index dd08b4ced01c..c7248ac6057b 100644\n> > --- a/Makefile\n> > +++ b/Makefile\n> > @@ -3292,11 +3292,11 @@ cover_db_html: cover_db\n> >  # are not necessarily appropriate for general builds, and that vary greatly\n> >  # depending on the compiler version used.\n> >  #\n> > -# An example command to build against libFuzzer from LLVM 4.0.0:\n> > +# An example command to build against libFuzzer from LLVM 11.0.0:\n> >  #\n> >  # make CC=clang CXX=clang++ \\\n> > -#      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n> > -#      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \\\n> > +#      CFLAGS=\"-fsanitize=fuzzer-no-link,address\" \\\n> > +#      LIB_FUZZING_ENGINE=\"-fsanitize=fuzzer\" \\\n> >  #      fuzz-all\n> >  #\n> >  FUZZ_CXXFLAGS ?= $(CFLAGS)\n> \n> LIB_FUZZING_ENGINE is used this way in the Makefile:\n> \n>     $(FUZZ_PROGRAMS): all\n>             $(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n>                     $(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@\n> \n> and it is somewhat annoying to see a compiler/linker option that\n> late on the command line, where readers would expect an object file\n> or a library archive would appear.\n\nYes, it appears that clang has changed how the fuzzing engine is\nselected, as this used to be just a library path (as you see in the\ndiff). We might as well move this option up with the rest of the flags.\n\n> It makes me wonder if we should\n> instead be doing something along the following line:\n> \n>  - empty LIB_FUZZING_ENGINE by default\n>  - add -fsanitize=fuzzer names to FUZZ_CXXFLAGS\n> \n> i.e.\n> \n> diff --git c/Makefile w/Makefile\n> index 4128b457e1..b5df76b33b 100644\n> --- c/Makefile\n> +++ w/Makefile\n> @@ -3306,14 +3306,15 @@ cover_db_html: cover_db\n>  # are not necessarily appropriate for general builds, and that vary greatly\n>  # depending on the compiler version used.\n>  #\n> -# An example command to build against libFuzzer from LLVM 4.0.0:\n> +# An example command to build against libFuzzer from LLVM 11.0.0:\n>  #\n>  # make CC=clang CXX=clang++ \\\n> -#      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n> -#      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \\\n> +#      CFLAGS=\"-fsanitize=fuzzer-no-link,address\" \\\n>  #      fuzz-all\n>  #\n>  FUZZ_CXXFLAGS ?= $(CFLAGS)\n> +FUZZ_CXXFLAGS += -fsanitize=fuzzer\n> +LIB_FUZZING_ENGINE =\n\nI don't think we want to mess with FUZZ_CXXFLAGS, as oss-fuzz may be\nadding conflicting -fsanitize args here. Having LIB_FUZZING_ENGINE\ndefault to empty should be fine though.\n\n>  \n>  .PHONY: fuzz-all\n>  \n> \n> In the meantime, I'll queue the version you sent as-is (modulo the\n> retitling).\n> \n> Thanks.\n> \n> \n"},{"id":"418701","messageId":"YEkVdx6PugLSX2UF@google.com","threadId":"55232","inReplyTo":"pull.889.v3.git.1615223682911.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] Makefile: update 'make fuzz-all' docs to reflect modern clang","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2021-03-10T18:52:39Z","receivedAt":"2021-03-10T18:53:34Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2021.03.08 17:14, Andrzej Hunt via GitGitGadget wrote:\n> From: Andrzej Hunt <ajrhunt@google.com>\n> \n> Clang no longer produces a libFuzzer.a. Instead, you can include\n> libFuzzer by using -fsanitize=fuzzer. Therefore we should use that in\n> the example command for building fuzzers.\n> \n> We also add -fsanitize=fuzzer-no-link to the CFLAGS to ensure that all\n> the required instrumentation is added when compiling git [1], and remove\n>  -fsanitize-coverage=trace-pc-guard as it is deprecated.\n> \n> I happen to have tested with LLVM 11 - however -fsanitize=fuzzer appears\n> to work in a wide range of reasonably modern clangs.\n> \n> (On my system: what used to be libFuzzer.a now lives under the following\n>  path, which is tricky albeit not impossible for a novice such as myself\n>  to find:\n> /usr/lib64/clang/11.0.0/lib/linux/libclang_rt.fuzzer-x86_64.a )\n> \n> [1] https://releases.llvm.org/11.0.0/docs/LibFuzzer.html#fuzzer-usage\n> \n> Signed-off-by: Andrzej Hunt <ajrhunt@google.com>\n> ---\n>     Update 'make fuzz-all' docs to reflect modern clang\n>     \n>     This version of the patch fixes the commit message as per Junio's\n>     feedback. Thank you!\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-889%2Fahunt%2Ffuzz-docs-v3\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-889/ahunt/fuzz-docs-v3\n> Pull-Request: https://github.com/gitgitgadget/git/pull/889\n> \n> Range-diff vs v2:\n> \n>  1:  f5b5a11966ca ! 1:  bc0d8b615410 Update 'make fuzz-all' docs to reflect modern clang\n>      @@ Metadata\n>       Author: Andrzej Hunt <ajrhunt@google.com>\n>       \n>        ## Commit message ##\n>      -    Update 'make fuzz-all' docs to reflect modern clang\n>      +    Makefile: update 'make fuzz-all' docs to reflect modern clang\n>       \n>      -    Clang no longer produces a libFuzzer.a, instead you can include\n>      -    libFuzzer by using -fsanitize=fuzzer. Therefore we should use\n>      -    that in the example command for building fuzzers.\n>      +    Clang no longer produces a libFuzzer.a. Instead, you can include\n>      +    libFuzzer by using -fsanitize=fuzzer. Therefore we should use that in\n>      +    the example command for building fuzzers.\n>       \n>      -    We also add -fsanitize=fuzzer-no-link to ensure that all the required\n>      -    instrumentation is added when compiling git [1], and remove\n>      +    We also add -fsanitize=fuzzer-no-link to the CFLAGS to ensure that all\n>      +    the required instrumentation is added when compiling git [1], and remove\n>            -fsanitize-coverage=trace-pc-guard as it is deprecated.\n>       \n>      -    I happen to have tested with LLVM 11 - however -fsanitize=fuzzer appears to\n>      -    work in a wide range of reasonably modern clangs.\n>      +    I happen to have tested with LLVM 11 - however -fsanitize=fuzzer appears\n>      +    to work in a wide range of reasonably modern clangs.\n>       \n>      -    (On my system: what used to be libFuzzer.a now lives under the following path,\n>      -     which is tricky albeit not impossible for a novice such as myself to find:\n>      +    (On my system: what used to be libFuzzer.a now lives under the following\n>      +     path, which is tricky albeit not impossible for a novice such as myself\n>      +     to find:\n>           /usr/lib64/clang/11.0.0/lib/linux/libclang_rt.fuzzer-x86_64.a )\n>       \n>           [1] https://releases.llvm.org/11.0.0/docs/LibFuzzer.html#fuzzer-usage\n> \n> \n>  Makefile | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n> \n> diff --git a/Makefile b/Makefile\n> index dfb0f1000fa3..f3dc2178324e 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -3299,11 +3299,11 @@ cover_db_html: cover_db\n>  # are not necessarily appropriate for general builds, and that vary greatly\n>  # depending on the compiler version used.\n>  #\n> -# An example command to build against libFuzzer from LLVM 4.0.0:\n> +# An example command to build against libFuzzer from LLVM 11.0.0:\n>  #\n>  # make CC=clang CXX=clang++ \\\n> -#      CFLAGS=\"-fsanitize-coverage=trace-pc-guard -fsanitize=address\" \\\n> -#      LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \\\n> +#      CFLAGS=\"-fsanitize=fuzzer-no-link,address\" \\\n> +#      LIB_FUZZING_ENGINE=\"-fsanitize=fuzzer\" \\\n>  #      fuzz-all\n>  #\n>  FUZZ_CXXFLAGS ?= $(CFLAGS)\n> \n> base-commit: be7935ed8bff19f481b033d0d242c5d5f239ed50\n> -- \n> gitgitgadget\n\nThis version looks good to me, although you may also want to make the\nchanges Junio suggested regarding LIB_FUZZING_ENGINE.\n\nThanks!\n"}]}