I am investigating some arm regressions on gcc trunk. There are other issues around those LTO test cases. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78529 Apart from the gcc lto issues, I noticed a new issues which I believe is bfd linker's problem. so I opened this separate bugzilla ticket. The cases are quit tricky. With user-defined library function and lto optimization, the compiler optimize the code with the knowledge of user-defined function, while linking with library defined function. The user defined function is abandoned. The issues in gcc is that, the register usage of library function implementation is different from the user defined function. This will cause runtime error. For arm target, there is another type of error. For example, gcc/testsuite/gcc.c-torture/execute/builtins/strlen-2.c The library implementation of strlen function is a thumb function while the user-defined one is in arm mode. Normally, a BL instruction is generated for the function call. Later, it's the linker who changed the BL into BLX if the destination of function call is a thumb function. The error is that, while linking with library defined function, the BL instruction is not converted into BLX given the fact that the destination is a thumb function. The symbol is over-written by the user define function. And the size of the symbol is not the size of actual linked function. I checked the documentation of --allow-multiple-definition option, '''Normally when a symbol is defined multiple times, the linker will report a fatal error. These options allow multiple definitions and the first definition will be used.''' The first definition is from the library, and the second one is from user defined code. This is the order the linker scans the object files. Following the document, apparently, the symbol shouldn't be overwritten. I assume this is a stand-alone bfd linker bug. I have debugged a little bit. in elf_link_add_object_symbols () _bfd_elf_merge_symbol() will be called to decided what is going to do if there is already a symbol in the hash table. and _bfd_generic_link_add_one_symbol() will do something to actually add the symbol to the global hash table. However, _bfd_generic_link_add_one_symbol() will do nothing if it is a multiple definition, while _bfd_elf_merge_symbol() decided the override the existing symbol with the new definition. _bfd_elf_merge_symbol() returns a OVERRIDE flag which indicates whether the old definition should override the new definition. To me, it seems OVERRIDE should be TRUE for this case. If their is an old definition and allow_multiple_definition is true, the function will return TRUE with OVERRIDE set to TRUE. But there are a lot of other cases in the function, I don't know if I have missed anything.
When you say "the first definition is from the library", I assume you're talking about a shared library. If so, a later definition for the same symbol in a regular object file should replace the shared library definition. That's the way ELF shared libraries are supposed to work. _bfd_elf_merge_symbol ought to twiddle the symbol to undefined (or new) so that the generic linker will accept the newly seen regular object file definition. The result ought to be the same as when then regular object file definition is seen first. In that case a later shared library definition is ignored. Whether --allow-multiple-definition is given or not should not affect symbol resolution, just whether an error is reported, as it would be for example when linking two regular object files each with a definition of the same symbol.
(In reply to Alan Modra from comment #1) > When you say "the first definition is from the library", I assume you're > talking about a shared library. If so, a later definition for the same > symbol in a regular object file should replace the shared library > definition. That's the way ELF shared libraries are supposed to work. > _bfd_elf_merge_symbol ought to twiddle the symbol to undefined (or new) so > that the generic linker will accept the newly seen regular object file > definition. The result ought to be the same as when then regular object > file definition is seen first. In that case a later shared library > definition is ignored. > > Whether --allow-multiple-definition is given or not should not affect symbol > resolution, just whether an error is reported, as it would be for example > when linking two regular object files each with a definition of the same > symbol. I agree for shared library case. The behavior I observed here is happening in arm-none-eabi toolchain with static newlib.
Created attachment 10242 [details] foo_arm.c
Created attachment 10243 [details] foo_thumb.c
Created attachment 10244 [details] main_arm.c
I create a test case to show the bug. arm-none-eabi-gcc main_arm.c -O2 -march=armv7-a -mfloat-abi=softfp -specs=aprofile-validation.specs -c -o main_arm.o arm-none-eabi-gcc -O2 -mthumb -march=armv7-a -mfloat-abi=softfp -specs=aprofile-validation.specs -c foo_arm.c -o foo_arm.o arm-none-eabi-gcc -O2 -mthumb -march=armv7-a -mfloat-abi=softfp -specs=aprofile-validation.specs -c foo_thumb.c -o foo_thumb.o arm-none-eabi-gcc main_arm.o foo_thumb.o foo_arm.o -specs=aprofile-validation.specs -o main.exe -Wl,--allow-multiple-definition In this case, we would expect in main.exe, the function get called is the thumb version, and BLX is used to do the function call. But BL is used to call the thumb function. I have checked weak symbol and dynamic symbol are properly handled.
I had a quick look at your example (typo on foo_arm.c command line, you don't want -mthumb there!), and while I don't know the arm backend that well, I think the problem is due to target_internal being set from the last definition seen.
(In reply to Alan Modra from comment #7) > I had a quick look at your example (typo on foo_arm.c command line, you > don't want -mthumb there!), and while I don't know the arm backend that > well, I think the problem is due to target_internal being set from the last > definition seen. Yes, the target_internal is override by the last definition. While _bfd_generic_link_add_one_symbol () doesn't update the symbol's definition section. It's still the old one. Because _bfd_generic_link_add_one_symbol () decided to not add the new definition. case MDEF: /* Handle a multiple definition. */ (*info->callbacks->multiple_definition) (info, h, abfd, section, value); break; The function simply returns when allow-multiple-definition is true.
The master branch has been updated by Renlin Li <renlin@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=93f4de3929aeb3e21d57950bfa96539599a92f2a commit 93f4de3929aeb3e21d57950bfa96539599a92f2a Author: Renlin Li <renlin.li@arm.com> Date: Tue Oct 24 12:42:30 2017 +0100 [BFD][PR21703]Override the new defined symbol with the old normal symbol when --allow-multiple-definition is provided. The behavior of _bfd_elf_merge_symbol and _bfd_generic_link_add_one_symbol is inconsistent. In multiple definition case, _bfd_elf_merge_symbol decided to override the old symbol definition with the new defintion, (size, type, target data) In _bfd_generic_link_add_one_symbol, it simply return without doing anything because of allow-multiple-definition is provided. This leaves the symbol in a wrong state. Here, following the documentation, I made this patch to force the old definition override the new definition if the old symbol is not dynamic or weak. Because, in those two cases, it's expected to do some merge. I have checked that, those two cases are properly handled. bfd/ PR ld/21703 * elflink.c (_bfd_elf_merge_symbol): Handle multiple definition case. ld/ PR ld/21703 * testsuite/ld-elf/elf.exp: Run new tests. * testsuite/ld-elf/pr21703-1.s: New. * testsuite/ld-elf/pr21703-2.s: New. * testsuite/ld-elf/pr21703-3.s: New. * testsuite/ld-elf/pr21703-4.s: New. * testsuite/ld-elf/pr21703-r.sd: New. * testsuite/ld-elf/pr21703-shared.sd: New. * testsuite/ld-elf/pr21703.sd: New. * testsuite/ld-elf/pr21703.ver: New.
(In reply to cvs-commit@gcc.gnu.org from comment #9) > The master branch has been updated by Renlin Li <renlin@sourceware.org>: > > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git; > h=93f4de3929aeb3e21d57950bfa96539599a92f2a > > commit 93f4de3929aeb3e21d57950bfa96539599a92f2a > Author: Renlin Li <renlin.li@arm.com> > Date: Tue Oct 24 12:42:30 2017 +0100 > > [BFD][PR21703]Override the new defined symbol with the old normal symbol > when --allow-multiple-definition is provided. > > The behavior of _bfd_elf_merge_symbol and > _bfd_generic_link_add_one_symbol is > inconsistent. > > In multiple definition case, _bfd_elf_merge_symbol decided to override > the old > symbol definition with the new defintion, (size, type, target data) > In _bfd_generic_link_add_one_symbol, it simply return without doing > anything > because of allow-multiple-definition is provided. > This leaves the symbol in a wrong state. > > Here, following the documentation, I made this patch to force the old > definition > override the new definition if the old symbol is not dynamic or weak. > Because, in those two cases, it's expected to do some merge. I have > checked > that, those two cases are properly handled. > > bfd/ > PR ld/21703 > * elflink.c (_bfd_elf_merge_symbol): Handle multiple definition case. > > ld/ > > PR ld/21703 > * testsuite/ld-elf/elf.exp: Run new tests. > * testsuite/ld-elf/pr21703-1.s: New. > * testsuite/ld-elf/pr21703-2.s: New. > * testsuite/ld-elf/pr21703-3.s: New. > * testsuite/ld-elf/pr21703-4.s: New. > * testsuite/ld-elf/pr21703-r.sd: New. > * testsuite/ld-elf/pr21703-shared.sd: New. > * testsuite/ld-elf/pr21703.sd: New. > * testsuite/ld-elf/pr21703.ver: New. Mark the PR as Fixed.
The master branch has been updated by Maciej W. Rozycki <macro@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=822520337789f93b528fe0babc7dcfb03bb50fcd commit 822520337789f93b528fe0babc7dcfb03bb50fcd Author: Maciej W. Rozycki <macro@mips.com> Date: Tue Jan 30 00:38:12 2018 +0000 MIPS/LD/testsuite: Adjust match patterns for special section indexes Update `readelf' symbol table dump match patterns to handle SHN_MIPS_DATA and SHN_MIPS_TEXT special section indexes produced by the IRIX ELF format variation used with a number of MIPS targets and printed by `readelf' as PRC[0xff02] and PRC[0xff01] respectively, correcting LD test suite failures: extra regexps in .../ld/testsuite/ld-elf/comm-data1.sd starting with "^ +[0-9]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo$" EOF from dump.out FAIL: Common symbol override test (auxiliary shared object build) extra regexps in .../ld/testsuite/ld-elf/pr21703-shared.sd starting with "^ +[0-9]+: +[0-9a-f]+ +4 +FUNC +GLOBAL +DEFAULT +[0-9] +foo@FOO$" EOF from dump.out FAIL: PR ld/21703 shared extra regexps in .../ld/testsuite/ld-elf/comm-data1.sd starting with "^ +[0-9]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo$" EOF from dump.out FAIL: MIPS o32/copyreloc common symbol override test (auxiliary shared object build) extra regexps in .../ld/testsuite/ld-elf/comm-data1.sd starting with "^ +[0-9]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo$" EOF from dump.out FAIL: MIPS o32/nocopyreloc common symbol override test (auxiliary shared object build) observed due to file contents like: 7: 5ffe02e8 0 OBJECT GLOBAL DEFAULT PRC[0xff02] foo shown by `readelf -s' vs: +[0-9]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo pattern expected, triggered by widening testing to these targets by commit 05a5feafdd38 ("Rewrite check_shared_lib_support"). ld/ * testsuite/ld-elf/comm-data1.sd: Alternatively accept `PRC[0xff02]' in place of a regular section index. * testsuite/ld-elf/pr21703-shared.sd: Likewise `PRC[0xff01]'.
The master branch has been updated by Maciej W. Rozycki <macro@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e54d3c943bb3999aeaf1667b4975dc035f7b554c commit e54d3c943bb3999aeaf1667b4975dc035f7b554c Author: Maciej W. Rozycki <macro@mips.com> Date: Fri Feb 2 18:08:04 2018 +0000 LD/testsuite: Pass $AFLAGS_PIC to GAS for PIC assembly Add $AFLAGS_PIC flags for PIC assembly to a number of tests missing them and remove `tic6x-*-*' XFAIL annotations from them, previously added to paper over: .../ld-new: warning: generating a shared library containing non-PID code error messages produced due to `-mpic -mpid=near' GAS options having not been used. Such errors now do not happen anymore, removing: XFAIL: Build shared library for pr14170 XFAIL: PR ld/21703 shared XFAIL: Build shared library for broken linker script test XFAIL: Build pr17068.so XFAIL: -Bsymbolic-functions XFAIL: Build pr20995.so XFAIL: Build pr22374 shared library with `tic6x-elf' and `tic6x-uclinux' targets. These tests now pass all except for: FAIL: PR ld/21703 shared which is now due to a different reason, as follows: extra regexps in .../ld/testsuite/ld-elf/pr21703-shared.sd starting with "^Symbol table '\.dynsym' contains [0-9]+ entries:$" EOF from dump.out FAIL: PR ld/21703 shared The addition of $AFLAGS_PIC requires the affected test cases to use the `list' command rather than `{}' characters to create a list, to avoid the quoting property `{}' also have in TCL. Consequently the change is slightly more extensive than it could otherwise be. ld/ * testsuite/ld-elf/shared.exp: Add $AFLAGS_PIC throughout to PIC assembly builds where missing and remove `tic6x-*-*' XFAIL markings accordingly.
The master branch has been updated by Alan Modra <amodra@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=18e404c4e2eb15443cc6dda93cbd38bdfeb17667 commit 18e404c4e2eb15443cc6dda93cbd38bdfeb17667 Author: Alan Modra <amodra@gmail.com> Date: Wed Apr 11 11:10:18 2018 +0930 Silence nds32 pic warnings Fixes: FAIL: Build pr22471a.so FAIL: Build pr22471b.so FAIL: Build pr22649-1.so FAIL: Build pr22649-2c.so FAIL: Build pr22649-2d.so FAIL: PR ld/20828 dynamic symbols with section GC (auxiliary shared library) FAIL: PR ld/20828 dynamic symbols with section GC (plain) FAIL: PR ld/20828 dynamic symbols with section GC (version script) FAIL: PR ld/20828 dynamic symbols with section GC (versioned shared library) FAIL: PR ld/20828 dynamic symbols with section GC (versioned) FAIL: PR ld/21233 dynamic symbols with section GC (auxiliary shared library) FAIL: Build pr22150.so FAIL: Build shared library for pr14170 FAIL: PR ld/21703 shared FAIL: Build shared library for broken linker script test FAIL: Build pr17068.so FAIL: -Bsymbolic-functions FAIL: Build pr20995.so FAIL: Build pr20995-2.so FAIL: Build pr22374 shared library * testsuite/ld-elf/shared.exp (AFLAGS_PIC): Add -mpic for nds32.
The master branch has been updated by Alan Modra <amodra@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=1336939d3cd4cb02670c48ef065bafdf0fdae300 commit 1336939d3cd4cb02670c48ef065bafdf0fdae300 Author: Alan Modra <amodra@gmail.com> Date: Wed May 29 22:54:37 2019 +0930 Support tcl procedure calls in run_dump_test xfail Also support tcl procedure calls in the run_ld_link_tests and run_ld_link_exec_tests optional "xfail" args. Implements "is_generic" and renames "is_generic_elf" to "uses_genelf", then uses these procs in lots of ld tests. moxie-elf -FAIL: ld-elf/orphan3 mt-elf -FAIL: ld-elf/merge s12z-elf -FAIL: ld-discard/extern s12z-elf -FAIL: ld-discard/start s12z-elf -FAIL: ld-discard/static s12z-elf -FAIL: PR ld/21703 s12z-elf -FAIL: PR ld/21703 -r s12z-elf -FAIL: Symbol flags copy s12z-elf -FAIL: ld-elf/group1 s12z-elf -FAIL: ld-elf/group3b s12z-elf -FAIL: ld-elf/group8a s12z-elf -FAIL: ld-elf/group8b s12z-elf -FAIL: ld-elf/group9a s12z-elf -FAIL: ld-elf/group9b s12z-elf -FAIL: ld-elf/linkonce2 s12z-elf -FAIL: ld-elf/merge2 s12z-elf -FAIL: ld-elf/merge3 s12z-elf -FAIL: ld-elf/pr12851 s12z-elf -FAIL: ld-elf/pr17550c s12z-elf -FAIL: ld-elf/pr17550d s12z-elf -FAIL: ld-elf/pr22677 s12z-elf -FAIL: ld-elf/pr22836-1a s12z-elf -FAIL: ld-elf/pr22836-1b s12z-elf -FAIL: ld-elf/warn1 s12z-elf -FAIL: ld-elf/warn3 binutils/ * testsuite/lib/binutils-common.exp (run_dump_test): Support tcl procedures for xfail args. ld/ * testsuite/lib/ld-lib.exp (run_ld_link_tests): Support procedure calls in optional "xfail" args. (run_ld_link_exec_tests): Likewise. (is_generic): New. (uses_genelf): Rename from is_generic_elf. Delete bogus semicolons. * testsuite/ld-scripts/align.exp: Rename is_generic_elf call. * testsuite/ld-elf/elf.exp: Use is_generic and uses_genelf. Delete xfail_implib var. * testsuite/ld-elf/sec64k.exp: Use is_generic. * testsuite/ld-elf/shared.exp: Likewise. * testsuite/ld-discard/extern.d: Use is_generic in xfail. * testsuite/ld-discard/start.d: Likewise. * testsuite/ld-discard/static.d: Likewise. * testsuite/ld-elf/attributes.d: Likewise. * testsuite/ld-elf/group1.d: Likewise. * testsuite/ld-elf/group3b.d: Likewise. * testsuite/ld-elf/group8a.d: Likewise. * testsuite/ld-elf/group8b.d: Likewise. * testsuite/ld-elf/group9a.d: Likewise. * testsuite/ld-elf/group9b.d: Likewise. * testsuite/ld-elf/linkonce2.d: Likewise. * testsuite/ld-elf/merge2.d: Likewise. * testsuite/ld-elf/merge3.d: Likewise. * testsuite/ld-elf/pr12851.d: Likewise. * testsuite/ld-elf/pr12975.d: Likewise. * testsuite/ld-elf/pr13177.d: Likewise. * testsuite/ld-elf/pr13195.d: Likewise. * testsuite/ld-elf/pr17550c.d: Likewise. * testsuite/ld-elf/pr17550d.d: Likewise. * testsuite/ld-elf/pr17615.d: Likewise. * testsuite/ld-elf/pr21562a.d: Likewise. * testsuite/ld-elf/pr21562b.d: Likewise. * testsuite/ld-elf/pr21562c.d: Likewise. * testsuite/ld-elf/pr21562d.d: Likewise. * testsuite/ld-elf/pr21562i.d: Likewise. * testsuite/ld-elf/pr21562j.d: Likewise. * testsuite/ld-elf/pr21562k.d: Likewise. * testsuite/ld-elf/pr21562l.d: Likewise. * testsuite/ld-elf/pr21562m.d: Likewise. * testsuite/ld-elf/pr21562n.d: Likewise. * testsuite/ld-elf/pr22677.d: Likewise. * testsuite/ld-elf/pr22836-1a.d: Likewise. * testsuite/ld-elf/pr22836-1b.d: Likewise. * testsuite/ld-elf/warn3.d: Likewise. * testsuite/ld-elf/warn1.d: Likewise and xfail sparc solaris targets rather than notarget. * testsuite/ld-elf/compressed1d.d: Use uses_genelf in xfail. * testsuite/ld-elf/orphan-10.d: Likewise. * testsuite/ld-elf/orphan-9.d: Likewise. * testsuite/ld-elf/orphan-region.d: Likewise. * testsuite/ld-elf/orphan.d: Likewise. * testsuite/ld-elf/orphan3.d: Likewise. * testsuite/ld-elf/pr20528a.d: Likewise. * testsuite/ld-elf/pr20528b.d: Likewise. * testsuite/ld-elf/pr23658-1a.d: Likewise. * testsuite/ld-elf/pr23658-1b.d: Likewise. * testsuite/ld-elf/pr349.d: Likewise. * testsuite/ld-elf/warn2.d: Likewise and xfail sparc solaris targets rather than notarget. * testsuite/ld-elf/merge.d: Correct ms1-*-* to mt-*-*.
The master branch has been updated by H.J. Lu <hjl@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=597344c9a4a110b662ed04fe115e6f887b42d366 commit 597344c9a4a110b662ed04fe115e6f887b42d366 Author: H.J. Lu <hjl.tools@gmail.com> Date: Sun Jun 7 14:48:13 2020 -0700 ld: Pass $LFLAGS to PR ld/21703 shared test $LFLAGS is needed for -shared test. This fixes FAIL: PR ld/21703 shared for tic6x-*-elf. * testsuite/ld-elf/shared.exp: Pass $LFLAGS to PR ld/21703 shared test.