public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Fix --gc-sections for C++ MIPS ELF
@ 2007-12-02 20:15 Richard Sandiford
  2007-12-02 20:18 ` [PATCH 1/7] " Richard Sandiford
                   ` (9 more replies)
  0 siblings, 10 replies; 15+ messages in thread
From: Richard Sandiford @ 2007-12-02 20:15 UTC (permalink / raw)
  To: binutils

--Wl,--gc-sections generates wrong code for C++ on MIPS ELF.  The problem
is that if:

  (a) all C++ CIEs have relocations against the personality routine
      itself, rather relocations against a pointer to the personality
      routine, and

  (b) as usual, there are no references to the personality routine's
      section outside .eh_frame

then we never mark the personality routine's section as needed.
The main code for marking .eh_frames is:

      /* Keep .gcc_except_table.* if the associated .text.* (or the
	 associated .gnu.linkonce.t.* if .text.* doesn't exist) is
	 marked.  This isn't very nice, but the proper solution,
	 splitting .eh_frame up and using comdat doesn't pan out
	 easily due to needing special relocs to handle the
	 difference of two symbols in separate sections.
	 Don't keep code sections referenced by .eh_frame.  */
#define TEXT_PREFIX			".text."
#define TEXT_PREFIX2			".gnu.linkonce.t."
#define GCC_EXCEPT_TABLE_PREFIX		".gcc_except_table."
      for (o = sub->sections; o != NULL; o = o->next)
	if (!o->gc_mark && o->gc_mark_from_eh && (o->flags & SEC_CODE) == 0)

and it ignores all relocations against code.

Non-PIC CIEs for targets like x86_64-linux-gnu also have direct references
to the personality routines.  I suspect the only reason -Wl,--gc-sections
-static-libgcc works for them is that libgcc's own CIEs use an indirect
reference, so the section gets marked that way.

A simple fix would be to use indirect references for MIPS ELF too,
but it would be nice to avoid the overhead.  It would also be nice to
make binutils work with older GCCs if possible.

Another reason to prefer a binutils change is that the current code
seems unsafe in ways that aren't explicitly mentioned in the comment
above.  The marking code assumes that FDEs against discarded sections will
themselves be discarded, but if an .eh_frame section contains something
unexpected, like a future augmentation type, elf-eh-frame.c will keep
the section as-is.  We would then get a link failure or silent wrong code.

Has there been any talk about parsing the CIEs and FDEs and marking
relocations against them individually?  I couldn't find anything in the
archives, so I gave it a go, and it seems to work.  The main problem was
that we currently don't parse the .eh_frame information until later in
the link process, so a fair amount of rejigging was needed.

Once we have access to the parsed information, we can mark an .eh_frame
relocation if:

  (i) it is associated with an FDE that is itself associated with
      a marked section or

  (ii) it is associated with the CIE for such an FDE.

I tested the changes by running the C++ and libstdc++-v3 testsuites
on x86_64-linux-gnu and mipsisa64-elf with -Wl,--gc-sections.
The C++ PCH tests failed, but the results were otherwise identical
to those without -Wl,--gc-sections.  There were also no regressions
in the binutils, gas and ld testsuites for x86_64-linux-gnu,
mipsisa64-elf and mips64-linux-gnu.

Because several changes are needed, I've tried to split things up for
ease of review.  I'll post each patch as a follow-up.

Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/7] Fix --gc-sections for C++ MIPS ELF
  2007-12-02 20:15 Fix --gc-sections for C++ MIPS ELF Richard Sandiford
@ 2007-12-02 20:18 ` Richard Sandiford
  2007-12-02 20:19 ` [PATCH 2/7] " Richard Sandiford
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2007-12-02 20:18 UTC (permalink / raw)
  To: binutils

The discard routines store some relocation information in a structure
called elf_reloc_cookie, which the .eh_frame parser uses to check that
the relocs are as expected.  If we're going to parse .eh_frame earlier,
it would be good to have cookies at that stage too.

_bfd_elf_gc_mark uses the same information that you get from an
elf_reloc_cookie, but stores it in separate variables.  This first
patch therefore splits out the cookie initialisation and finalisation
routines from bfd_elf_discard_info and uses them in _bfd_elf_gc_mark as
well.  The patch also splits out parts of _bfd_elf_gc_mark so that they
can be reused in later patches.

The patch fixes a potential memory leak, although it probably never
occured in practice.  _bfd_elf_gc_mark behaves as though it could be
the first routine to set symtab_hdr->contents:

      if (isym != NULL && symtab_hdr->contents != (unsigned char *) isym)
	{
	  if (! info->keep_memory)
	    free (isym);
	  else
	    symtab_hdr->contents = (unsigned char *) isym;
	}

but it does this after a loop that involves recursive calls to
_bfd_elf_gc_mark.  If one of those recursive calls was for the
same input bfd, that call would still see a null contents field,
and would therefore create a second copy of the memory.  I avoided
this by putting the symtab_hdr->contents assignment in the
initialisation code instead.

Richard


bfd/
	* elf-bfd.h (_bfd_elf_gc_mark_rsec, _bfd_elf_gc_mark_reloc): Declare.
	(_bfd_elf_gc_mark): Use elf_gc_mark_hook_fn.
	* elflink.c (init_reloc_cookie, fini_reloc_cookie)
	(init_reloc_cookie_rels, fini_reloc_cookie_rels): New functions,
	split out from...
	(bfd_elf_discard_info): ...here.
	(init_reloc_cookie_for_section): New function.
	(fini_reloc_cookie_for_section): Likewise.
	(_bfd_elf_gc_mark_rsec, _bfd_elf_gc_mark_reloc): New functions,
	split out from...
	(_bfd_elf_gc_mark): ...here.  Use init_reloc_cookie_for_section
	and fini_reloc_cookie_for_section.

Index: bfd/elf-bfd.h
===================================================================
*** bfd/elf-bfd.h	2007-12-02 16:59:44.000000000 +0000
--- bfd/elf-bfd.h	2007-12-02 16:59:46.000000000 +0000
*************** extern void _bfd_elf_set_osabi (bfd * , 
*** 1971,1980 ****
    (asection *, struct bfd_link_info *, Elf_Internal_Rela *,
     struct elf_link_hash_entry *, Elf_Internal_Sym *);
  
  extern bfd_boolean _bfd_elf_gc_mark
!   (struct bfd_link_info *, asection *,
!    asection * (*) (asection *, struct bfd_link_info *, Elf_Internal_Rela *,
! 		   struct elf_link_hash_entry *, Elf_Internal_Sym *));
  
  extern bfd_boolean bfd_elf_gc_common_finalize_got_offsets
    (bfd *, struct bfd_link_info *);
--- 1971,1986 ----
    (asection *, struct bfd_link_info *, Elf_Internal_Rela *,
     struct elf_link_hash_entry *, Elf_Internal_Sym *);
  
+ extern asection *_bfd_elf_gc_mark_rsec
+   (struct bfd_link_info *, asection *, elf_gc_mark_hook_fn,
+    struct elf_reloc_cookie *);
+ 
+ extern bfd_boolean _bfd_elf_gc_mark_reloc
+   (struct bfd_link_info *, asection *, elf_gc_mark_hook_fn,
+    struct elf_reloc_cookie *, bfd_boolean);
+ 
  extern bfd_boolean _bfd_elf_gc_mark
!   (struct bfd_link_info *, asection *, elf_gc_mark_hook_fn);
  
  extern bfd_boolean bfd_elf_gc_common_finalize_got_offsets
    (bfd *, struct bfd_link_info *);
Index: bfd/elflink.c
===================================================================
*** bfd/elflink.c	2007-12-02 16:59:44.000000000 +0000
--- bfd/elflink.c	2007-12-02 17:01:03.000000000 +0000
*************** bfd_elf_final_link (bfd *abfd, struct bf
*** 10940,10945 ****
--- 10940,11078 ----
    return FALSE;
  }
  \f
+ /* Initialize COOKIE for input bfd ABFD.  */
+ 
+ static bfd_boolean
+ init_reloc_cookie (struct elf_reloc_cookie *cookie,
+ 		   struct bfd_link_info *info, bfd *abfd)
+ {
+   Elf_Internal_Shdr *symtab_hdr;
+   const struct elf_backend_data *bed;
+ 
+   bed = get_elf_backend_data (abfd);
+   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
+ 
+   cookie->abfd = abfd;
+   cookie->sym_hashes = elf_sym_hashes (abfd);
+   cookie->bad_symtab = elf_bad_symtab (abfd);
+   if (cookie->bad_symtab)
+     {
+       cookie->locsymcount = symtab_hdr->sh_size / bed->s->sizeof_sym;
+       cookie->extsymoff = 0;
+     }
+   else
+     {
+       cookie->locsymcount = symtab_hdr->sh_info;
+       cookie->extsymoff = symtab_hdr->sh_info;
+     }
+ 
+   if (bed->s->arch_size == 32)
+     cookie->r_sym_shift = 8;
+   else
+     cookie->r_sym_shift = 32;
+ 
+   cookie->locsyms = (Elf_Internal_Sym *) symtab_hdr->contents;
+   if (cookie->locsyms == NULL && cookie->locsymcount != 0)
+     {
+       cookie->locsyms = bfd_elf_get_elf_syms (abfd, symtab_hdr,
+ 					      cookie->locsymcount, 0,
+ 					      NULL, NULL, NULL);
+       if (cookie->locsyms == NULL)
+ 	{
+ 	  info->callbacks->einfo (_("%P%X: can not read symbols: %E\n"));
+ 	  return FALSE;
+ 	}
+       if (info->keep_memory)
+ 	symtab_hdr->contents = (bfd_byte *) cookie->locsyms;
+     }
+   return TRUE;
+ }
+ 
+ /* Free the memory allocated by init_reloc_cookie, if appropriate.  */
+ 
+ static void
+ fini_reloc_cookie (struct elf_reloc_cookie *cookie, bfd *abfd)
+ {
+   Elf_Internal_Shdr *symtab_hdr;
+ 
+   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
+   if (cookie->locsyms != NULL
+       && symtab_hdr->contents != (unsigned char *) cookie->locsyms)
+     free (cookie->locsyms);
+ }
+ 
+ /* Initialize the relocation information in COOKIE for input section SEC
+    of input bfd ABFD.  */
+ 
+ static bfd_boolean
+ init_reloc_cookie_rels (struct elf_reloc_cookie *cookie,
+ 			struct bfd_link_info *info, bfd *abfd,
+ 			asection *sec)
+ {
+   const struct elf_backend_data *bed;
+ 
+   if (sec->reloc_count == 0)
+     {
+       cookie->rels = NULL;
+       cookie->relend = NULL;
+     }
+   else
+     {
+       bed = get_elf_backend_data (abfd);
+ 
+       cookie->rels = _bfd_elf_link_read_relocs (abfd, sec, NULL, NULL,
+ 						info->keep_memory);
+       if (cookie->rels == NULL)
+ 	return FALSE;
+       cookie->rel = cookie->rels;
+       cookie->relend = (cookie->rels
+ 			+ sec->reloc_count * bed->s->int_rels_per_ext_rel);
+     }
+   cookie->rel = cookie->rels;
+   return TRUE;
+ }
+ 
+ /* Free the memory allocated by init_reloc_cookie_rels,
+    if appropriate.  */
+ 
+ static void
+ fini_reloc_cookie_rels (struct elf_reloc_cookie *cookie,
+ 			asection *sec)
+ {
+   if (cookie->rels && elf_section_data (sec)->relocs != cookie->rels)
+     free (cookie->rels);
+ }
+ 
+ /* Initialize the whole of COOKIE for input section SEC.  */
+ 
+ static bfd_boolean
+ init_reloc_cookie_for_section (struct elf_reloc_cookie *cookie,
+ 			       struct bfd_link_info *info,
+ 			       asection *sec)
+ {
+   if (!init_reloc_cookie (cookie, info, sec->owner))
+     goto error1;
+   if (!init_reloc_cookie_rels (cookie, info, sec->owner, sec))
+     goto error2;
+   return TRUE;
+ 
+  error2:
+   fini_reloc_cookie (cookie, sec->owner);
+  error1:
+   return FALSE;
+ }
+ 
+ /* Free the memory allocated by init_reloc_cookie_for_section,
+    if appropriate.  */
+ 
+ static void
+ fini_reloc_cookie_for_section (struct elf_reloc_cookie *cookie,
+ 			       asection *sec)
+ {
+   fini_reloc_cookie_rels (cookie, sec);
+   fini_reloc_cookie (cookie, sec->owner);
+ }
+ \f
  /* Garbage collect unused sections.  */
  
  /* Default gc_mark_hook.  */
*************** _bfd_elf_gc_mark_hook (asection *sec,
*** 10972,10977 ****
--- 11105,11167 ----
    return NULL;
  }
  
+ /* COOKIE->rel describes a relocation against section SEC, which is
+    a section we've decided to keep.  Return the section that contains
+    the relocation symbol, or NULL if no section contains it.  */
+ 
+ asection *
+ _bfd_elf_gc_mark_rsec (struct bfd_link_info *info, asection *sec,
+ 		       elf_gc_mark_hook_fn gc_mark_hook,
+ 		       struct elf_reloc_cookie *cookie)
+ {
+   unsigned long r_symndx;
+   struct elf_link_hash_entry *h;
+ 
+   r_symndx = cookie->rel->r_info >> cookie->r_sym_shift;
+   if (r_symndx == 0)
+     return NULL;
+ 
+   if (r_symndx >= cookie->locsymcount
+       || ELF_ST_BIND (cookie->locsyms[r_symndx].st_info) != STB_LOCAL)
+     {
+       h = cookie->sym_hashes[r_symndx - cookie->extsymoff];
+       while (h->root.type == bfd_link_hash_indirect
+ 	     || h->root.type == bfd_link_hash_warning)
+ 	h = (struct elf_link_hash_entry *) h->root.u.i.link;
+       return (*gc_mark_hook) (sec, info, cookie->rel, h, NULL);
+     }
+ 
+   return (*gc_mark_hook) (sec, info, cookie->rel, NULL,
+ 			  &cookie->locsyms[r_symndx]);
+ }
+ 
+ /* COOKIE->rel describes a relocation against section SEC, which is
+    a section we've decided to keep.  Mark the section that contains
+    the relocation symbol.  IS_EH is true if the mark comes from
+    .eh_frame.  */
+ 
+ bfd_boolean
+ _bfd_elf_gc_mark_reloc (struct bfd_link_info *info,
+ 			asection *sec,
+ 			elf_gc_mark_hook_fn gc_mark_hook,
+ 			struct elf_reloc_cookie *cookie,
+ 			bfd_boolean is_eh)
+ {
+   asection *rsec;
+ 
+   rsec = _bfd_elf_gc_mark_rsec (info, sec, gc_mark_hook, cookie);
+   if (rsec && !rsec->gc_mark)
+     {
+       if (bfd_get_flavour (rsec->owner) != bfd_target_elf_flavour)
+ 	rsec->gc_mark = 1;
+       else if (is_eh)
+ 	rsec->gc_mark_from_eh = 1;
+       else if (!_bfd_elf_gc_mark (info, rsec, gc_mark_hook))
+ 	return FALSE;
+     }
+   return TRUE;
+ }
+ 
  /* The mark phase of garbage collection.  For a given section, mark
     it and any sections in this section's group, and all the sections
     which define symbols to which it refers.  */
*************** _bfd_elf_gc_mark (struct bfd_link_info *
*** 10998,11100 ****
    is_eh = strcmp (sec->name, ".eh_frame") == 0;
    if ((sec->flags & SEC_RELOC) != 0 && sec->reloc_count > 0)
      {
!       Elf_Internal_Rela *relstart, *rel, *relend;
!       Elf_Internal_Shdr *symtab_hdr;
!       struct elf_link_hash_entry **sym_hashes;
!       size_t nlocsyms;
!       size_t extsymoff;
!       bfd *input_bfd = sec->owner;
!       const struct elf_backend_data *bed = get_elf_backend_data (input_bfd);
!       Elf_Internal_Sym *isym = NULL;
!       int r_sym_shift;
  
!       symtab_hdr = &elf_tdata (input_bfd)->symtab_hdr;
!       sym_hashes = elf_sym_hashes (input_bfd);
! 
!       /* Read the local symbols.  */
!       if (elf_bad_symtab (input_bfd))
! 	{
! 	  nlocsyms = symtab_hdr->sh_size / bed->s->sizeof_sym;
! 	  extsymoff = 0;
! 	}
        else
- 	extsymoff = nlocsyms = symtab_hdr->sh_info;
- 
-       isym = (Elf_Internal_Sym *) symtab_hdr->contents;
-       if (isym == NULL && nlocsyms != 0)
- 	{
- 	  isym = bfd_elf_get_elf_syms (input_bfd, symtab_hdr, nlocsyms, 0,
- 				       NULL, NULL, NULL);
- 	  if (isym == NULL)
- 	    return FALSE;
- 	}
- 
-       /* Read the relocations.  */
-       relstart = _bfd_elf_link_read_relocs (input_bfd, sec, NULL, NULL,
- 					    info->keep_memory);
-       if (relstart == NULL)
- 	{
- 	  ret = FALSE;
- 	  goto out1;
- 	}
-       relend = relstart + sec->reloc_count * bed->s->int_rels_per_ext_rel;
- 
-       if (bed->s->arch_size == 32)
- 	r_sym_shift = 8;
-       else
- 	r_sym_shift = 32;
- 
-       for (rel = relstart; rel < relend; rel++)
- 	{
- 	  unsigned long r_symndx;
- 	  asection *rsec;
- 	  struct elf_link_hash_entry *h;
- 
- 	  r_symndx = rel->r_info >> r_sym_shift;
- 	  if (r_symndx == 0)
- 	    continue;
- 
- 	  if (r_symndx >= nlocsyms
- 	      || ELF_ST_BIND (isym[r_symndx].st_info) != STB_LOCAL)
- 	    {
- 	      h = sym_hashes[r_symndx - extsymoff];
- 	      while (h->root.type == bfd_link_hash_indirect
- 		     || h->root.type == bfd_link_hash_warning)
- 		h = (struct elf_link_hash_entry *) h->root.u.i.link;
- 	      rsec = (*gc_mark_hook) (sec, info, rel, h, NULL);
- 	    }
- 	  else
- 	    {
- 	      rsec = (*gc_mark_hook) (sec, info, rel, NULL, &isym[r_symndx]);
- 	    }
- 
- 	  if (rsec && !rsec->gc_mark)
- 	    {
- 	      if (bfd_get_flavour (rsec->owner) != bfd_target_elf_flavour)
- 		rsec->gc_mark = 1;
- 	      else if (is_eh)
- 		rsec->gc_mark_from_eh = 1;
- 	      else if (!_bfd_elf_gc_mark (info, rsec, gc_mark_hook))
- 		{
- 		  ret = FALSE;
- 		  goto out2;
- 		}
- 	    }
- 	}
- 
-     out2:
-       if (elf_section_data (sec)->relocs != relstart)
- 	free (relstart);
-     out1:
-       if (isym != NULL && symtab_hdr->contents != (unsigned char *) isym)
  	{
! 	  if (! info->keep_memory)
! 	    free (isym);
! 	  else
! 	    symtab_hdr->contents = (unsigned char *) isym;
  	}
      }
- 
    return ret;
  }
  
--- 11188,11209 ----
    is_eh = strcmp (sec->name, ".eh_frame") == 0;
    if ((sec->flags & SEC_RELOC) != 0 && sec->reloc_count > 0)
      {
!       struct elf_reloc_cookie cookie;
  
!       if (!init_reloc_cookie_for_section (&cookie, info, sec))
! 	ret = FALSE;
        else
  	{
! 	  for (; cookie.rel < cookie.relend; cookie.rel++)
! 	    if (!_bfd_elf_gc_mark_reloc (info, sec, gc_mark_hook,
! 					 &cookie, is_eh))
! 	      {
! 		ret = FALSE;
! 		break;
! 	      }
! 	  fini_reloc_cookie_for_section (&cookie, sec);
  	}
      }
    return ret;
  }
  
*************** bfd_elf_discard_info (bfd *output_bfd, s
*** 11777,11786 ****
  {
    struct elf_reloc_cookie cookie;
    asection *stab, *eh;
-   Elf_Internal_Shdr *symtab_hdr;
    const struct elf_backend_data *bed;
    bfd *abfd;
-   unsigned int count;
    bfd_boolean ret = FALSE;
  
    if (info->traditional_format
--- 11886,11893 ----
*************** bfd_elf_discard_info (bfd *output_bfd, s
*** 11819,11913 ****
  	  && bed->elf_backend_discard_info == NULL)
  	continue;
  
!       symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
!       cookie.abfd = abfd;
!       cookie.sym_hashes = elf_sym_hashes (abfd);
!       cookie.bad_symtab = elf_bad_symtab (abfd);
!       if (cookie.bad_symtab)
! 	{
! 	  cookie.locsymcount = symtab_hdr->sh_size / bed->s->sizeof_sym;
! 	  cookie.extsymoff = 0;
! 	}
!       else
! 	{
! 	  cookie.locsymcount = symtab_hdr->sh_info;
! 	  cookie.extsymoff = symtab_hdr->sh_info;
! 	}
! 
!       if (bed->s->arch_size == 32)
! 	cookie.r_sym_shift = 8;
!       else
! 	cookie.r_sym_shift = 32;
! 
!       cookie.locsyms = (Elf_Internal_Sym *) symtab_hdr->contents;
!       if (cookie.locsyms == NULL && cookie.locsymcount != 0)
! 	{
! 	  cookie.locsyms = bfd_elf_get_elf_syms (abfd, symtab_hdr,
! 						 cookie.locsymcount, 0,
! 						 NULL, NULL, NULL);
! 	  if (cookie.locsyms == NULL)
! 	    {
! 	      info->callbacks->einfo (_("%P%X: can not read symbols: %E\n"));
! 	      return FALSE;
! 	    }
! 	}
  
!       if (stab != NULL)
  	{
! 	  cookie.rels = NULL;
! 	  count = stab->reloc_count;
! 	  if (count != 0)
! 	    cookie.rels = _bfd_elf_link_read_relocs (abfd, stab, NULL, NULL,
! 						     info->keep_memory);
! 	  if (cookie.rels != NULL)
! 	    {
! 	      cookie.rel = cookie.rels;
! 	      cookie.relend = cookie.rels;
! 	      cookie.relend += count * bed->s->int_rels_per_ext_rel;
! 	      if (_bfd_discard_section_stabs (abfd, stab,
! 					      elf_section_data (stab)->sec_info,
! 					      bfd_elf_reloc_symbol_deleted_p,
! 					      &cookie))
! 		ret = TRUE;
! 	      if (elf_section_data (stab)->relocs != cookie.rels)
! 		free (cookie.rels);
! 	    }
  	}
  
!       if (eh != NULL)
  	{
- 	  cookie.rels = NULL;
- 	  count = eh->reloc_count;
- 	  if (count != 0)
- 	    cookie.rels = _bfd_elf_link_read_relocs (abfd, eh, NULL, NULL,
- 						     info->keep_memory);
- 	  cookie.rel = cookie.rels;
- 	  cookie.relend = cookie.rels;
- 	  if (cookie.rels != NULL)
- 	    cookie.relend += count * bed->s->int_rels_per_ext_rel;
- 
  	  if (_bfd_elf_discard_section_eh_frame (abfd, info, eh,
  						 bfd_elf_reloc_symbol_deleted_p,
  						 &cookie))
  	    ret = TRUE;
! 
! 	  if (cookie.rels != NULL
! 	      && elf_section_data (eh)->relocs != cookie.rels)
! 	    free (cookie.rels);
  	}
  
        if (bed->elf_backend_discard_info != NULL
  	  && (*bed->elf_backend_discard_info) (abfd, &cookie, info))
  	ret = TRUE;
  
!       if (cookie.locsyms != NULL
! 	  && symtab_hdr->contents != (unsigned char *) cookie.locsyms)
! 	{
! 	  if (! info->keep_memory)
! 	    free (cookie.locsyms);
! 	  else
! 	    symtab_hdr->contents = (unsigned char *) cookie.locsyms;
! 	}
      }
  
    if (info->eh_frame_hdr
--- 11926,11961 ----
  	  && bed->elf_backend_discard_info == NULL)
  	continue;
  
!       if (!init_reloc_cookie (&cookie, info, abfd))
! 	return FALSE;
  
!       if (stab != NULL
! 	  && stab->reloc_count > 0
! 	  && init_reloc_cookie_rels (&cookie, info, abfd, stab))
  	{
! 	  if (_bfd_discard_section_stabs (abfd, stab,
! 					  elf_section_data (stab)->sec_info,
! 					  bfd_elf_reloc_symbol_deleted_p,
! 					  &cookie))
! 	    ret = TRUE;
! 	  fini_reloc_cookie_rels (&cookie, stab);
  	}
  
!       if (eh != NULL
! 	  && init_reloc_cookie_rels (&cookie, info, abfd, eh))
  	{
  	  if (_bfd_elf_discard_section_eh_frame (abfd, info, eh,
  						 bfd_elf_reloc_symbol_deleted_p,
  						 &cookie))
  	    ret = TRUE;
! 	  fini_reloc_cookie_rels (&cookie, eh);
  	}
  
        if (bed->elf_backend_discard_info != NULL
  	  && (*bed->elf_backend_discard_info) (abfd, &cookie, info))
  	ret = TRUE;
  
!       fini_reloc_cookie (&cookie, abfd);
      }
  
    if (info->eh_frame_hdr

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/7] Fix --gc-sections for C++ MIPS ELF
  2007-12-02 20:15 Fix --gc-sections for C++ MIPS ELF Richard Sandiford
  2007-12-02 20:18 ` [PATCH 1/7] " Richard Sandiford
@ 2007-12-02 20:19 ` Richard Sandiford
  2007-12-02 20:28 ` [PATCH 3/7] " Richard Sandiford
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2007-12-02 20:19 UTC (permalink / raw)
  To: binutils

Later patches want to know the index of the first relocation against
a particular FDE or CIE, so this patch adds a reloc_index field to
eh_cie_fde.  We currently have:

  struct eh_cie_fde *cie_inf;
  unsigned int size;
  unsigned int offset;
  unsigned int new_offset;
  unsigned char fde_encoding;
  unsigned char lsda_encoding;
  unsigned char lsda_offset;
  unsigned int cie : 1;
  unsigned int removed : 1;
  unsigned int add_augmentation_size : 1;
  unsigned int add_fde_encoding : 1;
  unsigned int make_relative : 1;
  unsigned int make_lsda_relative : 1;
  unsigned int need_lsda_relative : 1;
  unsigned int per_encoding_relative : 1;
  unsigned int *set_loc;

i.e. 3 unsigned chars and 8 bits in an unsigned int, so we can
avoid growing the structure by turning the "unsigned char"s into
"unsigned int : 8"s.

Also, later patches want to add some CIE-specific information,
so this patch puts the FDE-specific cie_inf in a union.

Richard


bfd/
	* elf-bfd.h (eh_cie_fde): Put cie_inf in a union.  Add a reloc_index
	field.  Use bitfields for fde_encoding, lsda_encoding and lsda_offset.
	* elf-eh-frame.c (extra_augmentation_data_bytes): Adjust cie_inf
	accesses after the above change.
	(_bfd_elf_eh_frame_section_offset): Likewise.
	(_bfd_elf_write_section_eh_frame): Likewise.
	(_bfd_elf_discard_section_eh_frame): Likewise.  Set up reloc_index.

Index: bfd/elf-bfd.h
===================================================================
--- bfd/elf-bfd.h	2007-12-02 16:59:46.000000000 +0000
+++ bfd/elf-bfd.h	2007-12-02 17:01:08.000000000 +0000
@@ -264,14 +264,19 @@ struct elf_link_loaded_list
 /* Structures used by the eh_frame optimization code.  */
 struct eh_cie_fde
 {
-  /* For FDEs, this points to the CIE used.  */
-  struct eh_cie_fde *cie_inf;
+  union {
+    struct {
+      /* The CIE that we have chosen to use for this FDE.  */
+      struct eh_cie_fde *cie_inf;
+    } fde;
+  } u;
+  unsigned int reloc_index;
   unsigned int size;
   unsigned int offset;
   unsigned int new_offset;
-  unsigned char fde_encoding;
-  unsigned char lsda_encoding;
-  unsigned char lsda_offset;
+  unsigned int fde_encoding : 8;
+  unsigned int lsda_encoding : 8;
+  unsigned int lsda_offset : 8;
   unsigned int cie : 1;
   unsigned int removed : 1;
   unsigned int add_augmentation_size : 1;
Index: bfd/elf-eh-frame.c
===================================================================
--- bfd/elf-eh-frame.c	2007-12-02 16:59:44.000000000 +0000
+++ bfd/elf-eh-frame.c	2007-12-02 17:01:08.000000000 +0000
@@ -304,7 +304,7 @@ extra_augmentation_data_bytes (struct eh
     }
   else
     {
-      if (entry->cie_inf->add_augmentation_size)
+      if (entry->u.fde.cie_inf->add_augmentation_size)
 	size++;
     }
   return size;
@@ -572,6 +572,7 @@ #define GET_RELOC(buf)					\
 
       this_inf->offset = last_fde - ehbuf;
       this_inf->size = 4 + hdr_length;
+      this_inf->reloc_index = cookie->rel - cookie->rels;
 
       if (hdr_length == 0)
 	{
@@ -826,7 +827,7 @@ #define GET_RELOC(buf)					\
 		}
 	      ecie->usage_count++;
 	      hdr_info->fde_count++;
-	      this_inf->cie_inf = (void *) (ecie - ecies);
+	      this_inf->u.fde.cie_inf = (void *) (ecie - ecies);
 	    }
 
 	  /* Skip the initial location and address range.  */
@@ -954,8 +955,8 @@ #define GET_RELOC(buf)					\
       {
 	if (!ent->cie)
 	  {
-	    ecie = ecies + (bfd_hostptr_t) ent->cie_inf;
-	    ent->cie_inf = ecie->cie.cie_inf;
+	    ecie = ecies + (bfd_hostptr_t) ent->u.fde.cie_inf;
+	    ent->u.fde.cie_inf = ecie->cie.cie_inf;
 	  }
 	ent->new_offset = offset;
 	offset += size_of_output_cie_fde (ent, ptr_size);
@@ -1116,20 +1117,20 @@ _bfd_elf_eh_frame_section_offset (bfd *o
   /* If converting to DW_EH_PE_pcrel, there will be no need for run-time
      relocation against FDE's initial_location field.  */
   if (!sec_info->entry[mid].cie
-      && sec_info->entry[mid].cie_inf->make_relative
+      && sec_info->entry[mid].u.fde.cie_inf->make_relative
       && offset == sec_info->entry[mid].offset + 8)
     return (bfd_vma) -2;
 
   /* If converting LSDA pointers to DW_EH_PE_pcrel, there will be no need
      for run-time relocation against LSDA field.  */
   if (!sec_info->entry[mid].cie
-      && sec_info->entry[mid].cie_inf->make_lsda_relative
+      && sec_info->entry[mid].u.fde.cie_inf->make_lsda_relative
       && (offset == (sec_info->entry[mid].offset + 8
 		     + sec_info->entry[mid].lsda_offset))
-      && (sec_info->entry[mid].cie_inf->need_lsda_relative
+      && (sec_info->entry[mid].u.fde.cie_inf->need_lsda_relative
 	  || !hdr_info->offsets_adjusted))
     {
-      sec_info->entry[mid].cie_inf->need_lsda_relative = 1;
+      sec_info->entry[mid].u.fde.cie_inf->need_lsda_relative = 1;
       return (bfd_vma) -2;
     }
 
@@ -1138,7 +1139,7 @@ _bfd_elf_eh_frame_section_offset (bfd *o
   if (sec_info->entry[mid].set_loc
       && (sec_info->entry[mid].cie
 	  ? sec_info->entry[mid].make_relative
-	  : sec_info->entry[mid].cie_inf->make_relative)
+	  : sec_info->entry[mid].u.fde.cie_inf->make_relative)
       && (offset >= sec_info->entry[mid].offset + 8
 		    + sec_info->entry[mid].set_loc[1]))
     {
@@ -1375,10 +1376,12 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 	  bfd_vma value, address;
 	  unsigned int width;
 	  bfd_byte *start;
+	  struct eh_cie_fde *cie;
 
 	  /* Skip length.  */
+	  cie = ent->u.fde.cie_inf;
 	  buf += 4;
-	  value = ent->new_offset + 4 - ent->cie_inf->new_offset;
+	  value = ent->new_offset + 4 - cie->new_offset;
 	  bfd_put_32 (abfd, value, buf);
 	  buf += 4;
 	  width = get_DW_EH_PE_width (ent->fde_encoding, ptr_size);
@@ -1406,7 +1409,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 		  address += sec->output_section->vma + ent->offset + 8;
 		  break;
 		}
-	      if (ent->cie_inf->make_relative)
+	      if (cie->make_relative)
 		value -= sec->output_section->vma + ent->new_offset + 8;
 	      write_value (abfd, buf, value, width);
 	    }
@@ -1421,7 +1424,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 	    }
 
 	  if ((ent->lsda_encoding & 0xf0) == DW_EH_PE_pcrel
-	      || ent->cie_inf->need_lsda_relative)
+	      || cie->need_lsda_relative)
 	    {
 	      buf += ent->lsda_offset;
 	      width = get_DW_EH_PE_width (ent->lsda_encoding, ptr_size);
@@ -1431,13 +1434,13 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 		{
 		  if ((ent->lsda_encoding & 0xf0) == DW_EH_PE_pcrel)
 		    value += ent->offset - ent->new_offset;
-		  else if (ent->cie_inf->need_lsda_relative)
+		  else if (cie->need_lsda_relative)
 		    value -= (sec->output_section->vma + ent->new_offset + 8
 			      + ent->lsda_offset);
 		  write_value (abfd, buf, value, width);
 		}
 	    }
-	  else if (ent->cie_inf->add_augmentation_size)
+	  else if (cie->add_augmentation_size)
 	    {
 	      /* Skip the PC and length and insert a zero byte for the
 		 augmentation size.  */
@@ -1469,7 +1472,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 
 		  if ((ent->fde_encoding & 0xf0) == DW_EH_PE_pcrel)
 		    value += ent->offset + 8 - new_offset;
-		  if (ent->cie_inf->make_relative)
+		  if (cie->make_relative)
 		    value -= sec->output_section->vma + new_offset
 			     + ent->set_loc[cnt];
 		  write_value (abfd, buf, value, width);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/7] Fix --gc-sections for C++ MIPS ELF
  2007-12-02 20:15 Fix --gc-sections for C++ MIPS ELF Richard Sandiford
  2007-12-02 20:18 ` [PATCH 1/7] " Richard Sandiford
  2007-12-02 20:19 ` [PATCH 2/7] " Richard Sandiford
@ 2007-12-02 20:28 ` Richard Sandiford
  2007-12-02 20:31 ` [PATCH 4/7] " Richard Sandiford
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2007-12-02 20:28 UTC (permalink / raw)
  To: binutils

I imagine this might be the most controversial of the changes.
_bfd_elf_get_eh_frame_sec_info currently has three loops:

  - The main parsing loop.  It grows memory as needed and stores
    stores CIE offsets rather than CIE pointers in the FDEs' cie_inf
    fields.  It decides on the fly whether to keep an FDE or not.

  - A pass over the CIEs.  If a CIE is needed by at least one
    to-be-kept FDE, the pass tries to merge the CIE with one
    that is already in the global pool, adding a new CIE to the
    pool on failure.  The pass also copies some information
    from the temporary CIE structure to the eh_cie_fde.

  - A loop over the to-be-kept entries that assigns final offsets.
    It also points each to-be-kept FDE at its final CIE.

If we're going to parse .eh_frame earlier than we do now, we can't
always discard FDEs and CIEs on the fly; we need a separate pass
that lives _outside_ the parsing routines.  And that in turn means
that the parsing routines don't know for sure whether a CIE is
needed or not.  We could get around this by keeping the extended
CIE information around and leaving the discard routines to merge CIEs,
but I think it would be better to merge all CIEs upfront.  Both approaches
should give the same results: it's just a bookkeeping issue.

Also, later patches need to construct chains of eh_cie_fde entries,
so it would be better if we didn't dynamically grow the sec_info
structure in the main parsing loop.

This patch splits out the parsing code into a separate function
with two loops:

  - A simple pass over the section contents to count the CIEs and FDEs.
    Once we know how many there are, we can allocate the necessary
    memory upfront.

  - The main parsing loop.  It adds all CIEs to the global pool,
    if we're using such a pool, and uses a new eh_cie_fde u.cie.merged
    field to point to the representative of each CIE group.  (We don't
    know at this stage whether we'll keep that representative or not.)
    It points each FDE's u.fde.cie_inf field at the _local_ CIE,
    so that later patches can use the same reloc cookie to mark
    CIEs and FDEs; the u.cie.merged field then points to the CIE's
    group representative.

The EH discard routine then uses the cached section information.
It also has two loops:

  - A pass over the FDEs to decide which we need to keep.

  - A loop over the to-be-kept entries that assigns final offsets.

In the worst case, that's 4 O(n) loops rather than 3.  However,
the second of the original three loops only looked at CIEs, so it was
probably less expensive than the other 6 loops in the average case.
From that point of view, this patch could slow things down.

FWIW, the final patch in the series gets rid of offsets_adjusted,
thus saving a pass over the .eh_frames there.  I don't know if that's
enough mitigation.

The CIE marking code might look a little more convoluted than necessary.
The idea is to make sure that we only look at u.cie.merged when
removed == 1, which leads indirectly to the offsets_adjusted patch.

The u.cie.merged comment gets better in a later patch (or so I hope).

Richard


bfd/
	* elf-bfd.h (eh_cie_fde): Add u.cie.  Document how u.fde.cie_inf
	changes when removed == 0.
	(eh_frame_hdr_info): Add parsed_eh_frames.
	(_bfd_elf_begin_eh_frame_parsing): Declare.
	(_bfd_elf_parse_eh_frame): Declare.
	(_bfd_elf_end_eh_frame_parsing): Declare.
	* elf-eh-frame.c (_bfd_elf_begin_eh_frame_parsing): New function.
	(_bfd_elf_parse_eh_frame): New function, split out from
	_bfd_elf_discard_section_eh_frame.  Make a first pass through the
	buffer to calculate the number of entries and CIEs.  Allocate memory
	for them before the main loop.  Replace current extended cie
	representation with a pair of pointers, one to the local eh_cie_fde
	and one to the full struct cie.  Allocate a separate array of struct
	cies if not merging.  Merge CIEs during the main loop and set up each
	u.cie.merged field.  Point an FDE's cie_inf field directly at the
	local CIE.  Initially assume that all entries should be removed.
	(_bfd_elf_end_eh_frame_parsing): New function.
	(_bfd_elf_discard_section_eh_frame): Assume that the section has
	already been parsed.  Use a separate pass to mark entries that
	need to be kept.  Use the u.cie.merged field to track a CIE's
	group representative.
	* elflink.c (bfd_elf_discard_info): Call _bfd_elf_parse_eh_frame
	before _bfd_elf_discard_section_eh_frame.  Wrap loop with calls to
	_bfd_elf_begin_eh_frame_parsing and _bfd_elf_end_eh_frame_parsing.

Index: bfd/elf-bfd.h
===================================================================
*** bfd/elf-bfd.h	2007-12-02 17:01:08.000000000 +0000
--- bfd/elf-bfd.h	2007-12-02 17:21:33.000000000 +0000
*************** struct eh_cie_fde
*** 266,274 ****
  {
    union {
      struct {
!       /* The CIE that we have chosen to use for this FDE.  */
        struct eh_cie_fde *cie_inf;
      } fde;
    } u;
    unsigned int reloc_index;
    unsigned int size;
--- 266,287 ----
  {
    union {
      struct {
!       /* If REMOVED == 1, this is the CIE that the FDE originally used.
! 	 The CIE belongs to the same .eh_frame input section as the FDE.
! 
! 	 If REMOVED == 0, this is the CIE that we have chosen to use for
! 	 the output FDE.  The CIE's REMOVED field is also 0, but the CIE
! 	 might belong to a different .eh_frame input section from the FDE.  */
        struct eh_cie_fde *cie_inf;
      } fde;
+     struct {
+       /* In general, equivalent CIEs are grouped together, with one CIE
+ 	 representing all the others in a group.  If REMOVED == 0,
+ 	 this CIE is the group representative.  If REMOVED == 1,
+ 	 following this pointer brings us "closer" to the CIE's group
+ 	 representative, and reapplying always gives the representative.  */
+       struct eh_cie_fde *merged;
+     } cie;
    } u;
    unsigned int reloc_index;
    unsigned int size;
*************** struct eh_frame_hdr_info
*** 308,313 ****
--- 321,328 ----
    asection *hdr_sec;
    unsigned int fde_count, array_count;
    struct eh_frame_array_ent *array;
+   /* TRUE if all .eh_frames have been parsd.  */
+   bfd_boolean parsed_eh_frames;
    /* TRUE if .eh_frame_hdr should contain the sorted search table.
       We build it if we successfully read all .eh_frame input sections
       and recognize them.  */
*************** #define _bfd_elf_minisymbol_to_symbol _b
*** 1723,1728 ****
--- 1738,1750 ----
  extern void _bfd_elf_strtab_finalize
    (struct elf_strtab_hash *);
  
+ extern void _bfd_elf_begin_eh_frame_parsing
+   (struct bfd_link_info *info);
+ extern void _bfd_elf_parse_eh_frame
+   (bfd *, struct bfd_link_info *, asection *, struct elf_reloc_cookie *);
+ extern void _bfd_elf_end_eh_frame_parsing
+   (struct bfd_link_info *info);
+ 
  extern bfd_boolean _bfd_elf_discard_section_eh_frame
    (bfd *, struct bfd_link_info *, asection *,
     bfd_boolean (*) (bfd_vma, void *), struct elf_reloc_cookie *);
Index: bfd/elf-eh-frame.c
===================================================================
*** bfd/elf-eh-frame.c	2007-12-02 17:01:08.000000000 +0000
--- bfd/elf-eh-frame.c	2007-12-02 17:03:27.000000000 +0000
*************** skip_non_nops (bfd_byte *buf, bfd_byte *
*** 431,446 ****
    return last;
  }
  
! /* This function is called for each input file before the .eh_frame
!    section is relocated.  It discards duplicate CIEs and FDEs for discarded
!    functions.  The function returns TRUE iff any entries have been
!    deleted.  */
  
! bfd_boolean
! _bfd_elf_discard_section_eh_frame
!    (bfd *abfd, struct bfd_link_info *info, asection *sec,
!     bfd_boolean (*reloc_symbol_deleted_p) (bfd_vma, void *),
!     struct elf_reloc_cookie *cookie)
  {
  #define REQUIRE(COND)					\
    do							\
--- 431,456 ----
    return last;
  }
  
! /* Called before calling _bfd_elf_parse_eh_frame on every input bfd's
!    .eh_frame section.  */
  
! void
! _bfd_elf_begin_eh_frame_parsing (struct bfd_link_info *info)
! {
!   struct eh_frame_hdr_info *hdr_info;
! 
!   hdr_info = &elf_hash_table (info)->eh_info;
!   if (!hdr_info->parsed_eh_frames && !info->relocatable)
!     hdr_info->cies = htab_try_create (1, cie_hash, cie_eq, free);
! }
! 
! /* Try to parse .eh_frame section SEC, which belongs to ABFD.  Store the
!    information in the section's sec_info field on success.  COOKIE
!    describes the relocations in SEC.  */
! 
! void
! _bfd_elf_parse_eh_frame (bfd *abfd, struct bfd_link_info *info,
! 			 asection *sec, struct elf_reloc_cookie *cookie)
  {
  #define REQUIRE(COND)					\
    do							\
*************** #define REQUIRE(COND)					\
*** 448,492 ****
        goto free_no_table;				\
    while (0)
  
!   bfd_byte *ehbuf = NULL, *buf;
    bfd_byte *last_fde;
!   struct eh_cie_fde *ent, *this_inf;
    unsigned int hdr_length, hdr_id;
    struct extended_cie
      {
!       struct cie cie;
!       unsigned int offset;
!       unsigned int usage_count;
!       unsigned int entry;
      } *ecies = NULL, *ecie;
!   unsigned int ecie_count = 0, ecie_alloced = 0;
!   struct cie *cie;
    struct elf_link_hash_table *htab;
    struct eh_frame_hdr_info *hdr_info;
    struct eh_frame_sec_info *sec_info = NULL;
-   unsigned int offset;
    unsigned int ptr_size;
!   unsigned int entry_alloced;
  
    if (sec->size == 0)
      {
        /* This file does not contain .eh_frame information.  */
!       return FALSE;
      }
  
    if (bfd_is_abs_section (sec->output_section))
      {
        /* At least one of the sections is being discarded from the
  	 link, so we should just ignore them.  */
!       return FALSE;
      }
  
-   htab = elf_hash_table (info);
-   hdr_info = &htab->eh_info;
- 
-   if (hdr_info->cies == NULL && !info->relocatable)
-     hdr_info->cies = htab_try_create (1, cie_hash, cie_eq, free);
- 
    /* Read the frame unwind information from abfd.  */
  
    REQUIRE (bfd_malloc_and_get_section (abfd, sec, &ehbuf));
--- 458,499 ----
        goto free_no_table;				\
    while (0)
  
!   bfd_byte *ehbuf = NULL, *buf, *end;
    bfd_byte *last_fde;
!   struct eh_cie_fde *this_inf;
    unsigned int hdr_length, hdr_id;
    struct extended_cie
      {
!       struct cie *cie;
!       struct eh_cie_fde *local_cie;
      } *ecies = NULL, *ecie;
!   unsigned int ecie_count;
!   struct cie *cie, *local_cies = NULL, tmp_cie;
    struct elf_link_hash_table *htab;
    struct eh_frame_hdr_info *hdr_info;
    struct eh_frame_sec_info *sec_info = NULL;
    unsigned int ptr_size;
!   unsigned int num_cies;
!   unsigned int num_entries;
! 
!   htab = elf_hash_table (info);
!   hdr_info = &htab->eh_info;
!   if (hdr_info->parsed_eh_frames)
!     return;
  
    if (sec->size == 0)
      {
        /* This file does not contain .eh_frame information.  */
!       return;
      }
  
    if (bfd_is_abs_section (sec->output_section))
      {
        /* At least one of the sections is being discarded from the
  	 link, so we should just ignore them.  */
!       return;
      }
  
    /* Read the frame unwind information from abfd.  */
  
    REQUIRE (bfd_malloc_and_get_section (abfd, sec, &ehbuf));
*************** #define REQUIRE(COND)					\
*** 497,503 ****
      {
        /* Empty .eh_frame section.  */
        free (ehbuf);
!       return FALSE;
      }
  
    /* If .eh_frame section size doesn't fit into int, we cannot handle
--- 504,510 ----
      {
        /* Empty .eh_frame section.  */
        free (ehbuf);
!       return;
      }
  
    /* If .eh_frame section size doesn't fit into int, we cannot handle
*************** #define REQUIRE(COND)					\
*** 508,519 ****
  	      ->elf_backend_eh_frame_address_size (abfd, sec));
    REQUIRE (ptr_size != 0);
  
    buf = ehbuf;
    sec_info = bfd_zmalloc (sizeof (struct eh_frame_sec_info)
! 			  + 99 * sizeof (struct eh_cie_fde));
    REQUIRE (sec_info);
  
!   entry_alloced = 100;
  
  #define ENSURE_NO_RELOCS(buf)				\
    REQUIRE (!(cookie->rel < cookie->relend		\
--- 515,561 ----
  	      ->elf_backend_eh_frame_address_size (abfd, sec));
    REQUIRE (ptr_size != 0);
  
+   /* Go through the section contents and work out how many FDEs and
+      CIEs there are.  */
    buf = ehbuf;
+   end = ehbuf + sec->size;
+   num_cies = 0;
+   num_entries = 0;
+   while (buf != end)
+     {
+       num_entries++;
+ 
+       /* Read the length of the entry.  */
+       REQUIRE (skip_bytes (&buf, end, 4));
+       hdr_length = bfd_get_32 (abfd, buf - 4);
+ 
+       /* 64-bit .eh_frame is not supported.  */
+       REQUIRE (hdr_length != 0xffffffff);
+       if (hdr_length == 0)
+ 	break;
+ 
+       REQUIRE (skip_bytes (&buf, end, 4));
+       hdr_id = bfd_get_32 (abfd, buf - 4);
+       if (hdr_id == 0)
+ 	num_cies++;
+ 
+       REQUIRE (skip_bytes (&buf, end, hdr_length - 4));
+     }
+ 
    sec_info = bfd_zmalloc (sizeof (struct eh_frame_sec_info)
! 			  + (num_entries - 1) * sizeof (struct eh_cie_fde));
    REQUIRE (sec_info);
  
!   ecies = bfd_zmalloc (num_cies * sizeof (*ecies));
!   REQUIRE (ecies);
! 
!   /* If we're not merging CIE entries (such as for a relocatable link),
!      we need to have a "struct cie" for each CIE in this section.  */
!   if (hdr_info->cies == NULL)
!     {
!       local_cies = bfd_zmalloc (num_cies * sizeof (*local_cies));
!       REQUIRE (local_cies);
!     }
  
  #define ENSURE_NO_RELOCS(buf)				\
    REQUIRE (!(cookie->rel < cookie->relend		\
*************** #define GET_RELOC(buf)					\
*** 533,571 ****
  	== (bfd_size_type) ((buf) - ehbuf)))		\
     ? cookie->rel : NULL)
  
!   for (;;)
      {
        char *aug;
!       bfd_byte *start, *end, *insns, *insns_end;
        bfd_size_type length;
        unsigned int set_loc_count;
  
-       if (sec_info->count == entry_alloced)
- 	{
- 	  sec_info = bfd_realloc (sec_info,
- 				  sizeof (struct eh_frame_sec_info)
- 				  + ((entry_alloced + 99)
- 				     * sizeof (struct eh_cie_fde)));
- 	  REQUIRE (sec_info);
- 
- 	  memset (&sec_info->entry[entry_alloced], 0,
- 		  100 * sizeof (struct eh_cie_fde));
- 	  entry_alloced += 100;
- 	}
- 
        this_inf = sec_info->entry + sec_info->count;
        last_fde = buf;
  
-       if ((bfd_size_type) (buf - ehbuf) == sec->size)
- 	break;
- 
        /* Read the length of the entry.  */
        REQUIRE (skip_bytes (&buf, ehbuf + sec->size, 4));
        hdr_length = bfd_get_32 (abfd, buf - 4);
  
-       /* 64-bit .eh_frame is not supported.  */
-       REQUIRE (hdr_length != 0xffffffff);
- 
        /* The CIE/FDE must be fully contained in this input section.  */
        REQUIRE ((bfd_size_type) (buf - ehbuf) + hdr_length <= sec->size);
        end = buf + hdr_length;
--- 575,596 ----
  	== (bfd_size_type) ((buf) - ehbuf)))		\
     ? cookie->rel : NULL)
  
!   buf = ehbuf;
!   ecie_count = 0;
!   while ((bfd_size_type) (buf - ehbuf) != sec->size)
      {
        char *aug;
!       bfd_byte *start, *insns, *insns_end;
        bfd_size_type length;
        unsigned int set_loc_count;
  
        this_inf = sec_info->entry + sec_info->count;
        last_fde = buf;
  
        /* Read the length of the entry.  */
        REQUIRE (skip_bytes (&buf, ehbuf + sec->size, 4));
        hdr_length = bfd_get_32 (abfd, buf - 4);
  
        /* The CIE/FDE must be fully contained in this input section.  */
        REQUIRE ((bfd_size_type) (buf - ehbuf) + hdr_length <= sec->size);
        end = buf + hdr_length;
*************** #define GET_RELOC(buf)					\
*** 594,612 ****
  	  /* CIE  */
  	  this_inf->cie = 1;
  
! 	  if (ecie_count == ecie_alloced)
  	    {
! 	      ecies = bfd_realloc (ecies,
! 				   (ecie_alloced + 20) * sizeof (*ecies));
! 	      REQUIRE (ecies);
! 	      memset (&ecies[ecie_alloced], 0, 20 * sizeof (*ecies));
! 	      ecie_alloced += 20;
  	    }
! 
! 	  cie = &ecies[ecie_count].cie;
! 	  ecies[ecie_count].offset = this_inf->offset;
! 	  ecies[ecie_count++].entry = sec_info->count;
  	  cie->length = hdr_length;
  	  start = buf;
  	  REQUIRE (read_byte (&buf, end, &cie->version));
  
--- 619,637 ----
  	  /* CIE  */
  	  this_inf->cie = 1;
  
! 	  /* If we're merging CIEs, construct the struct cie in TMP_CIE;
! 	     we'll enter it into the global pool later.  Otherwise point
! 	     CIE to one of the section-local cie structures.  */
! 	  if (local_cies)
! 	    cie = local_cies + ecie_count;
! 	  else
  	    {
! 	      cie = &tmp_cie;
! 	      memset (cie, 0, sizeof (*cie));
  	    }
! 	  cie->cie_inf = this_inf;
  	  cie->length = hdr_length;
+ 	  cie->output_sec = sec->output_section;
  	  start = buf;
  	  REQUIRE (read_byte (&buf, end, &cie->version));
  
*************** #define GET_RELOC(buf)					\
*** 788,835 ****
  	  insns = buf;
  	  buf += initial_insn_length;
  	  ENSURE_NO_RELOCS (buf);
  	}
        else
  	{
  	  /* Find the corresponding CIE.  */
  	  unsigned int cie_offset = this_inf->offset + 4 - hdr_id;
  	  for (ecie = ecies; ecie < ecies + ecie_count; ++ecie)
! 	    if (cie_offset == ecie->offset)
  	      break;
  
  	  /* Ensure this FDE references one of the CIEs in this input
  	     section.  */
  	  REQUIRE (ecie != ecies + ecie_count);
! 	  cie = &ecie->cie;
  
  	  ENSURE_NO_RELOCS (buf);
  	  REQUIRE (GET_RELOC (buf));
  
- 	  if ((*reloc_symbol_deleted_p) (buf - ehbuf, cookie))
- 	    /* This is a FDE against a discarded section.  It should
- 	       be deleted.  */
- 	    this_inf->removed = 1;
- 	  else
- 	    {
- 	      if (info->shared
- 		  && (((cie->fde_encoding & 0xf0) == DW_EH_PE_absptr
- 		       && cie->make_relative == 0)
- 		      || (cie->fde_encoding & 0xf0) == DW_EH_PE_aligned))
- 		{
- 		  /* If a shared library uses absolute pointers
- 		     which we cannot turn into PC relative,
- 		     don't create the binary search table,
- 		     since it is affected by runtime relocations.  */
- 		  hdr_info->table = FALSE;
- 		  (*info->callbacks->einfo)
- 		    (_("%P: fde encoding in %B(%A) prevents .eh_frame_hdr"
- 		       " table being created.\n"), abfd, sec);
- 		}
- 	      ecie->usage_count++;
- 	      hdr_info->fde_count++;
- 	      this_inf->u.fde.cie_inf = (void *) (ecie - ecies);
- 	    }
- 
  	  /* Skip the initial location and address range.  */
  	  start = buf;
  	  length = get_DW_EH_PE_width (cie->fde_encoding, ptr_size);
--- 813,841 ----
  	  insns = buf;
  	  buf += initial_insn_length;
  	  ENSURE_NO_RELOCS (buf);
+ 
+ 	  this_inf->make_relative = cie->make_relative;
+ 	  this_inf->make_lsda_relative = cie->make_lsda_relative;
+ 	  this_inf->per_encoding_relative
+ 	    = (cie->per_encoding & 0x70) == DW_EH_PE_pcrel;
  	}
        else
  	{
  	  /* Find the corresponding CIE.  */
  	  unsigned int cie_offset = this_inf->offset + 4 - hdr_id;
  	  for (ecie = ecies; ecie < ecies + ecie_count; ++ecie)
! 	    if (cie_offset == ecie->local_cie->offset)
  	      break;
  
  	  /* Ensure this FDE references one of the CIEs in this input
  	     section.  */
  	  REQUIRE (ecie != ecies + ecie_count);
! 	  cie = ecie->cie;
! 	  this_inf->u.fde.cie_inf = ecie->local_cie;
  
  	  ENSURE_NO_RELOCS (buf);
  	  REQUIRE (GET_RELOC (buf));
  
  	  /* Skip the initial location and address range.  */
  	  start = buf;
  	  length = get_DW_EH_PE_width (cie->fde_encoding, ptr_size);
*************** #define GET_RELOC(buf)					\
*** 901,990 ****
  	    }
  	}
  
        this_inf->fde_encoding = cie->fde_encoding;
        this_inf->lsda_encoding = cie->lsda_encoding;
        sec_info->count++;
      }
  
    elf_section_data (sec)->sec_info = sec_info;
    sec->sec_info_type = ELF_INFO_TYPE_EH_FRAME;
  
!   /* Look at all CIEs in this section and determine which can be
!      removed as unused, which can be merged with previous duplicate
!      CIEs and which need to be kept.  */
!   for (ecie = ecies; ecie < ecies + ecie_count; ++ecie)
!     {
!       if (ecie->usage_count == 0)
! 	{
! 	  sec_info->entry[ecie->entry].removed = 1;
! 	  continue;
! 	}
!       ecie->cie.output_sec = sec->output_section;
!       ecie->cie.cie_inf = sec_info->entry + ecie->entry;
!       cie_compute_hash (&ecie->cie);
!       if (hdr_info->cies != NULL)
! 	{
! 	  void **loc = htab_find_slot_with_hash (hdr_info->cies, &ecie->cie,
! 						 ecie->cie.hash, INSERT);
! 	  if (loc != NULL)
! 	    {
! 	      if (*loc != HTAB_EMPTY_ENTRY)
! 		{
! 		  sec_info->entry[ecie->entry].removed = 1;
! 		  ecie->cie.cie_inf = ((struct cie *) *loc)->cie_inf;
! 		  continue;
! 		}
  
! 	      *loc = malloc (sizeof (struct cie));
! 	      if (*loc == NULL)
! 		*loc = HTAB_DELETED_ENTRY;
! 	      else
! 		memcpy (*loc, &ecie->cie, sizeof (struct cie));
! 	    }
! 	}
!       ecie->cie.cie_inf->make_relative = ecie->cie.make_relative;
!       ecie->cie.cie_inf->make_lsda_relative = ecie->cie.make_lsda_relative;
!       ecie->cie.cie_inf->per_encoding_relative
! 	= (ecie->cie.per_encoding & 0x70) == DW_EH_PE_pcrel;
      }
  
!   /* Ok, now we can assign new offsets.  */
!   offset = 0;
    for (ent = sec_info->entry; ent < sec_info->entry + sec_info->count; ++ent)
!     if (!ent->removed)
        {
! 	if (!ent->cie)
  	  {
! 	    ecie = ecies + (bfd_hostptr_t) ent->u.fde.cie_inf;
! 	    ent->u.fde.cie_inf = ecie->cie.cie_inf;
  	  }
  	ent->new_offset = offset;
  	offset += size_of_output_cie_fde (ent, ptr_size);
        }
  
-   /* Resize the sec as needed.  */
    sec->rawsize = sec->size;
    sec->size = offset;
- 
-   free (ehbuf);
-   if (ecies)
-     free (ecies);
    return offset != sec->rawsize;
- 
- free_no_table:
-   (*info->callbacks->einfo)
-     (_("%P: error in %B(%A); no .eh_frame_hdr table will be created.\n"),
-      abfd, sec);
-   if (ehbuf)
-     free (ehbuf);
-   if (sec_info)
-     free (sec_info);
-   if (ecies)
-     free (ecies);
-   hdr_info->table = FALSE;
-   return FALSE;
- 
- #undef REQUIRE
  }
  
  /* This function is called for .eh_frame_hdr section after
--- 907,1061 ----
  	    }
  	}
  
+       this_inf->removed = 1;
        this_inf->fde_encoding = cie->fde_encoding;
        this_inf->lsda_encoding = cie->lsda_encoding;
+       if (this_inf->cie)
+ 	{
+ 	  /* We have now finished constructing the struct cie.  */
+ 	  if (hdr_info->cies != NULL)
+ 	    {
+ 	      /* See if we can merge this CIE with an earlier one.  */
+ 	      void **loc;
+ 
+ 	      cie_compute_hash (cie);
+ 	      loc = htab_find_slot_with_hash (hdr_info->cies, cie,
+ 					      cie->hash, INSERT);
+ 	      REQUIRE (loc);
+ 	      if (*loc == HTAB_EMPTY_ENTRY)
+ 		{
+ 		  *loc = malloc (sizeof (struct cie));
+ 		  REQUIRE (*loc);
+ 		  memcpy (*loc, cie, sizeof (struct cie));
+ 		}
+ 	      cie = (struct cie *) *loc;
+ 	    }
+ 	  this_inf->u.cie.merged = cie->cie_inf;
+ 	  ecies[ecie_count].cie = cie;
+ 	  ecies[ecie_count++].local_cie = this_inf;
+ 	}
        sec_info->count++;
      }
+   BFD_ASSERT (sec_info->count == num_entries);
+   BFD_ASSERT (ecie_count == num_cies);
  
    elf_section_data (sec)->sec_info = sec_info;
    sec->sec_info_type = ELF_INFO_TYPE_EH_FRAME;
+   goto success;
  
!  free_no_table:
!   (*info->callbacks->einfo)
!     (_("%P: error in %B(%A); no .eh_frame_hdr table will be created.\n"),
!      abfd, sec);
!   hdr_info->table = FALSE;
!   if (sec_info)
!     free (sec_info);
!  success:
!   if (ehbuf)
!     free (ehbuf);
!   if (ecies)
!     free (ecies);
!   if (local_cies)
!     free (local_cies);
! #undef REQUIRE
! }
  
! /* Finish a pass over all .eh_frame sections.  */
! 
! void
! _bfd_elf_end_eh_frame_parsing (struct bfd_link_info *info)
! {
!   struct eh_frame_hdr_info *hdr_info;
! 
!   hdr_info = &elf_hash_table (info)->eh_info;
!   if (hdr_info->cies != NULL)
!     {
!       htab_delete (hdr_info->cies);
!       hdr_info->cies = NULL;
      }
+   hdr_info->parsed_eh_frames = TRUE;
+ }
  
! /* This function is called for each input file before the .eh_frame
!    section is relocated.  It discards duplicate CIEs and FDEs for discarded
!    functions.  The function returns TRUE iff any entries have been
!    deleted.  */
! 
! bfd_boolean
! _bfd_elf_discard_section_eh_frame
!    (bfd *abfd, struct bfd_link_info *info, asection *sec,
!     bfd_boolean (*reloc_symbol_deleted_p) (bfd_vma, void *),
!     struct elf_reloc_cookie *cookie)
! {
!   struct eh_cie_fde *ent, *cie, *merged;
!   struct eh_frame_sec_info *sec_info;
!   struct eh_frame_hdr_info *hdr_info;
!   unsigned int ptr_size, offset;
! 
!   sec_info = (struct eh_frame_sec_info *) elf_section_data (sec)->sec_info;
!   if (sec_info == NULL)
!     return FALSE;
! 
!   hdr_info = &elf_hash_table (info)->eh_info;
    for (ent = sec_info->entry; ent < sec_info->entry + sec_info->count; ++ent)
!     if (!ent->cie)
        {
! 	cookie->rel = cookie->rels + ent->reloc_index;
! 	BFD_ASSERT (cookie->rel < cookie->relend
! 		    && cookie->rel->r_offset == ent->offset + 8);
! 	if (!(*reloc_symbol_deleted_p) (ent->offset + 8, cookie))
  	  {
! 	    if (info->shared
! 		&& (((ent->fde_encoding & 0xf0) == DW_EH_PE_absptr
! 		     && ent->u.fde.cie_inf->make_relative == 0)
! 		    || (ent->fde_encoding & 0xf0) == DW_EH_PE_aligned))
! 	      {
! 		/* If a shared library uses absolute pointers
! 		   which we cannot turn into PC relative,
! 		   don't create the binary search table,
! 		   since it is affected by runtime relocations.  */
! 		hdr_info->table = FALSE;
! 		(*info->callbacks->einfo)
! 		  (_("%P: fde encoding in %B(%A) prevents .eh_frame_hdr"
! 		     " table being created.\n"), abfd, sec);
! 	      }
! 	    ent->removed = 0;
! 	    hdr_info->fde_count++;
! 
! 	    cie = ent->u.fde.cie_inf;
! 	    if (cie->removed)
! 	      {
! 		merged = cie->u.cie.merged;
! 		if (!merged->removed)
! 		  /* We have decided to keep the group representative.  */
! 		  ent->u.fde.cie_inf = merged;
! 		else if (merged->u.cie.merged != merged)
! 		  /* We didn't keep the original group representative,
! 		     but we did keep an alternative.  */
! 		  ent->u.fde.cie_inf = merged->u.cie.merged;
! 		else
! 		  {
! 		    /* Make the local CIE represent the merged group.  */
! 		    merged->u.cie.merged = cie;
! 		    cie->removed = 0;
! 		  }
! 	      }
  	  }
+       }
+ 
+   ptr_size = (get_elf_backend_data (sec->owner)
+ 	      ->elf_backend_eh_frame_address_size (sec->owner, sec));
+   offset = 0;
+   for (ent = sec_info->entry; ent < sec_info->entry + sec_info->count; ++ent)
+     if (!ent->removed)
+       {
  	ent->new_offset = offset;
  	offset += size_of_output_cie_fde (ent, ptr_size);
        }
  
    sec->rawsize = sec->size;
    sec->size = offset;
    return offset != sec->rawsize;
  }
  
  /* This function is called for .eh_frame_hdr section after
*************** _bfd_elf_discard_section_eh_frame_hdr (b
*** 1001,1012 ****
    htab = elf_hash_table (info);
    hdr_info = &htab->eh_info;
  
-   if (hdr_info->cies != NULL)
-     {
-       htab_delete (hdr_info->cies);
-       hdr_info->cies = NULL;
-     }
- 
    sec = hdr_info->hdr_sec;
    if (sec == NULL)
      return FALSE;
--- 1072,1077 ----
Index: bfd/elflink.c
===================================================================
*** bfd/elflink.c	2007-12-02 17:01:03.000000000 +0000
--- bfd/elflink.c	2007-12-02 17:03:27.000000000 +0000
*************** bfd_elf_discard_info (bfd *output_bfd, s
*** 11894,11899 ****
--- 11894,11900 ----
        || !is_elf_hash_table (info->hash))
      return FALSE;
  
+   _bfd_elf_begin_eh_frame_parsing (info);
    for (abfd = info->input_bfds; abfd != NULL; abfd = abfd->link_next)
      {
        if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)
*************** bfd_elf_discard_info (bfd *output_bfd, s
*** 11944,11949 ****
--- 11945,11951 ----
        if (eh != NULL
  	  && init_reloc_cookie_rels (&cookie, info, abfd, eh))
  	{
+ 	  _bfd_elf_parse_eh_frame (abfd, info, eh, &cookie);
  	  if (_bfd_elf_discard_section_eh_frame (abfd, info, eh,
  						 bfd_elf_reloc_symbol_deleted_p,
  						 &cookie))
*************** bfd_elf_discard_info (bfd *output_bfd, s
*** 11957,11962 ****
--- 11959,11965 ----
  
        fini_reloc_cookie (&cookie, abfd);
      }
+   _bfd_elf_end_eh_frame_parsing (info);
  
    if (info->eh_frame_hdr
        && !info->relocatable

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/7] Fix --gc-sections for C++ MIPS ELF
  2007-12-02 20:15 Fix --gc-sections for C++ MIPS ELF Richard Sandiford
                   ` (2 preceding siblings ...)
  2007-12-02 20:28 ` [PATCH 3/7] " Richard Sandiford
@ 2007-12-02 20:31 ` Richard Sandiford
  2007-12-02 20:35 ` [PATCH 5/7] " Richard Sandiford
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2007-12-02 20:31 UTC (permalink / raw)
  To: binutils

This is the main patch.  The core routine is mark_entry, which "marks
all relocations" against a CIE or FDE ("marking a relocation" being
shorthand for "marking the section in which a relocation's symbol
is defined").

The parsing routines now chain together the FDEs for each code section
into a list, and a new routine called _bfd_elf_gc_mark_fdes marks all
the FDEs for a particular code section.  CIEs are marked when one
their dependent FDEs is marked.

FDEs are only marked once, when their associated section is marked,
so there's no need for them to have a separate gc_mark field.  However,
CIEs might be processed many times, so a gc_mark field does seem
appropriate there.

bfd_elf_gc_sections tries to parse each input .eh_frame section.
It stores the .eh_frame section in a new ELF bfd field called
"eh_frame_section" on success, otherwise the .eh_frame section
is marked like any other section would be.

If eh_frame_section is nonnull, _bfd_elf_gc_mark marks all the FDEs
associated with the section it is marking.  It doesn't process the
relocations in eh_frame_section itself.

Richard


bfd/
	* elf-bfd.h (eh_cie_fde): Add u.fde.next_for_section and
	u.cie.gc_mark.
	(bfd_elf_section_data): Add fde_list.
	(elf_fde_list): New macro.
	(elf_obj_tdata): Add eh_frame_section.
	(elf_eh_frame_section): New macro.
	(_bfd_elf_gc_mark_reloc): Remove last parameter.
	(_bfd_elf_gc_mark_fdes): Declare.
	* elf-eh-frame.c (_bfd_elf_get_eh_frame_sec_info): Chain the FDEs
	for each input section.
	(mark_entry, _bfd_elf_gc_mark_fdes): New functions.
	* elflink.c (_bfd_elf_gc_mark_reloc): Remove is_eh parameter.
	(_bfd_elf_gc_mark): Update call accordingly.  Mark the relocations
	againts the section's FDEs.  Don't mark the bfd's elf_eh_frame_section.
	(bfd_elf_gc_sections): Parse each input bfd's .eh_frame before
	marking any input sections.  Remove the current EH handling.
	* section.c (bfd_section): Remove gc_mark_from_eh.
	* ecoff.c (bfd_debug_section): Update initializer accordingly.

Index: bfd/elf-bfd.h
===================================================================
--- bfd/elf-bfd.h	2007-12-02 17:21:33.000000000 +0000
+++ bfd/elf-bfd.h	2007-12-02 17:22:22.000000000 +0000
@@ -273,6 +273,7 @@ struct eh_cie_fde
 	 the output FDE.  The CIE's REMOVED field is also 0, but the CIE
 	 might belong to a different .eh_frame input section from the FDE.  */
       struct eh_cie_fde *cie_inf;
+      struct eh_cie_fde *next_for_section;
     } fde;
     struct {
       /* In general, equivalent CIEs are grouped together, with one CIE
@@ -281,6 +282,9 @@ struct eh_cie_fde
 	 following this pointer brings us "closer" to the CIE's group
 	 representative, and reapplying always gives the representative.  */
       struct eh_cie_fde *merged;
+
+      /* True if we have marked relocations associated with this CIE.  */
+      unsigned int gc_mark : 1;
     } cie;
   } u;
   unsigned int reloc_index;
@@ -1243,6 +1247,10 @@ struct bfd_elf_section_data
      the linker.  For the SHT_GROUP section, points at first member.  */
   asection *next_in_group;
 
+  /* The FDEs associated with this section.  The u.fde.next_in_section
+     field acts as a chain pointer.  */
+  struct eh_cie_fde *fde_list;
+
   /* A pointer used for various section optimizations.  */
   void *sec_info;
 };
@@ -1254,6 +1262,7 @@ #define elf_section_flags(sec) (elf_sect
 #define elf_group_name(sec)    (elf_section_data(sec)->group.name)
 #define elf_group_id(sec)      (elf_section_data(sec)->group.id)
 #define elf_next_in_group(sec) (elf_section_data(sec)->next_in_group)
+#define elf_fde_list(sec)      (elf_section_data(sec)->fde_list)
 #define elf_sec_group(sec)	(elf_section_data(sec)->sec_group)
 
 #define xvec_get_elf_backend_data(xvec) \
@@ -1459,6 +1468,9 @@ struct elf_obj_tdata
   asection *elf_data_section;
   asection *elf_text_section;
 
+  /* A pointer to the .eh_frame section.  */
+  asection *eh_frame_section;
+
   /* Whether a dyanmic object was specified normally on the linker
      command line, or was specified when --as-needed was in effect,
      or was found via a DT_NEEDED entry.  */
@@ -1504,6 +1516,8 @@ #define elf_dynsymtab(bfd)	(elf_tdata(bf
 #define elf_dynversym(bfd)	(elf_tdata(bfd) -> dynversym_section)
 #define elf_dynverdef(bfd)	(elf_tdata(bfd) -> dynverdef_section)
 #define elf_dynverref(bfd)	(elf_tdata(bfd) -> dynverref_section)
+#define elf_eh_frame_section(bfd) \
+				(elf_tdata(bfd) -> eh_frame_section)
 #define elf_num_locals(bfd)	(elf_tdata(bfd) -> num_locals)
 #define elf_num_globals(bfd)	(elf_tdata(bfd) -> num_globals)
 #define elf_section_syms(bfd)	(elf_tdata(bfd) -> section_syms)
@@ -2004,7 +2018,11 @@ extern void _bfd_elf_set_osabi (bfd * , 
 
 extern bfd_boolean _bfd_elf_gc_mark_reloc
   (struct bfd_link_info *, asection *, elf_gc_mark_hook_fn,
-   struct elf_reloc_cookie *, bfd_boolean);
+   struct elf_reloc_cookie *);
+
+extern bfd_boolean _bfd_elf_gc_mark_fdes
+  (struct bfd_link_info *, asection *, asection *, elf_gc_mark_hook_fn,
+   struct elf_reloc_cookie *);
 
 extern bfd_boolean _bfd_elf_gc_mark
   (struct bfd_link_info *, asection *, elf_gc_mark_hook_fn);
Index: bfd/elf-eh-frame.c
===================================================================
--- bfd/elf-eh-frame.c	2007-12-02 17:03:27.000000000 +0000
+++ bfd/elf-eh-frame.c	2007-12-02 17:26:13.000000000 +0000
@@ -475,6 +475,7 @@ #define REQUIRE(COND)					\
   unsigned int ptr_size;
   unsigned int num_cies;
   unsigned int num_entries;
+  elf_gc_mark_hook_fn gc_mark_hook;
 
   htab = elf_hash_table (info);
   hdr_info = &htab->eh_info;
@@ -577,6 +578,7 @@ #define GET_RELOC(buf)					\
 
   buf = ehbuf;
   ecie_count = 0;
+  gc_mark_hook = get_elf_backend_data (abfd)->gc_mark_hook;
   while ((bfd_size_type) (buf - ehbuf) != sec->size)
     {
       char *aug;
@@ -821,6 +823,8 @@ #define GET_RELOC(buf)					\
 	}
       else
 	{
+	  asection *rsec;
+
 	  /* Find the corresponding CIE.  */
 	  unsigned int cie_offset = this_inf->offset + 4 - hdr_id;
 	  for (ecie = ecies; ecie < ecies + ecie_count; ++ecie)
@@ -836,6 +840,12 @@ #define GET_RELOC(buf)					\
 	  ENSURE_NO_RELOCS (buf);
 	  REQUIRE (GET_RELOC (buf));
 
+	  /* Chain together the FDEs for each section.  */
+	  rsec = _bfd_elf_gc_mark_rsec (info, sec, gc_mark_hook, cookie);
+	  REQUIRE (rsec && rsec->owner == abfd);
+	  this_inf->u.fde.next_for_section = elf_fde_list (rsec);
+	  elf_fde_list (rsec) = this_inf;
+
 	  /* Skip the initial location and address range.  */
 	  start = buf;
 	  length = get_DW_EH_PE_width (cie->fde_encoding, ptr_size);
@@ -976,6 +986,55 @@ _bfd_elf_end_eh_frame_parsing (struct bf
   hdr_info->parsed_eh_frames = TRUE;
 }
 
+/* Mark all relocations against CIE or FDE ENT, which occurs in
+   .eh_frame section SEC.  COOKIE describes the relocations in SEC;
+   its "rel" field can be changed freely.  */
+
+static bfd_boolean
+mark_entry (struct bfd_link_info *info, asection *sec,
+	    struct eh_cie_fde *ent, elf_gc_mark_hook_fn gc_mark_hook,
+	    struct elf_reloc_cookie *cookie)
+{
+  for (cookie->rel = cookie->rels + ent->reloc_index;
+       cookie->rel < cookie->relend
+	 && cookie->rel->r_offset < ent->offset + ent->size;
+       cookie->rel++)
+    if (!_bfd_elf_gc_mark_reloc (info, sec, gc_mark_hook, cookie))
+      return FALSE;
+
+  return TRUE;
+}
+
+/* Mark all the relocations against FDEs that relate to code in input
+   section SEC.  The FDEs belong to .eh_frame section EH_FRAME, whose
+   relocations are described by COOKIE.  */
+
+bfd_boolean
+_bfd_elf_gc_mark_fdes (struct bfd_link_info *info, asection *sec,
+		       asection *eh_frame, elf_gc_mark_hook_fn gc_mark_hook,
+		       struct elf_reloc_cookie *cookie)
+{
+  struct eh_cie_fde *fde, *cie, *merged;
+
+  for (fde = elf_fde_list (sec); fde; fde = fde->u.fde.next_for_section)
+    {
+      if (!mark_entry (info, eh_frame, fde, gc_mark_hook, cookie))
+	return FALSE;
+
+      /* At this stage, all cie_inf fields point to local CIEs, so we
+	 can use the same cookie to refer to them.  */
+      cie = fde->u.fde.cie_inf;
+      merged = cie->u.cie.merged;
+      if (!merged->u.cie.gc_mark)
+	{
+	  merged->u.cie.gc_mark = 1;
+	  if (!mark_entry (info, eh_frame, cie, gc_mark_hook, cookie))
+	    return FALSE;
+	}
+    }
+  return TRUE;
+}
+
 /* This function is called for each input file before the .eh_frame
    section is relocated.  It discards duplicate CIEs and FDEs for discarded
    functions.  The function returns TRUE iff any entries have been
Index: bfd/elflink.c
===================================================================
--- bfd/elflink.c	2007-12-02 17:03:27.000000000 +0000
+++ bfd/elflink.c	2007-12-02 17:22:00.000000000 +0000
@@ -11137,15 +11137,13 @@ _bfd_elf_gc_mark_rsec (struct bfd_link_i
 
 /* COOKIE->rel describes a relocation against section SEC, which is
    a section we've decided to keep.  Mark the section that contains
-   the relocation symbol.  IS_EH is true if the mark comes from
-   .eh_frame.  */
+   the relocation symbol.  */
 
 bfd_boolean
 _bfd_elf_gc_mark_reloc (struct bfd_link_info *info,
 			asection *sec,
 			elf_gc_mark_hook_fn gc_mark_hook,
-			struct elf_reloc_cookie *cookie,
-			bfd_boolean is_eh)
+			struct elf_reloc_cookie *cookie)
 {
   asection *rsec;
 
@@ -11154,8 +11152,6 @@ _bfd_elf_gc_mark_reloc (struct bfd_link_
     {
       if (bfd_get_flavour (rsec->owner) != bfd_target_elf_flavour)
 	rsec->gc_mark = 1;
-      else if (is_eh)
-	rsec->gc_mark_from_eh = 1;
       else if (!_bfd_elf_gc_mark (info, rsec, gc_mark_hook))
 	return FALSE;
     }
@@ -11172,8 +11168,7 @@ _bfd_elf_gc_mark (struct bfd_link_info *
 		  elf_gc_mark_hook_fn gc_mark_hook)
 {
   bfd_boolean ret;
-  bfd_boolean is_eh;
-  asection *group_sec;
+  asection *group_sec, *eh_frame;
 
   sec->gc_mark = 1;
 
@@ -11185,8 +11180,10 @@ _bfd_elf_gc_mark (struct bfd_link_info *
 
   /* Look through the section relocs.  */
   ret = TRUE;
-  is_eh = strcmp (sec->name, ".eh_frame") == 0;
-  if ((sec->flags & SEC_RELOC) != 0 && sec->reloc_count > 0)
+  eh_frame = elf_eh_frame_section (sec->owner);
+  if ((sec->flags & SEC_RELOC) != 0
+      && sec->reloc_count > 0
+      && sec != eh_frame)
     {
       struct elf_reloc_cookie cookie;
 
@@ -11195,8 +11192,7 @@ _bfd_elf_gc_mark (struct bfd_link_info *
       else
 	{
 	  for (; cookie.rel < cookie.relend; cookie.rel++)
-	    if (!_bfd_elf_gc_mark_reloc (info, sec, gc_mark_hook,
-					 &cookie, is_eh))
+	    if (!_bfd_elf_gc_mark_reloc (info, sec, gc_mark_hook, &cookie))
 	      {
 		ret = FALSE;
 		break;
@@ -11204,6 +11200,22 @@ _bfd_elf_gc_mark (struct bfd_link_info *
 	  fini_reloc_cookie_for_section (&cookie, sec);
 	}
     }
+
+  if (ret && eh_frame && elf_fde_list (sec))
+    {
+      struct elf_reloc_cookie cookie;
+
+      if (!init_reloc_cookie_for_section (&cookie, info, eh_frame))
+	ret = FALSE;
+      else
+	{
+	  if (!_bfd_elf_gc_mark_fdes (info, sec, eh_frame,
+				      gc_mark_hook, &cookie))
+	    ret = FALSE;
+	  fini_reloc_cookie_for_section (&cookie, eh_frame);
+	}
+    }
+
   return ret;
 }
 
@@ -11469,6 +11481,25 @@ bfd_elf_gc_sections (bfd *abfd, struct b
       return TRUE;
     }
 
+  /* Try to parse each bfd's .eh_frame section.  Point elf_eh_frame_section
+     at the .eh_frame section if we can mark the FDEs individually.  */
+  _bfd_elf_begin_eh_frame_parsing (info);
+  for (sub = info->input_bfds; sub != NULL; sub = sub->link_next)
+    {
+      asection *sec;
+      struct elf_reloc_cookie cookie;
+
+      sec = bfd_get_section_by_name (sub, ".eh_frame");
+      if (sec && init_reloc_cookie_for_section (&cookie, info, sec))
+	{
+	  _bfd_elf_parse_eh_frame (sub, info, sec, &cookie);
+	  if (elf_section_data (sec)->sec_info)
+	    elf_eh_frame_section (sub) = sec;
+	  fini_reloc_cookie_for_section (&cookie, sec);
+	}
+    }
+  _bfd_elf_end_eh_frame_parsing (info);
+
   /* Apply transitive closure to the vtable entry usage info.  */
   elf_link_hash_traverse (elf_hash_table (info),
 			  elf_gc_propagate_vtable_entries_used,
@@ -11508,68 +11539,6 @@ bfd_elf_gc_sections (bfd *abfd, struct b
   if (bed->gc_mark_extra_sections)
     bed->gc_mark_extra_sections(info, gc_mark_hook);
 
-  /* ... again for sections marked from eh_frame.  */
-  for (sub = info->input_bfds; sub != NULL; sub = sub->link_next)
-    {
-      asection *o;
-
-      if (bfd_get_flavour (sub) != bfd_target_elf_flavour)
-	continue;
-
-      /* Keep .gcc_except_table.* if the associated .text.* (or the
-	 associated .gnu.linkonce.t.* if .text.* doesn't exist) is
-	 marked.  This isn't very nice, but the proper solution,
-	 splitting .eh_frame up and using comdat doesn't pan out
-	 easily due to needing special relocs to handle the
-	 difference of two symbols in separate sections.
-	 Don't keep code sections referenced by .eh_frame.  */
-#define TEXT_PREFIX			".text."
-#define TEXT_PREFIX2			".gnu.linkonce.t."
-#define GCC_EXCEPT_TABLE_PREFIX		".gcc_except_table."
-      for (o = sub->sections; o != NULL; o = o->next)
-	if (!o->gc_mark && o->gc_mark_from_eh && (o->flags & SEC_CODE) == 0)
-	  {
-	    if (CONST_STRNEQ (o->name, GCC_EXCEPT_TABLE_PREFIX))
-	      {
-		char *fn_name;
-		const char *sec_name;
-		asection *fn_text;
-		unsigned o_name_prefix_len , fn_name_prefix_len, tmp;
-
-		o_name_prefix_len = strlen (GCC_EXCEPT_TABLE_PREFIX);
-		sec_name = o->name + o_name_prefix_len;
-		fn_name_prefix_len = strlen (TEXT_PREFIX);
-		tmp = strlen (TEXT_PREFIX2);
-		if (tmp > fn_name_prefix_len)
-		  fn_name_prefix_len = tmp;
-		fn_name
-		  = bfd_malloc (fn_name_prefix_len + strlen (sec_name) + 1);
-		if (fn_name == NULL)
-		  return FALSE;
-
-		/* Try the first prefix.  */
-		sprintf (fn_name, "%s%s", TEXT_PREFIX, sec_name);
-		fn_text = bfd_get_section_by_name (sub, fn_name);
-
-		/* Try the second prefix.  */
-		if (fn_text == NULL)
-		  {
-		    sprintf (fn_name, "%s%s", TEXT_PREFIX2, sec_name);
-		    fn_text = bfd_get_section_by_name (sub, fn_name);
-		  }
-
-		free (fn_name);
-		if (fn_text == NULL || !fn_text->gc_mark)
-		  continue;
-	      }
-
-	    /* If not using specially named exception table section,
-	       then keep whatever we are using.  */
-	    if (!_bfd_elf_gc_mark (info, o, gc_mark_hook))
-	      return FALSE;
-	  }
-    }
-
   /* ... and mark SEC_EXCLUDE for those that go.  */
   return elf_gc_sweep (abfd, info);
 }
Index: bfd/section.c
===================================================================
--- bfd/section.c	2007-12-02 16:52:51.000000000 +0000
+++ bfd/section.c	2007-12-02 17:21:41.000000000 +0000
@@ -357,9 +357,8 @@ typedef asection, section prototypes, Se
 .     output sections that have an input section.  *}
 .  unsigned int linker_has_input : 1;
 .
-.  {* Mark flags used by some linker backends for garbage collection.  *}
+.  {* Mark flag used by some linker backends for garbage collection.  *}
 .  unsigned int gc_mark : 1;
-.  unsigned int gc_mark_from_eh : 1;
 .
 .  {* The following flags are used by the ELF linker. *}
 .
Index: bfd/ecoff.c
===================================================================
--- bfd/ecoff.c	2007-12-02 16:52:51.000000000 +0000
+++ bfd/ecoff.c	2007-12-02 17:21:41.000000000 +0000
@@ -55,8 +55,8 @@ static asection bfd_debug_section =
 {
   /* name,      id,  index, next, prev, flags, user_set_vma,       */
      "*DEBUG*", 0,   0,     NULL, NULL, 0,     0,
-  /* linker_mark, linker_has_input, gc_mark, gc_mark_from_eh,      */
-     0,           0,                1,       0,
+  /* linker_mark, linker_has_input, gc_mark,                       */
+     0,           0,                1,
   /* segment_mark, sec_info_type, use_rela_p, has_tls_reloc,       */
      0,            0,             0,          0,
   /* has_gp_reloc, need_finalize_relax, reloc_done,                */

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 5/7] Fix --gc-sections for C++ MIPS ELF
  2007-12-02 20:15 Fix --gc-sections for C++ MIPS ELF Richard Sandiford
                   ` (3 preceding siblings ...)
  2007-12-02 20:31 ` [PATCH 4/7] " Richard Sandiford
@ 2007-12-02 20:35 ` Richard Sandiford
  2007-12-02 20:37 ` [PATCH 6/7] " Richard Sandiford
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2007-12-02 20:35 UTC (permalink / raw)
  To: binutils

This patch and the next one reorganise eh_cie_fde so that the CIE-specific
flags are in the new u.cie structure.  I've split them up because this
first one is more involved, and it's quite likely I'm missing something.
(It's also a prerequisite for removing offsets_adjusted, because I'm
getting rid of one of the checks here.)

We currently have two flags: make_lsda_relative, which says that we
can make LSDA relocations relative if want to, and need_lsda_relative,
which says that we've decided to do that.  We need two fields in case
the LSDA is not a pointer:

     http://sourceware.org/ml/binutils/2004-09/msg00313.html

We set need_lsda_relative based on make_lsda_relative in
_bfd_elf_eh_frame_section_offset, but could we do it when
parsing the section instead?  The original patch linked
above used a make/need_* pair for make_relative as well as
make_lsda_relative, whereas we've now dropped need_relative.
This is now the only case where _bfd_elf_eh_frame_section_offset
changes the CIE and FDE information.

Richard


bfd/
	* elf-bfd.h (eh_cie_fde): Remove need_lsda_relative.
	Move make_lsda_relative to u.cie.
	* elf-eh-frame.c (cie): Rename make_lsda_relative to
	can_make_lsda_relative.
	(_bfd_elf_parse_eh_frame): Don't set the old eh_cie_fde
	make_lsda_relative field.  Update after cie renaming.
	Set u.cie.make_lsda_relative if can_make_lsda_relative
	and if we find a relocation against the LSDA.
	(_bfd_elf_discard_section_eh_frame): Copy make_lsda_relative when
	changing a CIE's group representative.
	(_bfd_elf_eh_frame_section_offset): Don't set need_ldsa_relative here.
	(_bfd_elf_write_section_eh_frame): Check u.cie.make_lsda_relative
	rather than need_lsda_relative.

Index: bfd/elf-bfd.h
===================================================================
--- bfd/elf-bfd.h	2007-12-02 17:22:22.000000000 +0000
+++ bfd/elf-bfd.h	2007-12-02 17:40:41.000000000 +0000
@@ -285,6 +285,11 @@ struct eh_cie_fde
 
       /* True if we have marked relocations associated with this CIE.  */
       unsigned int gc_mark : 1;
+
+      /* True if we have decided to turn an absolute LSDA encoding into
+	 a PC-relative one.  It is the group representative's setting
+	 that matters.  */
+      unsigned int make_lsda_relative : 1;
     } cie;
   } u;
   unsigned int reloc_index;
@@ -299,8 +304,6 @@ struct eh_cie_fde
   unsigned int add_augmentation_size : 1;
   unsigned int add_fde_encoding : 1;
   unsigned int make_relative : 1;
-  unsigned int make_lsda_relative : 1;
-  unsigned int need_lsda_relative : 1;
   unsigned int per_encoding_relative : 1;
   unsigned int *set_loc;
 };
Index: bfd/elf-eh-frame.c
===================================================================
--- bfd/elf-eh-frame.c	2007-12-02 17:26:13.000000000 +0000
+++ bfd/elf-eh-frame.c	2007-12-02 17:41:29.000000000 +0000
@@ -50,7 +50,7 @@ struct cie
   unsigned char fde_encoding;
   unsigned char initial_insn_length;
   unsigned char make_relative;
-  unsigned char make_lsda_relative;
+  unsigned char can_make_lsda_relative;
   unsigned char initial_instructions[50];
 };
 
@@ -799,7 +799,7 @@ #define GET_RELOC(buf)					\
 		  ->elf_backend_can_make_lsda_relative_eh_frame
 		  (abfd, info, sec))
 	      && (cie->lsda_encoding & 0xf0) == DW_EH_PE_absptr)
-	    cie->make_lsda_relative = 1;
+	    cie->can_make_lsda_relative = 1;
 
 	  /* If FDE encoding was not specified, it defaults to
 	     DW_EH_absptr.  */
@@ -817,7 +817,6 @@ #define GET_RELOC(buf)					\
 	  ENSURE_NO_RELOCS (buf);
 
 	  this_inf->make_relative = cie->make_relative;
-	  this_inf->make_lsda_relative = cie->make_lsda_relative;
 	  this_inf->per_encoding_relative
 	    = (cie->per_encoding & 0x70) == DW_EH_PE_pcrel;
 	}
@@ -862,6 +861,9 @@ #define GET_RELOC(buf)					\
 	     be adjusted if any future augmentations do the same thing.  */
 	  if (cie->lsda_encoding != DW_EH_PE_omit)
 	    {
+	      SKIP_RELOCS (buf);
+	      if (cie->can_make_lsda_relative && GET_RELOC (buf))
+		cie->cie_inf->u.cie.make_lsda_relative = 1;
 	      this_inf->lsda_offset = buf - start;
 	      /* If there's no 'z' augmentation, we don't know where the
 		 CFA insns begin.  Assume no padding.  */
@@ -1097,6 +1099,8 @@ _bfd_elf_gc_mark_fdes (struct bfd_link_i
 		    /* Make the local CIE represent the merged group.  */
 		    merged->u.cie.merged = cie;
 		    cie->removed = 0;
+		    cie->u.cie.make_lsda_relative
+		      = merged->u.cie.make_lsda_relative;
 		  }
 	      }
 	  }
@@ -1248,15 +1252,10 @@ _bfd_elf_eh_frame_section_offset (bfd *o
   /* If converting LSDA pointers to DW_EH_PE_pcrel, there will be no need
      for run-time relocation against LSDA field.  */
   if (!sec_info->entry[mid].cie
-      && sec_info->entry[mid].u.fde.cie_inf->make_lsda_relative
-      && (offset == (sec_info->entry[mid].offset + 8
-		     + sec_info->entry[mid].lsda_offset))
-      && (sec_info->entry[mid].u.fde.cie_inf->need_lsda_relative
-	  || !hdr_info->offsets_adjusted))
-    {
-      sec_info->entry[mid].u.fde.cie_inf->need_lsda_relative = 1;
-      return (bfd_vma) -2;
-    }
+      && sec_info->entry[mid].u.fde.cie_inf->u.cie.make_lsda_relative
+      && offset == (sec_info->entry[mid].offset + 8
+		    + sec_info->entry[mid].lsda_offset))
+    return (bfd_vma) -2;
 
   /* If converting to DW_EH_PE_pcrel, there will be no need for run-time
      relocation against DW_CFA_set_loc's arguments.  */
@@ -1394,7 +1393,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 	{
 	  /* CIE */
 	  if (ent->make_relative
-	      || ent->need_lsda_relative
+	      || ent->u.cie.make_lsda_relative
 	      || ent->per_encoding_relative)
 	    {
 	      char *aug;
@@ -1404,7 +1403,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 	      /* Need to find 'R' or 'L' augmentation's argument and modify
 		 DW_EH_PE_* value.  */
 	      action = ((ent->make_relative ? 1 : 0)
-			| (ent->need_lsda_relative ? 2 : 0)
+			| (ent->u.cie.make_lsda_relative ? 2 : 0)
 			| (ent->per_encoding_relative ? 4 : 0));
 	      extra_string = extra_augmentation_string_bytes (ent);
 	      extra_data = extra_augmentation_data_bytes (ent);
@@ -1548,7 +1547,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 	    }
 
 	  if ((ent->lsda_encoding & 0xf0) == DW_EH_PE_pcrel
-	      || cie->need_lsda_relative)
+	      || cie->u.cie.make_lsda_relative)
 	    {
 	      buf += ent->lsda_offset;
 	      width = get_DW_EH_PE_width (ent->lsda_encoding, ptr_size);
@@ -1558,7 +1557,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 		{
 		  if ((ent->lsda_encoding & 0xf0) == DW_EH_PE_pcrel)
 		    value += ent->offset - ent->new_offset;
-		  else if (cie->need_lsda_relative)
+		  else if (cie->u.cie.make_lsda_relative)
 		    value -= (sec->output_section->vma + ent->new_offset + 8
 			      + ent->lsda_offset);
 		  write_value (abfd, buf, value, width);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 6/7] Fix --gc-sections for C++ MIPS ELF
  2007-12-02 20:15 Fix --gc-sections for C++ MIPS ELF Richard Sandiford
                   ` (4 preceding siblings ...)
  2007-12-02 20:35 ` [PATCH 5/7] " Richard Sandiford
@ 2007-12-02 20:37 ` Richard Sandiford
  2007-12-02 20:39 ` [PATCH 7/7] " Richard Sandiford
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2007-12-02 20:37 UTC (permalink / raw)
  To: binutils

This patch moves the other CIE-specific flags to u.cie.  The following
flags only apply to CIEs:

  per_encoding_relative
  add_fde_encoding
  add_augmentation_size
  make_relative

The first two don't affect the FDE code at all, so they might as well
remain CIE-only.  The last two do affect the FDE code, so we could
either:

  (1) make the FDE flags a copy of the CIE flags or
  (2) keep the flags CIE-specific and move them to u.cie

There's not much in it, but (1) helps to simplify a couple of things,
so I went for that.  I'm happy to do (2) if you prefer.

Richard


bfd/
	* elf-bfd.h (eh_cie_fde): Move add_fde_encoding and
	per_encoding_relative to u.cie.  Add commentary.
	* elf-eh-frame.c (cie): Remove make_relative.
	(extra_augmentation_string_bytes): Update use of add_fde_encoding.
	(extra_augmentation_data_bytes): Likewise.  Use an FDE's own
	add_augmentation_size field, rather than referring to the CIE.
	(_bfd_elf_parse_eh_frame): Don't set the struct cie
	make_relative field; set the eh_cie_fde field directly.
	Update setting of add_fde_encoding and per_encoding_relative.
	Copy make_relative and add_augmentation_size from the CIE
	to the FDE.
	(_bfd_elf_discard_section_eh_frame): Use the FDE's own
	make_relative field.
	(_bfd_elf_eh_frame_section_offset): Likewise.
	(_bfd_elf_write_section_eh_frame): Update accesses to
	add_fde_encoding and per_encoding_relative.  Use the FDE's
	own make_relative and add_augmentation_size fields.

Index: bfd/elf-bfd.h
===================================================================
--- bfd/elf-bfd.h	2007-12-02 17:40:41.000000000 +0000
+++ bfd/elf-bfd.h	2007-12-02 17:46:04.000000000 +0000
@@ -290,6 +290,14 @@ struct eh_cie_fde
 	 a PC-relative one.  It is the group representative's setting
 	 that matters.  */
       unsigned int make_lsda_relative : 1;
+
+      /* True if the CIE contains personality data and if that data
+	 uses a PC-relative encoding.  */
+      unsigned int per_encoding_relative : 1;
+
+      /* True if we need to add an 'R' (FDE encoding) entry to the
+	 CIE's augmentation data.  */
+      unsigned int add_fde_encoding : 1;
     } cie;
   } u;
   unsigned int reloc_index;
@@ -299,12 +307,22 @@ struct eh_cie_fde
   unsigned int fde_encoding : 8;
   unsigned int lsda_encoding : 8;
   unsigned int lsda_offset : 8;
+
+  /* True if this entry represents a CIE, false if it represents an FDE.  */
   unsigned int cie : 1;
+
+  /* True if this entry is currently marked for removal.  */
   unsigned int removed : 1;
+
+  /* True if we need to add a 'z' (augmentation size) entry to the CIE's
+     augmentation data, and an associated byte to each of the CIE's FDEs.  */
   unsigned int add_augmentation_size : 1;
-  unsigned int add_fde_encoding : 1;
+
+  /* True if we have decided to convert absolute FDE relocations into
+     relative ones.  This applies to the first relocation in the FDE,
+     which is against the code that the FDE describes.  */
   unsigned int make_relative : 1;
-  unsigned int per_encoding_relative : 1;
+
   unsigned int *set_loc;
 };
 
Index: bfd/elf-eh-frame.c
===================================================================
--- bfd/elf-eh-frame.c	2007-12-02 17:41:29.000000000 +0000
+++ bfd/elf-eh-frame.c	2007-12-02 17:45:12.000000000 +0000
@@ -49,7 +49,6 @@ struct cie
   unsigned char lsda_encoding;
   unsigned char fde_encoding;
   unsigned char initial_insn_length;
-  unsigned char make_relative;
   unsigned char can_make_lsda_relative;
   unsigned char initial_instructions[50];
 };
@@ -283,7 +282,7 @@ extra_augmentation_string_bytes (struct 
     {
       if (entry->add_augmentation_size)
 	size++;
-      if (entry->add_fde_encoding)
+      if (entry->u.cie.add_fde_encoding)
 	size++;
     }
   return size;
@@ -295,18 +294,10 @@ extra_augmentation_string_bytes (struct 
 extra_augmentation_data_bytes (struct eh_cie_fde *entry)
 {
   unsigned int size = 0;
-  if (entry->cie)
-    {
-      if (entry->add_augmentation_size)
-	size++;
-      if (entry->add_fde_encoding)
-	size++;
-    }
-  else
-    {
-      if (entry->u.fde.cie_inf->add_augmentation_size)
-	size++;
-    }
+  if (entry->add_augmentation_size)
+    size++;
+  if (entry->cie && entry->u.cie.add_fde_encoding)
+    size++;
   return size;
 }
 
@@ -780,7 +771,7 @@ #define GET_RELOC(buf)					\
 		  (abfd, info, sec)))
 	    {
 	      if ((cie->fde_encoding & 0xf0) == DW_EH_PE_absptr)
-		cie->make_relative = 1;
+		this_inf->make_relative = 1;
 	      /* If the CIE doesn't already have an 'R' entry, it's fairly
 		 easy to add one, provided that there's no aligned data
 		 after the augmentation string.  */
@@ -789,8 +780,8 @@ #define GET_RELOC(buf)					\
 		{
 		  if (*cie->augmentation == 0)
 		    this_inf->add_augmentation_size = 1;
-		  this_inf->add_fde_encoding = 1;
-		  cie->make_relative = 1;
+		  this_inf->u.cie.add_fde_encoding = 1;
+		  this_inf->make_relative = 1;
 		}
 	    }
 
@@ -816,8 +807,7 @@ #define GET_RELOC(buf)					\
 	  buf += initial_insn_length;
 	  ENSURE_NO_RELOCS (buf);
 
-	  this_inf->make_relative = cie->make_relative;
-	  this_inf->per_encoding_relative
+	  this_inf->u.cie.per_encoding_relative
 	    = (cie->per_encoding & 0x70) == DW_EH_PE_pcrel;
 	}
       else
@@ -835,6 +825,9 @@ #define GET_RELOC(buf)					\
 	  REQUIRE (ecie != ecies + ecie_count);
 	  cie = ecie->cie;
 	  this_inf->u.fde.cie_inf = ecie->local_cie;
+	  this_inf->make_relative = ecie->local_cie->make_relative;
+	  this_inf->add_augmentation_size
+	    = ecie->local_cie->add_augmentation_size;
 
 	  ENSURE_NO_RELOCS (buf);
 	  REQUIRE (GET_RELOC (buf));
@@ -900,7 +893,7 @@ #define GET_RELOC(buf)					\
 	}
       if (set_loc_count
 	  && ((cie->fde_encoding & 0xf0) == DW_EH_PE_pcrel
-	      || cie->make_relative))
+	      || this_inf->make_relative))
 	{
 	  unsigned int cnt;
 	  bfd_byte *p;
@@ -1068,7 +1061,7 @@ _bfd_elf_gc_mark_fdes (struct bfd_link_i
 	  {
 	    if (info->shared
 		&& (((ent->fde_encoding & 0xf0) == DW_EH_PE_absptr
-		     && ent->u.fde.cie_inf->make_relative == 0)
+		     && ent->make_relative == 0)
 		    || (ent->fde_encoding & 0xf0) == DW_EH_PE_aligned))
 	      {
 		/* If a shared library uses absolute pointers
@@ -1245,7 +1238,7 @@ _bfd_elf_eh_frame_section_offset (bfd *o
   /* If converting to DW_EH_PE_pcrel, there will be no need for run-time
      relocation against FDE's initial_location field.  */
   if (!sec_info->entry[mid].cie
-      && sec_info->entry[mid].u.fde.cie_inf->make_relative
+      && sec_info->entry[mid].make_relative
       && offset == sec_info->entry[mid].offset + 8)
     return (bfd_vma) -2;
 
@@ -1260,9 +1253,7 @@ _bfd_elf_eh_frame_section_offset (bfd *o
   /* If converting to DW_EH_PE_pcrel, there will be no need for run-time
      relocation against DW_CFA_set_loc's arguments.  */
   if (sec_info->entry[mid].set_loc
-      && (sec_info->entry[mid].cie
-	  ? sec_info->entry[mid].make_relative
-	  : sec_info->entry[mid].u.fde.cie_inf->make_relative)
+      && sec_info->entry[mid].make_relative
       && (offset >= sec_info->entry[mid].offset + 8
 		    + sec_info->entry[mid].set_loc[1]))
     {
@@ -1394,7 +1385,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 	  /* CIE */
 	  if (ent->make_relative
 	      || ent->u.cie.make_lsda_relative
-	      || ent->per_encoding_relative)
+	      || ent->u.cie.per_encoding_relative)
 	    {
 	      char *aug;
 	      unsigned int action, extra_string, extra_data;
@@ -1404,7 +1395,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 		 DW_EH_PE_* value.  */
 	      action = ((ent->make_relative ? 1 : 0)
 			| (ent->u.cie.make_lsda_relative ? 2 : 0)
-			| (ent->per_encoding_relative ? 4 : 0));
+			| (ent->u.cie.per_encoding_relative ? 4 : 0));
 	      extra_string = extra_augmentation_string_bytes (ent);
 	      extra_data = extra_augmentation_data_bytes (ent);
 
@@ -1434,7 +1425,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 		  *aug++ = 'z';
 		  *buf++ = extra_data - 1;
 		}
-	      if (ent->add_fde_encoding)
+	      if (ent->u.cie.add_fde_encoding)
 		{
 		  BFD_ASSERT (action & 1);
 		  *aug++ = 'R';
@@ -1459,7 +1450,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 		    per_width = get_DW_EH_PE_width (per_encoding, ptr_size);
 		    BFD_ASSERT (per_width != 0);
 		    BFD_ASSERT (((per_encoding & 0x70) == DW_EH_PE_pcrel)
-				== ent->per_encoding_relative);
+				== ent->u.cie.per_encoding_relative);
 		    if ((per_encoding & 0xf0) == DW_EH_PE_aligned)
 		      buf = (contents
 			     + ((buf - contents + per_width - 1)
@@ -1532,7 +1523,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 		  address += sec->output_section->vma + ent->offset + 8;
 		  break;
 		}
-	      if (cie->make_relative)
+	      if (ent->make_relative)
 		value -= sec->output_section->vma + ent->new_offset + 8;
 	      write_value (abfd, buf, value, width);
 	    }
@@ -1563,7 +1554,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 		  write_value (abfd, buf, value, width);
 		}
 	    }
-	  else if (cie->add_augmentation_size)
+	  else if (ent->add_augmentation_size)
 	    {
 	      /* Skip the PC and length and insert a zero byte for the
 		 augmentation size.  */
@@ -1595,7 +1586,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 
 		  if ((ent->fde_encoding & 0xf0) == DW_EH_PE_pcrel)
 		    value += ent->offset + 8 - new_offset;
-		  if (cie->make_relative)
+		  if (ent->make_relative)
 		    value -= sec->output_section->vma + new_offset
 			     + ent->set_loc[cnt];
 		  write_value (abfd, buf, value, width);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 7/7] Fix --gc-sections for C++ MIPS ELF
  2007-12-02 20:15 Fix --gc-sections for C++ MIPS ELF Richard Sandiford
                   ` (5 preceding siblings ...)
  2007-12-02 20:37 ` [PATCH 6/7] " Richard Sandiford
@ 2007-12-02 20:39 ` Richard Sandiford
  2007-12-02 21:55 ` Jakub Jelinek
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2007-12-02 20:39 UTC (permalink / raw)
  To: binutils

The u.cie.merged field is only really useful when REMOVED == 1;
it is basically spare space if we have decided to keep the CIE.
We know when setting REMOVED to 0 what the CIE's section is,
so if we store it in this spare space, we could avoid the need
for offsets_adjusted.

Richard


bfd/
	* elf-bfd.h (eh_cie_fde): Replace u.cie.merged with a union of
	a merged field and a sec field.
	(eh_frame_hdr_info): Remove offsets_adjusted.
	* elf-eh-frame.c (_bfd_elf_parse_eh_frame): Update accesses to
	the CIE merged field.
	(_bfd_elf_gc_mark_fdes): Likewise.
	(_bfd_elf_discard_section_eh_frame): Likewise.  Set u.cie.u.sec
	when clearing the removed flag.
	(_bfd_elf_eh_frame_section_offset): Remove offsets_adjusted handling.
	(_bfd_elf_write_section_eh_frame): Likewise.  Apply output_offsets
	where appropriate.  

Index: bfd/elf-bfd.h
===================================================================
--- bfd/elf-bfd.h	2007-12-02 17:46:04.000000000 +0000
+++ bfd/elf-bfd.h	2007-12-02 17:57:08.000000000 +0000
@@ -277,11 +277,20 @@ struct eh_cie_fde
     } fde;
     struct {
       /* In general, equivalent CIEs are grouped together, with one CIE
-	 representing all the others in a group.  If REMOVED == 0,
-	 this CIE is the group representative.  If REMOVED == 1,
-	 following this pointer brings us "closer" to the CIE's group
-	 representative, and reapplying always gives the representative.  */
-      struct eh_cie_fde *merged;
+	 representing all the others in a group.
+
+	 If REMOVED == 0, this CIE is the group representative, and
+	 U.SEC points to the .eh_frame section that contains the CIE.
+
+	 If REMOVED == 1, this CIE is the group representative if
+	 U.MERGED is a self pointer.  Otherwise, following U.MERGED
+	 brings us "closer" to the CIE's group representative;
+	 if U.MERGED is not the group representative, then
+	 U.MERGED->U.MERGED is.  */
+      union {
+ 	struct eh_cie_fde *merged;
+ 	asection *sec;
+      } u;
 
       /* True if we have marked relocations associated with this CIE.  */
       unsigned int gc_mark : 1;
@@ -352,7 +361,6 @@ struct eh_frame_hdr_info
      We build it if we successfully read all .eh_frame input sections
      and recognize them.  */
   bfd_boolean table;
-  bfd_boolean offsets_adjusted;
 };
 
 /* ELF linker hash table.  */
Index: bfd/elf-eh-frame.c
===================================================================
--- bfd/elf-eh-frame.c	2007-12-02 17:45:12.000000000 +0000
+++ bfd/elf-eh-frame.c	2007-12-02 17:56:21.000000000 +0000
@@ -935,7 +935,7 @@ #define GET_RELOC(buf)					\
 		}
 	      cie = (struct cie *) *loc;
 	    }
-	  this_inf->u.cie.merged = cie->cie_inf;
+	  this_inf->u.cie.u.merged = cie->cie_inf;
 	  ecies[ecie_count].cie = cie;
 	  ecies[ecie_count++].local_cie = this_inf;
 	}
@@ -1019,7 +1019,7 @@ _bfd_elf_gc_mark_fdes (struct bfd_link_i
       /* At this stage, all cie_inf fields point to local CIEs, so we
 	 can use the same cookie to refer to them.  */
       cie = fde->u.fde.cie_inf;
-      merged = cie->u.cie.merged;
+      merged = cie->u.cie.u.merged;
       if (!merged->u.cie.gc_mark)
 	{
 	  merged->u.cie.gc_mark = 1;
@@ -1079,19 +1079,20 @@ _bfd_elf_gc_mark_fdes (struct bfd_link_i
 	    cie = ent->u.fde.cie_inf;
 	    if (cie->removed)
 	      {
-		merged = cie->u.cie.merged;
+		merged = cie->u.cie.u.merged;
 		if (!merged->removed)
 		  /* We have decided to keep the group representative.  */
 		  ent->u.fde.cie_inf = merged;
-		else if (merged->u.cie.merged != merged)
+		else if (merged->u.cie.u.merged != merged)
 		  /* We didn't keep the original group representative,
 		     but we did keep an alternative.  */
-		  ent->u.fde.cie_inf = merged->u.cie.merged;
+		  ent->u.fde.cie_inf = merged->u.cie.u.merged;
 		else
 		  {
 		    /* Make the local CIE represent the merged group.  */
-		    merged->u.cie.merged = cie;
+		    merged->u.cie.u.merged = cie;
 		    cie->removed = 0;
+		    cie->u.cie.u.sec = sec;
 		    cie->u.cie.make_lsda_relative
 		      = merged->u.cie.make_lsda_relative;
 		  }
@@ -1211,8 +1212,6 @@ _bfd_elf_eh_frame_section_offset (bfd *o
 
   htab = elf_hash_table (info);
   hdr_info = &htab->eh_info;
-  if (hdr_info->offsets_adjusted)
-    offset += sec->output_offset;
 
   lo = 0;
   hi = sec_info->count;
@@ -1265,8 +1264,6 @@ _bfd_elf_eh_frame_section_offset (bfd *o
 	  return (bfd_vma) -2;
     }
 
-  if (hdr_info->offsets_adjusted)
-    offset -= sec->output_offset;
   /* Any new augmentation bytes go before the first relocation.  */
   return (offset + sec_info->entry[mid].new_offset
 	  - sec_info->entry[mid].offset
@@ -1301,38 +1298,6 @@ _bfd_elf_write_section_eh_frame (bfd *ab
   htab = elf_hash_table (info);
   hdr_info = &htab->eh_info;
 
-  /* First convert all offsets to output section offsets, so that a
-     CIE offset is valid if the CIE is used by a FDE from some other
-     section.  This can happen when duplicate CIEs are deleted in
-     _bfd_elf_discard_section_eh_frame.  We do all sections here because
-     this function might not be called on sections in the same order as
-     _bfd_elf_discard_section_eh_frame.  */
-  if (!hdr_info->offsets_adjusted)
-    {
-      bfd *ibfd;
-      asection *eh;
-      struct eh_frame_sec_info *eh_inf;
-
-      for (ibfd = info->input_bfds; ibfd != NULL; ibfd = ibfd->link_next)
-	{
-	  if (bfd_get_flavour (ibfd) != bfd_target_elf_flavour
-	      || (ibfd->flags & DYNAMIC) != 0)
-	    continue;
-
-	  eh = bfd_get_section_by_name (ibfd, ".eh_frame");
-	  if (eh == NULL || eh->sec_info_type != ELF_INFO_TYPE_EH_FRAME)
-	    continue;
-
-	  eh_inf = elf_section_data (eh)->sec_info;
-	  for (ent = eh_inf->entry; ent < eh_inf->entry + eh_inf->count; ++ent)
-	    {
-	      ent->offset += eh->output_offset;
-	      ent->new_offset += eh->output_offset;
-	    }
-	}
-      hdr_info->offsets_adjusted = TRUE;
-    }
-
   if (hdr_info->table && hdr_info->array == NULL)
     hdr_info->array
       = bfd_malloc (hdr_info->fde_count * sizeof(*hdr_info->array));
@@ -1346,13 +1311,11 @@ _bfd_elf_write_section_eh_frame (bfd *ab
      not reordered  */
   for (ent = sec_info->entry + sec_info->count; ent-- != sec_info->entry;)
     if (!ent->removed && ent->new_offset > ent->offset)
-      memmove (contents + ent->new_offset - sec->output_offset,
-	       contents + ent->offset - sec->output_offset, ent->size);
+      memmove (contents + ent->new_offset, contents + ent->offset, ent->size);
 
   for (ent = sec_info->entry; ent < sec_info->entry + sec_info->count; ++ent)
     if (!ent->removed && ent->new_offset < ent->offset)
-      memmove (contents + ent->new_offset - sec->output_offset,
-	       contents + ent->offset - sec->output_offset, ent->size);
+      memmove (contents + ent->new_offset, contents + ent->offset, ent->size);
 
   for (ent = sec_info->entry; ent < sec_info->entry + sec_info->count; ++ent)
     {
@@ -1369,7 +1332,7 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 	  continue;
 	}
 
-      buf = contents + ent->new_offset - sec->output_offset;
+      buf = contents + ent->new_offset;
       end = buf + ent->size;
       new_size = size_of_output_cie_fde (ent, ptr_size);
 
@@ -1495,7 +1458,8 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 	  /* Skip length.  */
 	  cie = ent->u.fde.cie_inf;
 	  buf += 4;
-	  value = ent->new_offset + 4 - cie->new_offset;
+	  value = ((ent->new_offset + sec->output_offset + 4)
+		   - (cie->new_offset + cie->u.cie.u.sec->output_offset));
 	  bfd_put_32 (abfd, value, buf);
 	  buf += 4;
 	  width = get_DW_EH_PE_width (ent->fde_encoding, ptr_size);
@@ -1520,11 +1484,15 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 		  break;
 		case DW_EH_PE_pcrel:
 		  value += ent->offset - ent->new_offset;
-		  address += sec->output_section->vma + ent->offset + 8;
+		  address += (sec->output_section->vma
+			      + sec->output_offset
+			      + ent->offset + 8);
 		  break;
 		}
 	      if (ent->make_relative)
-		value -= sec->output_section->vma + ent->new_offset + 8;
+		value -= (sec->output_section->vma
+			  + sec->output_offset
+			  + ent->new_offset + 8);
 	      write_value (abfd, buf, value, width);
 	    }
 
@@ -1534,7 +1502,9 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 	    {
 	      hdr_info->array[hdr_info->array_count].initial_loc = address;
 	      hdr_info->array[hdr_info->array_count++].fde
-		= sec->output_section->vma + ent->new_offset;
+		= (sec->output_section->vma
+		   + sec->output_offset
+		   + ent->new_offset);
 	    }
 
 	  if ((ent->lsda_encoding & 0xf0) == DW_EH_PE_pcrel
@@ -1549,8 +1519,9 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 		  if ((ent->lsda_encoding & 0xf0) == DW_EH_PE_pcrel)
 		    value += ent->offset - ent->new_offset;
 		  else if (cie->u.cie.make_lsda_relative)
-		    value -= (sec->output_section->vma + ent->new_offset + 8
-			      + ent->lsda_offset);
+		    value -= (sec->output_section->vma
+			      + sec->output_offset
+			      + ent->new_offset + 8 + ent->lsda_offset);
 		  write_value (abfd, buf, value, width);
 		}
 	    }
@@ -1587,8 +1558,9 @@ _bfd_elf_write_section_eh_frame (bfd *ab
 		  if ((ent->fde_encoding & 0xf0) == DW_EH_PE_pcrel)
 		    value += ent->offset + 8 - new_offset;
 		  if (ent->make_relative)
-		    value -= sec->output_section->vma + new_offset
-			     + ent->set_loc[cnt];
+		    value -= (sec->output_section->vma
+			      + sec->output_offset
+			      + new_offset + ent->set_loc[cnt]);
 		  write_value (abfd, buf, value, width);
 		}
 	    }

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix --gc-sections for C++ MIPS ELF
  2007-12-02 20:15 Fix --gc-sections for C++ MIPS ELF Richard Sandiford
                   ` (6 preceding siblings ...)
  2007-12-02 20:39 ` [PATCH 7/7] " Richard Sandiford
@ 2007-12-02 21:55 ` Jakub Jelinek
  2007-12-04 10:08   ` Richard Sandiford
  2007-12-02 22:26 ` Eric Botcazou
  2007-12-15  1:46 ` Alan Modra
  9 siblings, 1 reply; 15+ messages in thread
From: Jakub Jelinek @ 2007-12-02 21:55 UTC (permalink / raw)
  To: binutils, rsandifo

On Sun, Dec 02, 2007 at 08:15:15PM +0000, Richard Sandiford wrote:
> I tested the changes by running the C++ and libstdc++-v3 testsuites
> on x86_64-linux-gnu and mipsisa64-elf with -Wl,--gc-sections.
> The C++ PCH tests failed, but the results were otherwise identical
> to those without -Wl,--gc-sections.  There were also no regressions
> in the binutils, gas and ld testsuites for x86_64-linux-gnu,
> mipsisa64-elf and mips64-linux-gnu.

Another desirable test would be to build libgcj and glibc (as examples of
really huge .eh_frame, including hand written .eh_frame, gas generated
.cfi_* stuff and gcc generated stuff) on x86_64-linux, i686-linux, ppc-linux
at least and comparing the resulting .eh_frame between unpatched and patched
ld.

	Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix --gc-sections for C++ MIPS ELF
  2007-12-02 20:15 Fix --gc-sections for C++ MIPS ELF Richard Sandiford
                   ` (7 preceding siblings ...)
  2007-12-02 21:55 ` Jakub Jelinek
@ 2007-12-02 22:26 ` Eric Botcazou
  2007-12-04 10:02   ` [PATCH 8/7] " Richard Sandiford
  2007-12-15  1:46 ` Alan Modra
  9 siblings, 1 reply; 15+ messages in thread
From: Eric Botcazou @ 2007-12-02 22:26 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: binutils

> Non-PIC CIEs for targets like x86_64-linux-gnu also have direct references
> to the personality routines.  I suspect the only reason -Wl,--gc-sections
> -static-libgcc works for them is that libgcc's own CIEs use an indirect
> reference, so the section gets marked that way.

The section is supposed to be always marked, see scripttempl/elf.sc.

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 8/7] Fix --gc-sections for C++ MIPS ELF
  2007-12-02 22:26 ` Eric Botcazou
@ 2007-12-04 10:02   ` Richard Sandiford
  2007-12-04 11:39     ` Eric Botcazou
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Sandiford @ 2007-12-04 10:02 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: binutils

Eric Botcazou <ebotcazou@adacore.com> writes:
>> Non-PIC CIEs for targets like x86_64-linux-gnu also have direct references
>> to the personality routines.  I suspect the only reason -Wl,--gc-sections
>> -static-libgcc works for them is that libgcc's own CIEs use an indirect
>> reference, so the section gets marked that way.
>
> The section is supposed to be always marked, see scripttempl/elf.sc.

Gah!  Sorry, I'm an idiot.  I should have known to look for that.
So we could fix the original problem by modifying the libgloss
scripts instead.

I still think the patches make the linker more robust though,
both because it can then cope with scripts that don't have the KEEPs[*],
and because it no longer discards sections referenced by unparseable
.eh_frames.  There's also the theoretical advantage that we can get
rid of personality routines we don't need, although I doubt that occurs
very often in practice.  So I'd still like to go with these patches.

 [*] As a datapoint, current libgloss has 71 *.ld scripts, only 4 of
     which keep personality sections.  I imagine quite a few users have
     their own local scripts too.

Anyway, there's now an unscheduled eighth patch in the series.  Thanks
for the pointer!  I repeated the original testing with this patch also
applied, and there were no changes.

Richard


ld/scripttempl/
	* armbpabi.sc (.text): Remove KEEP (*(.text.*personality*)).
	(.data): KEEP (*(.gnu.linkonce.d.*personality*)).
	* elf.sc: As for armbpabi.sc.
	* elfxtensa.sc: Likewise.
	* mep.sc: Likewise.

Index: ld/scripttempl/armbpabi.sc
===================================================================
--- ld/scripttempl/armbpabi.sc	2007-12-02 22:47:25.000000000 +0000
+++ ld/scripttempl/armbpabi.sc	2007-12-02 22:47:38.000000000 +0000
@@ -183,7 +183,6 @@ cat <<EOF
   {
     ${RELOCATING+${TEXT_START_SYMBOLS}}
     *(.text .stub${RELOCATING+ .text.* .gnu.linkonce.t.*})
-    KEEP (*(.text.*personality*))
     /* .gnu.warning sections are handled specially by elf32.em.  */
     *(.gnu.warning)
     ${RELOCATING+${OTHER_TEXT_SECTIONS}}
@@ -271,7 +270,6 @@ cat <<EOF
   {
     ${RELOCATING+${DATA_START_SYMBOLS}}
     *(.data${RELOCATING+ .data.* .gnu.linkonce.d.*})
-    KEEP (*(.gnu.linkonce.d.*personality*))
     ${CONSTRUCTING+SORT(CONSTRUCTORS)}
   }
   .data1        ${RELOCATING-0} : { *(.data1) }
Index: ld/scripttempl/elf.sc
===================================================================
--- ld/scripttempl/elf.sc	2007-12-02 22:47:44.000000000 +0000
+++ ld/scripttempl/elf.sc	2007-12-02 22:47:58.000000000 +0000
@@ -350,7 +350,6 @@ cat <<EOF
   {
     ${RELOCATING+${TEXT_START_SYMBOLS}}
     *(.text .stub${RELOCATING+ .text.* .gnu.linkonce.t.*})
-    KEEP (*(.text.*personality*))
     /* .gnu.warning sections are handled specially by elf32.em.  */
     *(.gnu.warning)
     ${RELOCATING+${OTHER_TEXT_SECTIONS}}
@@ -427,7 +426,6 @@ cat <<EOF
   {
     ${RELOCATING+${DATA_START_SYMBOLS}}
     *(.data${RELOCATING+ .data.* .gnu.linkonce.d.*})
-    ${RELOCATING+KEEP (*(.gnu.linkonce.d.*personality*))}
     ${CONSTRUCTING+SORT(CONSTRUCTORS)}
   }
   .data1        ${RELOCATING-0} : { *(.data1) }
Index: ld/scripttempl/elfxtensa.sc
===================================================================
--- ld/scripttempl/elfxtensa.sc	2007-12-02 22:48:02.000000000 +0000
+++ ld/scripttempl/elfxtensa.sc	2007-12-02 22:48:12.000000000 +0000
@@ -366,7 +366,6 @@ cat <<EOF
 
     ${RELOCATING+${TEXT_START_SYMBOLS}}
     *(.literal .text .stub${RELOCATING+ .literal.* .text.* .gnu.linkonce.literal.* .gnu.linkonce.t.*.literal .gnu.linkonce.t.*})
-    KEEP (*(.text.*personality*))
     /* .gnu.warning sections are handled specially by elf32.em.  */
     *(.gnu.warning)
     ${RELOCATING+${OTHER_TEXT_SECTIONS}}
@@ -446,7 +445,6 @@ cat <<EOF
   {
     ${RELOCATING+${DATA_START_SYMBOLS}}
     *(.data${RELOCATING+ .data.* .gnu.linkonce.d.*})
-    ${RELOCATING+KEEP (*(.gnu.linkonce.d.*personality*))}
     ${CONSTRUCTING+SORT(CONSTRUCTORS)}
   }
   .data1        ${RELOCATING-0} : { *(.data1) }
Index: ld/scripttempl/mep.sc
===================================================================
--- ld/scripttempl/mep.sc	2007-12-02 22:48:16.000000000 +0000
+++ ld/scripttempl/mep.sc	2007-12-02 22:48:28.000000000 +0000
@@ -300,7 +300,6 @@ cat <<EOF
   {
     ${RELOCATING+${TEXT_START_SYMBOLS}}
     *(.text .stub${RELOCATING+ .text.* .gnu.linkonce.t.*})
-    KEEP (*(.text.*personality*))
     /* .gnu.warning sections are handled specially by elf32.em.  */
     *(.gnu.warning)
     ${RELOCATING+${OTHER_TEXT_SECTIONS}}
@@ -360,7 +359,6 @@ cat <<EOF
   {
     ${RELOCATING+${DATA_START_SYMBOLS}}
     *(.data${RELOCATING+ .data.* .gnu.linkonce.d.*})
-    KEEP (*(.gnu.linkonce.d.*personality*))
     ${CONSTRUCTING+SORT(CONSTRUCTORS)}
   }
   .data1        ${RELOCATING-0} : { *(.data1) }

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix --gc-sections for C++ MIPS ELF
  2007-12-02 21:55 ` Jakub Jelinek
@ 2007-12-04 10:08   ` Richard Sandiford
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2007-12-04 10:08 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: binutils

Jakub Jelinek <jakub@redhat.com> writes:
> On Sun, Dec 02, 2007 at 08:15:15PM +0000, Richard Sandiford wrote:
>> I tested the changes by running the C++ and libstdc++-v3 testsuites
>> on x86_64-linux-gnu and mipsisa64-elf with -Wl,--gc-sections.
>> The C++ PCH tests failed, but the results were otherwise identical
>> to those without -Wl,--gc-sections.  There were also no regressions
>> in the binutils, gas and ld testsuites for x86_64-linux-gnu,
>> mipsisa64-elf and mips64-linux-gnu.
>
> Another desirable test would be to build libgcj and glibc (as examples of
> really huge .eh_frame, including hand written .eh_frame, gas generated
> .cfi_* stuff and gcc generated stuff) on x86_64-linux, i686-linux, ppc-linux
> at least and comparing the resulting .eh_frame between unpatched and patched
> ld.

OK.  I don't have access to ppc-linux, but I tried it on the other two.
Specifically, with and without the patches, I:

  - built binutils from scratch and installed it into fixed location 1
  - ran a full gcc bootstrap and regression test from fixed location 2
    and installed into fixed location 3.  I did this with fixed location 1
    at the head of the path, and with --with-{as,ld} pointing at the
    assembler and linker in fixed location 1.
  - built glibc with fixed location 1 and fixed location 3 at the head
    of the path.
  - moved fixed location 3 and the glibc build directory away for later
    comparison.

I double-checked that the right binutils and gccs were being used
in each step.

The two libc.sos were identical.  The libgcj.sos differed only in
debugging information, due to things like different random seeds;
the stripped versions were also identical.  FWIW, libstdc++-v3
(which is built with -Wl,--gc-sections) stayed the same too.

The gcc regression-test results were also the same for both builds.
I extended the -Wl,--gc-sections testing on i686-pc-linux-gnu
(adding to the original x86_64-linux-gnu and mipsisa64-elf).

The patched binutils included the eighth patch that I just posted.

Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 8/7] Fix --gc-sections for C++ MIPS ELF
  2007-12-04 10:02   ` [PATCH 8/7] " Richard Sandiford
@ 2007-12-04 11:39     ` Eric Botcazou
  0 siblings, 0 replies; 15+ messages in thread
From: Eric Botcazou @ 2007-12-04 11:39 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: binutils

> I still think the patches make the linker more robust though,
> both because it can then cope with scripts that don't have the KEEPs[*],
> and because it no longer discards sections referenced by unparseable
> .eh_frames.

No discussion about that.  Your implementation is the proper one and makes 
the kludgy approach I initiated totally obsolete.

> Anyway, there's now an unscheduled eighth patch in the series.  Thanks
> for the pointer!

Thanks for implementing the feature for real. ;-)

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix --gc-sections for C++ MIPS ELF
  2007-12-02 20:15 Fix --gc-sections for C++ MIPS ELF Richard Sandiford
                   ` (8 preceding siblings ...)
  2007-12-02 22:26 ` Eric Botcazou
@ 2007-12-15  1:46 ` Alan Modra
  2007-12-15  9:46   ` Richard Sandiford
  9 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2007-12-15  1:46 UTC (permalink / raw)
  To: binutils, rsandifo

This all looks reasonable to me.  OK to apply.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Fix --gc-sections for C++ MIPS ELF
  2007-12-15  1:46 ` Alan Modra
@ 2007-12-15  9:46   ` Richard Sandiford
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2007-12-15  9:46 UTC (permalink / raw)
  To: binutils

Alan Modra <amodra@bigpond.net.au> writes:
> This all looks reasonable to me.  OK to apply.

Applied.  Thanks for reviewing that lot. ;)  For the record,
I added an explicit:

  /* Unused bits.  */
  unsigned int pad1 : 4;

to eh_cie_fde to try to make sure that the new :8 bitfields were
byte-aligned on all hosts.  That was the only change from the
posted patches.

Please shout if you find a problem.

Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2007-12-15  9:46 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-12-02 20:15 Fix --gc-sections for C++ MIPS ELF Richard Sandiford
2007-12-02 20:18 ` [PATCH 1/7] " Richard Sandiford
2007-12-02 20:19 ` [PATCH 2/7] " Richard Sandiford
2007-12-02 20:28 ` [PATCH 3/7] " Richard Sandiford
2007-12-02 20:31 ` [PATCH 4/7] " Richard Sandiford
2007-12-02 20:35 ` [PATCH 5/7] " Richard Sandiford
2007-12-02 20:37 ` [PATCH 6/7] " Richard Sandiford
2007-12-02 20:39 ` [PATCH 7/7] " Richard Sandiford
2007-12-02 21:55 ` Jakub Jelinek
2007-12-04 10:08   ` Richard Sandiford
2007-12-02 22:26 ` Eric Botcazou
2007-12-04 10:02   ` [PATCH 8/7] " Richard Sandiford
2007-12-04 11:39     ` Eric Botcazou
2007-12-15  1:46 ` Alan Modra
2007-12-15  9:46   ` Richard Sandiford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).