PATCH: PR ld/6931: COMDAT group is broken

PATCH: PR ld/6931: COMDAT group is broken

[H.J. Lu]

bfd_elf_set_group_contents needs to handle group signature properly for
ld -r. The sh_info field of group section has the symbol table index 
of group signature. We need to get the symbol table index of group
signature in output file. We start with the symbol table index for
group signature in input file. If it is a global symbol, we can get
its index from ELF symbol hash table. If it is a local symbol, we
have to first get the symbol table entry for group signature in
input file and then compute the symbol table index in output file.

OK to install?


H.J.
--
bfd/

2008-10-02  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/6931
	* elf.c (bfd_elf_set_group_contents): Properly get the symbol
	table index for group signature for ld -r.

ld/testsuite/

2008-10-02  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/6931
	* ld-elf/group4.d: New.
	* ld-elf/group5.d: Likewise.
	* ld-elf/group6.d: Likewise.

--- binutils/bfd/elf.c.grp	2008-10-02 13:52:45.000000000 -0700
+++ binutils/bfd/elf.c	2008-10-02 14:00:14.000000000 -0700
@@ -2715,11 +2715,73 @@ bfd_elf_set_group_contents (bfd *abfd, a
   if (symindx == 0)
     {
       /* If called from the assembler, swap_out_syms will have set up
-	 elf_section_syms;  If called for "ld -r", use target_index.  */
+	 elf_section_syms.  */
       if (elf_section_syms (abfd) != NULL)
 	symindx = elf_section_syms (abfd)[sec->index]->udata.i;
       else
-	symindx = sec->target_index;
+	{
+	  struct bfd_link_order *lo;
+	  asection *input;
+	  Elf_Internal_Shdr *symtab_hdr;
+
+	  /* Handle "ld -r".  */
+	  input = NULL;
+	  for (lo = sec->map_head.link_order; lo != NULL; lo = lo->next)
+	    if (lo->type == bfd_indirect_link_order)
+	      {
+		/* There should be only one input section.  */
+		BFD_ASSERT (input == NULL);
+
+		input = lo->u.indirect.section;
+		/* Get the symbol table index for group signature in
+		   input file.  */
+		symindx = elf_section_data(input)->this_hdr.sh_info;
+	      }
+
+	  BFD_ASSERT (input != NULL);
+
+	  symtab_hdr = &elf_symtab_hdr (input->owner);
+	  if (symindx < symtab_hdr->sh_info)
+	    {
+	      unsigned char esym[sizeof (Elf64_External_Sym)];
+	      Elf_External_Sym_Shndx eshndx;
+	      Elf_Internal_Sym isym;
+	      Elf_Internal_Shdr *elf_sect;
+	      int target_index;
+
+	      /* Get the symbol table entry for group signature in
+		 input file.  */
+	      if (bfd_elf_get_elf_syms (input->owner, symtab_hdr, 1, 
+					symindx, &isym, esym, &eshndx)
+		  == NULL)
+		abort ();
+
+	      /* Get the group signature section in input file.  */
+	      elf_sect = elf_elfsections (input->owner)[isym.st_shndx];
+
+	      /* Compute the symbol table index for group signature
+		 in output file.  */
+	      target_index
+		= elf_sect->bfd_section->output_section->target_index;
+	      if (ELF_ST_TYPE (isym.st_info) == STT_SECTION)
+		symindx = target_index;
+	      else
+		symindx += target_index;
+	    }
+	  else
+	    {
+	      struct elf_link_hash_entry **sym_hashes;
+	      struct elf_link_hash_entry *h;
+
+	      sym_hashes = elf_sym_hashes (input->owner);
+	      h = sym_hashes[symindx - symtab_hdr->sh_info];
+	      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;
+	      symindx = h->indx;
+	    }
+	}
+      BFD_ASSERT (symindx != 0);
     }
   elf_section_data (sec)->this_hdr.sh_info = symindx;
 
--- binutils/ld/testsuite/ld-elf/group4.d.grp	2008-10-02 13:52:46.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/group4.d	2008-10-02 13:52:46.000000000 -0700
@@ -0,0 +1,16 @@
+#source: ../../../binutils/testsuite/binutils-all/group-2.s
+#ld: -r
+#readelf: -Sg --wide
+
+#...
+  \[[ 0-9]+\] .group[ \t]+GROUP[ \t]+.*
+#...
+  \[[ 0-9]+\] \.text.*[ \t]+PROGBITS[ \t0-9a-f]+AXG[ \t]+.*
+#...
+  \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG[ \t]+.*
+#...
+COMDAT group section \[[ 0-9]+\] `.group' \[.text.foo\] contains 2 sections:
+   \[Index\]    Name
+   \[[ 0-9]+\]   .text.*
+   \[[ 0-9]+\]   .data.*
+#pass
--- binutils/ld/testsuite/ld-elf/group5.d.grp	2008-10-02 13:52:46.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/group5.d	2008-10-02 13:52:46.000000000 -0700
@@ -0,0 +1,16 @@
+#source: ../../../binutils/testsuite/binutils-all/group-3.s
+#ld: -r
+#readelf: -Sg --wide
+
+#...
+  \[[ 0-9]+\] .group[ \t]+GROUP[ \t]+.*
+#...
+  \[[ 0-9]+\] \.text.*[ \t]+PROGBITS[ \t0-9a-f]+AXG[ \t]+.*
+#...
+  \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG[ \t]+.*
+#...
+COMDAT group section \[[ 0-9]+\] `.group' \[foo\] contains 2 sections:
+   \[Index\]    Name
+   \[[ 0-9]+\]   .text.*
+   \[[ 0-9]+\]   .data.*
+#pass
--- binutils/ld/testsuite/ld-elf/group6.d.grp	2008-10-02 13:52:46.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/group6.d	2008-10-02 13:52:46.000000000 -0700
@@ -0,0 +1,16 @@
+#source: ../../../binutils/testsuite/binutils-all/group-4.s
+#ld: -r
+#readelf: -Sg --wide
+
+#...
+  \[[ 0-9]+\] .group[ \t]+GROUP[ \t]+.*
+#...
+  \[[ 0-9]+\] \.text.*[ \t]+PROGBITS[ \t0-9a-f]+AXG[ \t]+.*
+#...
+  \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG[ \t]+.*
+#...
+COMDAT group section \[[ 0-9]+\] `.group' \[foo\] contains 2 sections:
+   \[Index\]    Name
+   \[[ 0-9]+\]   .text.*
+   \[[ 0-9]+\]   .data.*
+#pass

Re: PATCH: PR ld/6931: COMDAT group is broken

[H.J. Lu]

It doesn't work on

[hjl@gnu-6 ld]$ cat /tmp/group-4.s
	.section .text.foo4,"axG",%progbits,foo4,comdat
foo4:
	.word 0
	.section .data.foo4,"awG",%progbits,foo4,comdat
bar4:
	.word 0
[hjl@gnu-6 ld]$ gcc -c /tmp/group-4.s
[hjl@gnu-6 ld]$  ./ld-new -r group-4.o
[hjl@gnu-6 ld]$ readelf -Sg a.out
There are 10 section headers, starting at offset 0xa0:

Section Headers:
  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
  [ 0]                   NULL             0000000000000000  00000000
       0000000000000000  0000000000000000           0     0     0
  [ 1] .group            GROUP            0000000000000000  00000040
       000000000000000c  0000000000000004           8     8     4
  [ 2] .text             PROGBITS         0000000000000000  0000004c
       0000000000000000  0000000000000000  AX       0     0     4
  [ 3] .text.foo4        PROGBITS         0000000000000000  0000004c
       0000000000000002  0000000000000000 AXG       0     0     1
  [ 4] .data             PROGBITS         0000000000000000  00000050
       0000000000000000  0000000000000000  WA       0     0     4
  [ 5] .data.foo4        PROGBITS         0000000000000000  00000050
       0000000000000002  0000000000000000 WAG       0     0     1
  [ 6] .bss              NOBITS           0000000000000000  00000054
       0000000000000000  0000000000000000  WA       0     0     4
  [ 7] .shstrtab         STRTAB           0000000000000000  00000054
       0000000000000049  0000000000000000           0     0     1
  [ 8] .symtab           SYMTAB           0000000000000000  00000320
       00000000000000d8  0000000000000018           9     9     8
  [ 9] .strtab           STRTAB           0000000000000000  000003f8
       000000000000000b  0000000000000000           0     0     1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings)
  I (info), L (link order), G (group), x (unknown)
  O (extra OS processing required) o (OS specific), p (processor specific)

COMDAT group section [    1] `.group' [bar4] contains 2 sections:
   [Index]    Name
   [    3]   .text.foo4
   [    5]   .data.foo4
[hjl@gnu-6 ld]$


H.J.

On Thu, Oct 2, 2008 at 2:42 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> bfd_elf_set_group_contents needs to handle group signature properly for
> ld -r. The sh_info field of group section has the symbol table index
> of group signature. We need to get the symbol table index of group
> signature in output file. We start with the symbol table index for
> group signature in input file. If it is a global symbol, we can get
> its index from ELF symbol hash table. If it is a local symbol, we
> have to first get the symbol table entry for group signature in
> input file and then compute the symbol table index in output file.
>
> OK to install?
>
>
> H.J.
> --
> bfd/
>
> 2008-10-02  H.J. Lu  <hongjiu.lu@intel.com>
>
>        PR ld/6931
>        * elf.c (bfd_elf_set_group_contents): Properly get the symbol
>        table index for group signature for ld -r.
>
> ld/testsuite/
>
> 2008-10-02  H.J. Lu  <hongjiu.lu@intel.com>
>
>        PR ld/6931
>        * ld-elf/group4.d: New.
>        * ld-elf/group5.d: Likewise.
>        * ld-elf/group6.d: Likewise.
>
> --- binutils/bfd/elf.c.grp      2008-10-02 13:52:45.000000000 -0700
> +++ binutils/bfd/elf.c  2008-10-02 14:00:14.000000000 -0700
> @@ -2715,11 +2715,73 @@ bfd_elf_set_group_contents (bfd *abfd, a
>   if (symindx == 0)
>     {
>       /* If called from the assembler, swap_out_syms will have set up
> -        elf_section_syms;  If called for "ld -r", use target_index.  */
> +        elf_section_syms.  */
>       if (elf_section_syms (abfd) != NULL)
>        symindx = elf_section_syms (abfd)[sec->index]->udata.i;
>       else
> -       symindx = sec->target_index;
> +       {
> +         struct bfd_link_order *lo;
> +         asection *input;
> +         Elf_Internal_Shdr *symtab_hdr;
> +
> +         /* Handle "ld -r".  */
> +         input = NULL;
> +         for (lo = sec->map_head.link_order; lo != NULL; lo = lo->next)
> +           if (lo->type == bfd_indirect_link_order)
> +             {
> +               /* There should be only one input section.  */
> +               BFD_ASSERT (input == NULL);
> +
> +               input = lo->u.indirect.section;
> +               /* Get the symbol table index for group signature in
> +                  input file.  */
> +               symindx = elf_section_data(input)->this_hdr.sh_info;
> +             }
> +
> +         BFD_ASSERT (input != NULL);
> +
> +         symtab_hdr = &elf_symtab_hdr (input->owner);
> +         if (symindx < symtab_hdr->sh_info)
> +           {
> +             unsigned char esym[sizeof (Elf64_External_Sym)];
> +             Elf_External_Sym_Shndx eshndx;
> +             Elf_Internal_Sym isym;
> +             Elf_Internal_Shdr *elf_sect;
> +             int target_index;
> +
> +             /* Get the symbol table entry for group signature in
> +                input file.  */
> +             if (bfd_elf_get_elf_syms (input->owner, symtab_hdr, 1,
> +                                       symindx, &isym, esym, &eshndx)
> +                 == NULL)
> +               abort ();
> +
> +             /* Get the group signature section in input file.  */
> +             elf_sect = elf_elfsections (input->owner)[isym.st_shndx];
> +
> +             /* Compute the symbol table index for group signature
> +                in output file.  */
> +             target_index
> +               = elf_sect->bfd_section->output_section->target_index;
> +             if (ELF_ST_TYPE (isym.st_info) == STT_SECTION)
> +               symindx = target_index;
> +             else
> +               symindx += target_index;
> +           }
> +         else
> +           {
> +             struct elf_link_hash_entry **sym_hashes;
> +             struct elf_link_hash_entry *h;
> +
> +             sym_hashes = elf_sym_hashes (input->owner);
> +             h = sym_hashes[symindx - symtab_hdr->sh_info];
> +             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;
> +             symindx = h->indx;
> +           }
> +       }
> +      BFD_ASSERT (symindx != 0);
>     }
>   elf_section_data (sec)->this_hdr.sh_info = symindx;
>
> --- binutils/ld/testsuite/ld-elf/group4.d.grp   2008-10-02 13:52:46.000000000 -0700
> +++ binutils/ld/testsuite/ld-elf/group4.d       2008-10-02 13:52:46.000000000 -0700
> @@ -0,0 +1,16 @@
> +#source: ../../../binutils/testsuite/binutils-all/group-2.s
> +#ld: -r
> +#readelf: -Sg --wide
> +
> +#...
> +  \[[ 0-9]+\] .group[ \t]+GROUP[ \t]+.*
> +#...
> +  \[[ 0-9]+\] \.text.*[ \t]+PROGBITS[ \t0-9a-f]+AXG[ \t]+.*
> +#...
> +  \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG[ \t]+.*
> +#...
> +COMDAT group section \[[ 0-9]+\] `.group' \[.text.foo\] contains 2 sections:
> +   \[Index\]    Name
> +   \[[ 0-9]+\]   .text.*
> +   \[[ 0-9]+\]   .data.*
> +#pass
> --- binutils/ld/testsuite/ld-elf/group5.d.grp   2008-10-02 13:52:46.000000000 -0700
> +++ binutils/ld/testsuite/ld-elf/group5.d       2008-10-02 13:52:46.000000000 -0700
> @@ -0,0 +1,16 @@
> +#source: ../../../binutils/testsuite/binutils-all/group-3.s
> +#ld: -r
> +#readelf: -Sg --wide
> +
> +#...
> +  \[[ 0-9]+\] .group[ \t]+GROUP[ \t]+.*
> +#...
> +  \[[ 0-9]+\] \.text.*[ \t]+PROGBITS[ \t0-9a-f]+AXG[ \t]+.*
> +#...
> +  \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG[ \t]+.*
> +#...
> +COMDAT group section \[[ 0-9]+\] `.group' \[foo\] contains 2 sections:
> +   \[Index\]    Name
> +   \[[ 0-9]+\]   .text.*
> +   \[[ 0-9]+\]   .data.*
> +#pass
> --- binutils/ld/testsuite/ld-elf/group6.d.grp   2008-10-02 13:52:46.000000000 -0700
> +++ binutils/ld/testsuite/ld-elf/group6.d       2008-10-02 13:52:46.000000000 -0700
> @@ -0,0 +1,16 @@
> +#source: ../../../binutils/testsuite/binutils-all/group-4.s
> +#ld: -r
> +#readelf: -Sg --wide
> +
> +#...
> +  \[[ 0-9]+\] .group[ \t]+GROUP[ \t]+.*
> +#...
> +  \[[ 0-9]+\] \.text.*[ \t]+PROGBITS[ \t0-9a-f]+AXG[ \t]+.*
> +#...
> +  \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG[ \t]+.*
> +#...
> +COMDAT group section \[[ 0-9]+\] `.group' \[foo\] contains 2 sections:
> +   \[Index\]    Name
> +   \[[ 0-9]+\]   .text.*
> +   \[[ 0-9]+\]   .data.*
> +#pass
>



-- 
H.J.

Re: PATCH: PR ld/6931: COMDAT group is broken

[Alan Modra]

The elf.c and elflink.c patches below teach the ELF backend linker to
handle group signature symbols for ld -r.  The genelf.em patch does
the same for ELF targets that use the generic linker.

The linker.c change fixes the group section contents for the generic
linker, a nasty hack but nothing else seemed nice either.  Prior to
this change, the generic linker simply copied group section contents
to output for ld -r, which, since section indices almost always
change, broke groups.  The generic linker also mashed together group
sections due to lack of unique sections for groups.  Moving this logic
from elf32.em into ldlang.c cures that particular problem.

Finally, I tweaked the various emulation's place_orphan code to not
use an existing output section statement if it did not already have a
bfd section.  This is because a new bfd section will be created after
all other existing sections, which is probably the wrong ordering.
lang_place_orphan knows how to rearrange sections.

bfd/
	* elf.c (bfd_elf_set_group_contents): Assign sh_info for ld -r when
	the signature symbol is global.
	* elflink.c (elf_link_input_bfd): Ensure group signature symbol
	is output when ld -r.  Set group sh_info when local.
	* linker.c (default_indirect_link_order): Handle group sections
	specially.
ld/
	* ldemul.c (ldemul_place_orphan): Add "name" param.
	* ldemul.h (ldemul_place_orphan): Update prototype.
	(struct ld_emulation_xfer_struct <place_orphan>): Likewise.
	* ldlang.c (lang_place_orphans): Generate unique section names here..
	* emultempl/elf32.em (place_orphan): ..rather than here.  Don't
	directly use an existing output section statement that has no
	bfd section.
	* emultempl/pe.em (place_orphan): Likewise.
	* emultempl/pep.em (place_orphan): Likewise.
	* emultempl/beos.em (place_orphan): Adjust.
	* emultempl/spuelf.em (spu_place_special_section): Adjust
	place_orphan call.
	* emultempl/genelf.em (gld${EMULATION_NAME}_after_open): New function.
	(LDEMUL_AFTER_OPEN): Define.

Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.464
diff -u -p -r1.464 elf.c
--- bfd/elf.c	29 Sep 2008 14:12:02 -0000	1.464
+++ bfd/elf.c	3 Oct 2008 08:38:51 -0000
@@ -2681,13 +2681,15 @@ elf_fake_sections (bfd *abfd, asection *
     *failedptr = TRUE;
 }
 
-/* Fill in the contents of a SHT_GROUP section.  */
+/* Fill in the contents of a SHT_GROUP section.  Called from
+   _bfd_elf_compute_section_file_positions for gas, objcopy, and
+   when ELF targets use the generic linker, ld.  Called for ld -r
+   from bfd_elf_final_link.  */
 
 void
 bfd_elf_set_group_contents (bfd *abfd, asection *sec, void *failedptrarg)
 {
   bfd_boolean *failedptr = failedptrarg;
-  unsigned long symindx;
   asection *elt, *first;
   unsigned char *loc;
   bfd_boolean gas;
@@ -2698,20 +2700,49 @@ bfd_elf_set_group_contents (bfd *abfd, a
       || *failedptr)
     return;
 
-  symindx = 0;
-  if (elf_group_id (sec) != NULL)
-    symindx = elf_group_id (sec)->udata.i;
-
-  if (symindx == 0)
-    {
-      /* If called from the assembler, swap_out_syms will have set up
-	 elf_section_syms;  If called for "ld -r", use target_index.  */
-      if (elf_section_syms (abfd) != NULL)
-	symindx = elf_section_syms (abfd)[sec->index]->udata.i;
-      else
-	symindx = sec->target_index;
+  if (elf_section_data (sec)->this_hdr.sh_info == 0)
+    {
+      unsigned long symindx = 0;
+
+      /* elf_group_id will have been set up by objcopy and the
+	 generic linker.  */
+      if (elf_group_id (sec) != NULL)
+	symindx = elf_group_id (sec)->udata.i;
+
+      if (symindx == 0)
+	{
+	  /* If called from the assembler, swap_out_syms will have set up
+	     elf_section_syms.  */
+	  BFD_ASSERT (elf_section_syms (abfd) != NULL);
+	  symindx = elf_section_syms (abfd)[sec->index]->udata.i;
+	}
+      elf_section_data (sec)->this_hdr.sh_info = symindx;
+    }
+  else if (elf_section_data (sec)->this_hdr.sh_info == (unsigned int) -2)
+    {
+      /* The ELF backend linker sets sh_info to -2 when the group
+	 signature symbol is global, and thus the index can't be
+	 set until all local symbols are output.  */
+      asection *igroup = elf_sec_group (elf_next_in_group (sec));
+      struct bfd_elf_section_data *sec_data = elf_section_data (igroup);
+      unsigned long symndx = sec_data->this_hdr.sh_info;
+      unsigned long extsymoff = 0;
+      struct elf_link_hash_entry *h;
+
+      if (!elf_bad_symtab (igroup->owner))
+	{
+	  Elf_Internal_Shdr *symtab_hdr;
+
+	  symtab_hdr = &elf_tdata (igroup->owner)->symtab_hdr;
+	  extsymoff = symtab_hdr->sh_info;
+	}
+      h = elf_sym_hashes (igroup->owner)[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;
+
+      elf_section_data (sec)->this_hdr.sh_info = h->indx;
     }
-  elf_section_data (sec)->this_hdr.sh_info = symindx;
 
   /* The contents won't be allocated for "ld -r" or objcopy.  */
   gas = TRUE;
Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.312
diff -u -p -r1.312 elflink.c
--- bfd/elflink.c	30 Sep 2008 04:47:50 -0000	1.312
+++ bfd/elflink.c	3 Oct 2008 08:38:58 -0000
@@ -9062,6 +9062,63 @@ elf_link_input_bfd (struct elf_final_lin
 	  continue;
 	}
 
+      if (finfo->info->relocatable
+	  && (o->flags & (SEC_LINKER_CREATED | SEC_GROUP)) == SEC_GROUP)
+	{
+	  /* Deal with the group signature symbol.  */
+	  struct bfd_elf_section_data *sec_data = elf_section_data (o);
+	  unsigned long symndx = sec_data->this_hdr.sh_info;
+	  asection *osec = o->output_section;
+
+	  if (symndx >= locsymcount
+	      || (elf_bad_symtab (input_bfd)
+		  && finfo->sections[symndx] == NULL))
+	    {
+	      struct elf_link_hash_entry *h = sym_hashes[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;
+	      /* Arrange for symbol to be output.  */
+	      h->indx = -2;
+	      elf_section_data (osec)->this_hdr.sh_info = -2;
+	    }
+	  else if (ELF_ST_TYPE (isymbuf[symndx].st_info) == STT_SECTION)
+	    {
+	      /* We'll use the output section target_index.  */
+	      asection *sec = finfo->sections[symndx]->output_section;
+	      elf_section_data (osec)->this_hdr.sh_info = sec->target_index;
+	    }
+	  else
+	    {
+	      if (finfo->indices[symndx] == -1)
+		{
+		  /* Otherwise output the local symbol now.  */
+		  Elf_Internal_Sym sym = isymbuf[symndx];
+		  asection *sec = finfo->sections[symndx]->output_section;
+		  const char *name;
+
+		  name = bfd_elf_string_from_elf_section (input_bfd,
+							  symtab_hdr->sh_link,
+							  sym.st_name);
+		  if (name == NULL)
+		    return FALSE;
+
+		  sym.st_shndx = _bfd_elf_section_from_bfd_section (output_bfd,
+								    sec);
+		  if (sym.st_shndx == SHN_BAD)
+		    return FALSE;
+
+		  sym.st_value += o->output_offset;
+
+		  finfo->indices[symndx] = bfd_get_symcount (output_bfd);
+		  if (! elf_link_output_sym (finfo, name, &sym, o, NULL))
+		    return FALSE;
+		}
+	      elf_section_data (osec)->this_hdr.sh_info
+		= finfo->indices[symndx];
+	    }
+	}
+
       if ((o->flags & SEC_HAS_CONTENTS) == 0
 	  || (o->size == 0 && (o->flags & SEC_RELOC) == 0))
 	continue;
Index: bfd/linker.c
===================================================================
RCS file: /cvs/src/src/bfd/linker.c,v
retrieving revision 1.65
diff -u -p -r1.65 linker.c
--- bfd/linker.c	17 Aug 2008 03:12:49 -0000	1.65
+++ bfd/linker.c	3 Oct 2008 08:38:59 -0000
@@ -2796,18 +2796,36 @@ default_indirect_link_order (bfd *output
 	}
     }
 
-  /* Get and relocate the section contents.  */
-  sec_size = (input_section->rawsize > input_section->size
-	      ? input_section->rawsize
-	      : input_section->size);
-  contents = bfd_malloc (sec_size);
-  if (contents == NULL && sec_size != 0)
-    goto error_return;
-  new_contents = (bfd_get_relocated_section_contents
-		  (output_bfd, info, link_order, contents, info->relocatable,
-		   _bfd_generic_link_get_symbols (input_bfd)));
-  if (!new_contents)
-    goto error_return;
+  if ((output_section->flags & (SEC_GROUP | SEC_LINKER_CREATED)) == SEC_GROUP
+      && input_section->size != 0)
+    {
+      /* Group section contents are set by bfd_elf_set_group_contents.  */
+      if (!output_bfd->output_has_begun)
+	{
+	  /* FIXME: This hack ensures bfd_elf_set_group_contents is called.  */
+	  if (!bfd_set_section_contents (output_bfd, output_section, "", 0, 1))
+	    goto error_return;
+	}
+      new_contents = output_section->contents;
+      BFD_ASSERT (new_contents != NULL);
+      BFD_ASSERT (input_section->output_offset == 0);
+    }
+  else
+    {
+      /* Get and relocate the section contents.  */
+      sec_size = (input_section->rawsize > input_section->size
+		  ? input_section->rawsize
+		  : input_section->size);
+      contents = bfd_malloc (sec_size);
+      if (contents == NULL && sec_size != 0)
+	goto error_return;
+      new_contents = (bfd_get_relocated_section_contents
+		      (output_bfd, info, link_order, contents,
+		       info->relocatable,
+		       _bfd_generic_link_get_symbols (input_bfd)));
+      if (!new_contents)
+	goto error_return;
+    }
 
   /* Output the section contents.  */
   loc = input_section->output_offset * bfd_octets_per_byte (output_bfd);
Index: ld/ldemul.c
===================================================================
RCS file: /cvs/src/src/ld/ldemul.c,v
retrieving revision 1.27
diff -u -p -r1.27 ldemul.c
--- ld/ldemul.c	15 Feb 2008 03:35:53 -0000	1.27
+++ ld/ldemul.c	3 Oct 2008 08:39:00 -0000
@@ -120,10 +120,10 @@ ldemul_open_dynamic_archive (const char 
 }
 
 bfd_boolean
-ldemul_place_orphan (asection *s)
+ldemul_place_orphan (asection *s, const char *name)
 {
   if (ld_emulation->place_orphan)
-    return (*ld_emulation->place_orphan) (s);
+    return (*ld_emulation->place_orphan) (s, name);
   return FALSE;
 }
 
Index: ld/ldemul.h
===================================================================
RCS file: /cvs/src/src/ld/ldemul.h,v
retrieving revision 1.19
diff -u -p -r1.19 ldemul.h
--- ld/ldemul.h	6 Jul 2007 14:09:41 -0000	1.19
+++ ld/ldemul.h	3 Oct 2008 08:39:00 -0000
@@ -59,7 +59,7 @@ extern void ldemul_set_symbols
 extern void ldemul_create_output_section_statements
   (void);
 extern bfd_boolean ldemul_place_orphan
-  (asection *);
+  (asection *, const char *);
 extern bfd_boolean ldemul_parse_args
   (int, char **);
 extern void ldemul_add_options
@@ -152,7 +152,7 @@ typedef struct ld_emulation_xfer_struct 
      the default action should be taken.  This field may be NULL, in
      which case the default action will always be taken.  */
   bfd_boolean (*place_orphan)
-    (asection *);
+    (asection *, const char *);
 
   /* Run after assigning parsing with the args, but before
      reading the script.  Used to initialize symbols used in the script.  */
Index: ld/ldlang.c
===================================================================
RCS file: /cvs/src/src/ld/ldlang.c,v
retrieving revision 1.292
diff -u -p -r1.292 ldlang.c
--- ld/ldlang.c	7 Sep 2008 04:02:30 -0000	1.292
+++ ld/ldlang.c	3 Oct 2008 08:39:04 -0000
@@ -5652,14 +5652,29 @@ lang_place_orphans (void)
 					default_common_section);
 		    }
 		}
-	      else if (ldemul_place_orphan (s))
-		;
 	      else
 		{
-		  lang_output_section_statement_type *os;
+		  const char *name = s->name;
 
-		  os = lang_output_section_statement_lookup (s->name, 0, TRUE);
-		  lang_add_section (&os->children, s, os);
+		  if ((config.unique_orphan_sections
+		       || unique_section_p (s))
+		      && bfd_get_section_by_name (link_info.output_bfd,
+						  name) != NULL)
+		    {
+		      static int count = 1;
+		      name = bfd_get_unique_section_name (link_info.output_bfd,
+							  name, &count);
+		      if (name == NULL)
+			einfo ("%F%P: place_orphan failed: %E\n");
+		    }
+
+		  if (!ldemul_place_orphan (s, name))
+		    {
+		      lang_output_section_statement_type *os;
+		      os = lang_output_section_statement_lookup (name, 0,
+								 TRUE);
+		      lang_add_section (&os->children, s, os);
+		    }
 		}
 	    }
 	}
Index: ld/emultempl/beos.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/beos.em,v
retrieving revision 1.40
diff -u -p -r1.40 beos.em
--- ld/emultempl/beos.em	8 Sep 2008 16:07:46 -0000	1.40
+++ ld/emultempl/beos.em	3 Oct 2008 08:39:04 -0000
@@ -665,9 +665,8 @@ gld_${EMULATION_NAME}_before_allocation 
    which are not mentioned in the linker script.  */
 
 static bfd_boolean
-gld${EMULATION_NAME}_place_orphan (asection *s)
+gld${EMULATION_NAME}_place_orphan (asection *s, const char *secname)
 {
-  const char *secname;
   char *output_secname, *ps;
   lang_output_section_statement_type *os;
   lang_statement_union_type *l;
@@ -682,8 +681,6 @@ gld${EMULATION_NAME}_place_orphan (asect
   if (link_info.relocatable)
     return FALSE;
 
-  secname = bfd_get_section_name (s->owner, s);
-
   /* Everything from the '\$' on gets deleted so don't allow '\$' as the
      first character.  */
   if (*secname == '\$')
Index: ld/emultempl/elf32.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/elf32.em,v
retrieving revision 1.192
diff -u -p -r1.192 elf32.em
--- ld/emultempl/elf32.em	7 Sep 2008 04:02:31 -0000	1.192
+++ ld/emultempl/elf32.em	3 Oct 2008 08:39:05 -0000
@@ -62,7 +62,7 @@ fragment <<EOF
 static void gld${EMULATION_NAME}_before_parse (void);
 static void gld${EMULATION_NAME}_after_open (void);
 static void gld${EMULATION_NAME}_before_allocation (void);
-static bfd_boolean gld${EMULATION_NAME}_place_orphan (asection *s);
+static bfd_boolean gld${EMULATION_NAME}_place_orphan (asection *, const char *);
 static void gld${EMULATION_NAME}_finish (void);
 
 EOF
@@ -1635,7 +1635,7 @@ output_rel_find (asection *sec, int isdy
    sections in the right segment.  */
 
 static bfd_boolean
-gld${EMULATION_NAME}_place_orphan (asection *s)
+gld${EMULATION_NAME}_place_orphan (asection *s, const char *secname)
 {
   static struct orphan_save hold[] =
     {
@@ -1673,15 +1673,12 @@ gld${EMULATION_NAME}_place_orphan (asect
     };
   static int orphan_init_done = 0;
   struct orphan_save *place;
-  const char *secname;
   lang_output_section_statement_type *after;
   lang_output_section_statement_type *os;
   int isdyn = 0;
   int iself = s->owner->xvec->flavour == bfd_target_elf_flavour;
   unsigned int sh_type = iself ? elf_section_type (s) : SHT_NULL;
 
-  secname = bfd_get_section_name (s->owner, s);
-
   if (! link_info.relocatable
       && link_info.combreloc
       && (s->flags & SEC_ALLOC))
@@ -1707,28 +1704,24 @@ gld${EMULATION_NAME}_place_orphan (asect
 	}
     }
 
-  if (isdyn || (!config.unique_orphan_sections && !unique_section_p (s)))
-    {
-      /* Look through the script to see where to place this section.  */
-      os = lang_output_section_find (secname);
+  /* Look through the script to see where to place this section.  */
+  os = lang_output_section_find (secname);
 
-      if (os != NULL
-	  && (os->bfd_section == NULL
-	      || os->bfd_section->flags == 0
-	      || (_bfd_elf_match_sections_by_type (link_info.output_bfd,
-						   os->bfd_section,
-						   s->owner, s)
-		  && ((s->flags ^ os->bfd_section->flags)
-		      & (SEC_LOAD | SEC_ALLOC)) == 0)))
-	{
-	  /* We already have an output section statement with this
-	     name, and its bfd section, if any, has compatible flags.
-	     If the section already exists but does not have any flags
-	     set, then it has been created by the linker, probably as a
-	     result of a --section-start command line switch.  */
-	  lang_add_section (&os->children, s, os);
-	  return TRUE;
-	}
+  if (os != NULL
+      && os->bfd_section != NULL
+      && (os->bfd_section->flags == 0
+	  || (_bfd_elf_match_sections_by_type (link_info.output_bfd,
+					       os->bfd_section, s->owner, s)
+	      && ((s->flags ^ os->bfd_section->flags)
+		  & (SEC_LOAD | SEC_ALLOC)) == 0)))
+    {
+      /* We already have an output section statement with this
+	 name, and its bfd section has compatible flags.
+	 If the section already exists but does not have any flags
+	 set, then it has been created by the linker, probably as a
+	 result of a --section-start command line switch.  */
+      lang_add_section (&os->children, s, os);
+      return TRUE;
     }
 
   if (!orphan_init_done)
@@ -1748,7 +1741,7 @@ gld${EMULATION_NAME}_place_orphan (asect
      sections into the .text section to get them out of the way.  */
   if (link_info.executable
       && ! link_info.relocatable
-      && CONST_STRNEQ (secname, ".gnu.warning.")
+      && CONST_STRNEQ (s->name, ".gnu.warning.")
       && hold[orphan_text].os != NULL)
     {
       lang_add_section (&hold[orphan_text].os->children, s,
@@ -1803,18 +1796,6 @@ gld${EMULATION_NAME}_place_orphan (asect
 	after = &lang_output_section_statement.head->output_section_statement;
     }
 
-  /* Choose a unique name for the section.  This will be needed if the
-     same section name appears in the input file with different
-     loadable or allocatable characteristics.  */
-  if (bfd_get_section_by_name (link_info.output_bfd, secname) != NULL)
-    {
-      static int count = 1;
-      secname = bfd_get_unique_section_name (link_info.output_bfd,
-					     secname, &count);
-      if (secname == NULL)
-	einfo ("%F%P: place_orphan failed: %E\n");
-    }
-
   lang_insert_orphan (s, secname, after, place, NULL, NULL);
 
   return TRUE;
Index: ld/emultempl/genelf.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/genelf.em,v
retrieving revision 1.3
diff -u -p -r1.3 genelf.em
--- ld/emultempl/genelf.em	19 Jul 2007 19:56:10 -0000	1.3
+++ ld/emultempl/genelf.em	3 Oct 2008 08:39:05 -0000
@@ -34,7 +34,27 @@ gld${EMULATION_NAME}_finish (void)
   gld${EMULATION_NAME}_map_segments (FALSE);
   finish_default ();
 }
+
+static void
+gld${EMULATION_NAME}_after_open (void)
+{
+  bfd *ibfd;
+  asection *sec;
+  asymbol **syms;
+
+  if (link_info.relocatable)
+    for (ibfd = link_info.input_bfds; ibfd != NULL; ibfd = ibfd->link_next)
+      if ((syms = bfd_get_outsymbols (ibfd)) != NULL
+	  && bfd_get_flavour (ibfd) == bfd_target_elf_flavour)
+	for (sec = ibfd->sections; sec != NULL; sec = sec->next)
+	  if ((sec->flags & (SEC_GROUP | SEC_LINKER_CREATED)) == SEC_GROUP)
+	    {
+	      struct bfd_elf_section_data *sec_data = elf_section_data (sec);
+	      elf_group_id (sec) = syms[sec_data->this_hdr.sh_info - 1];
+	    }
+}
 EOF
 # Put these extra routines in ld_${EMULATION_NAME}_emulation
 #
 LDEMUL_FINISH=gld${EMULATION_NAME}_finish
+LDEMUL_AFTER_OPEN=gld${EMULATION_NAME}_after_open
Index: ld/emultempl/mmo.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/mmo.em,v
retrieving revision 1.22
diff -u -p -r1.22 mmo.em
--- ld/emultempl/mmo.em	15 Feb 2008 03:35:53 -0000	1.22
+++ ld/emultempl/mmo.em	3 Oct 2008 08:39:05 -0000
@@ -47,7 +47,7 @@ fragment <<EOF
    from elf32.em.  */
 
 static bfd_boolean
-mmo_place_orphan (asection *s)
+mmo_place_orphan (asection *s, const char *secname)
 {
   static struct orphan_save hold_text =
     {
@@ -56,7 +56,6 @@ mmo_place_orphan (asection *s)
       0, 0, 0, 0
     };
   struct orphan_save *place;
-  const char *secname;
   lang_output_section_statement_type *after;
   lang_output_section_statement_type *os;
 
@@ -66,7 +65,6 @@ mmo_place_orphan (asection *s)
     return FALSE;
 
   /* Only care for sections we're going to load.  */
-  secname = s->name;
   os = lang_output_section_find (secname);
 
   /* We have an output section by this name.  Place the section inside it
Index: ld/emultempl/pe.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/pe.em,v
retrieving revision 1.134
diff -u -p -r1.134 pe.em
--- ld/emultempl/pe.em	9 Sep 2008 09:45:31 -0000	1.134
+++ ld/emultempl/pe.em	3 Oct 2008 08:39:06 -0000
@@ -1613,24 +1613,20 @@ gld_${EMULATION_NAME}_finish (void)
    sort_sections.  */
 
 static bfd_boolean
-gld_${EMULATION_NAME}_place_orphan (asection *s)
+gld_${EMULATION_NAME}_place_orphan (asection *s, const char *secname)
 {
-  const char *secname;
-  const char *orig_secname;
+  const char *orig_secname = secname;
   char *dollar = NULL;
   lang_output_section_statement_type *os;
   lang_statement_list_type add_child;
 
-  secname = bfd_get_section_name (s->owner, s);
-
   /* Look through the script to see where to place this section.  */
-  orig_secname = secname;
   if (!link_info.relocatable
       && (dollar = strchr (secname, '$')) != NULL)
     {
-      size_t len = dollar - orig_secname;
+      size_t len = dollar - secname;
       char *newname = xmalloc (len + 1);
-      memcpy (newname, orig_secname, len);
+      memcpy (newname, secname, len);
       newname[len] = '\0';
       secname = newname;
     }
@@ -1640,13 +1636,13 @@ gld_${EMULATION_NAME}_place_orphan (asec
   lang_list_init (&add_child);
 
   if (os != NULL
-      && (os->bfd_section == NULL
-	  || os->bfd_section->flags == 0
+      && os->bfd_section != NULL
+      && (os->bfd_section->flags == 0
 	  || ((s->flags ^ os->bfd_section->flags)
 	      & (SEC_LOAD | SEC_ALLOC)) == 0))
     {
       /* We already have an output section statement with this
-	 name, and its bfd section, if any, has compatible flags.
+	 name, and its bfd section has compatible flags.
 	 If the section already exists but does not have any flags set,
 	 then it has been created by the linker, probably as a result of
 	 a --section-start command line switch.  */
@@ -1723,18 +1719,6 @@ gld_${EMULATION_NAME}_place_orphan (asec
 		     ->output_section_statement);
 	}
 
-      /* Choose a unique name for the section.  This will be needed if the
-	 same section name appears in the input file with different
-	 loadable or allocatable characteristics.  */
-      if (bfd_get_section_by_name (link_info.output_bfd, secname) != NULL)
-	{
-	  static int count = 1;
-	  secname = bfd_get_unique_section_name (link_info.output_bfd,
-						 secname, &count);
-	  if (secname == NULL)
-	    einfo ("%F%P: place_orphan failed: %E\n");
-	}
-
       /* All sections in an executable must be aligned to a page boundary.  */
       address = exp_unop (ALIGN_K, exp_nameop (NAME, "__section_alignment__"));
       os = lang_insert_orphan (s, secname, after, place, address, &add_child);
Index: ld/emultempl/pep.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/pep.em,v
retrieving revision 1.13
diff -u -p -r1.13 pep.em
--- ld/emultempl/pep.em	9 Sep 2008 09:45:31 -0000	1.13
+++ ld/emultempl/pep.em	3 Oct 2008 08:39:08 -0000
@@ -1372,24 +1372,20 @@ gld_${EMULATION_NAME}_finish (void)
    sort_sections.  */
 
 static bfd_boolean
-gld_${EMULATION_NAME}_place_orphan (asection *s)
+gld_${EMULATION_NAME}_place_orphan (asection *s, const char *secname)
 {
-  const char *secname;
-  const char *orig_secname;
+  const char *orig_secname = secname;
   char *dollar = NULL;
   lang_output_section_statement_type *os;
   lang_statement_list_type add_child;
 
-  secname = bfd_get_section_name (s->owner, s);
-
   /* Look through the script to see where to place this section.  */
-  orig_secname = secname;
   if (!link_info.relocatable
       && (dollar = strchr (secname, '$')) != NULL)
     {
-      size_t len = dollar - orig_secname;
+      size_t len = dollar - secname;
       char *newname = xmalloc (len + 1);
-      memcpy (newname, orig_secname, len);
+      memcpy (newname, secname, len);
       newname[len] = '\0';
       secname = newname;
     }
@@ -1399,13 +1395,13 @@ gld_${EMULATION_NAME}_place_orphan (asec
   lang_list_init (&add_child);
 
   if (os != NULL
-      && (os->bfd_section == NULL
-	  || os->bfd_section->flags == 0
+      && os->bfd_section != NULL
+      && (os->bfd_section->flags == 0
 	  || ((s->flags ^ os->bfd_section->flags)
 	      & (SEC_LOAD | SEC_ALLOC)) == 0))
     {
       /* We already have an output section statement with this
-	 name, and its bfd section, if any, has compatible flags.
+	 name, and its bfd section has compatible flags.
 	 If the section already exists but does not have any flags set,
 	 then it has been created by the linker, probably as a result of
 	 a --section-start command line switch.  */
@@ -1482,18 +1478,6 @@ gld_${EMULATION_NAME}_place_orphan (asec
 		     ->output_section_statement);
 	}
 
-      /* Choose a unique name for the section.  This will be needed if the
-	 same section name appears in the input file with different
-	 loadable or allocatable characteristics.  */
-      if (bfd_get_section_by_name (link_info.output_bfd, secname) != NULL)
-	{
-	  static int count = 1;
-	  secname = bfd_get_unique_section_name (link_info.output_bfd,
-						 secname, &count);
-	  if (secname == NULL)
-	    einfo ("%F%P: place_orphan failed: %E\n");
-	}
-
       /* All sections in an executable must be aligned to a page boundary.  */
       address = exp_unop (ALIGN_K, exp_nameop (NAME, "__section_alignment__"));
       os = lang_insert_orphan (s, secname, after, place, address, &add_child);
Index: ld/emultempl/spuelf.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/spuelf.em,v
retrieving revision 1.26
diff -u -p -r1.26 spuelf.em
--- ld/emultempl/spuelf.em	2 Aug 2008 16:25:44 -0000	1.26
+++ ld/emultempl/spuelf.em	3 Oct 2008 08:39:08 -0000
@@ -114,12 +114,7 @@ spu_place_special_section (asection *s, 
 
   os = lang_output_section_find (o != NULL ? o->name : output_name);
   if (os == NULL)
-    {
-      const char *save = s->name;
-      s->name = output_name;
-      gld${EMULATION_NAME}_place_orphan (s);
-      s->name = save;
-    }
+    gld${EMULATION_NAME}_place_orphan (s, output_name);
   else if (o != NULL && os->children.head != NULL)
     {
       lang_statement_list_type add;

-- 
Alan Modra
Australia Development Lab, IBM

Re: PATCH: PR ld/6931: COMDAT group is broken

[Alan Modra]

On Fri, Oct 03, 2008 at 07:16:16PM +0930, Alan Modra wrote:
> Finally, I tweaked the various emulation's place_orphan code to not
> use an existing output section statement if it did not already have a
> bfd section.  This is because a new bfd section will be created after
> all other existing sections, which is probably the wrong ordering.
> lang_place_orphan knows how to rearrange sections.

One testcase expected the wrong order.

	* ld-powerpc/vxworks1-lib.rd: Update.

Index: ld/testsuite/ld-powerpc/vxworks1-lib.rd
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-powerpc/vxworks1-lib.rd,v
retrieving revision 1.4
diff -u -p -r1.4 vxworks1-lib.rd
--- ld/testsuite/ld-powerpc/vxworks1-lib.rd	15 May 2007 12:22:34 -0000	1.4
+++ ld/testsuite/ld-powerpc/vxworks1-lib.rd	3 Oct 2008 12:59:44 -0000
@@ -1,9 +1,4 @@
 
-Relocation section '\.rela\.plt' at offset .* contains 2 entries:
- Offset     Info    Type            Sym\.Value  Sym\. Name \+ Addend
-0009040c  .*15 R_PPC_JMP_SLOT    00000000   sexternal \+ 0
-00090410  .*15 R_PPC_JMP_SLOT    00080c48   sglobal \+ 0
-
 Relocation section '\.rela\.dyn' at offset .* contains 5 entries:
  Offset     Info    Type            Sym\.Value  Sym\. Name \+ Addend
 00090800  00000016 R_PPC_RELATIVE * 00080c44
@@ -11,3 +6,8 @@ Relocation section '\.rela\.dyn' at offs
 00080c12  .*04 R_PPC_ADDR16_LO   00000000   __GOTT_BASE__ \+ 0
 00080c16  .*03 R_PPC_ADDR16      00000000   __GOTT_INDEX__ \+ 0
 00090414  .*14 R_PPC_GLOB_DAT    00090c00   x \+ 0
+
+Relocation section '\.rela\.plt' at offset .* contains 2 entries:
+ Offset     Info    Type            Sym\.Value  Sym\. Name \+ Addend
+0009040c  .*15 R_PPC_JMP_SLOT    00000000   sexternal \+ 0
+00090410  .*15 R_PPC_JMP_SLOT    00080c48   sglobal \+ 0

-- 
Alan Modra
Australia Development Lab, IBM

Re: PATCH: PR ld/6931: COMDAT group is broken

[H.J. Lu]

On Fri, Oct 03, 2008 at 07:16:16PM +0930, Alan Modra wrote:
> The elf.c and elflink.c patches below teach the ELF backend linker to
> handle group signature symbols for ld -r.  The genelf.em patch does
> the same for ELF targets that use the generic linker.
> 
> The linker.c change fixes the group section contents for the generic
> linker, a nasty hack but nothing else seemed nice either.  Prior to
> this change, the generic linker simply copied group section contents
> to output for ld -r, which, since section indices almost always
> change, broke groups.  The generic linker also mashed together group
> sections due to lack of unique sections for groups.  Moving this logic
> from elf32.em into ldlang.c cures that particular problem.
> 
> Finally, I tweaked the various emulation's place_orphan code to not
> use an existing output section statement if it did not already have a
> bfd section.  This is because a new bfd section will be created after
> all other existing sections, which is probably the wrong ordering.
> lang_place_orphan knows how to rearrange sections.
> 

I checked in this patch to update the testcases.


H.J.
---
binutils/testsuite/

2008-10-03  H.J. Lu  <hongjiu.lu@intel.com>

	* binutils-all/group.s: Updated.
	* binutils-all/group-2.s: Likewise.
	* binutils-all/group-3.s: Likewise.
	* binutils-all/group-4.s: Likewise.
	* binutils-all/strip-7.d: Likewise.
	* binutils-all/strip-9.d: Likewise.

ld/testsuite/

2008-10-03  H.J. Lu  <hongjiu.lu@intel.com>

	* ld-elf/group5.d: Updated.
	* ld-elf/group6.d: Likewise.

--- binutils/binutils/testsuite/binutils-all/group-2.s.grp	2008-10-01 14:54:59.000000000 -0700
+++ binutils/binutils/testsuite/binutils-all/group-2.s	2008-10-03 07:23:24.000000000 -0700
@@ -1,8 +1,8 @@
 	.section .text.foo,"axG",%progbits,.text.foo,comdat
-	.global foo
-foo:
+	.global foo2
+foo2:
 	.word 0
 	.section .data.bar,"awG",%progbits,.text.foo,comdat
-	.global bar
-bar:
+	.global bar2
+bar2:
 	.word 0
--- binutils/binutils/testsuite/binutils-all/group-3.s.grp	2008-10-01 16:10:48.000000000 -0700
+++ binutils/binutils/testsuite/binutils-all/group-3.s	2008-10-02 17:17:01.000000000 -0700
@@ -1,8 +1,8 @@
-	.section .text,"axG",%progbits,foo,comdat
-	.global foo
-foo:
+	.section .text.foo3,"axG",%progbits,foo3,comdat
+	.global foo3
+foo3:
 	.word 0
-	.section .data,"awG",%progbits,foo,comdat
-	.global bar
-bar:
+	.section .data.bar3,"awG",%progbits,foo3,comdat
+	.global bar3
+bar3:
 	.word 0
--- binutils/binutils/testsuite/binutils-all/group-4.s.grp	2008-10-01 18:06:12.000000000 -0700
+++ binutils/binutils/testsuite/binutils-all/group-4.s	2008-10-02 17:17:16.000000000 -0700
@@ -1,6 +1,6 @@
-	.section .text,"axG",%progbits,foo,comdat
-foo:
+	.section .text.foo4,"axG",%progbits,foo4,comdat
+foo4:
 	.word 0
-	.section .data,"awG",%progbits,foo,comdat
-bar:
+	.section .data.foo4,"awG",%progbits,foo4,comdat
+bar4:
 	.word 0
--- binutils/binutils/testsuite/binutils-all/group.s.grp	2005-10-20 10:06:40.000000000 -0700
+++ binutils/binutils/testsuite/binutils-all/group.s	2008-10-02 17:12:54.000000000 -0700
@@ -1,8 +1,8 @@
-	.section .text,"axG",%progbits,foo_group,comdat
+	.section .text.foo,"axG",%progbits,foo_group,comdat
 	.global foo
 foo:
 	.word 0
-	.section .data,"awG",%progbits,foo_group,comdat
+	.section .data.foo,"awG",%progbits,foo_group,comdat
 	.global bar
 bar:
 	.word 0
--- binutils/binutils/testsuite/binutils-all/strip-7.d.grp	2008-10-01 16:10:48.000000000 -0700
+++ binutils/binutils/testsuite/binutils-all/strip-7.d	2008-10-03 07:23:42.000000000 -0700
@@ -11,7 +11,7 @@
 #...
   \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG[ \t]+.*
 #...
-COMDAT group section \[[ 0-9]+\] `.group' \[foo\] contains 2 sections:
+COMDAT group section \[[ 0-9]+\] `.group' \[foo3\] contains 2 sections:
    \[Index\]    Name
    \[[ 0-9]+\]   .text.*
    \[[ 0-9]+\]   .data.*
--- binutils/binutils/testsuite/binutils-all/strip-9.d.grp	2008-10-01 18:06:12.000000000 -0700
+++ binutils/binutils/testsuite/binutils-all/strip-9.d	2008-10-03 07:23:50.000000000 -0700
@@ -11,7 +11,7 @@
 #...
   \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG[ \t]+.*
 #...
-COMDAT group section \[[ 0-9]+\] `.group' \[foo\] contains 2 sections:
+COMDAT group section \[[ 0-9]+\] `.group' \[foo4\] contains 2 sections:
    \[Index\]    Name
    \[[ 0-9]+\]   .text.*
    \[[ 0-9]+\]   .data.*
--- binutils/ld/testsuite/ld-elf/group5.d.grp	2008-10-03 07:08:38.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/group5.d	2008-10-03 07:58:17.000000000 -0700
@@ -9,7 +9,7 @@
 #...
   \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG[ \t]+.*
 #...
-COMDAT group section \[[ 0-9]+\] `.group' \[foo\] contains 2 sections:
+COMDAT group section \[[ 0-9]+\] `.group' \[foo3\] contains 2 sections:
    \[Index\]    Name
    \[[ 0-9]+\]   .text.*
    \[[ 0-9]+\]   .data.*
--- binutils/ld/testsuite/ld-elf/group6.d.grp	2008-10-03 07:08:38.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/group6.d	2008-10-03 07:58:24.000000000 -0700
@@ -9,7 +9,7 @@
 #...
   \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG[ \t]+.*
 #...
-COMDAT group section \[[ 0-9]+\] `.group' \[foo\] contains 2 sections:
+COMDAT group section \[[ 0-9]+\] `.group' \[foo4\] contains 2 sections:
    \[Index\]    Name
    \[[ 0-9]+\]   .text.*
    \[[ 0-9]+\]   .data.*

Re: PATCH: PR ld/6931: COMDAT group is broken

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 3273 bytes --]

On Fri, Oct 3, 2008 at 2:46 AM, Alan Modra <amodra@bigpond.net.au> wrote:
> The elf.c and elflink.c patches below teach the ELF backend linker to
> handle group signature symbols for ld -r.  The genelf.em patch does
> the same for ELF targets that use the generic linker.
>
> The linker.c change fixes the group section contents for the generic
> linker, a nasty hack but nothing else seemed nice either.  Prior to
> this change, the generic linker simply copied group section contents
> to output for ld -r, which, since section indices almost always
> change, broke groups.  The generic linker also mashed together group
> sections due to lack of unique sections for groups.  Moving this logic
> from elf32.em into ldlang.c cures that particular problem.
>
> Finally, I tweaked the various emulation's place_orphan code to not
> use an existing output section statement if it did not already have a
> bfd section.  This is because a new bfd section will be created after
> all other existing sections, which is probably the wrong ordering.
> lang_place_orphan knows how to rearrange sections.
>
> bfd/
>        * elf.c (bfd_elf_set_group_contents): Assign sh_info for ld -r when
>        the signature symbol is global.
>        * elflink.c (elf_link_input_bfd): Ensure group signature symbol
>        is output when ld -r.  Set group sh_info when local.
>        * linker.c (default_indirect_link_order): Handle group sections
>        specially.
> ld/
>        * ldemul.c (ldemul_place_orphan): Add "name" param.
>        * ldemul.h (ldemul_place_orphan): Update prototype.
>        (struct ld_emulation_xfer_struct <place_orphan>): Likewise.
>        * ldlang.c (lang_place_orphans): Generate unique section names here..
>        * emultempl/elf32.em (place_orphan): ..rather than here.  Don't
>        directly use an existing output section statement that has no
>        bfd section.
>        * emultempl/pe.em (place_orphan): Likewise.
>        * emultempl/pep.em (place_orphan): Likewise.
>        * emultempl/beos.em (place_orphan): Adjust.
>        * emultempl/spuelf.em (spu_place_special_section): Adjust
>        place_orphan call.
>        * emultempl/genelf.em (gld${EMULATION_NAME}_after_open): New function.
>        (LDEMUL_AFTER_OPEN): Define.
>

It is unsafe to use an existing section symbol as group signature
since "ld -r" may change section symbol if there is a duplicate.
This patch disallows using section symbol as group signature.
OK to install?


-- 
H.J.
---

bfd/

2008-10-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/6931
	* elflink.c (elf_link_input_bfd): Disallow using section symbol
	as group signaure.

binutils/testsuite/

2008-10-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/6931
	* binutils-all/group-2.s: Don't use section symbol as group
	signature.

	* binutils-all/strip-5.d: Updated.

gas/

2008-10-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/6931
	* config/obj-elf.c (elf_frob_file): Disallow using section
	symbol as group signature.

gas/testsuite/

2008-10-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/6931
	* gas/elf/elf.exp: Run group2.

	* gas/elf/group2.l: New.
	* gas/elf/group2.s: Likewise.

ld/testsuite/

2008-10-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/6931
	* ld-elf/group4.d: Updated.

	* ld-elf/group7.d: New.

[-- Attachment #2: binutils-grp-3.patch --]
[-- Type: application/octet-stream, Size: 5849 bytes --]

bfd/

2008-10-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/6931
	* elflink.c (elf_link_input_bfd): Disallow using section symbol
	as group signature.

binutils/testsuite/

2008-10-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/6931
	* binutils-all/group-2.s: Don't use section symbol as group
	signature.

	* binutils-all/strip-5.d: Updated.

gas/

2008-10-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/6931
	* config/obj-elf.c (elf_frob_file): Disallow using section
	symbol as group signature.

gas/testsuite/

2008-10-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/6931
	* gas/elf/elf.exp: Run group2.

	* gas/elf/group2.l: New.
	* gas/elf/group2.s: Likewise.

ld/testsuite/

2008-10-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/6931
	* ld-elf/group4.d: Updated.

	* ld-elf/group7.d: New.

--- binutils/bfd/elflink.c.grp	2008-10-03 11:32:52.000000000 -0700
+++ binutils/bfd/elflink.c	2008-10-03 12:30:36.000000000 -0700
@@ -9170,7 +9170,21 @@ elf_link_input_bfd (struct elf_final_lin
 	  else if (ELF_ST_TYPE (isymbuf[symndx].st_info) == STT_SECTION)
 	    {
 	      /* We'll use the output section target_index.  */
-	      asection *sec = finfo->sections[symndx]->output_section;
+	      asection *sec = finfo->sections[symndx];
+
+	      /* PR ld/6931. Since "ld -r" may change section symbol,
+		 we can't use section symbol as group signature unless
+		 it is our section symbol.  */
+	      if (sec != o)
+		{
+		  (*_bfd_error_handler)
+		    (_("error: section %A in %B uses section %A symbol "
+		       "as group signature"),
+		     o, input_bfd, sec);
+		  return FALSE;
+		}
+
+	      sec = sec->output_section;
 	      elf_section_data (osec)->this_hdr.sh_info = sec->target_index;
 	    }
 	  else
--- binutils/binutils/testsuite/binutils-all/group-2.s.grp	2008-10-03 09:03:13.000000000 -0700
+++ binutils/binutils/testsuite/binutils-all/group-2.s	2008-10-03 11:52:33.000000000 -0700
@@ -1,8 +1,7 @@
-	.section .text.foo,"axG",%progbits,.text.foo,comdat
+	.section .text.foo,"axG",%progbits,bar2,comdat
 	.global foo2
 foo2:
 	.word 0
-	.section .data.bar,"awG",%progbits,.text.foo,comdat
-	.global bar2
+	.section .data.bar,"awG",%progbits,bar2,comdat
 bar2:
 	.word 0
--- binutils/binutils/testsuite/binutils-all/strip-5.d.grp	2008-10-01 14:54:59.000000000 -0700
+++ binutils/binutils/testsuite/binutils-all/strip-5.d	2008-10-03 11:52:59.000000000 -0700
@@ -11,7 +11,7 @@
 #...
   \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG[ \t]+.*
 #...
-COMDAT group section \[[ 0-9]+\] `.group' \[.text.foo\] contains 2 sections:
+COMDAT group section \[[ 0-9]+\] `.group' \[bar2\] contains 2 sections:
    \[Index\]    Name
    \[[ 0-9]+\]   .text.*
    \[[ 0-9]+\]   .data.*
--- binutils/gas/config/obj-elf.c.grp	2008-10-03 11:32:52.000000000 -0700
+++ binutils/gas/config/obj-elf.c	2008-10-03 11:46:30.000000000 -0700
@@ -2095,6 +2095,11 @@ elf_frob_file (void)
 	      || (sy->sy_next != NULL
 		  && sy->sy_next->sy_previous == sy)))
 	{
+	  /* PR ld/6931.  Since "ld -r" may change section symbol name,
+	     we can't use section symbol as group signature.  */
+	  if (sy->bsym->flags & BSF_SECTION_SYM)
+	    as_bad (_("can't use section symbol `%s' as group signature"),
+		    group_name);
 	  has_sym = 1;
 	  sec_name = ".group";
 	}
--- binutils/gas/testsuite/gas/elf/elf.exp.grp	2008-09-16 09:16:34.000000000 -0700
+++ binutils/gas/testsuite/gas/elf/elf.exp	2008-10-03 11:57:41.000000000 -0700
@@ -117,4 +117,5 @@ if { ([istarget "*-*-*elf*"]		
     run_elf_list_test "type" "" "" "-s" "| grep \"1 \\\[FONTC\\\]\""
     run_dump_test "section6" 
     run_dump_test "section7" 
+    run_list_test "group2" "-al"
 }
--- binutils/gas/testsuite/gas/elf/group2.l.grp	2008-10-03 12:00:28.000000000 -0700
+++ binutils/gas/testsuite/gas/elf/group2.l	2008-10-03 12:00:06.000000000 -0700
@@ -0,0 +1,8 @@
+.*: Assembler messages:
+.*:2: Error: .*
+GAS LISTING .*
+
+
+[ 	]*1[ 	]+\.section       \.text,"axG",%progbits,\.text,comdat
+[ 	]*2[ 	]+\?\?\?\? 01       		\.byte	1
+\*\*\*\*  Error:can't use section symbol `\.text' as group signature
--- binutils/gas/testsuite/gas/elf/group2.s.grp	2008-10-03 12:00:31.000000000 -0700
+++ binutils/gas/testsuite/gas/elf/group2.s	2008-10-03 11:55:40.000000000 -0700
@@ -0,0 +1,2 @@
+	.section       .text,"axG",%progbits,.text,comdat
+	.byte	1
--- binutils/ld/testsuite/ld-elf/group4.d.grp	2008-10-03 07:08:38.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/group4.d	2008-10-03 11:54:04.000000000 -0700
@@ -9,7 +9,7 @@
 #...
   \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG[ \t]+.*
 #...
-COMDAT group section \[[ 0-9]+\] `.group' \[.text.foo\] contains 2 sections:
+COMDAT group section \[[ 0-9]+\] `.group' \[bar2\] contains 2 sections:
    \[Index\]    Name
    \[[ 0-9]+\]   .text.*
    \[[ 0-9]+\]   .data.*
--- binutils/ld/testsuite/ld-elf/group7.d.grp	2008-10-03 12:05:09.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/group7.d	2008-10-03 12:04:45.000000000 -0700
@@ -0,0 +1,28 @@
+#source: ../../../binutils/testsuite/binutils-all/group.s
+#source: ../../../binutils/testsuite/binutils-all/group-2.s
+#source: ../../../binutils/testsuite/binutils-all/group-3.s
+#source: ../../../binutils/testsuite/binutils-all/group-4.s
+#ld: -r
+#readelf: -g --wide
+
+#...
+COMDAT group section \[[ 0-9]+\] `foo_group' \[foo_group\] contains 2 sections:
+   \[Index\]    Name
+   \[[ 0-9]+\]   .text.*
+   \[[ 0-9]+\]   .data.*
+#...
+COMDAT group section \[[ 0-9]+\] `.group.*' \[bar2\] contains 2 sections:
+   \[Index\]    Name
+   \[[ 0-9]+\]   .text.*
+   \[[ 0-9]+\]   .data.*
+#...
+COMDAT group section \[[ 0-9]+\] `.group.*' \[foo3\] contains 2 sections:
+   \[Index\]    Name
+   \[[ 0-9]+\]   .text.*
+   \[[ 0-9]+\]   .data.*
+#...
+COMDAT group section \[[ 0-9]+\] `.group.*' \[foo4\] contains 2 sections:
+   \[Index\]    Name
+   \[[ 0-9]+\]   .text.*
+   \[[ 0-9]+\]   .data.*
+#pass

Re: PATCH: PR ld/6931: COMDAT group is broken

[Alan Modra]

On Fri, Oct 03, 2008 at 12:41:01PM -0700, H.J. Lu wrote:
> It is unsafe to use an existing section symbol as group signature
> since "ld -r" may change section symbol if there is a duplicate.
> This patch disallows using section symbol as group signature.
> OK to install?

This fixes the latest problem HJ has found with section groups and
ld -r.  To stop ld -r merging sections that are part of a group, ld
generated unique names for them.  If a renamed section symbol is used
as the group signature, then the group is broken.  So, don't rename
sections.  This was suprisingly easy to do with the existing SPECIAL
section support.

	* ldemul.c (ldemul_place_orphan): Add "constraint" param.
	* ldemul.h (ldemul_place_orphan): Update prototype.
	(struct ld_emulation_xfer_struct <place_orphan>): Likewise add param.
	* ldlang.c (unique_section_p): Make static.
	(lang_output_section_statement_lookup): Optimise creation of SPECIAL
	sections.
	(lang_insert_orphan): Add "constraint" param.  Pass to
	lang_enter_output_section_statement.
	(init_os): Don't use an existing bfd section for SPECIAL sections.
	(lang_place_orphans): Don't rename unique output sections, instead
	mark their output section statements SPECIAL.
	* ldlang.h (lang_insert_orphan): Update prototype.
	(unique_section_p): Delete.
	* emultempl/beos.em (place_orphan): Add "constraint" param.
	* emultempl/elf32.em (place_orphan): Likewise.  Don't match existing
	output sections if set.
	* emultempl/pe.em (place_orphan): Likewise.
	* emultempl/pep.em (place_orphan): Likewise.
	* emultempl/mmo.em (mmo_place_orphan): Update.
	* emultempl/spuelf.em (spu_place_special_section): Update.

Index: ld/ldemul.c
===================================================================
RCS file: /cvs/src/src/ld/ldemul.c,v
retrieving revision 1.28
diff -u -p -r1.28 ldemul.c
--- ld/ldemul.c	3 Oct 2008 09:40:49 -0000	1.28
+++ ld/ldemul.c	4 Oct 2008 01:17:49 -0000
@@ -120,10 +120,10 @@ ldemul_open_dynamic_archive (const char 
 }
 
 bfd_boolean
-ldemul_place_orphan (asection *s, const char *name)
+ldemul_place_orphan (asection *s, const char *name, int constraint)
 {
   if (ld_emulation->place_orphan)
-    return (*ld_emulation->place_orphan) (s, name);
+    return (*ld_emulation->place_orphan) (s, name, constraint);
   return FALSE;
 }
 
Index: ld/ldemul.h
===================================================================
RCS file: /cvs/src/src/ld/ldemul.h,v
retrieving revision 1.20
diff -u -p -r1.20 ldemul.h
--- ld/ldemul.h	3 Oct 2008 09:40:49 -0000	1.20
+++ ld/ldemul.h	4 Oct 2008 01:17:49 -0000
@@ -59,7 +59,7 @@ extern void ldemul_set_symbols
 extern void ldemul_create_output_section_statements
   (void);
 extern bfd_boolean ldemul_place_orphan
-  (asection *, const char *);
+  (asection *, const char *, int);
 extern bfd_boolean ldemul_parse_args
   (int, char **);
 extern void ldemul_add_options
@@ -152,7 +152,7 @@ typedef struct ld_emulation_xfer_struct 
      the default action should be taken.  This field may be NULL, in
      which case the default action will always be taken.  */
   bfd_boolean (*place_orphan)
-    (asection *, const char *);
+    (asection *, const char *, int);
 
   /* Run after assigning parsing with the args, but before
      reading the script.  Used to initialize symbols used in the script.  */
Index: ld/ldlang.c
===================================================================
RCS file: /cvs/src/src/ld/ldlang.c,v
retrieving revision 1.293
diff -u -p -r1.293 ldlang.c
--- ld/ldlang.c	3 Oct 2008 09:40:49 -0000	1.293
+++ ld/ldlang.c	4 Oct 2008 01:17:53 -0000
@@ -193,7 +193,7 @@ input_statement_is_archive_path (const c
   return match;
 }
 
-bfd_boolean
+static bfd_boolean
 unique_section_p (const asection *sec)
 {
   struct unique_sections *unam;
@@ -1278,19 +1278,25 @@ lang_output_section_statement_lookup (co
       struct out_section_hash_entry *last_ent;
       unsigned long hash = entry->root.hash;
 
-      do
-	{
-	  if (entry->s.output_section_statement.constraint >= 0
-	      && (constraint == 0
-		  || (constraint == entry->s.output_section_statement.constraint
-		      && constraint != SPECIAL)))
-	    return &entry->s.output_section_statement;
-	  last_ent = entry;
-	  entry = (struct out_section_hash_entry *) entry->root.next;
-	}
-      while (entry != NULL
-	     && entry->root.hash == hash
-	     && strcmp (name, entry->s.output_section_statement.name) == 0);
+      if (create && constraint == SPECIAL)
+	/* Not traversing to the end reverses the order of the second
+	   and subsequent SPECIAL sections in the hash table chain,
+	   but that shouldn't matter.  */
+	last_ent = entry;
+      else
+	do
+	  {
+	    if (entry->s.output_section_statement.constraint >= 0
+		&& (constraint == 0
+		    || (constraint
+			== entry->s.output_section_statement.constraint)))
+	      return &entry->s.output_section_statement;
+	    last_ent = entry;
+	    entry = (struct out_section_hash_entry *) entry->root.next;
+	  }
+	while (entry != NULL
+	       && entry->root.hash == hash
+	       && strcmp (name, entry->s.output_section_statement.name) == 0);
 
       if (!create)
 	return NULL;
@@ -1556,6 +1562,7 @@ insert_os_after (lang_output_section_sta
 lang_output_section_statement_type *
 lang_insert_orphan (asection *s,
 		    const char *secname,
+		    int constraint,
 		    lang_output_section_statement_type *after,
 		    struct orphan_save *place,
 		    etree_type *address,
@@ -1611,7 +1618,7 @@ lang_insert_orphan (asection *s,
   os_tail = ((lang_output_section_statement_type **)
 	     lang_output_section_statement.tail);
   os = lang_enter_output_section_statement (secname, address, 0, NULL, NULL,
-					    NULL, 0);
+					    NULL, constraint);
 
   if (add_child == NULL)
     add_child = &os->children;
@@ -1914,10 +1921,11 @@ init_os (lang_output_section_statement_t
   if (strcmp (s->name, DISCARD_SECTION_NAME) == 0)
     einfo (_("%P%F: Illegal use of `%s' section\n"), DISCARD_SECTION_NAME);
 
-  s->bfd_section = bfd_get_section_by_name (link_info.output_bfd, s->name);
+  if (s->constraint != SPECIAL)
+    s->bfd_section = bfd_get_section_by_name (link_info.output_bfd, s->name);
   if (s->bfd_section == NULL)
-    s->bfd_section = bfd_make_section_with_flags (link_info.output_bfd,
-						  s->name, flags);
+    s->bfd_section = bfd_make_section_anyway_with_flags (link_info.output_bfd,
+							 s->name, flags);
   if (s->bfd_section == NULL)
     {
       einfo (_("%P%F: output format %s cannot represent section called %s\n"),
@@ -5655,23 +5663,16 @@ lang_place_orphans (void)
 	      else
 		{
 		  const char *name = s->name;
+		  int constraint = 0;
 
-		  if ((config.unique_orphan_sections
-		       || unique_section_p (s))
-		      && bfd_get_section_by_name (link_info.output_bfd,
-						  name) != NULL)
-		    {
-		      static int count = 1;
-		      name = bfd_get_unique_section_name (link_info.output_bfd,
-							  name, &count);
-		      if (name == NULL)
-			einfo ("%F%P: place_orphan failed: %E\n");
-		    }
+		  if (config.unique_orphan_sections || unique_section_p (s))
+		    constraint = SPECIAL;
 
-		  if (!ldemul_place_orphan (s, name))
+		  if (!ldemul_place_orphan (s, name, constraint))
 		    {
 		      lang_output_section_statement_type *os;
-		      os = lang_output_section_statement_lookup (name, 0,
+		      os = lang_output_section_statement_lookup (name,
+								 constraint,
 								 TRUE);
 		      lang_add_section (&os->children, s, os);
 		    }
Index: ld/ldlang.h
===================================================================
RCS file: /cvs/src/src/ld/ldlang.h,v
retrieving revision 1.79
diff -u -p -r1.79 ldlang.h
--- ld/ldlang.h	7 Sep 2008 04:02:30 -0000	1.79
+++ ld/ldlang.h	4 Oct 2008 01:17:53 -0000
@@ -536,7 +536,7 @@ extern lang_output_section_statement_typ
   (const asection *, lang_output_section_statement_type **,
    lang_match_sec_type_func);
 extern lang_output_section_statement_type *lang_insert_orphan
-  (asection *, const char *, lang_output_section_statement_type *,
+  (asection *, const char *, int, lang_output_section_statement_type *,
    struct orphan_save *, etree_type *, lang_statement_list_type *);
 extern lang_input_statement_type *lang_add_input_file
   (const char *, lang_input_file_enum_type, const char *);
@@ -604,8 +604,6 @@ extern void lang_register_vers_node
 extern void lang_append_dynamic_list (struct bfd_elf_version_expr *);
 extern void lang_append_dynamic_list_cpp_typeinfo (void);
 extern void lang_append_dynamic_list_cpp_new (void);
-bfd_boolean unique_section_p
-  (const asection *);
 extern void lang_add_unique
   (const char *);
 extern const char *lang_get_output_target
Index: ld/emultempl/beos.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/beos.em,v
retrieving revision 1.41
diff -u -p -r1.41 beos.em
--- ld/emultempl/beos.em	3 Oct 2008 09:40:49 -0000	1.41
+++ ld/emultempl/beos.em	4 Oct 2008 01:17:54 -0000
@@ -665,7 +665,9 @@ gld_${EMULATION_NAME}_before_allocation 
    which are not mentioned in the linker script.  */
 
 static bfd_boolean
-gld${EMULATION_NAME}_place_orphan (asection *s, const char *secname)
+gld${EMULATION_NAME}_place_orphan (asection *s,
+				   const char *secname,
+				   int constraint)
 {
   char *output_secname, *ps;
   lang_output_section_statement_type *os;
@@ -694,7 +696,7 @@ gld${EMULATION_NAME}_place_orphan (asect
   output_secname = xstrdup (secname);
   ps = strchr (output_secname + 1, '\$');
   *ps = 0;
-  os = lang_output_section_statement_lookup (output_secname, 0, TRUE);
+  os = lang_output_section_statement_lookup (output_secname, constraint, TRUE);
 
   /* Find the '\$' wild statement for this section.  We currently require the
      linker script to explicitly mention "*(.foo\$)".
Index: ld/emultempl/elf32.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/elf32.em,v
retrieving revision 1.193
diff -u -p -r1.193 elf32.em
--- ld/emultempl/elf32.em	3 Oct 2008 09:40:49 -0000	1.193
+++ ld/emultempl/elf32.em	4 Oct 2008 01:33:57 -0000
@@ -62,7 +62,8 @@ fragment <<EOF
 static void gld${EMULATION_NAME}_before_parse (void);
 static void gld${EMULATION_NAME}_after_open (void);
 static void gld${EMULATION_NAME}_before_allocation (void);
-static bfd_boolean gld${EMULATION_NAME}_place_orphan (asection *, const char *);
+static bfd_boolean gld${EMULATION_NAME}_place_orphan
+  (asection *, const char *, int);
 static void gld${EMULATION_NAME}_finish (void);
 
 EOF
@@ -1635,7 +1636,9 @@ output_rel_find (asection *sec, int isdy
    sections in the right segment.  */
 
 static bfd_boolean
-gld${EMULATION_NAME}_place_orphan (asection *s, const char *secname)
+gld${EMULATION_NAME}_place_orphan (asection *s,
+				   const char *secname,
+				   int constraint)
 {
   static struct orphan_save hold[] =
     {
@@ -1705,9 +1708,8 @@ gld${EMULATION_NAME}_place_orphan (asect
     }
 
   /* Look through the script to see where to place this section.  */
-  os = lang_output_section_find (secname);
-
-  if (os != NULL
+  if (constraint == 0
+      && (os = lang_output_section_find (secname)) != NULL
       && os->bfd_section != NULL
       && (os->bfd_section->flags == 0
 	  || (_bfd_elf_match_sections_by_type (link_info.output_bfd,
@@ -1796,7 +1798,7 @@ gld${EMULATION_NAME}_place_orphan (asect
 	after = &lang_output_section_statement.head->output_section_statement;
     }
 
-  lang_insert_orphan (s, secname, after, place, NULL, NULL);
+  lang_insert_orphan (s, secname, constraint, after, place, NULL, NULL);
 
   return TRUE;
 }
Index: ld/emultempl/mmo.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/mmo.em,v
retrieving revision 1.22
diff -u -p -r1.22 mmo.em
--- ld/emultempl/mmo.em	15 Feb 2008 03:35:53 -0000	1.22
+++ ld/emultempl/mmo.em	4 Oct 2008 01:17:55 -0000
@@ -47,7 +47,9 @@ fragment <<EOF
    from elf32.em.  */
 
 static bfd_boolean
-mmo_place_orphan (asection *s)
+mmo_place_orphan (asection *s,
+		  const char *secname,
+		  int constraint ATTRIBUTE_UNUSED)
 {
   static struct orphan_save hold_text =
     {
@@ -56,7 +58,6 @@ mmo_place_orphan (asection *s)
       0, 0, 0, 0
     };
   struct orphan_save *place;
-  const char *secname;
   lang_output_section_statement_type *after;
   lang_output_section_statement_type *os;
 
@@ -66,7 +67,6 @@ mmo_place_orphan (asection *s)
     return FALSE;
 
   /* Only care for sections we're going to load.  */
-  secname = s->name;
   os = lang_output_section_find (secname);
 
   /* We have an output section by this name.  Place the section inside it
@@ -93,7 +93,7 @@ mmo_place_orphan (asection *s)
 
   /* If there's an output section by this name, we'll use it, regardless
      of section flags, in contrast to what's done in elf32.em.  */
-  os = lang_insert_orphan (s, secname, after, place, NULL, NULL);
+  os = lang_insert_orphan (s, secname, 0, after, place, NULL, NULL);
 
   /* We need an output section for .text as a root, so if there was none
      (might happen with a peculiar linker script such as in "map
Index: ld/emultempl/pe.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/pe.em,v
retrieving revision 1.135
diff -u -p -r1.135 pe.em
--- ld/emultempl/pe.em	3 Oct 2008 09:40:49 -0000	1.135
+++ ld/emultempl/pe.em	4 Oct 2008 01:17:56 -0000
@@ -1613,7 +1613,9 @@ gld_${EMULATION_NAME}_finish (void)
    sort_sections.  */
 
 static bfd_boolean
-gld_${EMULATION_NAME}_place_orphan (asection *s, const char *secname)
+gld_${EMULATION_NAME}_place_orphan (asection *s,
+				    const char *secname,
+				    int constraint)
 {
   const char *orig_secname = secname;
   char *dollar = NULL;
@@ -1631,11 +1633,10 @@ gld_${EMULATION_NAME}_place_orphan (asec
       secname = newname;
     }
 
-  os = lang_output_section_find (secname);
-
   lang_list_init (&add_child);
 
-  if (os != NULL
+  if (constraint == 0
+      && (os = lang_output_section_find (secname)) != NULL
       && os->bfd_section != NULL
       && (os->bfd_section->flags == 0
 	  || ((s->flags ^ os->bfd_section->flags)
@@ -1721,7 +1722,8 @@ gld_${EMULATION_NAME}_place_orphan (asec
 
       /* All sections in an executable must be aligned to a page boundary.  */
       address = exp_unop (ALIGN_K, exp_nameop (NAME, "__section_alignment__"));
-      os = lang_insert_orphan (s, secname, after, place, address, &add_child);
+      os = lang_insert_orphan (s, secname, constraint, after, place, address,
+			       &add_child);
     }
 
   {
Index: ld/emultempl/pep.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/pep.em,v
retrieving revision 1.14
diff -u -p -r1.14 pep.em
--- ld/emultempl/pep.em	3 Oct 2008 09:40:49 -0000	1.14
+++ ld/emultempl/pep.em	4 Oct 2008 01:17:57 -0000
@@ -1372,7 +1372,9 @@ gld_${EMULATION_NAME}_finish (void)
    sort_sections.  */
 
 static bfd_boolean
-gld_${EMULATION_NAME}_place_orphan (asection *s, const char *secname)
+gld_${EMULATION_NAME}_place_orphan (asection *s,
+				    const char *secname,
+				    int constraint)
 {
   const char *orig_secname = secname;
   char *dollar = NULL;
@@ -1390,11 +1392,10 @@ gld_${EMULATION_NAME}_place_orphan (asec
       secname = newname;
     }
 
-  os = lang_output_section_find (secname);
-
   lang_list_init (&add_child);
 
-  if (os != NULL
+  if (constraint == 0
+      && (os = lang_output_section_find (secname)) != NULL
       && os->bfd_section != NULL
       && (os->bfd_section->flags == 0
 	  || ((s->flags ^ os->bfd_section->flags)
@@ -1480,7 +1481,8 @@ gld_${EMULATION_NAME}_place_orphan (asec
 
       /* All sections in an executable must be aligned to a page boundary.  */
       address = exp_unop (ALIGN_K, exp_nameop (NAME, "__section_alignment__"));
-      os = lang_insert_orphan (s, secname, after, place, address, &add_child);
+      os = lang_insert_orphan (s, secname, constraint, after, place, address,
+			       &add_child);
     }
 
   {
Index: ld/emultempl/spuelf.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/spuelf.em,v
retrieving revision 1.27
diff -u -p -r1.27 spuelf.em
--- ld/emultempl/spuelf.em	3 Oct 2008 09:40:49 -0000	1.27
+++ ld/emultempl/spuelf.em	4 Oct 2008 01:17:58 -0000
@@ -114,7 +114,7 @@ spu_place_special_section (asection *s, 
 
   os = lang_output_section_find (o != NULL ? o->name : output_name);
   if (os == NULL)
-    gld${EMULATION_NAME}_place_orphan (s, output_name);
+    gld${EMULATION_NAME}_place_orphan (s, output_name, 0);
   else if (o != NULL && os->children.head != NULL)
     {
       lang_statement_list_type add;
-- 
Alan Modra
Australia Development Lab, IBM

Re: PATCH: PR ld/6931: COMDAT group is broken

[H.J. Lu]

On Sat, Oct 04, 2008 at 03:43:42PM +0930, Alan Modra wrote:
> On Fri, Oct 03, 2008 at 12:41:01PM -0700, H.J. Lu wrote:
> > It is unsafe to use an existing section symbol as group signature
> > since "ld -r" may change section symbol if there is a duplicate.
> > This patch disallows using section symbol as group signature.
> > OK to install?
> 
> This fixes the latest problem HJ has found with section groups and
> ld -r.  To stop ld -r merging sections that are part of a group, ld
> generated unique names for them.  If a renamed section symbol is used
> as the group signature, then the group is broken.  So, don't rename
> sections.  This was suprisingly easy to do with the existing SPECIAL
> section support.
> 

I checked in this patch to add a testcase.

Thanks.


H.J.
----
2008-10-04  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/6931
	* ld-elf/group7.d: New.

--- ld-elf/group7.d.grp	2008-10-04 07:49:53.000000000 -0700
+++ ld-elf/group7.d	2008-10-04 07:52:00.000000000 -0700
@@ -0,0 +1,28 @@
+#source: ../../../binutils/testsuite/binutils-all/group.s
+#source: ../../../binutils/testsuite/binutils-all/group-2.s
+#source: ../../../binutils/testsuite/binutils-all/group-3.s
+#source: ../../../binutils/testsuite/binutils-all/group-4.s
+#ld: -r
+#readelf: -g --wide
+
+#...
+COMDAT group section \[[ 0-9]+\] `foo_group' \[foo_group\] contains 2 sections:
+   \[Index\]    Name
+   \[[ 0-9]+\]   .text.foo
+   \[[ 0-9]+\]   .data.foo
+#...
+COMDAT group section \[[ 0-9]+\] `.group' \[.text.foo\] contains 2 sections:
+   \[Index\]    Name
+   \[[ 0-9]+\]   .text.foo
+   \[[ 0-9]+\]   .data.bar
+#...
+COMDAT group section \[[ 0-9]+\] `.group' \[foo3\] contains 2 sections:
+   \[Index\]    Name
+   \[[ 0-9]+\]   .text.foo3
+   \[[ 0-9]+\]   .data.bar3
+#...
+COMDAT group section \[[ 0-9]+\] `.group' \[foo4\] contains 2 sections:
+   \[Index\]    Name
+   \[[ 0-9]+\]   .text.foo4
+   \[[ 0-9]+\]   .data.foo4
+#pass