{"thread":{"id":"62681","subject":"Git v2.46.0 and --allow-multiple-definition linker flag","startedAt":"2024-12-21T23:23:58Z","lastAt":"2025-01-14T11:03:25Z","messageCount":5,"participants":["James Mills","Junio C Hamano","Josh Steadmon","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"509487","messageId":"CALGqR9+bH3nMrGqPQ18aqs-epSHRFQOtnd6Da55=KrtSu+Jrkg@mail.gmail.com","threadId":"62681","inReplyTo":null,"subject":"Git v2.46.0 and --allow-multiple-definition linker flag","fromName":"James Mills","fromEmail":"prologic@shortcircuit.net.au","sentAt":"2024-12-21T23:23:19Z","receivedAt":"2024-12-21T23:23:58Z","isPatch":false,"sender":{"key":"prologic@shortcircuit.net.au","avatar":null},"body":"Hi Folks 👋\n\nBeen a while since I've emailed one of the large(r) open source lists,\nso please excuse me I may be a bit rusty.\n\nFor some years now I've been building a very tiny custom LInux\ndistribution called µLInux (micro Linux) over at\nhttps://git.mills.io/prologic/ulinux -- I primarily target the x86_64\narchitecture and its intended use is mostly as lightweight VM(s) that\nboot really quickly but are otherwise populated with \"just enough\"\nUNIX tools, a small package manager and few other bits to get\ninteresting things going.\n\nToday I noticed that when upgrading the zlib (✅) and git (❌) ports\nthat the latest version of Git v2.47.2 does not link and I observe the\nfollowing linker error, mostly what looks like a missing\ncapability/flag of the TinyC compiler that is used as the linker\n(LD=tcc):\n\ntcc: error: unsupported linker option '--allow-multiple-definition'\n\nI bisected the Git releases and traced the introduction of this new\nflag to v2.46.0\n\nI can't find any details of this flag really or when this was\nintroduced in the GNU binutils and so far I haven't asked if the Tiny\nC devs intend to support this option (yet).\n\nJust wondering, can we consider backing this out? From my very brief\nresearch on the web this seems to allow the linker to permit multiple\ndefinitions of a function. I'm not even sure why that would be\ndesirable to be honest (but I haven't been a C/C++ developer basically\never, I know enough to be dangerous!)\n\nKind regards\n\nJames\n\n-- James Mills / prologic\n\nJoin Yarn.social today! The only decentralised social media that\nrespects your privacy and freedoms!\n\nE:    prologic@shortcircuit.net.au\nW:    prologic.shortcircuit.net.au\nBlog: Read my Blog\nYarn: @prologic@twtxt.net\n"},{"id":"509488","messageId":"xmqqbjx4bgae.fsf@gitster.g","threadId":"62681","inReplyTo":"CALGqR9+bH3nMrGqPQ18aqs-epSHRFQOtnd6Da55=KrtSu+Jrkg@mail.gmail.com","subject":"Re: Git v2.46.0 and --allow-multiple-definition linker flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-22T02:30:33Z","receivedAt":"2024-12-22T02:30:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"James Mills <prologic@shortcircuit.net.au> writes:\n\n> tcc: error: unsupported linker option '--allow-multiple-definition'\n>\n> I bisected the Git releases and traced the introduction of this new\n> flag to v2.46.0\n>\n> I can't find any details of this flag really or when this was\n> introduced in the GNU binutils and so far I haven't asked if the Tiny\n> C devs intend to support this option (yet).\n\nWould\n\n    $ make LINK_FUZZ_PROGRAMS=\"\"\n\nhelp?\n\nThe platform-specific tweak defined in config.mak.uname file assumes\nthat you have glibc plus gcc or clang with usual binutils niceties\nonce you claim that you are Linux.  It lumps all different variants\nof Linux into a single ball of wax and defines LINK_FUZZ_PROGRAMS\nMakefile macro, which is a bit unfortunate.\n\n\n"},{"id":"509489","messageId":"xmqq7c7sbfhi.fsf@gitster.g","threadId":"62681","inReplyTo":"xmqqbjx4bgae.fsf@gitster.g","subject":"Re: Git v2.46.0 and --allow-multiple-definition linker flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-22T02:47:53Z","receivedAt":"2024-12-22T02:47:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> James Mills <prologic@shortcircuit.net.au> writes:\n>\n>> tcc: error: unsupported linker option '--allow-multiple-definition'\n>>\n>> I bisected the Git releases and traced the introduction of this new\n>> flag to v2.46.0\n>>\n>> I can't find any details of this flag really or when this was\n>> introduced in the GNU binutils and so far I haven't asked if the Tiny\n>> C devs intend to support this option (yet).\n>\n> Would\n>\n>     $ make LINK_FUZZ_PROGRAMS=\"\"\n>\n> help?\n>\n> The platform-specific tweak defined in config.mak.uname file assumes\n> that you have glibc plus gcc or clang with usual binutils niceties\n> once you claim that you are Linux.  It lumps all different variants\n> of Linux into a single ball of wax and defines LINK_FUZZ_PROGRAMS\n> Makefile macro, which is a bit unfortunate.\n\nHaving said that, I am not sure if the commit that introduced the\nfuzz-all linkage rules, which is 8b9a42bf (fuzz: fix fuzz test build\nrules, 2024-01-19), needed to do so in the first place.  With this\ntrivial patch at the end of the message applied on top of 8b9a42bf\n(or more recent 'master', for that matter), \"make fuzz-all\" seems to\nlink things just fine.\n\nNote that \"make CC=clang fuzz-all\" in the older code used to fail\ndue to something else from gcc+ (which recent build no longer makes\nmandatory), but in today's code with gcc/clang/ld available locally,\nthe linker flag does not seem to be needed for \"make fuzz-all\".\n\nJosh, do you offhand know what made us add the option?\n\nThanks.\n\ndiff --git c/Makefile w/Makefile\nindex c0cbed69d8..5af8935968 100644\n--- c/Makefile\n+++ w/Makefile\n@@ -3848,7 +3848,6 @@ FUZZ_CXXFLAGS ?= $(ALL_CFLAGS)\n \n $(FUZZ_PROGRAMS): %: %.o oss-fuzz/dummy-cmd-main.o $(GITLIBS) GIT-LDFLAGS\n \t$(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) -o $@ $(ALL_LDFLAGS) \\\n-\t\t-Wl,--allow-multiple-definition \\\n \t\t$(filter %.o,$^) $(filter %.a,$^) $(LIBS) $(LIB_FUZZING_ENGINE)\n \n fuzz-all: $(FUZZ_PROGRAMS)\n"},{"id":"510443","messageId":"egtxf4f3dufiz56g276lt4qtediarj5kkuqbv222edrwcgf5dk@ocnbky74w3tv","threadId":"62681","inReplyTo":"xmqq7c7sbfhi.fsf@gitster.g","subject":"Re: Git v2.46.0 and --allow-multiple-definition linker flag","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2025-01-13T17:54:07Z","receivedAt":"2025-01-13T17:54:14Z","isPatch":false,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2024.12.21 18:47, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > James Mills <prologic@shortcircuit.net.au> writes:\n> >\n> >> tcc: error: unsupported linker option '--allow-multiple-definition'\n> >>\n> >> I bisected the Git releases and traced the introduction of this new\n> >> flag to v2.46.0\n> >>\n> >> I can't find any details of this flag really or when this was\n> >> introduced in the GNU binutils and so far I haven't asked if the Tiny\n> >> C devs intend to support this option (yet).\n> >\n> > Would\n> >\n> >     $ make LINK_FUZZ_PROGRAMS=\"\"\n> >\n> > help?\n> >\n> > The platform-specific tweak defined in config.mak.uname file assumes\n> > that you have glibc plus gcc or clang with usual binutils niceties\n> > once you claim that you are Linux.  It lumps all different variants\n> > of Linux into a single ball of wax and defines LINK_FUZZ_PROGRAMS\n> > Makefile macro, which is a bit unfortunate.\n> \n> Having said that, I am not sure if the commit that introduced the\n> fuzz-all linkage rules, which is 8b9a42bf (fuzz: fix fuzz test build\n> rules, 2024-01-19), needed to do so in the first place.  With this\n> trivial patch at the end of the message applied on top of 8b9a42bf\n> (or more recent 'master', for that matter), \"make fuzz-all\" seems to\n> link things just fine.\n> \n> Note that \"make CC=clang fuzz-all\" in the older code used to fail\n> due to something else from gcc+ (which recent build no longer makes\n> mandatory), but in today's code with gcc/clang/ld available locally,\n> the linker flag does not seem to be needed for \"make fuzz-all\".\n> \n> Josh, do you offhand know what made us add the option?\n> \n> Thanks.\n> \n> diff --git c/Makefile w/Makefile\n> index c0cbed69d8..5af8935968 100644\n> --- c/Makefile\n> +++ w/Makefile\n> @@ -3848,7 +3848,6 @@ FUZZ_CXXFLAGS ?= $(ALL_CFLAGS)\n>  \n>  $(FUZZ_PROGRAMS): %: %.o oss-fuzz/dummy-cmd-main.o $(GITLIBS) GIT-LDFLAGS\n>  \t$(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) -o $@ $(ALL_LDFLAGS) \\\n> -\t\t-Wl,--allow-multiple-definition \\\n>  \t\t$(filter %.o,$^) $(filter %.a,$^) $(LIBS) $(LIB_FUZZING_ENGINE)\n>  \n>  fuzz-all: $(FUZZ_PROGRAMS)\n\nSorry for the delayed response, I'm finally back to work after holidays\n/ sick leave.\n\nUnfortunately, the --allow-multiple-definitions flag is still required.\n\nWhen simply running \"make fuzz-all\" with no other args (with or without\nthe patch above), we do not actually produce a working fuzzer:\n\n$ make clean\n$ make fuzz-all\n$ ./oss-fuzz/fuzz-date\nBUG: oss-fuzz/dummy-cmd-main.c:12: We should not execute cmd_main() from a fuzz target\nzsh: IOT instruction  ./oss-fuzz/fuzz-date\n\nThis is because we're linking with common-main.o and\noss-fuzz/dummy-cmd-main.o. commain-main.o:main() calls cmd_main(), which\nit finds in oss-fuzz/dummy-cmd-main.o. If we instead build with a\nfuzzing engine enabled:\n\n$ make clean\n$ make CC=clang FUZZ_CXX=clang++ \\\n     CFLAGS=\"-fsanitize=fuzzer-no-link,address\" \\\n     LIB_FUZZING_ENGINE=\"-fsanitize=fuzzer,address\" \\\n     fuzz-all\n$ ./oss-fuzz/fuzz-date\nINFO: Running with entropic power schedule (0xFF, 100).\nINFO: Seed: 2884061060\nINFO: Loaded 1 modules   (55020 inline 8-bit counters): 55020 [0x562aff0d2840, 0x562aff0dff2c),\nINFO: Loaded 1 PC tables (55020 PCs): 55020 [0x562aff0dff30,0x562aff1b6df0),\nINFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes\nINFO: A corpus is not provided, starting from an empty corpus\n[...]\n\n\nThis works because the fuzzing engine provides its own main() which\noverrides ours, and so we never end up calling\ndummy-cmd-main.o:cmd_main().\n\nHowever, that build will fail if we remove the\n--allow-multiple-definition flag:\n\n$ make clean\n$ make CC=clang FUZZ_CXX=clang++ \\\n     CFLAGS=\"-fsanitize=fuzzer-no-link,address\" \\\n     LIB_FUZZING_ENGINE=\"-fsanitize=fuzzer,address\" \\\n     fuzz-all\n[...]\n    LINK oss-fuzz/fuzz-commit-graph\n/usr/bin/ld: common-main.o: in function `main':\ncommon-main.c:(.text.main[main]+0x0): multiple definition of `main'; /usr/lib/llvm-16/lib/clang/16/lib/linux/libclang_rt.fuzzer-x86_64.a(FuzzerMain.cpp.o):(.text.main+0x0): first defined here\nclang: error: linker command failed with exit code 1 (use -v to see invocation)\nmake: *** [Makefile:3890: oss-fuzz/fuzz-commit-graph] Error 1\n\n\nAs Junio says, a short term fix would be to build with\nLINK_FUZZ_PROGRAMS=\"\". A better solution would be to make\nconfig.mak.uname smarter about whether to enable this by default. I see\nthat we use \"detect-compiler\" in config.mak.dev, would it make sense to\ncheck this in config.mak.uname as well?\n"},{"id":"510493","messageId":"20250114110323.GD882468@coredump.intra.peff.net","threadId":"62681","inReplyTo":"egtxf4f3dufiz56g276lt4qtediarj5kkuqbv222edrwcgf5dk@ocnbky74w3tv","subject":"Re: Git v2.46.0 and --allow-multiple-definition linker flag","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-14T11:03:23Z","receivedAt":"2025-01-14T11:03:25Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 13, 2025 at 09:54:07AM -0800, Josh Steadmon wrote:\n\n> As Junio says, a short term fix would be to build with\n> LINK_FUZZ_PROGRAMS=\"\". A better solution would be to make\n> config.mak.uname smarter about whether to enable this by default. I see\n> that we use \"detect-compiler\" in config.mak.dev, would it make sense to\n> check this in config.mak.uname as well?\n\nIt feels like the original sin here is defining main() in our library in\nthe first place, if there are programs that may not want it. But we\ndon't actually put it into libgit.a; the object file is just mentioned\nin the $(GITLIBS) variable of the Makefile.\n\nWe could split that out like this:\n\ndiff --git a/Makefile b/Makefile\nindex 97e8385b66..f098ca5a5c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1371,7 +1371,8 @@ UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-reftable.o\n \n # xdiff and reftable libs may in turn depend on what is in libgit.a\n-GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(LIB_FILE)\n+GITLIBS = $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(LIB_FILE)\n+PROGRAM_GITLIBS = common-main.o $(GITLIBS)\n EXTLIBS =\n \n GIT_USER_AGENT = git/$(GIT_VERSION)\n\nand then depend on $(PROGRAM_GITLIBS) as appropriate. Or if oss-fuzz is\nthe only special case here, then perhaps we could just teach it to\nsuppress the extra main:\n\ndiff --git a/Makefile b/Makefile\nindex 97e8385b66..06431170de 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3852,9 +3852,8 @@ FUZZ_CXXFLAGS ?= $(ALL_CFLAGS)\n .PHONY: fuzz-all\n fuzz-all: $(FUZZ_PROGRAMS)\n \n-$(FUZZ_PROGRAMS): %: %.o oss-fuzz/dummy-cmd-main.o $(GITLIBS) GIT-LDFLAGS\n+$(FUZZ_PROGRAMS): %: %.o $(filter-out common-main.o,$(GITLIBS)) GIT-LDFLAGS\n \t$(QUIET_LINK)$(FUZZ_CXX) $(FUZZ_CXXFLAGS) -o $@ $(ALL_LDFLAGS) \\\n-\t\t-Wl,--allow-multiple-definition \\\n \t\t$(filter %.o,$^) $(filter %.a,$^) $(LIBS) $(LIB_FUZZING_ENGINE)\n \n $(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o $(UNIT_TEST_OBJS) \\\n\nYou'd need two fixes to go with that:\n\n  1. common-main was originally supposed to just be about overriding\n     main(). But it has since grown common_exit(), which does not have\n     the same linking properties (we override exit() calls at the macro\n     layer instead). That should be defined in libgit.a somewhere.\n\n  2. When building the test-compile fuzzers, now you'll need to\n     provide an actual main() function. I guess we can determine that by\n     the presence of LIB_FUZZING_ENGINE? So maybe:\n\ndiff --git a/Makefile b/Makefile\nindex 97e8385b66..ba3faf9b9e 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3852,9 +3852,15 @@ FUZZ_CXXFLAGS ?= $(ALL_CFLAGS)\n .PHONY: fuzz-all\n fuzz-all: $(FUZZ_PROGRAMS)\n \n-$(FUZZ_PROGRAMS): %: %.o oss-fuzz/dummy-cmd-main.o $(GITLIBS) GIT-LDFLAGS\n+ifdef LIB_FUZZING_ENGINE\n+# assume the fuzzing engine supplies main()\n+FUZZ_GITLIBS = $(filter-out common-main.o, $(GITLIBS))\n+else\n+FUZZ_GITLIBS = oss-fuzz/dummy-cmd-main.o $(GITLIBS)\n+endif\n+\n+$(FUZZ_PROGRAMS): %: %.o $(FUZZ_GITLIBS) GIT-LDFLAGS\n \t$(QUIET_LINK)$(FUZZ_CXX) $(FUZZ_CXXFLAGS) -o $@ $(ALL_LDFLAGS) \\\n-\t\t-Wl,--allow-multiple-definition \\\n \t\t$(filter %.o,$^) $(filter %.a,$^) $(LIBS) $(LIB_FUZZING_ENGINE)\n \n $(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o $(UNIT_TEST_OBJS) \\\n\n-Peff\n"}]}