Git v2.46.0 and --allow-multiple-definition linker flag

Hi Folks 👋

Been a while since I've emailed one of the large(r) open source lists, so please excuse me I may be a bit rusty.

For some years now I've been building a very tiny custom LInux distribution called µLInux (micro Linux) over at https://git.mills.io/prologic/ulinux -- I primarily target the x86_64 architecture and its intended use is mostly as lightweight VM(s) that boot really quickly but are otherwise populated with "just enough" UNIX tools, a small package manager and few other bits to get interesting things going.

Today I noticed that when upgrading the zlib (✅) and git (❌) ports that the latest version of Git v2.47.2 does not link and I observe the following linker error, mostly what looks like a missing capability/flag of the TinyC compiler that is used as the linker (LD=tcc):

tcc: error: unsupported linker option '--allow-multiple-definition'

I bisected the Git releases and traced the introduction of this new flag to v2.46.0

I can't find any details of this flag really or when this was introduced in the GNU binutils and so far I haven't asked if the Tiny C devs intend to support this option (yet).

Just wondering, can we consider backing this out? From my very brief research on the web this seems to allow the linker to permit multiple definitions of a function. I'm not even sure why that would be desirable to be honest (but I haven't been a C/C++ developer basically ever, I know enough to be dangerous!)

Kind regards

James

-- James Mills / prologic

Join Yarn.social today! The only decentralised social media that respects your privacy and freedoms!

E:    prologic@shortcircuit.net.au
W:    prologic.shortcircuit.net.au
Blog: Read my Blog
Yarn: @prologic@twtxt.net

James Mills <prologic@shortcircuit.net.au> writes:

Would

    $ make LINK_FUZZ_PROGRAMS=""

help?

The platform-specific tweak defined in config.mak.uname file assumes that you have glibc plus gcc or clang with usual binutils niceties once you claim that you are Linux. It lumps all different variants of Linux into a single ball of wax and defines LINK_FUZZ_PROGRAMS Makefile macro, which is a bit unfortunate.

Junio C Hamano <gitster@pobox.com> writes:

Having said that, I am not sure if the commit that introduced the fuzz-all linkage rules, which is 8b9a42bf (fuzz: fix fuzz test build rules, 2024-01-19), needed to do so in the first place. With this trivial patch at the end of the message applied on top of 8b9a42bf (or more recent 'master', for that matter), "make fuzz-all" seems to link things just fine.

Note that "make CC=clang fuzz-all" in the older code used to fail due to something else from gcc+ (which recent build no longer makes mandatory), but in today's code with gcc/clang/ld available locally, the linker flag does not seem to be needed for "make fuzz-all".

Josh, do you offhand know what made us add the option?

Thanks.

On 2024.12.21 18:47, Junio C Hamano wrote:

Sorry for the delayed response, I'm finally back to work after holidays / sick leave.

Unfortunately, the --allow-multiple-definitions flag is still required.

When simply running "make fuzz-all" with no other args (with or without the patch above), we do not actually produce a working fuzzer:

$ make clean
$ make fuzz-all
$ ./oss-fuzz/fuzz-date
BUG: oss-fuzz/dummy-cmd-main.c:12: We should not execute cmd_main() from a fuzz target
zsh: IOT instruction  ./oss-fuzz/fuzz-date

This is because we're linking with common-main.o and oss-fuzz/dummy-cmd-main.o. commain-main.o:main() calls cmd_main(), which it finds in oss-fuzz/dummy-cmd-main.o. If we instead build with a fuzzing engine enabled:

$ make clean
$ make CC=clang FUZZ_CXX=clang++ \
     CFLAGS="-fsanitize=fuzzer-no-link,address" \
     LIB_FUZZING_ENGINE="-fsanitize=fuzzer,address" \
     fuzz-all
$ ./oss-fuzz/fuzz-date
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 2884061060
INFO: Loaded 1 modules   (55020 inline 8-bit counters): 55020 [0x562aff0d2840, 0x562aff0dff2c),
INFO: Loaded 1 PC tables (55020 PCs): 55020 [0x562aff0dff30,0x562aff1b6df0),
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: A corpus is not provided, starting from an empty corpus
[...]

This works because the fuzzing engine provides its own main() which overrides ours, and so we never end up calling dummy-cmd-main.o:cmd_main().

However, that build will fail if we remove the --allow-multiple-definition flag:

$ make clean
$ make CC=clang FUZZ_CXX=clang++ \
     CFLAGS="-fsanitize=fuzzer-no-link,address" \
     LIB_FUZZING_ENGINE="-fsanitize=fuzzer,address" \
     fuzz-all
[...]
    LINK oss-fuzz/fuzz-commit-graph
/usr/bin/ld: common-main.o: in function `main':
common-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
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [Makefile:3890: oss-fuzz/fuzz-commit-graph] Error 1

As Junio says, a short term fix would be to build with LINK_FUZZ_PROGRAMS="". A better solution would be to make config.mak.uname smarter about whether to enable this by default. I see that we use "detect-compiler" in config.mak.dev, would it make sense to check this in config.mak.uname as well?

On Mon, Jan 13, 2025 at 09:54:07AM -0800, Josh Steadmon wrote:

It feels like the original sin here is defining main() in our library in the first place, if there are programs that may not want it. But we don't actually put it into libgit.a; the object file is just mentioned in the $(GITLIBS) variable of the Makefile.

We could split that out like this: