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

From: Josh Steadmon <steadmon@google.com>
Date: Jan 13, 2025, 17:54 UTC
Message-ID: <egtxf4f3dufiz56g276lt4qtediarj5kkuqbv222edrwcgf5dk@ocnbky74w3tv>
In-Reply-To: <xmqq7c7sbfhi.fsf@gitster.g>

On 2024.12.21 18:47, Junio C Hamano wrote:

Show 53 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > James Mills <prologic@shortcircuit.net.au> writes:
> >
> >> 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).
> >
> > 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.
> 
> 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.
> 
> diff --git c/Makefile w/Makefile
> index c0cbed69d8..5af8935968 100644
> --- c/Makefile
> +++ w/Makefile
> @@ -3848,7 +3848,6 @@ FUZZ_CXXFLAGS ?= $(ALL_CFLAGS)
>  
>  $(FUZZ_PROGRAMS): %: %.o oss-fuzz/dummy-cmd-main.o $(GITLIBS) GIT-LDFLAGS
>  	$(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) -o $@ $(ALL_LDFLAGS) \
> -		-Wl,--allow-multiple-definition \
>  		$(filter %.o,$^) $(filter %.a,$^) $(LIBS) $(LIB_FUZZING_ENGINE)
>  
>  fuzz-all: $(FUZZ_PROGRAMS)

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?