bfd_make_section

bfd_make_section

[Alan Modra]

Near the end of 2001, I modified bfd to use a hash table for searches
of sections by name.  In the process, the return from bfd_make_section
on finding an existing section, was accidentally changed.  This function
is supposed to return NULL when the section already exists, to catch
places where it's an error to try to make a section twice.

This fix will almost certainly expose errors in some ports, as I found
with ppc32 trying to create .rela.got twice.  If you see new ld
testsuite failures, particularly bootstrap ones, it's probably due to
the same thing.

	* section.c (bfd_make_section): Return NULL for existing section.

Index: bfd/section.c
===================================================================
RCS file: /cvs/src/src/bfd/section.c,v
retrieving revision 1.57
diff -u -p -r1.57 section.c
--- bfd/section.c	29 Jun 2003 10:06:39 -0000	1.57
+++ bfd/section.c	4 Jul 2003 02:00:16 -0000
@@ -1001,7 +1001,7 @@ bfd_make_section (bfd *abfd, const char 
   if (newsect->name != NULL)
     {
       /* Section already exists.  */
-      return newsect;
+      return NULL;
     }
 
   newsect->name = name;

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: bfd_make_section

[Alan Modra]

On Fri, Jul 04, 2003 at 01:42:56PM +0930, Alan Modra wrote:
> This fix will almost certainly expose errors in some ports, as I found
> with ppc32 trying to create .rela.got twice.  If you see new ld

	* elf32-sparc.c (elf32_sparc_check_relocs): Don't call
	create_got_section if we already have done so.
	* elf32-sh.c (sh_elf_create_dynamic_sections): Likewise.
	(sh_elf_check_relocs): Likewise.
	(sh_elf_adjust_dynamic_symbol): Delete "dynobj" var.  Use
	htab->root.dynobj instead.
	(sh_elf_check_relocs): Likewise.
	(sh_elf_finish_dynamic_sections): Likewise.

Index: bfd/elf32-sparc.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-sparc.c,v
retrieving revision 1.51
diff -u -p -r1.51 elf32-sparc.c
--- bfd/elf32-sparc.c	25 Jun 2003 06:40:24 -0000	1.51
+++ bfd/elf32-sparc.c	4 Jul 2003 04:47:57 -0000
@@ -1024,10 +1024,13 @@ elf32_sparc_check_relocs (abfd, info, se
 	      }
 	  }
 
-	  if (htab->elf.dynobj == NULL)
-	    htab->elf.dynobj = abfd;
-	  if (!create_got_section (htab->elf.dynobj, info))
-	    return FALSE;
+	  if (htab->sgot == NULL)
+	    {
+	      if (htab->elf.dynobj == NULL)
+		htab->elf.dynobj = abfd;
+	      if (!create_got_section (htab->elf.dynobj, info))
+		return FALSE;
+	    }
 	  break;
 
 	case R_SPARC_TLS_GD_CALL:
Index: bfd/elf32-sh.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-sh.c,v
retrieving revision 1.81
diff -u -p -r1.81 elf32-sh.c
--- bfd/elf32-sh.c	25 Jun 2003 06:40:20 -0000	1.81
+++ bfd/elf32-sh.c	4 Jul 2003 04:48:00 -0000
@@ -3805,7 +3805,8 @@ sh_elf_create_dynamic_sections (abfd, in
       || ! bfd_set_section_alignment (abfd, s, ptralign))
     return FALSE;
 
-  if (! create_got_section (abfd, info))
+  if (htab->sgot == NULL
+      && !create_got_section (abfd, info))
     return FALSE;
 
   {
@@ -3891,10 +3892,10 @@ sh_elf_adjust_dynamic_symbol (info, h)
   asection *s;
   unsigned int power_of_two;
 
-  dynobj = elf_hash_table (info)->dynobj;
+  htab = sh_elf_hash_table (info);
 
   /* Make sure we know what is going on here.  */
-  BFD_ASSERT (dynobj != NULL
+  BFD_ASSERT (htab->root.dynobj != NULL
 	      && ((h->elf_link_hash_flags & ELF_LINK_HASH_NEEDS_PLT)
 		  || h->weakdef != NULL
 		  || ((h->elf_link_hash_flags
@@ -3996,7 +3997,6 @@ sh_elf_adjust_dynamic_symbol (info, h)
      both the dynamic object and the regular object will refer to the
      same memory location for the variable.  */
 
-  htab = sh_elf_hash_table (info);
   s = htab->sdynbss;
   BFD_ASSERT (s != NULL);
 
@@ -4022,9 +4022,9 @@ sh_elf_adjust_dynamic_symbol (info, h)
 
   /* Apply the required alignment.  */
   s->_raw_size = BFD_ALIGN (s->_raw_size, (bfd_size_type) (1 << power_of_two));
-  if (power_of_two > bfd_get_section_alignment (dynobj, s))
+  if (power_of_two > bfd_get_section_alignment (htab->root.dynobj, s))
     {
-      if (! bfd_set_section_alignment (dynobj, s, power_of_two))
+      if (! bfd_set_section_alignment (htab->root.dynobj, s, power_of_two))
 	return FALSE;
     }
 
@@ -6199,7 +6199,6 @@ sh_elf_check_relocs (abfd, info, sec, re
   struct elf_sh_link_hash_table *htab;
   const Elf_Internal_Rela *rel;
   const Elf_Internal_Rela *rel_end;
-  bfd *dynobj;
   bfd_vma *local_got_offsets;
   asection *sgot;
   asection *srelgot;
@@ -6221,7 +6220,6 @@ sh_elf_check_relocs (abfd, info, sec, re
     sym_hashes_end -= symtab_hdr->sh_info;
 
   htab = sh_elf_hash_table (info);
-  dynobj = htab->root.dynobj;
   local_got_offsets = elf_local_got_offsets (abfd);
 
   rel_end = relocs + sec->reloc_count;
@@ -6295,10 +6293,13 @@ sh_elf_check_relocs (abfd, info, sec, re
 	    case R_SH_TLS_GD_32:
 	    case R_SH_TLS_LD_32:
 	    case R_SH_TLS_IE_32:
-	      if (dynobj == NULL)
-		htab->root.dynobj = dynobj = abfd;
-	      if (! create_got_section (dynobj, info))
-		return FALSE;
+	      if (htab->sgot == NULL)
+		{
+		  if (htab->root.dynobj == NULL)
+		    htab->root.dynobj = abfd;
+		  if (!create_got_section (htab->root.dynobj, info))
+		    return FALSE;
+		}
 	      break;
 
 	    default:
@@ -6535,8 +6536,8 @@ sh_elf_check_relocs (abfd, info, sec, re
 	      struct elf_sh_dyn_relocs *p;
 	      struct elf_sh_dyn_relocs **head;
 
-	      if (dynobj == NULL)
-		htab->root.dynobj = dynobj = abfd;
+	      if (htab->root.dynobj == NULL)
+		htab->root.dynobj = abfd;
 
 	      /* When creating a shared object, we must copy these
 		 reloc types into the output file.  We create a reloc
@@ -6556,19 +6557,21 @@ sh_elf_check_relocs (abfd, info, sec, re
 			      && strcmp (bfd_get_section_name (abfd, sec),
 					 name + 5) == 0);
 
-		  sreloc = bfd_get_section_by_name (dynobj, name);
+		  sreloc = bfd_get_section_by_name (htab->root.dynobj, name);
 		  if (sreloc == NULL)
 		    {
 		      flagword flags;
 
-		      sreloc = bfd_make_section (dynobj, name);
+		      sreloc = bfd_make_section (htab->root.dynobj, name);
 		      flags = (SEC_HAS_CONTENTS | SEC_READONLY
 			       | SEC_IN_MEMORY | SEC_LINKER_CREATED);
 		      if ((sec->flags & SEC_ALLOC) != 0)
 			flags |= SEC_ALLOC | SEC_LOAD;
 		      if (sreloc == NULL
-			  || ! bfd_set_section_flags (dynobj, sreloc, flags)
-			  || ! bfd_set_section_alignment (dynobj, sreloc, 2))
+			  || ! bfd_set_section_flags (htab->root.dynobj,
+						      sreloc, flags)
+			  || ! bfd_set_section_alignment (htab->root.dynobj,
+							  sreloc, 2))
 			return FALSE;
 		    }
 		  if (sec->flags & SEC_READONLY)
@@ -6598,7 +6601,7 @@ sh_elf_check_relocs (abfd, info, sec, re
 	      if (p == NULL || p->sec != sec)
 		{
 		  bfd_size_type amt = sizeof (*p);
-		  p = ((struct elf_sh_dyn_relocs *) bfd_alloc (dynobj, amt));
+		  p = bfd_alloc (htab->root.dynobj, amt);
 		  if (p == NULL)
 		    return FALSE;
 		  p->next = *head;
@@ -6788,10 +6791,8 @@ sh_elf_finish_dynamic_symbol (output_bfd
      Elf_Internal_Sym *sym;
 {
   struct elf_sh_link_hash_table *htab;
-  bfd *dynobj;
 
   htab = sh_elf_hash_table (info);
-  dynobj = htab->root.dynobj;
 
   if (h->plt.offset != (bfd_vma) -1)
     {
@@ -7076,15 +7077,12 @@ sh_elf_finish_dynamic_sections (output_b
      struct bfd_link_info *info;
 {
   struct elf_sh_link_hash_table *htab;
-  bfd *dynobj;
   asection *sgot;
   asection *sdyn;
 
   htab = sh_elf_hash_table (info);
-  dynobj = htab->root.dynobj;
-
   sgot = htab->sgotplt;
-  sdyn = bfd_get_section_by_name (dynobj, ".dynamic");
+  sdyn = bfd_get_section_by_name (htab->root.dynobj, ".dynamic");
 
   if (htab->root.dynamic_sections_created)
     {
@@ -7103,7 +7101,7 @@ sh_elf_finish_dynamic_sections (output_b
 	  const char *name;
 #endif
 
-	  bfd_elf32_swap_dyn_in (dynobj, dyncon, &dyn);
+	  bfd_elf32_swap_dyn_in (htab->root.dynobj, dyncon, &dyn);
 
 	  switch (dyn.d_tag)
 	    {

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: bfd_make_section

[kaz Kojima]

Hi,

Alan Modra <amodra@bigpond.net.au> wrote:
> On Fri, Jul 04, 2003 at 01:42:56PM +0930, Alan Modra wrote:
>> This fix will almost certainly expose errors in some ports, as I found
>> with ppc32 trying to create .rela.got twice.  If you see new ld
> 
> 	* elf32-sparc.c (elf32_sparc_check_relocs): Don't call
> 	create_got_section if we already have done so.
> 	* elf32-sh.c (sh_elf_create_dynamic_sections): Likewise.
> 	(sh_elf_check_relocs): Likewise.
> 	(sh_elf_adjust_dynamic_symbol): Delete "dynobj" var.  Use
> 	htab->root.dynobj instead.
> 	(sh_elf_check_relocs): Likewise.
> 	(sh_elf_finish_dynamic_sections): Likewise.

I've found that similar problems cause 2 new regressions on ld test and
glibc build failure on sh4-unknown-linux-gnu. With the attached patch,
these failures went away. I'd like to check it in soon.

Regards,
	kaz
--
2003-07-04  Kaz Kojima  <kkojima@rr.iij4u.or.jp>

	* elf32-sh.c (sh_elf_create_dynamic_sections): Don't call
	bfd_make_section for existing sections.

diff -u3prN ORIG/src/bfd/elf32-sh.c LOCAL/src/bfd/elf32-sh.c
--- ORIG/src/bfd/elf32-sh.c	Fri Jul  4 14:27:21 2003
+++ LOCAL/src/bfd/elf32-sh.c	Fri Jul  4 14:30:14 2003
@@ -3768,12 +3768,16 @@ sh_elf_create_dynamic_sections (abfd, in
   if (bed->plt_readonly)
     pltflags |= SEC_READONLY;
 
-  s = bfd_make_section (abfd, ".plt");
-  htab->splt = s;
-  if (s == NULL
-      || ! bfd_set_section_flags (abfd, s, pltflags)
-      || ! bfd_set_section_alignment (abfd, s, bed->plt_alignment))
-    return FALSE;
+  s = htab->splt;
+  if (s == NULL)
+    {
+      s = bfd_make_section (abfd, ".plt");
+      htab->splt = s;
+      if (s == NULL
+	  || ! bfd_set_section_flags (abfd, s, pltflags)
+	  || ! bfd_set_section_alignment (abfd, s, bed->plt_alignment))
+	return FALSE;
+    }
 
   if (bed->want_plt_sym)
     {
@@ -3797,13 +3801,17 @@ sh_elf_create_dynamic_sections (abfd, in
 	return FALSE;
     }
 
-  s = bfd_make_section (abfd,
-			bed->default_use_rela_p ? ".rela.plt" : ".rel.plt");
-  htab->srelplt = s;
-  if (s == NULL
-      || ! bfd_set_section_flags (abfd, s, flags | SEC_READONLY)
-      || ! bfd_set_section_alignment (abfd, s, ptralign))
-    return FALSE;
+  if (htab->srelplt == NULL)
+    {
+      s = bfd_make_section (abfd,
+			    (bed->default_use_rela_p ?
+			     ".rela.plt" : ".rel.plt"));
+      htab->srelplt = s;
+      if (s == NULL
+	  || ! bfd_set_section_flags (abfd, s, flags | SEC_READONLY)
+	  || ! bfd_set_section_alignment (abfd, s, ptralign))
+	return FALSE;
+    }
 
   if (htab->sgot == NULL
       && !create_got_section (abfd, info))
@@ -3825,6 +3833,8 @@ sh_elf_create_dynamic_sections (abfd, in
 	relname = (char *) bfd_malloc ((bfd_size_type) strlen (secname) + 6);
 	strcpy (relname, ".rela");
 	strcat (relname, secname);
+	if (bfd_get_section_by_name (abfd, secname))
+	  continue;
 	s = bfd_make_section (abfd, relname);
 	if (s == NULL
 	    || ! bfd_set_section_flags (abfd, s, flags | SEC_READONLY)
@@ -3841,11 +3851,14 @@ sh_elf_create_dynamic_sections (abfd, in
 	 image and use a R_*_COPY reloc to tell the dynamic linker to
 	 initialize them at run time.  The linker script puts the .dynbss
 	 section into the .bss section of the final image.  */
-      s = bfd_make_section (abfd, ".dynbss");
-      htab->sdynbss = s;
-      if (s == NULL
-	  || ! bfd_set_section_flags (abfd, s, SEC_ALLOC))
-	return FALSE;
+      if (htab->sdynbss == NULL)
+	{
+	  s = bfd_make_section (abfd, ".dynbss");
+	  htab->sdynbss = s;
+	  if (s == NULL
+	      || ! bfd_set_section_flags (abfd, s, SEC_ALLOC))
+	    return FALSE;
+	}
 
       /* The .rel[a].bss section holds copy relocs.  This section is not
 	 normally needed.  We need to create it here, though, so that the
@@ -3858,7 +3871,7 @@ sh_elf_create_dynamic_sections (abfd, in
 	 be needed, we can discard it later.  We will never need this
 	 section when generating a shared object, since they do not use
 	 copy relocs.  */
-      if (! info->shared)
+      if (! info->shared && htab->srelbss == NULL)
 	{
 	  s = bfd_make_section (abfd,
 				(bed->default_use_rela_p

Re: bfd_make_section

[Alan Modra]

Hi,

On Fri, Jul 04, 2003 at 03:26:34PM +0900, kaz Kojima wrote:
> 	* elf32-sh.c (sh_elf_create_dynamic_sections): Don't call
> 	bfd_make_section for existing sections.

Wouldn't a test of htab->root.dynamic_sections_created near the start of
sh_elf_create_dynamic_sections avoid all these extra tests?

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: bfd_make_section

[kaz Kojima]

Alan Modra <amodra@bigpond.net.au> wrote:
>> 	* elf32-sh.c (sh_elf_create_dynamic_sections): Don't call
>> 	bfd_make_section for existing sections.
>
> Wouldn't a test of htab->root.dynamic_sections_created near the start of
> sh_elf_create_dynamic_sections avoid all these extra tests?

Oops. It cleans all changes except one hunk of my previous patch.
The appended patch reverts unnecessary tests and adds a test for
htab->root.dynamic_sections_created.
Thanks a lot.

Regards,
	kaz
--
2003-07-04  Kaz Kojima  <kkojima@rr.iij4u.or.jp>

	* elf32-sh.c (sh_elf_create_dynamic_sections): Return if dynamic sections
	created already. Remove unnecessary tests of the previous change.

--- ORIG/src/bfd/elf32-sh.c	Fri Jul  4 16:49:27 2003
+++ LOCAL/src/bfd/elf32-sh.c	Fri Jul  4 17:05:10 2003
@@ -3754,6 +3754,8 @@ sh_elf_create_dynamic_sections (abfd, in
     }
 
   htab = sh_elf_hash_table (info);
+  if (htab->root.dynamic_sections_created)
+    return TRUE;
 
   /* We need to create .plt, .rel[a].plt, .got, .got.plt, .dynbss, and
      .rel[a].bss sections.  */
@@ -3768,16 +3770,12 @@ sh_elf_create_dynamic_sections (abfd, in
   if (bed->plt_readonly)
     pltflags |= SEC_READONLY;
 
-  s = htab->splt;
-  if (s == NULL)
-    {
-      s = bfd_make_section (abfd, ".plt");
-      htab->splt = s;
-      if (s == NULL
-	  || ! bfd_set_section_flags (abfd, s, pltflags)
-	  || ! bfd_set_section_alignment (abfd, s, bed->plt_alignment))
-	return FALSE;
-    }
+  s = bfd_make_section (abfd, ".plt");
+  htab->splt = s;
+  if (s == NULL
+      || ! bfd_set_section_flags (abfd, s, pltflags)
+      || ! bfd_set_section_alignment (abfd, s, bed->plt_alignment))
+    return FALSE;
 
   if (bed->want_plt_sym)
     {
@@ -3801,17 +3799,13 @@ sh_elf_create_dynamic_sections (abfd, in
 	return FALSE;
     }
 
-  if (htab->srelplt == NULL)
-    {
-      s = bfd_make_section (abfd,
-			    (bed->default_use_rela_p ?
-			     ".rela.plt" : ".rel.plt"));
-      htab->srelplt = s;
-      if (s == NULL
-	  || ! bfd_set_section_flags (abfd, s, flags | SEC_READONLY)
-	  || ! bfd_set_section_alignment (abfd, s, ptralign))
-	return FALSE;
-    }
+  s = bfd_make_section (abfd,
+			bed->default_use_rela_p ? ".rela.plt" : ".rel.plt");
+  htab->srelplt = s;
+  if (s == NULL
+      || ! bfd_set_section_flags (abfd, s, flags | SEC_READONLY)
+      || ! bfd_set_section_alignment (abfd, s, ptralign))
+    return FALSE;
 
   if (htab->sgot == NULL
       && !create_got_section (abfd, info))
@@ -3851,14 +3845,11 @@ sh_elf_create_dynamic_sections (abfd, in
 	 image and use a R_*_COPY reloc to tell the dynamic linker to
 	 initialize them at run time.  The linker script puts the .dynbss
 	 section into the .bss section of the final image.  */
-      if (htab->sdynbss == NULL)
-	{
-	  s = bfd_make_section (abfd, ".dynbss");
-	  htab->sdynbss = s;
-	  if (s == NULL
-	      || ! bfd_set_section_flags (abfd, s, SEC_ALLOC))
-	    return FALSE;
-	}
+      s = bfd_make_section (abfd, ".dynbss");
+      htab->sdynbss = s;
+      if (s == NULL
+	  || ! bfd_set_section_flags (abfd, s, SEC_ALLOC))
+	return FALSE;
 
       /* The .rel[a].bss section holds copy relocs.  This section is not
 	 normally needed.  We need to create it here, though, so that the
@@ -3871,7 +3862,7 @@ sh_elf_create_dynamic_sections (abfd, in
 	 be needed, we can discard it later.  We will never need this
 	 section when generating a shared object, since they do not use
 	 copy relocs.  */
-      if (! info->shared && htab->srelbss == NULL)
+      if (! info->shared)
 	{
 	  s = bfd_make_section (abfd,
 				(bed->default_use_rela_p

Re: bfd_make_section

[kaz Kojima]

kaz Kojima <kkojima@rr.iij4u.or.jp> wrote:
> 	* elf32-sh.c (sh_elf_create_dynamic_sections): Return if dynamic sections
> 	created already. Remove unnecessary tests of the previous change.

Ugh. cut&paste failure. I've checked the patch in with the following
ChangeLog entry.

	* elf32-sh.c (sh_elf_create_dynamic_sections): Return if dynamic
	sections were created already.  Remove unnecessary tests of
	the previous change.

Regards,
	kaz