[PATCH] bfd/ld bug when using linker plugins and dynobj is dynamic

[Kwok Cheung Yeung <kcy at codesourcery dot com>]

This error was encountered when running the GCC DejaGNU LTO tests (in 
gcc/testsuite/gcc.dg/lto/*) with uClibc on MIPS:

mips-linux-gnu-gcc c_lto_20081024_0.o -O0 -flto -flto-partition=none 
-Wl,-dynamic-linker,mips-linux-gnu/libc/uclibc/lib/ld-uClibc.so.0 
-Wl,-rpath,mips-linux-gnu/libc/uclibc/lib/ 
-Wl,-rpath,mips-linux-gnu/libc/uclibc/usr/lib/ -muclibc -o 
gcc-dg-lto-20081024-01.exe
mips-linux-gnu/libc/uclibc/lib/libgcc_s.so.1: could not read symbols: 
Nonrepresentable section on output
collect2: error: ld returned 1 exit status
compiler exited with status 1

Stepping through ld with GDB, I find the following sequence of events leading up 
to the error:

1) elf_link_add_object_symbols is called with the BFD for libgcc_s.so.1.
2) Since it is a dynamic object, bfd_section_list_clear is called to clear the 
section list.
3) Since htab->dynobj==NULL, it is set to the current BFD during the call to 
elf_add_dt_needed_tag.
4) Execution continues normally until...
5) elf_link_add_object_symbols is called with the same BFD again as part of the 
rescan triggered by the plugin (in lang_process in ld/ldlang.c).
6) bfd_get_section_by_name (abfd, ".dynamic") picks up the .dynamic section 
generated by the linker.
7) _bfd_elf_section_from_bfd_section fails to find the ELF section for the BFD 
section, resulting in a SHN_BAD.

I've fixed the problem in this patch by:

1) Recording the shindex of the dynamic section in the elf_tdata as the BFD 
section is constructed in bfd_section_from_shdr.
2) Use that to look up .dynamic in elf_link_add_object_symbols.
3) Not clearing the section list if abfd==htab->dynobj (since the places where 
this could be set occur after the call to bfd_section_list_clear, this must be a 
later pass after the list was already cleared, so the sections now in the list 
must be linker-generated).

Are there any problems with this approach?

I haven't seen this anywhere else so far - on MIPS with glibc for example, 
dynobj gets set to the BFD for crti.o. There doesn't seem to be anything 
explicitly stopping the BFD of a dynamic library becoming the dynobj though 
(apart from static objects tending to get there first), so this could 
potentially occur on any architecture.

Regards

Kwok Cheung Yeung

2013-06-01  Kwok Cheung Yeung  <kcy@codesourcery.com>

	bfd/
	* elf-bfd.h (struct elf_obj_tdata): Add new field.
	(elf_dynamic): New macro.
	* elf.c (bfd_section_from_shdr): Set elf_dynamic when a dynamic section
	is encountered.
	* elflink.c (elf_link_add_object_symbols): Lookup BFD and ELF sections
	corresponding to dynamic section using elf_dynamic rather than by
	name.  Do not clear section list if the current BFD is used to store
	linker-generated sections.

Index: bfd/elf-bfd.h
===================================================================
RCS file: /cvs/src/src/bfd/elf-bfd.h,v
retrieving revision 1.372
diff -u -p -r1.372 elf-bfd.h
--- bfd/elf-bfd.h	7 May 2013 17:03:51 -0000	1.372
+++ bfd/elf-bfd.h	1 Jun 2013 00:58:11 -0000
@@ -1656,6 +1656,7 @@ struct elf_obj_tdata

   unsigned int symtab_section, symtab_shndx_section, dynsymtab_section;
   unsigned int dynversym_section, dynverdef_section, dynverref_section;
+  unsigned int dynamic_section;

   /* An identifier used to distinguish different target
      specific extensions to this structure.  */
@@ -1707,6 +1708,7 @@ struct elf_obj_tdata
 #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_dynamic(bfd)	(elf_tdata(bfd) -> dynamic_section)
 #define elf_eh_frame_section(bfd) \
 				(elf_tdata(bfd) -> eh_frame_section)
 #define elf_section_syms(bfd)	(elf_tdata(bfd) -> o->section_syms)
Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.591
diff -u -p -r1.591 elf.c
--- bfd/elf.c	27 Mar 2013 13:37:50 -0000	1.591
+++ bfd/elf.c	1 Jun 2013 00:58:11 -0000
@@ -1580,6 +1580,7 @@ bfd_section_from_shdr (bfd *abfd, unsign
     case SHT_DYNAMIC:	/* Dynamic linking information.  */
       if (! _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex))
 	return FALSE;
+      elf_dynamic (abfd) = shindex;
       if (hdr->sh_link > elf_numsections (abfd))
 	{
 	  /* PR 10478: Accept Solaris binaries with a sh_link
Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.494
diff -u -p -r1.494 elflink.c
--- bfd/elflink.c	8 May 2013 23:31:38 -0000	1.494
+++ bfd/elflink.c	1 Jun 2013 00:58:13 -0000
@@ -3502,12 +3502,11 @@ elf_link_add_object_symbols (bfd *abfd,
 		    & (DYN_AS_NEEDED | DYN_DT_NEEDED
 		       | DYN_NO_NEEDED)) == 0;

-      s = bfd_get_section_by_name (abfd, ".dynamic");
+      s = bfd_section_from_elf_index (abfd, elf_dynamic (abfd));
       if (s != NULL)
 	{
 	  bfd_byte *dynbuf;
 	  bfd_byte *extdyn;
-	  unsigned int elfsec;
 	  unsigned long shlink;

 	  if (!bfd_malloc_and_get_section (abfd, s, &dynbuf))
@@ -3517,10 +3516,7 @@ error_free_dyn:
 	      goto error_return;
 	    }

-	  elfsec = _bfd_elf_section_from_bfd_section (abfd, s);
-	  if (elfsec == SHN_BAD)
-	    goto error_free_dyn;
-	  shlink = elf_elfsections (abfd)[elfsec]->sh_link;
+	  shlink = elf_elfsections (abfd)[elf_dynamic (abfd)]->sh_link;

 	  for (extdyn = dynbuf;
 	       extdyn < dynbuf + s->size;
@@ -3635,12 +3631,15 @@ error_free_dyn:

       /* We do not want to include any of the sections in a dynamic
 	 object in the output file.  We hack by simply clobbering the
-	 list of sections in the BFD.  This could be handled more
-	 cleanly by, say, a new section flag; the existing
-	 SEC_NEVER_LOAD flag is not the one we want, because that one
-	 still implies that the section takes up space in the output
-	 file.  */
-      bfd_section_list_clear (abfd);
+	 list of sections in the BFD, unless this BFD is being used to
+	 hold linker-generated sections (which implies that the
+	 original section list has already been clobbered).  This
+	 could be handled more cleanly by, say, a new section flag; the
+	 existing SEC_NEVER_LOAD flag is not the one we want, because
+	 that one still implies that the section takes up space in the
+	 output file.  */
+      if (htab->dynobj != abfd)
+        bfd_section_list_clear (abfd);

       /* Find the name to use in a DT_NEEDED entry that refers to this
 	 object.  If the object has a DT_SONAME entry, we use it.