This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
RFC: [PATCH] New attempt at fixing MIPS --gc-sections et al.
- From: David Daney <ddaney at avtrex dot com>
- To: binutils at sources dot redhat dot com
- Date: Tue, 16 Aug 2005 15:55:31 -0700
- Subject: RFC: [PATCH] New attempt at fixing MIPS --gc-sections et al.
This is a follow up to the patches I posted in these threads:
http://sourceware.org/ml/binutils/2005-07/msg00521.html
http://sourceware.org/ml/binutils/2005-08/msg00040.html
http://sourceware.org/ml/binutils/2005-07/msg00511.html
The main problem I was having (and that this patch fixes) was that some
symbols defined in GCed sections, were being emitted into the dynamic
symbol table as undefined symbols. This caused errors in the runtime
linker as the symbols were not defined anywhere.
A secondary problem (worked around in the patch found in the third
hyper-link above) was that the magic '_gp_disp' and '__gnu_local_gp'
symbols were being emitted into the dynamic symbol table even though
they are resolved by the linker.
The theory behind the patch is this:
A new field (do_not_emit) is added to the elf_link_hash_entry structure.
When it is set, the corresponding symbol will not be emitted into the
linker output.
The do_not_emit field is then set in elf_gc_sweep_symbol if the symbol's
section was GCed. Similarly it is set for the magic '_gp_disp' and
'__gnu_local_gp' symbols.
I have run make -k check with no regressions on a mipsel-linux cross
build running on x86 FC3. I have two large multi-got executables that
can now be successfully built/run with gcc -ffunction-sections
-fdata-sections and ld --gc-sections. The space savings in one of them
is about 20%, so I am quite happy with the results.
I am only seeking comments at this time because this is a quite
intrusive patch, and I have only tried it on mipsel-linux targets.
There are several potential problems:
1) struct elf_link_hash_entry may be 4 bytes larger (I am not sure about
this though).
2) Non-MIPS targets may not work anymore with --gc-sections. For MIPS
the GOT generation code needed some patching. I suspect that other
targets might need similar things as well, but I don't really have any
good way to test them.
Let me know what you think.
2005-08-16 David Daney <ddaney@avtrex.com>
* elf-bfd.h (struct elf_link_hash_entry.do_not_emit): New field.
* elflink.c (elf_link_renumber_hash_table_dynsyms): Do nothing if
do_not_emit is true.
(elf_link_renumber_local_hash_table_dynsyms): Ditto.
(elf_collect_hash_codes): Ditto.
(elf_link_output_extsym): Ditto.
(elf_gc_sweep_symbol): Set do_not_emit if symbol is in a
SEC_EXCLUDE section.
(elf_gc_sweep): Add new parameter *output_bfd. Call
_bfd_elf_link_renumber_dynsyms.
(bfd_elf_gc_sections): Pass new output_bfd parameter to elf_gc_sweep.
* elfxx-mips.c (mips_elf_sort_hash_table_f): Do nothing if
do_not_emit is true.
(mips_elf_record_global_got_symbol): Ditto.
(mips_elf_make_got_per_bfd): Ditto.
(mips_elf_set_global_got_offset): Ditto.
(mips_elf_calculate_relocation): Set do_not_emit for '__gnu_local_gp'
and '_gp_disp'.
(_bfd_mips_elf_finish_dynamic_symbol): Do nothing if do_not_emit
is true.
David Daney.
? gc-sections.d
? doc/bfd.info
Index: elf-bfd.h
===================================================================
RCS file: /cvs/src/src/bfd/elf-bfd.h,v
retrieving revision 1.195
diff -c -p -r1.195 elf-bfd.h
*** elf-bfd.h 15 Aug 2005 15:39:07 -0000 1.195
--- elf-bfd.h 16 Aug 2005 22:04:39 -0000
*************** struct elf_link_hash_entry
*** 169,174 ****
--- 169,178 ----
/* Symbol is referenced with a relocation where C/C++ pointer equality
matters. */
unsigned int pointer_equality_needed : 1;
+ /* Symbol should not be emitted into the symbol tables. It is
+ either in a garbage collected section, or a magic symbol that is
+ resolved by the linker. */
+ unsigned int do_not_emit : 1;
/* String table index in .dynstr if this is a dynamic symbol. */
unsigned long dynstr_index;
Index: elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.188
diff -c -p -r1.188 elflink.c
*** elflink.c 15 Aug 2005 15:39:07 -0000 1.188
--- elflink.c 16 Aug 2005 22:04:40 -0000
*************** elf_link_renumber_hash_table_dynsyms (st
*** 623,629 ****
if (h->root.type == bfd_link_hash_warning)
h = (struct elf_link_hash_entry *) h->root.u.i.link;
! if (h->forced_local)
return TRUE;
if (h->dynindx != -1)
--- 623,629 ----
if (h->root.type == bfd_link_hash_warning)
h = (struct elf_link_hash_entry *) h->root.u.i.link;
! if (h->forced_local || h->do_not_emit)
return TRUE;
if (h->dynindx != -1)
*************** elf_link_renumber_local_hash_table_dynsy
*** 645,651 ****
if (h->root.type == bfd_link_hash_warning)
h = (struct elf_link_hash_entry *) h->root.u.i.link;
! if (!h->forced_local)
return TRUE;
if (h->dynindx != -1)
--- 645,651 ----
if (h->root.type == bfd_link_hash_warning)
h = (struct elf_link_hash_entry *) h->root.u.i.link;
! if (!h->forced_local || h->do_not_emit)
return TRUE;
if (h->dynindx != -1)
*************** elf_collect_hash_codes (struct elf_link_
*** 4740,4746 ****
h = (struct elf_link_hash_entry *) h->root.u.i.link;
/* Ignore indirect symbols. These are added by the versioning code. */
! if (h->dynindx == -1)
return TRUE;
name = h->root.root.string;
--- 4740,4746 ----
h = (struct elf_link_hash_entry *) h->root.u.i.link;
/* Ignore indirect symbols. These are added by the versioning code. */
! if (h->dynindx == -1 || h->do_not_emit)
return TRUE;
name = h->root.root.string;
*************** elf_link_output_extsym (struct elf_link_
*** 6350,6355 ****
--- 6350,6358 ----
return TRUE;
}
+ if (h->do_not_emit)
+ return TRUE;
+
/* Decide whether to output this symbol in this pass. */
if (eoinfo->localsyms)
{
*************** elf_gc_sweep_symbol (struct elf_link_has
*** 8860,8865 ****
--- 8863,8876 ----
if (h->root.type == bfd_link_hash_warning)
h = (struct elf_link_hash_entry *) h->root.u.i.link;
+ /* If the symbol is defined in a garbage collected section, do not
+ emit it into the symbol tables. */
+ if (h->root.u.def.section->flags & SEC_EXCLUDE)
+ {
+ h->do_not_emit = 1;
+ return TRUE;
+ }
+
if (h->dynindx != -1
&& ((h->root.type != bfd_link_hash_defined
&& h->root.type != bfd_link_hash_defweak)
*************** typedef bfd_boolean (*gc_sweep_hook_fn)
*** 8875,8881 ****
(bfd *, struct bfd_link_info *, asection *, const Elf_Internal_Rela *);
static bfd_boolean
! elf_gc_sweep (struct bfd_link_info *info, gc_sweep_hook_fn gc_sweep_hook)
{
bfd *sub;
--- 8886,8894 ----
(bfd *, struct bfd_link_info *, asection *, const Elf_Internal_Rela *);
static bfd_boolean
! elf_gc_sweep (bfd *output_bfd,
! struct bfd_link_info *info,
! gc_sweep_hook_fn gc_sweep_hook)
{
bfd *sub;
*************** elf_gc_sweep (struct bfd_link_info *info
*** 8936,8951 ****
static symbol table as well? */
{
int i = 0;
!
elf_link_hash_traverse (elf_hash_table (info), elf_gc_sweep_symbol, &i);
! /* There is an unused NULL entry at the head of the table which
! we must account for in our count. Unless there weren't any
! symbols, which means we'll have no table at all. */
! if (i != 0)
! ++i;
!
! elf_hash_table (info)->dynsymcount = i;
}
return TRUE;
--- 8949,8962 ----
static symbol table as well? */
{
int i = 0;
! unsigned long j;
!
elf_link_hash_traverse (elf_hash_table (info), elf_gc_sweep_symbol, &i);
! /* elf_gc_sweep_symbol does not have the necessary logic to correctly
! number and count the dynsyms. We use the proper tool to do this. */
! _bfd_elf_link_renumber_dynsyms (output_bfd, info, &j);
!
}
return TRUE;
*************** bfd_elf_gc_sections (bfd *abfd, struct b
*** 9184,9190 ****
}
/* ... and mark SEC_EXCLUDE for those that go. */
! if (!elf_gc_sweep (info, get_elf_backend_data (abfd)->gc_sweep_hook))
return FALSE;
return TRUE;
--- 9195,9201 ----
}
/* ... and mark SEC_EXCLUDE for those that go. */
! if (!elf_gc_sweep (abfd, info, get_elf_backend_data (abfd)->gc_sweep_hook))
return FALSE;
return TRUE;
Index: elfxx-mips.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-mips.c,v
retrieving revision 1.148
diff -c -p -r1.148 elfxx-mips.c
*** elfxx-mips.c 1 Aug 2005 11:59:31 -0000 1.148
--- elfxx-mips.c 16 Aug 2005 22:04:42 -0000
*************** mips_elf_sort_hash_table_f (struct mips_
*** 2583,2589 ****
/* Symbols without dynamic symbol table entries aren't interesting
at all. */
! if (h->root.dynindx == -1)
return TRUE;
/* Global symbols that need GOT entries that are not explicitly
--- 2583,2589 ----
/* Symbols without dynamic symbol table entries aren't interesting
at all. */
! if (h->root.dynindx == -1 || h->root.do_not_emit)
return TRUE;
/* Global symbols that need GOT entries that are not explicitly
*************** mips_elf_record_global_got_symbol (struc
*** 2623,2628 ****
--- 2623,2631 ----
{
struct mips_got_entry entry, **loc;
+ if (h->do_not_emit)
+ return TRUE;
+
/* A global symbol in the GOT must also be in the dynamic symbol
table. */
if (h->dynindx == -1)
*************** mips_elf_make_got_per_bfd (void **entryp
*** 2792,2797 ****
--- 2795,2804 ----
struct mips_elf_bfd2got_hash bfdgot_entry, *bfdgot;
void **bfdgotp;
+
+ if (entry->d.h->root.do_not_emit)
+ return 1;
+
/* Find the got_info for this GOT entry's input bfd. Create one if
none exists. */
bfdgot_entry.bfd = entry->abfd;
*************** mips_elf_set_global_got_offset (void **e
*** 3049,3055 ****
entry->symndx == -1 ? &entry->d.h->root : NULL);
if (entry->abfd != NULL && entry->symndx == -1
! && entry->d.h->root.dynindx != -1
&& entry->d.h->tls_type == GOT_NORMAL)
{
if (g)
--- 3056,3062 ----
entry->symndx == -1 ? &entry->d.h->root : NULL);
if (entry->abfd != NULL && entry->symndx == -1
! && entry->d.h->root.dynindx != -1 && !entry->d.h->root.do_not_emit
&& entry->d.h->tls_type == GOT_NORMAL)
{
if (g)
*************** mips_elf_calculate_relocation (bfd *abfd
*** 3747,3758 ****
return bfd_reloc_notsupported;
gp_disp_p = TRUE;
}
! /* See if this is the special _gp symbol. Note that such a
! symbol must always be a global symbol. */
else if (strcmp (*namep, "__gnu_local_gp") == 0)
! gnu_local_gp_p = TRUE;
!
/* If this symbol is defined, calculate its address. Note that
_gp_disp is a magic symbol, always implicitly defined by the
--- 3754,3772 ----
return bfd_reloc_notsupported;
gp_disp_p = TRUE;
+ /* Since it is a magic symbol resolved here in the linker,
+ do not allow it to escape into the symbol tables. */
+ h->root.do_not_emit = 1;
}
! /* See if this is the special _gp_local_gp symbol. Note that
! such a symbol must always be a global symbol. */
else if (strcmp (*namep, "__gnu_local_gp") == 0)
! {
! gnu_local_gp_p = TRUE;
! /* Since it is a magic symbol resolved here in the linker,
! do not allow it to escape into the symbol tables. */
! h->root.do_not_emit = 1;
! }
/* If this symbol is defined, calculate its address. Note that
_gp_disp is a magic symbol, always implicitly defined by the
*************** _bfd_mips_elf_finish_dynamic_symbol (bfd
*** 7425,7431 ****
MIPS_ELF_PUT_WORD (output_bfd, value, sgot->contents + offset);
}
! if (g->next && h->dynindx != -1 && h->type != STT_TLS)
{
struct mips_got_entry e, *p;
bfd_vma entry;
--- 7439,7445 ----
MIPS_ELF_PUT_WORD (output_bfd, value, sgot->contents + offset);
}
! if (g->next && h->dynindx != -1 && !h->do_not_emit && h->type != STT_TLS)
{
struct mips_got_entry e, *p;
bfd_vma entry;