When a __start_ symbol is defined both .o and .so, --gc-sections drops the definition on .o: [hjl@gnu-6 gc-4]$ cat a.s .globl _start _start: .dc.a __start__foo .dc.a bar .section _foo,"aw",%progbits foo: .ascii "This is " [hjl@gnu-6 gc-4]$ cat c.s .globl bar bar: .dc.a __start__foo .section _foo,"aw",%progbits foo: .ascii "This is " [hjl@gnu-6 gc-4]$ make gcc -B./ -fdata-sections -c -o a.o a.s gcc -B./ -fdata-sections -c -o c.o c.s ./ld -shared --gc-sections -o c.so c.o ./ld --gc-sections -o x a.o c.so ./ld: warning: type and size of dynamic symbol `__start__foo' are not defined ./ld: warning: type and size of dynamic symbol `bar' are not defined ./ld -o y a.o c.so ./ld: warning: type and size of dynamic symbol `bar' are not defined objdump -s -j _foo y y: file format elf64-x86-64 Contents of section _foo: 600338 54686973 20697320 This is objdump -s -j _foo x x: file format elf64-x86-64 objdump: section '_foo' mentioned in a -j option, but not found in any input file Makefile:11: recipe for target 'all' failed make: *** [all] Error 1 [hjl@gnu-6 gc-4]$
Hi H.J. This seems like a very obscure case. It only applies to orphan sections, and only when the sole symbol referred to in that section is the linker created start (or stop) symbol. Is there a real world case where this problem is actually likely to occur ? if not, then I am of a mind to ignore this problem... Cheers Nick
(In reply to Nick Clifton from comment #1) > Hi H.J. > > This seems like a very obscure case. It only applies to orphan sections, > and only when the sole symbol referred to in that section is the linker > created start (or stop) symbol. Is there a real world case where this > problem is actually likely to occur ? if not, then I am of a mind to > ignore this problem... I don't mind adding abort if you don't think it will ever happen.
(In reply to H.J. Lu from comment #2) > I don't mind adding abort if you don't think it will ever happen. Where would you add the abort ?
(In reply to Nick Clifton from comment #3) > (In reply to H.J. Lu from comment #2) > > > I don't mind adding abort if you don't think it will ever happen. > > Where would you add the abort ? Somewhere in _bfd_elf_is_start_stop?
(In reply to H.J. Lu from comment #4) > Somewhere in _bfd_elf_is_start_stop? I don't think that will work. We only want to trigger if an orphan section is being referenced solely via its start/stop symbols, and I don't think that we can determine this in _bfd_elf_is_start_stop. Still if you have a potential patch to suggest I would be happy to take a look. I think that the patch should produce a warning/error message, rather than an abort however, since the user is not doing anything wrong. Cheers Nick
(In reply to Nick Clifton from comment #5) > (In reply to H.J. Lu from comment #4) > > > Somewhere in _bfd_elf_is_start_stop? > > I don't think that will work. We only want to trigger if an orphan section > is being referenced solely via its start/stop symbols, and I don't think that > we can determine this in _bfd_elf_is_start_stop. > > Still if you have a potential patch to suggest I would be happy to take a > look. I don't have a real patch. This patch: https://sourceware.org/ml/binutils/2016-04/msg00449.html contains changes to _bfd_elf_is_start_stop to detect such case. It may be used to issue an error. > I think that the patch should produce a warning/error message, rather than an > abort however, since the user is not doing anything wrong. An error is better.
Created attachment 9270 [details] Proposed patch Hi H.J. OK - based upon your patch, but simplified quite a lot, here is a patch which I think will produce the error message we want. Please could you give it a try and if you are happy with it let me know, so that I can check it in. Cheers Nick
(In reply to Nick Clifton from comment #7) > Created attachment 9270 [details] > Proposed patch > > Hi H.J. > > OK - based upon your patch, but simplified quite a lot, here is a patch > which > I think will produce the error message we want. Please could you give it > a try > and if you are happy with it let me know, so that I can check it in. > > Cheers > Nick if (h->root.type != bfd_link_hash_undefined && h->root.type != bfd_link_hash_undefweak) - return NULL; + { + /* PR 20022 */ + if (sec_name && info->gc_sections && h->root.u.undef.section == NULL) + info->callbacks->einfo (_("%X%P: error: undefined orphan section symbol %s suggests that garbage collection failed\n"), + h->root.root.string); + return NULL; } Is it safe to check h->root.u.undef.section for defined symbol?
Created attachment 9274 [details] Revised patch Hi H.J. Of course, you are right. Plus the error message was wrong - it is not undefined orphan section symbols that are the problem, it is defined ones. Plus, really this ought to be a warning, not an error, since garbage collection will work if the orphan section is referred to by non start/stop symbols. So here is a revised patch. What do you think ? Cheers Nick
(In reply to Nick Clifton from comment #9) > Created attachment 9274 [details] > Revised patch > > Hi H.J. > > Of course, you are right. Plus the error message was wrong - it is not > undefined orphan section symbols that are the problem, it is defined ones. > Plus, really this ought to be a warning, not an error, since garbage > collection will work if the orphan section is referred to by non start/stop > symbols. > > So here is a revised patch. What do you think ? > _bfd_elf_is_start_stop is called only with --gc-sections. There is no need to check info->gc_sections. Also the defined __start_foo isn't a problem. The problem happens only when __start_foo is defined in a shared object.
The master branch has been updated by H.J. Lu <hjl@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=cbd0eecf261c2447781f8c89b0d955ee66fae7e9 commit cbd0eecf261c2447781f8c89b0d955ee66fae7e9 Author: H.J. Lu <hjl.tools@gmail.com> Date: Tue Jun 13 08:53:22 2017 -0700 Always define referenced __start_SECNAME/__stop_SECNAME Currently, linker will define __start_SECNAME and __stop_SECNAME symbols only for orphaned sections. However, during garbage collection, ELF linker marks all sections with references to __start_SECNAME and __stop_SECNAME symbols as used even when section SECNAME isn't an orphaned section and linker won't define __start_SECNAME nor __stop_SECNAME. And ELF linker stores the first input section whose name matches __start_SECNAME or __stop_SECNAME in u.undef.section for garbage collection. If these symbols are provided in linker script, u.undef.section is set to the section where they will defined by linker script, which leads to the incorrect output. This patch changes linker to always define referenced __start_SECNAME and __stop_SECNAME if the input section name is the same as the output section name, which is always true for orphaned sections, and SECNAME is a C identifier. Also __start_SECNAME and __stop_SECNAME symbols are marked as hidden by ELF linker so that __start_SECNAME and __stop_SECNAME symbols for section SECNAME in different modules are unique. For garbage collection, ELF linker stores the first matched input section in the unused vtable field. bfd/ PR ld/20022 PR ld/21557 PR ld/21562 PR ld/21571 * elf-bfd.h (elf_link_hash_entry): Add start_stop. Change the vtable field to a union. (_bfd_elf_is_start_stop): Removed. * elf32-i386.c (elf_i386_convert_load_reloc): Also check for __start_SECNAME and __stop_SECNAME symbols. * elf64-x86-64.c (elf_x86_64_convert_load_reloc): Likewise. * elflink.c (_bfd_elf_is_start_stop): Removed. (_bfd_elf_gc_mark_rsec): Check start_stop instead of calling _bfd_elf_is_start_stop. (elf_gc_propagate_vtable_entries_used): Skip __start_SECNAME and __stop_SECNAME symbols. Updated. (elf_gc_smash_unused_vtentry_relocs): Likewise. (bfd_elf_gc_record_vtinherit): Likewise. (bfd_elf_gc_record_vtentry): Likewise. ld/ PR ld/20022 PR ld/21557 PR ld/21562 PR ld/21571 * ld.texinfo: Update __start_SECNAME/__stop_SECNAME symbols. * ldlang.c (lang_insert_orphan): Move handling of __start_SECNAME and __stop_SECNAME symbols to ... (lang_set_startof): Here. Also define __start_SECNAME and __stop_SECNAME for -Ur. * emultempl/elf32.em (gld${EMULATION_NAME}_after_open): Mark referenced __start_SECNAME and __stop_SECNAME symbols as hidden and set start_stop for garbage collection. * testsuite/ld-elf/pr21562a.d: New file. * testsuite/ld-elf/pr21562a.s: Likewise. * testsuite/ld-elf/pr21562a.t: Likewise. * testsuite/ld-elf/pr21562b.d: Likewise. * testsuite/ld-elf/pr21562b.s: Likewise. * testsuite/ld-elf/pr21562b.t: Likewise. * testsuite/ld-elf/pr21562c.d: Likewise. * testsuite/ld-elf/pr21562c.t: Likewise. * testsuite/ld-elf/pr21562d.d: Likewise. * testsuite/ld-elf/pr21562d.t: Likewise. * testsuite/ld-elf/pr21562e.d: Likewise. * testsuite/ld-elf/pr21562f.d: Likewise. * testsuite/ld-elf/pr21562g.d: Likewise. * testsuite/ld-elf/pr21562h.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-gc/pr20022.d: Likewise. * testsuite/ld-gc/pr20022a.s: Likewise. * testsuite/ld-gc/pr20022b.s: Likewise. * testsuite/ld-gc/gc.exp: Run PR ld/20022 tests. * testsuite/ld-gc/pr19161.d: Also accept local __start_SECNAME symbol. * testsuite/ld-gc/start.d: Likewise. * testsuite/ld-x86-64/lea1a.d: Updated. * testsuite/ld-x86-64/lea1b.d: Updated. * testsuite/ld-x86-64/lea1d.d: Updated. * testsuite/ld-x86-64/lea1e.d: Likewise.
Fixed.
The master branch has been updated by H.J. Lu <hjl@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=99031bafd6e81a41553803886c6b245cb0ab89d9 commit 99031bafd6e81a41553803886c6b245cb0ab89d9 Author: H.J. Lu <hjl.tools@gmail.com> Date: Wed Jun 14 06:11:48 2017 -0700 xfail pr20022.d on targets without dynamic relocs in .text ld-gc/pr20022.d requires support for dynamic relocations in .text section. PR ld/20022 * testsuite/ld-gc/pr20022.d: Skip on targets without dynamic relocations in .text section.
The master branch has been updated by Alan Modra <amodra@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7dba9362c172f1073487536eb137feb2da30b0ff commit 7dba9362c172f1073487536eb137feb2da30b0ff Author: Alan Modra <amodra@gmail.com> Date: Fri Jun 16 19:41:41 2017 +0930 Rewrite __start and __stop symbol handling This arranges for __start and __stop symbols to be defined before garbage collection, for all target formats. That should allow the COFF and PE --gc-sections to keep a singleton orphan input section, a feature lost by 2017-06-13 commit cbd0eecf26. The fancier ELF treatment of keeping all input sections associated with a __start or __stop symbol, from 2015-10-23 commit 1cce69b9dc, is retained. .startof. and .sizeof. symbols are deliberately not defined before garbage collection, so these won't affect garbage collection of sections. The patch also ensures __start, __stop, .startof. and .sizeof. symbols are defined before target size_dynamic_sections is called, albeit with a preliminary value, so that target code doesn't need to cope with a symbol changing from undefined at size_dynamic_sections to defined at relocate_section. Also, a number of problems with the testcases have been fixed. PR ld/20022 PR ld/21557 PR ld/21562 PR ld/21571 include/ * bfdlink.h (struct bfd_link_hash_entry): Delete undef.section. bfd/ * targets.c (struct bfd_target): Add _bfd_define_start_stop. (BFD_JUMP_TABLE_LINK): Likewise. * elf-bfd.h (bfd_elf_define_start_stop): Declare. * elflink.c (_bfd_elf_gc_mark_rsec): Update comment. (bfd_elf_define_start_stop): New function. * linker.c (bfd_generic_define_start_stop): New function. * coff64-rs6000.c (rs6000_xcoff64_vec, rs6000_xcoff64_aix_vec): Init new field. * aout-adobe.c (aout_32_bfd_define_start_stop): Define. * aout-target.h (MY_bfd_define_start_stop): Define. * aout-tic30.c (MY_bfd_define_start_stop): Define. * binary.c (binary_bfd_define_start_stop): Define. * bout.c (b_out_bfd_define_start_stop): Define. * coff-alpha.c (_bfd_ecoff_bfd_define_start_stop): Define. * coff-mips.c (_bfd_ecoff_bfd_define_start_stop): Define. * coff-rs6000.c (_bfd_xcoff_bfd_define_start_stop): Define. * coffcode.h (coff_bfd_define_start_stop): Define. * elfxx-target.h (bfd_elfNN_bfd_define_start_stop): Define. * i386msdos.c (msdos_bfd_define_start_stop): Define. * i386os9k.c (os9k_bfd_define_start_stop): Define. * ieee.c (ieee_bfd_define_start_stop): Define. * ihex.c (ihex_bfd_define_start_stop): Define. * libbfd-in.h (_bfd_nolink_bfd_define_start_stop): Define. * mach-o-target.c (bfd_mach_o_bfd_define_start_stop): Define. * mmo.c (mmo_bfd_define_start_stop): Define. * nlm-target.h (nlm_bfd_define_start_stop): Define. * oasys.c (oasys_bfd_define_start_stop): Define. * pef.c (bfd_pef_bfd_define_start_stop): Define. * plugin.c (bfd_plugin_bfd_define_start_stop): Define. * ppcboot.c (ppcboot_bfd_define_start_stop): Define. * som.c (som_bfd_define_start_stop): Define. * srec.c (srec_bfd_define_start_stop): Define. * tekhex.c (tekhex_bfd_define_start_stop): Define. * versados.c (versados_bfd_define_start_stop): Define. * vms-alpha.c (vms_bfd_define_start_stop): Define. (alpha_vms_bfd_define_start_stop): Define. * xsym.c (bfd_sym_bfd_define_start_stop): Define. * bfd-in2.h: Regenerate. * libbfd.h: Regenerate. ld/ * emultempl/elf32.em (gld${EMULATION_NAME}_after_open): Don't set __start/__stop syms here. * ldlang.c (lang_set_startof): Delete. (start_stop_syms, start_stop_count, start_stop_alloc): New vars. (lang_define_start_stop, lang_init_start_stop, foreach_start_stop, undef_start_stop, lang_undef_start_stop, lang_init_startof_sizeof, set_start_stop, lang_finalize_start_stop): New functions. (lang_process): Call _start_stop functions. * testsuite/ld-elf/pr21562a.d: Use xfail rather than notarget. Correct typos and list of xfail targets. * testsuite/ld-elf/pr21562b.d: Likewise. * testsuite/ld-elf/pr21562c.d: Likewise. * testsuite/ld-elf/pr21562d.d: Likewise. * testsuite/ld-elf/pr21562e.d: Likewise. * testsuite/ld-elf/pr21562f.d: Likewise. * testsuite/ld-elf/pr21562g.d: Likewise. * testsuite/ld-elf/pr21562h.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/sizeofa.d: Likewise. Adjust to pass for generic ELF. * testsuite/ld-elf/sizeofb.d: Likewise. * testsuite/ld-elf/startofa.d: Likewise. * testsuite/ld-elf/startofb.d: Likewise.