RFC: COMDAT group names become anonymouse local symbols

RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

When assembling this trivial file:

	.section	A,"axG",%progbits,B,comdat
        # Note that there is no definition of the symbol B.     		

GAS generates a COMDAT group whose "signature" (in the sense of the
ELF spec) is an anonymous local symbol:

Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 1] B                 GROUP           0000000000000000 000040 000008 04      7   5  4

Symbol table '.symtab' contains 6 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     5: 0000000000000000     0 SECTION LOCAL  DEFAULT    1 

This seems bad.  The documentation for the .section directive says
that B is the group name, and since the linker's collapsing of COMDAT
groups is done based on the *signature*, not the name referred to by
the section header itself, this seems wrong.

On the other hand, perhaps GCC shouldn't generate code like this.  (It
does, at present, for certain C++ inputs.)  The assembler certainly
shouldn't create a global symbol named "B".  If the assembler were to
create a local symbol with the name "B", would the linker still
combine this COMDAT group with another group in another object file
also named "B", even if both had different local symbols as their
signatures?

Thanks,

--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFC: COMDAT group names become anonymouse local symbols

[Cary Coutant]

> When assembling this trivial file:
>
>        .section        A,"axG",%progbits,B,comdat
>        # Note that there is no definition of the symbol B.
>
> GAS generates a COMDAT group whose "signature" (in the sense of the
> ELF spec) is an anonymous local symbol:
>
> Section Headers:
>  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
>  [ 1] B                 GROUP           0000000000000000 000040 000008 04      7   5  4
>
> Symbol table '.symtab' contains 6 entries:
>   Num:    Value          Size Type    Bind   Vis      Ndx Name
>     5: 0000000000000000     0 SECTION LOCAL  DEFAULT    1
>
> This seems bad.  The documentation for the .section directive says
> that B is the group name, and since the linker's collapsing of COMDAT
> groups is done based on the *signature*, not the name referred to by
> the section header itself, this seems wrong.
>
> On the other hand, perhaps GCC shouldn't generate code like this.  (It
> does, at present, for certain C++ inputs.)  The assembler certainly
> shouldn't create a global symbol named "B".  If the assembler were to
> create a local symbol with the name "B", would the linker still
> combine this COMDAT group with another group in another object file
> also named "B", even if both had different local symbols as their
> signatures?

It should; I'm pretty sure that gold will do this correctly, but I
can't say for sure for gnu ld.

If gcc were to generate the local symbol, it looks like gas will do
the right thing and set the name of the SHT_GROUP section to ".group"
with a link field pointing to the key symbol.

I'm guessing that this might be an artifact from gas support for HP's
original comdat implementation on PA-RISC 2.0, where the comdat key
was the name of the SHT_GROUP section.

(The point of using a symbol is that the comdat keys are often
extremely long and are usually already in the symbol table. With my
original design of COMDAT groups at HP, we found that we were bloating
the section string table with strings that were already in the symbol
string table. We fixed this during the Itanium ABI discussions.)

-cary

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

On 10/20/2010 11:05 AM, Cary Coutant wrote:

>>        .section        A,"axG",%progbits,B,comdat
>>        # Note that there is no definition of the symbol B.

>> On the other hand, perhaps GCC shouldn't generate code like this.

> If gcc were to generate the local symbol, it looks like gas will do
> the right thing and set the name of the SHT_GROUP section to ".group"
> with a link field pointing to the key symbol.

So, do you consider this a GCC bug?  Or a GAS bug?

I think we can fix the end-user C++ behavior in either way, but I guess
I feel like this is a GAS bug.  Why should someone writing assembly code
have to define "B"?  Telling the assembler that "B" is the signature of
the group seems like it should be enough.  What do you think?

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFC: COMDAT group names become anonymouse local symbols

[Alan Modra]

On Tue, Oct 19, 2010 at 01:08:38PM -0700, Mark Mitchell wrote:
> 
> When assembling this trivial file:
> 
> 	.section	A,"axG",%progbits,B,comdat
>         # Note that there is no definition of the symbol B.     		
> 
> GAS generates a COMDAT group whose "signature" (in the sense of the
> ELF spec) is an anonymous local symbol:
> 
> Section Headers:
>   [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
>   [ 1] B                 GROUP           0000000000000000 000040 000008 04      7   5  4
> 
> Symbol table '.symtab' contains 6 entries:
>    Num:    Value          Size Type    Bind   Vis      Ndx Name
>      5: 0000000000000000     0 SECTION LOCAL  DEFAULT    1 
> 
> This seems bad.  The documentation for the .section directive says
> that B is the group name, and since the linker's collapsing of COMDAT
> groups is done based on the *signature*, not the name referred to by
> the section header itself, this seems wrong.

I don't see anything bad about this (except possibly if you are using
a linker other than GNU ld or gold).  That isn't an "anonymous" local
symbol.  That is the section symbol for the group section named "B".
GNU ld saves .strtab space by leaving st_name zero on section symbols;
The name of such a symbol is given by the section sh_name.  So the
group signature is indeed "B".

In cases like this, where a non-section symbol having the same name a
the group signature does not already exist, gas could create a normal
local symbol just for the group name.  You'd then get the group name
in .strtab *and* you'd have ".group" in .shstrtab.  With the current
gas fallback, you just have the group name in .shstrtab.  I'll grant
that is not much of a saving, but when I first wrote the gas group
support, gas always named the group section with the name of the group
signature.  It seemed reasonable to leave that as a fallback when a
non-section symbol with the group name does not exist.

> On the other hand, perhaps GCC shouldn't generate code like this.  (It
> does, at present, for certain C++ inputs.)  The assembler certainly
> shouldn't create a global symbol named "B".  If the assembler were to
> create a local symbol with the name "B", would the linker still
> combine this COMDAT group with another group in another object file
> also named "B", even if both had different local symbols as their
> signatures?

Yes.

-- 
Alan Modra
Australia Development Lab, IBM

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

On 10/20/2010 5:18 PM, Alan Modra wrote:

>> This seems bad.  The documentation for the .section directive says
>> that B is the group name, and since the linker's collapsing of COMDAT
>> groups is done based on the *signature*, not the name referred to by
>> the section header itself, this seems wrong.

> I don't see anything bad about this (except possibly if you are using
> a linker other than GNU ld or gold).  That isn't an "anonymous" local
> symbol.  That is the section symbol for the group section named "B".
> GNU ld saves .strtab space by leaving st_name zero on section symbols;
> The name of such a symbol is given by the section sh_name.  So the
> group signature is indeed "B".

The concern here is indeed about compatibility with other linkers.  At
least one linker does not treat the name of the group as "B" in this
case -- and therefore does not correctly collapse COMDAT groups
generated by GAS.

I guess the question is what exactly this bit of the ELF specification
means:

"The name of a symbol from one of the containing object's symbol tables
provides a signature for the section group.  The section header of the
SHT_GROUP section specifies the identifying symbol entry, as described
above: the sh_link member contains the section header index of the
symbol table section that contains the entry. The sh_info member
contains the symbol table index of the identifying entry.

Here, the symbol is the section symbol.  The section symbol has no name,
in the sense that its st_name value is zero.  In fact, the ELF
specification says, in the documentation for "st_name":

"If the value is non-zero, it represents a string table index that gives
the symbol name. Otherwise, the symbol table entry has no name."

I don't see anything that says that the symbol name is implicitly the
name of the associated section; in other words, I don't see how to
justify your statement that "the name of such a symbol is given by the
section sn_name".  So, it seems to me that a literal reading of the ELF
specification says that what Binutils is doing isn't valid and that we
should change the GAS behavior to do:

> In cases like this, where a non-section symbol having the same name a
> the group signature does not already exist, gas could create a normal
> local symbol just for the group name.

Thoughts?

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFC: COMDAT group names become anonymouse local symbols

[Ian Lance Taylor]

Mark Mitchell <mark@codesourcery.com> writes:

> I guess the question is what exactly this bit of the ELF specification
> means:
>
> "The name of a symbol from one of the containing object's symbol tables
> provides a signature for the section group.  The section header of the
> SHT_GROUP section specifies the identifying symbol entry, as described
> above: the sh_link member contains the section header index of the
> symbol table section that contains the entry. The sh_info member
> contains the symbol table index of the identifying entry.
>
> Here, the symbol is the section symbol.  The section symbol has no name,
> in the sense that its st_name value is zero.  In fact, the ELF
> specification says, in the documentation for "st_name":
>
> "If the value is non-zero, it represents a string table index that gives
> the symbol name. Otherwise, the symbol table entry has no name."
>
> I don't see anything that says that the symbol name is implicitly the
> name of the associated section; in other words, I don't see how to
> justify your statement that "the name of such a symbol is given by the
> section sn_name".  So, it seems to me that a literal reading of the ELF
> specification says that what Binutils is doing isn't valid and that we
> should change the GAS behavior to do:
>
>> In cases like this, where a non-section symbol having the same name a
>> the group signature does not already exist, gas could create a normal
>> local symbol just for the group name.
>
> Thoughts?

I note that the corresponding code in gold says this:

  // It seems that some versions of gas will create a section group
  // associated with a section symbol, and then fail to give a name to
  // the section symbol.  In such a case, use the name of the section.

So gold has a specific workaround for this gas behaviour.

The gas behaviour makes sense in that it gives you a slightly smaller
object file, although that is partially offset by the need for the
linker to do extra work to dig up the section name from .shstrtab rather
than the symbol name from .strtab.  But I agree that it does not meet
the letter of the ELF standard.

Ian

Re: RFC: COMDAT group names become anonymouse local symbols

[Alan Modra]

On Wed, Oct 20, 2010 at 05:31:27PM -0700, Mark Mitchell wrote:
> The concern here is indeed about compatibility with other linkers.  At
> least one linker does not treat the name of the group as "B" in this
> case -- and therefore does not correctly collapse COMDAT groups
> generated by GAS.

Hmm, OK.

> I don't see anything that says that the symbol name is implicitly the
> name of the associated section; in other words, I don't see how to
> justify your statement that "the name of such a symbol is given by the
> section sn_name".

Correct, but this is how GNU ld and other binutils have behaved for a
long time.  For example, gas allows access to a section symbol (having
st_name zero) with

 .data
 .long .data

for which readelf shows:

Relocation section '.rel.data' at offset 0x1ec contains 1 entries:
 Offset     Info    Type                Sym. Value  Symbol's Name
00000000  00000201 R_386_32               00000000   .data

There are no unwind sections in this file.

Symbol table '.symtab' contains 4 entries:
   Num:    Value  Size Type    Bind   Vis      Ndx Name
     0: 00000000     0 NOTYPE  LOCAL  DEFAULT  UND 
     1: 00000000     0 SECTION LOCAL  DEFAULT    1 
     2: 00000000     0 SECTION LOCAL  DEFAULT    2 
     3: 00000000     0 SECTION LOCAL  DEFAULT    4 

Note the symbol index on the reloc, the zero st_name in .symtab,
but the reloc is said to be against ".data".

It would be reasonable to say that this behaviour is a GNU extension
to the ELF spec.  We do quite a lot of that.  :-)

As far as changing this to suit some other linker, I'm definitely not
motivated to do the work myself as I don't see this as a bug in gas.
It's a bug in the other linker!

-- 
Alan Modra
Australia Development Lab, IBM

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

On 10/20/2010 6:04 PM, Ian Lance Taylor wrote:

> The gas behaviour makes sense in that it gives you a slightly smaller
> object file, although that is partially offset by the need for the
> linker to do extra work to dig up the section name from .shstrtab rather
> than the symbol name from .strtab.  But I agree that it does not meet
> the letter of the ELF standard.

So, would a patch to make GAS create a local symbol for the group name
be acceptable?  Or would it be rejected on the grounds that it makes
object files bigger?

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFC: COMDAT group names become anonymouse local symbols

[H.J. Lu]

On Wed, Oct 20, 2010 at 9:41 PM, Mark Mitchell <mark@codesourcery.com> wrote:
> On 10/20/2010 6:04 PM, Ian Lance Taylor wrote:
>
>> The gas behaviour makes sense in that it gives you a slightly smaller
>> object file, although that is partially offset by the need for the
>> linker to do extra work to dig up the section name from .shstrtab rather
>> than the symbol name from .strtab.  But I agree that it does not meet
>> the letter of the ELF standard.
>
> So, would a patch to make GAS create a local symbol for the group name
> be acceptable?  Or would it be rejected on the grounds that it makes
> object files bigger?
>

Can't we use ".group" as section name for all group sections?


-- 
H.J.

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

On 10/20/2010 10:19 PM, H.J. Lu wrote:

>>> The gas behaviour makes sense in that it gives you a slightly smaller
>>> object file, although that is partially offset by the need for the
>>> linker to do extra work to dig up the section name from .shstrtab rather
>>> than the symbol name from .strtab.  But I agree that it does not meet
>>> the letter of the ELF standard.
>>
>> So, would a patch to make GAS create a local symbol for the group name
>> be acceptable?  Or would it be rejected on the grounds that it makes
>> object files bigger?

> Can't we use ".group" as section name for all group sections?

Yes, but that by itself doesn't do anything to solve the problem.  The
issue is whether the group signature symbol must have a name, not what
the name on the section is.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFC: COMDAT group names become anonymouse local symbols

[H.J. Lu]

On Wed, Oct 20, 2010 at 10:27 PM, Mark Mitchell <mark@codesourcery.com> wrote:
> On 10/20/2010 10:19 PM, H.J. Lu wrote:
>
>>>> The gas behaviour makes sense in that it gives you a slightly smaller
>>>> object file, although that is partially offset by the need for the
>>>> linker to do extra work to dig up the section name from .shstrtab rather
>>>> than the symbol name from .strtab.  But I agree that it does not meet
>>>> the letter of the ELF standard.
>>>
>>> So, would a patch to make GAS create a local symbol for the group name
>>> be acceptable?  Or would it be rejected on the grounds that it makes
>>> object files bigger?
>
>> Can't we use ".group" as section name for all group sections?
>
> Yes, but that by itself doesn't do anything to solve the problem.  The
> issue is whether the group signature symbol must have a name, not what
> the name on the section is.
>

The symbol table entry of group signature doesn't need a name
since it isn't a real symbol. Having a symbol name for group signature
sounds a bad idea.



-- 
H.J.

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

On 10/20/2010 10:41 PM, H.J. Lu wrote:

> The symbol table entry of group signature doesn't need a name
> since it isn't a real symbol. Having a symbol name for group signature
> sounds a bad idea.

Have you read through the previous messages in this thread?  It seems to
me that there's a consensus that in fact the ELF specification requires
that group signature symbols have a name, and there's certainly a
practical problem in that at least one linker relies on that.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFC: COMDAT group names become anonymouse local symbols

[Cary Coutant]

>>> On the other hand, perhaps GCC shouldn't generate code like this.
>
>> If gcc were to generate the local symbol, it looks like gas will do
>> the right thing and set the name of the SHT_GROUP section to ".group"
>> with a link field pointing to the key symbol.
>
> So, do you consider this a GCC bug?  Or a GAS bug?
>
> I think we can fix the end-user C++ behavior in either way, but I guess
> I feel like this is a GAS bug.  Why should someone writing assembly code
> have to define "B"?  Telling the assembler that "B" is the signature of
> the group seems like it should be enough.  What do you think?

Judging by the responses so far, it sounds like the GAS behavior to
leave the section symbol's name blank is intended behavior, even if
not completely legal according to the ELF spec. But the real issue is
whether using the section symbol as the comdat key is the right thing
to do, rather than creating a local symbol for the purpose. For
consistency, I'd argue that SHT_GROUP sections ought to be named
independently of whether or not a symbol for the comdat key exists,
and that GAS ought to simply generate a new local symbol if necessary.

Since no one has offered any support for my wild guess about this
being an artifact of support for the old HP comdat design, I think
it's safe to discount that and just change GAS.

On the other hand, I wouldn't object to changing both GCC and GAS:
change GCC to generate the symbol in all cases, and change GAS to
complain about a reference to an undefined symbol for the example you
gave.

-cary

Re: RFC: COMDAT group names become anonymouse local symbols

[Alan Modra]

On Wed, Oct 20, 2010 at 09:41:20PM -0700, Mark Mitchell wrote:
> So, would a patch to make GAS create a local symbol for the group name
> be acceptable?  Or would it be rejected on the grounds that it makes
> object files bigger?

I won't object.  In fact, a patch to do that should simplify the group
support.

-- 
Alan Modra
Australia Development Lab, IBM

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

On 10/20/2010 11:24 PM, Alan Modra wrote:

>> So, would a patch to make GAS create a local symbol for the group name
>> be acceptable?  Or would it be rejected on the grounds that it makes
>> object files bigger?
> 
> I won't object.  In fact, a patch to do that should simplify the group
> support.

Thank you.

I tried to change obj-elf.c:elf_frob_file to create the symbol if the
test for its presence comes back false -- but calling symbol_new trapped
because the symbol table is already frozen at that point.

What API should I be using at that point to create the symbol?

Thank you,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFC: COMDAT group names become anonymouse local symbols

[Daniel Jacobowitz]

On Thu, Oct 21, 2010 at 09:41:12AM -0700, Mark Mitchell wrote:
> On 10/20/2010 11:24 PM, Alan Modra wrote:
> 
> >> So, would a patch to make GAS create a local symbol for the group name
> >> be acceptable?  Or would it be rejected on the grounds that it makes
> >> object files bigger?
> > 
> > I won't object.  In fact, a patch to do that should simplify the group
> > support.
> 
> Thank you.
> 
> I tried to change obj-elf.c:elf_frob_file to create the symbol if the
> test for its presence comes back false -- but calling symbol_new trapped
> because the symbol table is already frozen at that point.
> 
> What API should I be using at that point to create the symbol?

You'll have to do it earlier.  Can you do it when the directive was
seen?  Or, will that lead to multiple definition errors if the symbol
is defined later in the file?  If so you'll have to frob before the
symbol table is frozen: obj_adjust_symtab.

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

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

On 10/21/2010 11:50 AM, Daniel Jacobowitz wrote:

> You'll have to do it earlier.  Can you do it when the directive was
> seen?  Or, will that lead to multiple definition errors if the symbol
> is defined later in the file?  If so you'll have to frob before the
> symbol table is frozen: obj_adjust_symtab.

Thanks for the hints!

Creating it at the point the directive is seen does indeed lead to
multiple-definition errors.  The attached patch, however, seems to work.

I need to write test cases, ChangeLog, and run the testsuite -- but I
thought it would be worth posting here to get comments since I'm a GAS
novice.  Does this look sensible?  Are the arguments to symbol_new for
segment and fragment plausible?

Thank you,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

[-- Attachment #2: gas.patch --]
[-- Type: text/plain, Size: 5170 bytes --]

# binutils diff
pushd /scratch/mitchell/builds/fsf-mainline/src/binutils-mainline
cvs -q diff
Index: gas/config/obj-elf.c
===================================================================
RCS file: /cvs/src/src/gas/config/obj-elf.c,v
retrieving revision 1.131
diff -c -5 -p -r1.131 obj-elf.c
*** gas/config/obj-elf.c	16 Sep 2010 23:55:09 -0000	1.131
--- gas/config/obj-elf.c	21 Oct 2010 20:18:59 -0000
*************** static void free_section_idx (const char
*** 2079,2088 ****
--- 2079,2125 ----
  {
    free ((unsigned int *) val);
  }
  
  void
+ elf_adjust_symtab (void)
+ {
+   struct group_list list;
+   unsigned int i;
+ 
+   /* Go find section groups.  */
+   list.num_group = 0;
+   list.head = NULL;
+   list.elt_count = NULL;
+   list.indexes  = hash_new ();
+   bfd_map_over_sections (stdoutput, build_group_lists, &list);
+   
+   /* Make sure that the group signature symbol for each group has the
+      intended name.  */
+   for (i = 0; i < list.num_group; i++)
+     {
+       const char *group_name = elf_group_name (list.head[i]);
+       symbolS *symbolP;
+       /* See if the symbol already exists.  */
+       symbolP = symbol_find_exact (group_name);
+       if (symbolP
+ 	  && (symbolP == symbol_lastP
+ 	      || (symbolP->sy_next != NULL
+ 		  && symbolP->sy_next->sy_previous == symbolP)))
+ 	continue;
+       /* The symbol does not already exist; we must create it now.  */
+       symbolP = symbol_new (group_name, now_seg, (valueT) 0, frag_now);
+       symbol_get_obj (symbolP)->local = 1;
+       symbol_table_insert (symbolP);
+     }
+ 
+   /* Cleanup hash.  */
+   hash_traverse (list.indexes, free_section_idx);
+   hash_die (list.indexes);
+ }
+ 
+ void
  elf_frob_file (void)
  {
    struct group_list list;
    unsigned int i;
  
*************** elf_frob_file (void)
*** 2104,2114 ****
        const char *group_name = elf_group_name (list.head[i]);
        const char *sec_name;
        asection *s;
        flagword flags;
        struct symbol *sy;
-       int has_sym;
        bfd_size_type size;
  
        flags = SEC_READONLY | SEC_HAS_CONTENTS | SEC_IN_MEMORY | SEC_GROUP;
        for (s = list.head[i]; s != NULL; s = elf_next_in_group (s))
  	if ((s->flags ^ flags) & SEC_LINK_ONCE)
--- 2141,2150 ----
*************** elf_frob_file (void)
*** 2120,2140 ****
  			 group_name);
  		break;
  	      }
  	  }
  
!       sec_name = group_name;
        sy = symbol_find_exact (group_name);
!       has_sym = 0;
!       if (sy != NULL
! 	  && (sy == symbol_lastP
! 	      || (sy->sy_next != NULL
! 		  && sy->sy_next->sy_previous == sy)))
! 	{
! 	  has_sym = 1;
! 	  sec_name = ".group";
! 	}
        s = subseg_force_new (sec_name, 0);
        if (s == NULL
  	  || !bfd_set_section_flags (stdoutput, s, flags)
  	  || !bfd_set_section_alignment (stdoutput, s, 2))
  	{
--- 2156,2170 ----
  			 group_name);
  		break;
  	      }
  	  }
  
!       sec_name = ".group";
        sy = symbol_find_exact (group_name);
!       /* If the symbol did not already exist, it has been created in
! 	 elf_adjust_symtab.  */
!       gas_assert (sy != NULL);
        s = subseg_force_new (sec_name, 0);
        if (s == NULL
  	  || !bfd_set_section_flags (stdoutput, s, flags)
  	  || !bfd_set_section_alignment (stdoutput, s, 2))
  	{
*************** elf_frob_file (void)
*** 2143,2154 ****
  	}
        elf_section_type (s) = SHT_GROUP;
  
        /* Pass a pointer to the first section in this group.  */
        elf_next_in_group (s) = list.head[i];
!       if (has_sym)
! 	elf_group_id (s) = sy->bsym;
  
        size = 4 * (list.elt_count[i] + 1);
        bfd_set_section_size (stdoutput, s, size);
        s->contents = (unsigned char *) frag_more (size);
        frag_now->fr_fix = frag_now_fix_octets ();
--- 2173,2183 ----
  	}
        elf_section_type (s) = SHT_GROUP;
  
        /* Pass a pointer to the first section in this group.  */
        elf_next_in_group (s) = list.head[i];
!       elf_group_id (s) = sy->bsym;
  
        size = 4 * (list.elt_count[i] + 1);
        bfd_set_section_size (stdoutput, s, size);
        s->contents = (unsigned char *) frag_more (size);
        frag_now->fr_fix = frag_now_fix_octets ();
Index: gas/config/obj-elf.h
===================================================================
RCS file: /cvs/src/src/gas/config/obj-elf.h,v
retrieving revision 1.37
diff -c -5 -p -r1.37 obj-elf.h
*** gas/config/obj-elf.h	13 Jan 2010 14:08:52 -0000	1.37
--- gas/config/obj-elf.h	21 Oct 2010 20:18:59 -0000
*************** void elf_copy_symbol_attributes (symbolS
*** 195,204 ****
--- 195,209 ----
  #ifndef OBJ_COPY_SYMBOL_ATTRIBUTES
  #define OBJ_COPY_SYMBOL_ATTRIBUTES(DEST, SRC) \
    (elf_copy_symbol_attributes (DEST, SRC))
  #endif
  
+ void elf_adjust_symtab (void);
+ #ifndef obj_adjust_symtab
+ #define obj_adjust_symtab	elf_adjust_symtab
+ #endif
+ 
  #ifndef SEPARATE_STAB_SECTIONS
  /* Avoid ifndef each separate macro setting by wrapping the whole of the
     stab group on the assumption that whoever sets SEPARATE_STAB_SECTIONS
     caters to ECOFF_DEBUGGING and the right setting of INIT_STAB_SECTIONS
     and OBJ_PROCESS_STAB too, without needing the tweaks below.  */
popd

Re: RFC: COMDAT group names become anonymouse local symbols

[Alan Modra]

On Thu, Oct 21, 2010 at 01:21:59PM -0700, Mark Mitchell wrote:
> I need to write test cases, ChangeLog, and run the testsuite -- but I
> thought it would be worth posting here to get comments since I'm a GAS
> novice.  Does this look sensible?  Are the arguments to symbol_new for
> segment and fragment plausible?

I think you'll find that build_group_lists should only run once.  Move
the group handling code in elf_frob_file to elf_adjust_symtab, then
tweak it to create a symbol when necessary.  Create the sym after
subseg_force_new so as to set segment and frag to the group section.

-- 
Alan Modra
Australia Development Lab, IBM

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

On 10/21/2010 3:19 PM, Alan Modra wrote:

> I think you'll find that build_group_lists should only run once.

It looks like it works OK to run it twice (it doesn't affect global data
AFAICT), but I will try what you suggest.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFC: COMDAT group names become anonymouse local symbols

[Alan Modra]

On Thu, Oct 21, 2010 at 03:41:51PM -0700, Mark Mitchell wrote:
> On 10/21/2010 3:19 PM, Alan Modra wrote:
> 
> > I think you'll find that build_group_lists should only run once.
> 
> It looks like it works OK to run it twice (it doesn't affect global data
> AFAICT), but I will try what you suggest.

My mistake.  I was thinking that
      elf_next_in_group (sec) = list->head[*elem_idx];
would mess with the list, but of course it doesn't.  Still, it's worth
moving everything to elf_set_symtab.

-- 
Alan Modra
Australia Development Lab, IBM

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

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

On 10/21/2010 4:11 PM, Alan Modra wrote:

> Still, it's worth moving everything to elf_set_symtab.

How about this version?  This passes the GAS testsuite on
x86_64-unknown-linux-gnu; I've got G++ and libstdc++ tests running as we
speak.

Thank you,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

[-- Attachment #2: gas.patch --]
[-- Type: text/plain, Size: 14407 bytes --]

2010-10-21  Mark Mitchell  <mark@codesourcery.com>

	* config/obj-elf.c (elf_adjust_symtab): New.  Move group section
	processing here from elf_frob_file.  Ensure that group signature
	symbols have the name of the group.
	(elf_frob_file): Move group section processing to
	elf_adjust_symtab.
	* config/obj-elf.h (elf_adjust_symtab): Declare.
	(obj_adjust_symtab): Define.

2010-10-21  Mark Mitchell  <mark@codesourcery.com>

	* gas/elf/elf.exp: Add group0c test.
	* gas/elf/group0c.d: New.
	* gas/elf/group0a.d: Expect ".group" for the name of group
	sections.
	* gas/elf/group0b.d: Likewise.
	* gas/elf/group1a.d: Likewise.
	* gas/elf/group1b.d: Likewise.
	* gas/elf/groupautoa.d: Likewise.
	* gas/elf/groupautob.d: Likewise.
	* gas/elf/section4.d: Likewise.

Index: gas/config/obj-elf.c
===================================================================
RCS file: /cvs/src/src/gas/config/obj-elf.c,v
retrieving revision 1.131
diff -c -5 -p -r1.131 obj-elf.c
*** gas/config/obj-elf.c	16 Sep 2010 23:55:09 -0000	1.131
--- gas/config/obj-elf.c	22 Oct 2010 03:16:59 -0000
*************** static void free_section_idx (const char
*** 2079,2114 ****
  {
    free ((unsigned int *) val);
  }
  
  void
! elf_frob_file (void)
  {
    struct group_list list;
    unsigned int i;
  
-   bfd_map_over_sections (stdoutput, adjust_stab_sections, NULL);
- 
    /* Go find section groups.  */
    list.num_group = 0;
    list.head = NULL;
    list.elt_count = NULL;
!   list.indexes  = hash_new ();
    bfd_map_over_sections (stdoutput, build_group_lists, &list);
! 
!   /* Make the SHT_GROUP sections that describe each section group.  We
!      can't set up the section contents here yet, because elf section
!      indices have yet to be calculated.  elf.c:set_group_contents does
!      the rest of the work.  */
    for (i = 0; i < list.num_group; i++)
      {
        const char *group_name = elf_group_name (list.head[i]);
        const char *sec_name;
        asection *s;
        flagword flags;
        struct symbol *sy;
-       int has_sym;
        bfd_size_type size;
  
        flags = SEC_READONLY | SEC_HAS_CONTENTS | SEC_IN_MEMORY | SEC_GROUP;
        for (s = list.head[i]; s != NULL; s = elf_next_in_group (s))
  	if ((s->flags ^ flags) & SEC_LINK_ONCE)
--- 2079,2109 ----
  {
    free ((unsigned int *) val);
  }
  
  void
! elf_adjust_symtab (void)
  {
    struct group_list list;
    unsigned int i;
  
    /* Go find section groups.  */
    list.num_group = 0;
    list.head = NULL;
    list.elt_count = NULL;
!   list.indexes = hash_new ();
    bfd_map_over_sections (stdoutput, build_group_lists, &list);
!   
!   /* Make sure that the group signature symbol for each group has the
!      intended name.  */
    for (i = 0; i < list.num_group; i++)
      {
        const char *group_name = elf_group_name (list.head[i]);
        const char *sec_name;
        asection *s;
        flagword flags;
        struct symbol *sy;
        bfd_size_type size;
  
        flags = SEC_READONLY | SEC_HAS_CONTENTS | SEC_IN_MEMORY | SEC_GROUP;
        for (s = list.head[i]; s != NULL; s = elf_next_in_group (s))
  	if ((s->flags ^ flags) & SEC_LINK_ONCE)
*************** elf_frob_file (void)
*** 2120,2139 ****
  			 group_name);
  		break;
  	      }
  	  }
  
!       sec_name = group_name;
        sy = symbol_find_exact (group_name);
!       has_sym = 0;
!       if (sy != NULL
! 	  && (sy == symbol_lastP
! 	      || (sy->sy_next != NULL
! 		  && sy->sy_next->sy_previous == sy)))
  	{
! 	  has_sym = 1;
! 	  sec_name = ".group";
  	}
        s = subseg_force_new (sec_name, 0);
        if (s == NULL
  	  || !bfd_set_section_flags (stdoutput, s, flags)
  	  || !bfd_set_section_alignment (stdoutput, s, 2))
--- 2115,2135 ----
  			 group_name);
  		break;
  	      }
  	  }
  
!       sec_name = ".group";
        sy = symbol_find_exact (group_name);
!       if (!sy
! 	  || (sy != symbol_lastP
! 	      && (sy->sy_next == NULL
! 		  || sy->sy_next->sy_previous != sy)))
  	{
! 	  /* Create the symbol now.  */
! 	  sy = symbol_new (group_name, now_seg, (valueT) 0, frag_now);
! 	  symbol_get_obj (sy)->local = 1;
! 	  symbol_table_insert (sy);
  	}
        s = subseg_force_new (sec_name, 0);
        if (s == NULL
  	  || !bfd_set_section_flags (stdoutput, s, flags)
  	  || !bfd_set_section_alignment (stdoutput, s, 2))
*************** elf_frob_file (void)
*** 2143,2171 ****
  	}
        elf_section_type (s) = SHT_GROUP;
  
        /* Pass a pointer to the first section in this group.  */
        elf_next_in_group (s) = list.head[i];
!       if (has_sym)
! 	elf_group_id (s) = sy->bsym;
  
        size = 4 * (list.elt_count[i] + 1);
        bfd_set_section_size (stdoutput, s, size);
        s->contents = (unsigned char *) frag_more (size);
        frag_now->fr_fix = frag_now_fix_octets ();
        frag_wane (frag_now);
      }
  
- #ifdef elf_tc_final_processing
-   elf_tc_final_processing ();
- #endif
- 
    /* Cleanup hash.  */
    hash_traverse (list.indexes, free_section_idx);
    hash_die (list.indexes);
  }
  
  /* It removes any unneeded versioned symbols from the symbol table.  */
  
  void
  elf_frob_file_before_adjust (void)
  {
--- 2139,2172 ----
  	}
        elf_section_type (s) = SHT_GROUP;
  
        /* Pass a pointer to the first section in this group.  */
        elf_next_in_group (s) = list.head[i];
!       elf_group_id (s) = sy->bsym;
  
        size = 4 * (list.elt_count[i] + 1);
        bfd_set_section_size (stdoutput, s, size);
        s->contents = (unsigned char *) frag_more (size);
        frag_now->fr_fix = frag_now_fix_octets ();
        frag_wane (frag_now);
      }
  
    /* Cleanup hash.  */
    hash_traverse (list.indexes, free_section_idx);
    hash_die (list.indexes);
  }
  
+ void
+ elf_frob_file (void)
+ {
+   bfd_map_over_sections (stdoutput, adjust_stab_sections, NULL);
+ 
+ #ifdef elf_tc_final_processing
+   elf_tc_final_processing ();
+ #endif
+ }
+ 
  /* It removes any unneeded versioned symbols from the symbol table.  */
  
  void
  elf_frob_file_before_adjust (void)
  {
Index: gas/config/obj-elf.h
===================================================================
RCS file: /cvs/src/src/gas/config/obj-elf.h,v
retrieving revision 1.37
diff -c -5 -p -r1.37 obj-elf.h
*** gas/config/obj-elf.h	13 Jan 2010 14:08:52 -0000	1.37
--- gas/config/obj-elf.h	22 Oct 2010 03:16:59 -0000
*************** void elf_copy_symbol_attributes (symbolS
*** 195,204 ****
--- 195,209 ----
  #ifndef OBJ_COPY_SYMBOL_ATTRIBUTES
  #define OBJ_COPY_SYMBOL_ATTRIBUTES(DEST, SRC) \
    (elf_copy_symbol_attributes (DEST, SRC))
  #endif
  
+ void elf_adjust_symtab (void);
+ #ifndef obj_adjust_symtab
+ #define obj_adjust_symtab	elf_adjust_symtab
+ #endif
+ 
  #ifndef SEPARATE_STAB_SECTIONS
  /* Avoid ifndef each separate macro setting by wrapping the whole of the
     stab group on the assumption that whoever sets SEPARATE_STAB_SECTIONS
     caters to ECOFF_DEBUGGING and the right setting of INIT_STAB_SECTIONS
     and OBJ_PROCESS_STAB too, without needing the tweaks below.  */
Index: gas/testsuite/gas/elf/elf.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/elf.exp,v
retrieving revision 1.67
diff -c -5 -p -r1.67 elf.exp
*** gas/testsuite/gas/elf/elf.exp	20 Sep 2010 16:07:27 -0000	1.67
--- gas/testsuite/gas/elf/elf.exp	22 Oct 2010 03:16:59 -0000
*************** if { ([istarget "*-*-*elf*"]
*** 102,111 ****
--- 102,112 ----
  	    run_dump_test "file"
  	}
      }
      run_dump_test "group0a"
      run_dump_test "group0b"
+     run_dump_test "group0c"
      run_dump_test "group1a"
      run_dump_test "group1b"
      run_dump_test "groupautoa"
      run_dump_test "groupautob"
      case $target_triplet in {
Index: gas/testsuite/gas/elf/group0a.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/group0a.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 group0a.d
*** gas/testsuite/gas/elf/group0a.d	25 Nov 2004 00:56:00 -0000	1.2
--- gas/testsuite/gas/elf/group0a.d	22 Oct 2010 03:16:59 -0000
***************
*** 1,10 ****
  #readelf: -SW
  #name: group section
  #source: group0.s
  
  #...
! [ 	]*\[.*\][ 	]+\.foo_group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.foo[ 	]+PROGBITS.*[ 	]+AXG[ 	]+.*
  [ 	]*\[.*\][ 	]+\.bar[ 	]+PROGBITS.*[ 	]+AG[ 	]+.*
  #pass
--- 1,10 ----
  #readelf: -SW
  #name: group section
  #source: group0.s
  
  #...
! [ 	]*\[.*\][ 	]+\.group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.foo[ 	]+PROGBITS.*[ 	]+AXG[ 	]+.*
  [ 	]*\[.*\][ 	]+\.bar[ 	]+PROGBITS.*[ 	]+AG[ 	]+.*
  #pass
Index: gas/testsuite/gas/elf/group0b.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/group0b.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 group0b.d
*** gas/testsuite/gas/elf/group0b.d	25 May 2005 06:30:25 -0000	1.2
--- gas/testsuite/gas/elf/group0b.d	22 Oct 2010 03:16:59 -0000
***************
*** 1,10 ****
  #readelf: -g
  #name: group section
  #source: group0.s
  
  #...
! COMDAT group section \[    1\] `.foo_group' \[.foo_group\] contains 2 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.foo
  [ 	]+\[.*\][ 	]+.bar
  #pass
--- 1,10 ----
  #readelf: -g
  #name: group section
  #source: group0.s
  
  #...
! COMDAT group section \[    1\] `\.group' \[.foo_group\] contains 2 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.foo
  [ 	]+\[.*\][ 	]+.bar
  #pass
Index: gas/testsuite/gas/elf/group0c.d
===================================================================
RCS file: gas/testsuite/gas/elf/group0c.d
diff -N gas/testsuite/gas/elf/group0c.d
*** /dev/null	1 Jan 1970 00:00:00 -0000
--- gas/testsuite/gas/elf/group0c.d	22 Oct 2010 03:16:59 -0000
***************
*** 0 ****
--- 1,7 ----
+ #readelf: -sW
+ #name: group section name
+ #source: group0.s
+ 
+ #...
+ .*NOTYPE[ 	]+LOCAL[ 	]+DEFAULT[ 	]+[0-9]+[ 	]+\.foo_group
+ #pass
Index: gas/testsuite/gas/elf/group1a.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/group1a.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 group1a.d
*** gas/testsuite/gas/elf/group1a.d	25 Nov 2004 00:56:00 -0000	1.2
--- gas/testsuite/gas/elf/group1a.d	22 Oct 2010 03:16:59 -0000
***************
*** 1,11 ****
  #readelf: -SW
  #name: group section with multiple sections of same name
  #source: group1.s
  
  #...
! [ 	]*\[.*\][ 	]+\.foo_group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AX[ 	]+.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AXG[ 	]+.*
  #pass
--- 1,11 ----
  #readelf: -SW
  #name: group section with multiple sections of same name
  #source: group1.s
  
  #...
! [ 	]*\[.*\][ 	]+\.group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AX[ 	]+.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AXG[ 	]+.*
  #pass
Index: gas/testsuite/gas/elf/group1b.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/group1b.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 group1b.d
*** gas/testsuite/gas/elf/group1b.d	25 May 2005 06:30:25 -0000	1.2
--- gas/testsuite/gas/elf/group1b.d	22 Oct 2010 03:16:59 -0000
***************
*** 1,9 ****
  #readelf: -g
  #name: group section with multiple sections of same name
  #source: group1.s
  
  #...
! COMDAT group section \[    1\] `.foo_group' \[.foo_group\] contains 1 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.text
  #pass
--- 1,9 ----
  #readelf: -g
  #name: group section with multiple sections of same name
  #source: group1.s
  
  #...
! COMDAT group section \[    1\] `\.group' \[.foo_group\] contains 1 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.text
  #pass
Index: gas/testsuite/gas/elf/groupautoa.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/groupautoa.d,v
retrieving revision 1.1
diff -c -5 -p -r1.1 groupautoa.d
*** gas/testsuite/gas/elf/groupautoa.d	18 Aug 2010 00:43:46 -0000	1.1
--- gas/testsuite/gas/elf/groupautoa.d	22 Oct 2010 03:16:59 -0000
***************
*** 1,11 ****
  #readelf: -SW
  #name: automatic section group
  #source: groupauto.s
  
  #...
! [ 	]*\[.*\][ 	]+some_group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AX[ 	]+.*
  #...
  [ 	]*\[.*\][ 	]+\.foo[ 	]+PROGBITS.*[ 	]+A[ 	]+.*
  #...
--- 1,11 ----
  #readelf: -SW
  #name: automatic section group
  #source: groupauto.s
  
  #...
! [ 	]*\[.*\][ 	]+\.group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AX[ 	]+.*
  #...
  [ 	]*\[.*\][ 	]+\.foo[ 	]+PROGBITS.*[ 	]+A[ 	]+.*
  #...
Index: gas/testsuite/gas/elf/groupautob.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/groupautob.d,v
retrieving revision 1.1
diff -c -5 -p -r1.1 groupautob.d
*** gas/testsuite/gas/elf/groupautob.d	18 Aug 2010 00:43:46 -0000	1.1
--- gas/testsuite/gas/elf/groupautob.d	22 Oct 2010 03:16:59 -0000
***************
*** 1,10 ****
  #readelf: -g
  #name: automatic section group
  #source: groupauto.s
  
  #...
! COMDAT group section \[    1\] `some_group' \[some_group\] contains 2 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.text
  [ 	]+\[.*\][ 	]+.note.bar
  #pass
--- 1,10 ----
  #readelf: -g
  #name: automatic section group
  #source: groupauto.s
  
  #...
! COMDAT group section \[    1\] `\.group' \[some_group\] contains 2 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.text
  [ 	]+\[.*\][ 	]+.note.bar
  #pass
Index: gas/testsuite/gas/elf/section4.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/section4.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 section4.d
*** gas/testsuite/gas/elf/section4.d	25 Nov 2004 00:56:00 -0000	1.2
--- gas/testsuite/gas/elf/section4.d	22 Oct 2010 03:16:59 -0000
***************
*** 1,10 ****
  #readelf: --sections
  #name: label arithmetic with multiple same-name sections
  
  #...
! [ 	]*\[.*\][ 	]+foo[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*
  #...
  [ 	]*\[.*\][ 	]+\.data[ 	]+PROGBITS.*
  #...
--- 1,10 ----
  #readelf: --sections
  #name: label arithmetic with multiple same-name sections
  
  #...
! [ 	]*\[.*\][ 	]+\.group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*
  #...
  [ 	]*\[.*\][ 	]+\.data[ 	]+PROGBITS.*
  #...

Re: RFC: COMDAT group names become anonymouse local symbols

[Alan Modra]

On Thu, Oct 21, 2010 at 08:23:44PM -0700, Mark Mitchell wrote:
> !   /* Make the SHT_GROUP sections that describe each section group.  We
> !      can't set up the section contents here yet, because elf section
> !      indices have yet to be calculated.  elf.c:set_group_contents does
> !      the rest of the work.  */

Keep this comment, please.

> !       sec_name = ".group";
>         sy = symbol_find_exact (group_name);
> !       if (!sy
> ! 	  || (sy != symbol_lastP
> ! 	      && (sy->sy_next == NULL
> ! 		  || sy->sy_next->sy_previous != sy)))
>   	{
> ! 	  /* Create the symbol now.  */
> ! 	  sy = symbol_new (group_name, now_seg, (valueT) 0, frag_now);
> ! 	  symbol_get_obj (sy)->local = 1;
> ! 	  symbol_table_insert (sy);
>   	}
>         s = subseg_force_new (sec_name, 0);
>         if (s == NULL
>   	  || !bfd_set_section_flags (stdoutput, s, flags)
>   	  || !bfd_set_section_alignment (stdoutput, s, 2))

Create symbol after this point, so that now_seg and frag_now are in
the group section.  It doesn't really matter which section you use for
the symbol, but the way you have it you'll get some random section.
In fact, you may as well move the sym check and create to..

> !       elf_group_id (s) = sy->bsym;

..just before here.  Which reminds me, use symbol_get_bfdsym here
since we are before set_symtab.

-- 
Alan Modra
Australia Development Lab, IBM

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

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

On 10/21/2010 9:33 PM, Alan Modra wrote:

> Create symbol after this point, so that now_seg and frag_now are in
> the group section.  It doesn't really matter which section you use for
> the symbol, but the way you have it you'll get some random section.
> In fact, you may as well move the sym check and create to..
> 
>> !       elf_group_id (s) = sy->bsym;
> 
> ..just before here.  Which reminds me, use symbol_get_bfdsym here
> since we are before set_symtab.

How about this version?

Thanks for for helping me to work through this problem.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

[-- Attachment #2: gas.patch --]
[-- Type: text/plain, Size: 14732 bytes --]

2010-10-21  Mark Mitchell  <mark@codesourcery.com>

	* config/obj-elf.c (elf_adjust_symtab): New.  Move group section
	processing here from elf_frob_file.  Ensure that group signature
	symbols have the name of the group.
	(elf_frob_file): Move group section processing to
	elf_adjust_symtab.
	* config/obj-elf.h (elf_adjust_symtab): Declare.
	(obj_adjust_symtab): Define.

2010-10-21  Mark Mitchell  <mark@codesourcery.com>

	* gas/elf/elf.exp: Add group0c test.
	* gas/elf/group0c.d: New.
	* gas/elf/group0a.d: Expect ".group" for the name of group
	sections.
	* gas/elf/group0b.d: Likewise.
	* gas/elf/group1a.d: Likewise.
	* gas/elf/group1b.d: Likewise.
	* gas/elf/groupautoa.d: Likewise.
	* gas/elf/groupautob.d: Likewise.
	* gas/elf/section4.d: Likewise.

2010-10-18  Kai Tietz  <kaI.tietz@onevision.com>
Index: gas/config/obj-elf.c
===================================================================
RCS file: /cvs/src/src/gas/config/obj-elf.c,v
retrieving revision 1.131
diff -c -5 -p -r1.131 obj-elf.c
*** gas/config/obj-elf.c	16 Sep 2010 23:55:09 -0000	1.131
--- gas/config/obj-elf.c	22 Oct 2010 06:06:43 -0000
*************** static void free_section_idx (const char
*** 2079,2114 ****
  {
    free ((unsigned int *) val);
  }
  
  void
! elf_frob_file (void)
  {
    struct group_list list;
    unsigned int i;
  
-   bfd_map_over_sections (stdoutput, adjust_stab_sections, NULL);
- 
    /* Go find section groups.  */
    list.num_group = 0;
    list.head = NULL;
    list.elt_count = NULL;
!   list.indexes  = hash_new ();
    bfd_map_over_sections (stdoutput, build_group_lists, &list);
! 
    /* Make the SHT_GROUP sections that describe each section group.  We
       can't set up the section contents here yet, because elf section
       indices have yet to be calculated.  elf.c:set_group_contents does
       the rest of the work.  */
!   for (i = 0; i < list.num_group; i++)
      {
        const char *group_name = elf_group_name (list.head[i]);
        const char *sec_name;
        asection *s;
        flagword flags;
        struct symbol *sy;
-       int has_sym;
        bfd_size_type size;
  
        flags = SEC_READONLY | SEC_HAS_CONTENTS | SEC_IN_MEMORY | SEC_GROUP;
        for (s = list.head[i]; s != NULL; s = elf_next_in_group (s))
  	if ((s->flags ^ flags) & SEC_LINK_ONCE)
--- 2079,2111 ----
  {
    free ((unsigned int *) val);
  }
  
  void
! elf_adjust_symtab (void)
  {
    struct group_list list;
    unsigned int i;
  
    /* Go find section groups.  */
    list.num_group = 0;
    list.head = NULL;
    list.elt_count = NULL;
!   list.indexes = hash_new ();
    bfd_map_over_sections (stdoutput, build_group_lists, &list);
!   
    /* Make the SHT_GROUP sections that describe each section group.  We
       can't set up the section contents here yet, because elf section
       indices have yet to be calculated.  elf.c:set_group_contents does
       the rest of the work.  */
!  for (i = 0; i < list.num_group; i++)
      {
        const char *group_name = elf_group_name (list.head[i]);
        const char *sec_name;
        asection *s;
        flagword flags;
        struct symbol *sy;
        bfd_size_type size;
  
        flags = SEC_READONLY | SEC_HAS_CONTENTS | SEC_IN_MEMORY | SEC_GROUP;
        for (s = list.head[i]; s != NULL; s = elf_next_in_group (s))
  	if ((s->flags ^ flags) & SEC_LINK_ONCE)
*************** elf_frob_file (void)
*** 2120,2140 ****
  			 group_name);
  		break;
  	      }
  	  }
  
!       sec_name = group_name;
!       sy = symbol_find_exact (group_name);
!       has_sym = 0;
!       if (sy != NULL
! 	  && (sy == symbol_lastP
! 	      || (sy->sy_next != NULL
! 		  && sy->sy_next->sy_previous == sy)))
! 	{
! 	  has_sym = 1;
! 	  sec_name = ".group";
! 	}
        s = subseg_force_new (sec_name, 0);
        if (s == NULL
  	  || !bfd_set_section_flags (stdoutput, s, flags)
  	  || !bfd_set_section_alignment (stdoutput, s, 2))
  	{
--- 2117,2127 ----
  			 group_name);
  		break;
  	      }
  	  }
  
!       sec_name = ".group";
        s = subseg_force_new (sec_name, 0);
        if (s == NULL
  	  || !bfd_set_section_flags (stdoutput, s, flags)
  	  || !bfd_set_section_alignment (stdoutput, s, 2))
  	{
*************** elf_frob_file (void)
*** 2143,2171 ****
  	}
        elf_section_type (s) = SHT_GROUP;
  
        /* Pass a pointer to the first section in this group.  */
        elf_next_in_group (s) = list.head[i];
!       if (has_sym)
! 	elf_group_id (s) = sy->bsym;
  
        size = 4 * (list.elt_count[i] + 1);
        bfd_set_section_size (stdoutput, s, size);
        s->contents = (unsigned char *) frag_more (size);
        frag_now->fr_fix = frag_now_fix_octets ();
        frag_wane (frag_now);
      }
  
- #ifdef elf_tc_final_processing
-   elf_tc_final_processing ();
- #endif
- 
    /* Cleanup hash.  */
    hash_traverse (list.indexes, free_section_idx);
    hash_die (list.indexes);
  }
  
  /* It removes any unneeded versioned symbols from the symbol table.  */
  
  void
  elf_frob_file_before_adjust (void)
  {
--- 2130,2176 ----
  	}
        elf_section_type (s) = SHT_GROUP;
  
        /* Pass a pointer to the first section in this group.  */
        elf_next_in_group (s) = list.head[i];
!       /* Make sure that the signature symbol for the group has the
! 	 name of the group.  */
!       sy = symbol_find_exact (group_name);
!       if (!sy
! 	  || (sy != symbol_lastP
! 	      && (sy->sy_next == NULL
! 		  || sy->sy_next->sy_previous != sy)))
! 	{
! 	  /* Create the symbol now.  */
! 	  sy = symbol_new (group_name, now_seg, (valueT) 0, frag_now);
! 	  symbol_get_obj (sy)->local = 1;
! 	  symbol_table_insert (sy);
! 	}
!       elf_group_id (s) = symbol_get_bfdsym (sy);
  
        size = 4 * (list.elt_count[i] + 1);
        bfd_set_section_size (stdoutput, s, size);
        s->contents = (unsigned char *) frag_more (size);
        frag_now->fr_fix = frag_now_fix_octets ();
        frag_wane (frag_now);
      }
  
    /* Cleanup hash.  */
    hash_traverse (list.indexes, free_section_idx);
    hash_die (list.indexes);
  }
  
+ void
+ elf_frob_file (void)
+ {
+   bfd_map_over_sections (stdoutput, adjust_stab_sections, NULL);
+ 
+ #ifdef elf_tc_final_processing
+   elf_tc_final_processing ();
+ #endif
+ }
+ 
  /* It removes any unneeded versioned symbols from the symbol table.  */
  
  void
  elf_frob_file_before_adjust (void)
  {
Index: gas/config/obj-elf.h
===================================================================
RCS file: /cvs/src/src/gas/config/obj-elf.h,v
retrieving revision 1.37
diff -c -5 -p -r1.37 obj-elf.h
*** gas/config/obj-elf.h	13 Jan 2010 14:08:52 -0000	1.37
--- gas/config/obj-elf.h	22 Oct 2010 06:06:43 -0000
*************** void elf_copy_symbol_attributes (symbolS
*** 195,204 ****
--- 195,209 ----
  #ifndef OBJ_COPY_SYMBOL_ATTRIBUTES
  #define OBJ_COPY_SYMBOL_ATTRIBUTES(DEST, SRC) \
    (elf_copy_symbol_attributes (DEST, SRC))
  #endif
  
+ void elf_adjust_symtab (void);
+ #ifndef obj_adjust_symtab
+ #define obj_adjust_symtab	elf_adjust_symtab
+ #endif
+ 
  #ifndef SEPARATE_STAB_SECTIONS
  /* Avoid ifndef each separate macro setting by wrapping the whole of the
     stab group on the assumption that whoever sets SEPARATE_STAB_SECTIONS
     caters to ECOFF_DEBUGGING and the right setting of INIT_STAB_SECTIONS
     and OBJ_PROCESS_STAB too, without needing the tweaks below.  */
Index: gas/testsuite/gas/elf/elf.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/elf.exp,v
retrieving revision 1.67
diff -c -5 -p -r1.67 elf.exp
*** gas/testsuite/gas/elf/elf.exp	20 Sep 2010 16:07:27 -0000	1.67
--- gas/testsuite/gas/elf/elf.exp	22 Oct 2010 06:06:43 -0000
*************** if { ([istarget "*-*-*elf*"]
*** 102,111 ****
--- 102,112 ----
  	    run_dump_test "file"
  	}
      }
      run_dump_test "group0a"
      run_dump_test "group0b"
+     run_dump_test "group0c"
      run_dump_test "group1a"
      run_dump_test "group1b"
      run_dump_test "groupautoa"
      run_dump_test "groupautob"
      case $target_triplet in {
Index: gas/testsuite/gas/elf/group0a.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/group0a.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 group0a.d
*** gas/testsuite/gas/elf/group0a.d	25 Nov 2004 00:56:00 -0000	1.2
--- gas/testsuite/gas/elf/group0a.d	22 Oct 2010 06:06:43 -0000
***************
*** 1,10 ****
  #readelf: -SW
  #name: group section
  #source: group0.s
  
  #...
! [ 	]*\[.*\][ 	]+\.foo_group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.foo[ 	]+PROGBITS.*[ 	]+AXG[ 	]+.*
  [ 	]*\[.*\][ 	]+\.bar[ 	]+PROGBITS.*[ 	]+AG[ 	]+.*
  #pass
--- 1,10 ----
  #readelf: -SW
  #name: group section
  #source: group0.s
  
  #...
! [ 	]*\[.*\][ 	]+\.group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.foo[ 	]+PROGBITS.*[ 	]+AXG[ 	]+.*
  [ 	]*\[.*\][ 	]+\.bar[ 	]+PROGBITS.*[ 	]+AG[ 	]+.*
  #pass
Index: gas/testsuite/gas/elf/group0b.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/group0b.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 group0b.d
*** gas/testsuite/gas/elf/group0b.d	25 May 2005 06:30:25 -0000	1.2
--- gas/testsuite/gas/elf/group0b.d	22 Oct 2010 06:06:43 -0000
***************
*** 1,10 ****
  #readelf: -g
  #name: group section
  #source: group0.s
  
  #...
! COMDAT group section \[    1\] `.foo_group' \[.foo_group\] contains 2 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.foo
  [ 	]+\[.*\][ 	]+.bar
  #pass
--- 1,10 ----
  #readelf: -g
  #name: group section
  #source: group0.s
  
  #...
! COMDAT group section \[    1\] `\.group' \[.foo_group\] contains 2 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.foo
  [ 	]+\[.*\][ 	]+.bar
  #pass
Index: gas/testsuite/gas/elf/group0c.d
===================================================================
RCS file: gas/testsuite/gas/elf/group0c.d
diff -N gas/testsuite/gas/elf/group0c.d
*** /dev/null	1 Jan 1970 00:00:00 -0000
--- gas/testsuite/gas/elf/group0c.d	22 Oct 2010 06:06:43 -0000
***************
*** 0 ****
--- 1,7 ----
+ #readelf: -sW
+ #name: group section name
+ #source: group0.s
+ 
+ #...
+ .*NOTYPE[ 	]+LOCAL[ 	]+DEFAULT[ 	]+[0-9]+[ 	]+\.foo_group
+ #pass
Index: gas/testsuite/gas/elf/group1a.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/group1a.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 group1a.d
*** gas/testsuite/gas/elf/group1a.d	25 Nov 2004 00:56:00 -0000	1.2
--- gas/testsuite/gas/elf/group1a.d	22 Oct 2010 06:06:43 -0000
***************
*** 1,11 ****
  #readelf: -SW
  #name: group section with multiple sections of same name
  #source: group1.s
  
  #...
! [ 	]*\[.*\][ 	]+\.foo_group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AX[ 	]+.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AXG[ 	]+.*
  #pass
--- 1,11 ----
  #readelf: -SW
  #name: group section with multiple sections of same name
  #source: group1.s
  
  #...
! [ 	]*\[.*\][ 	]+\.group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AX[ 	]+.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AXG[ 	]+.*
  #pass
Index: gas/testsuite/gas/elf/group1b.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/group1b.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 group1b.d
*** gas/testsuite/gas/elf/group1b.d	25 May 2005 06:30:25 -0000	1.2
--- gas/testsuite/gas/elf/group1b.d	22 Oct 2010 06:06:43 -0000
***************
*** 1,9 ****
  #readelf: -g
  #name: group section with multiple sections of same name
  #source: group1.s
  
  #...
! COMDAT group section \[    1\] `.foo_group' \[.foo_group\] contains 1 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.text
  #pass
--- 1,9 ----
  #readelf: -g
  #name: group section with multiple sections of same name
  #source: group1.s
  
  #...
! COMDAT group section \[    1\] `\.group' \[.foo_group\] contains 1 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.text
  #pass
Index: gas/testsuite/gas/elf/groupautoa.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/groupautoa.d,v
retrieving revision 1.1
diff -c -5 -p -r1.1 groupautoa.d
*** gas/testsuite/gas/elf/groupautoa.d	18 Aug 2010 00:43:46 -0000	1.1
--- gas/testsuite/gas/elf/groupautoa.d	22 Oct 2010 06:06:43 -0000
***************
*** 1,11 ****
  #readelf: -SW
  #name: automatic section group
  #source: groupauto.s
  
  #...
! [ 	]*\[.*\][ 	]+some_group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AX[ 	]+.*
  #...
  [ 	]*\[.*\][ 	]+\.foo[ 	]+PROGBITS.*[ 	]+A[ 	]+.*
  #...
--- 1,11 ----
  #readelf: -SW
  #name: automatic section group
  #source: groupauto.s
  
  #...
! [ 	]*\[.*\][ 	]+\.group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AX[ 	]+.*
  #...
  [ 	]*\[.*\][ 	]+\.foo[ 	]+PROGBITS.*[ 	]+A[ 	]+.*
  #...
Index: gas/testsuite/gas/elf/groupautob.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/groupautob.d,v
retrieving revision 1.1
diff -c -5 -p -r1.1 groupautob.d
*** gas/testsuite/gas/elf/groupautob.d	18 Aug 2010 00:43:46 -0000	1.1
--- gas/testsuite/gas/elf/groupautob.d	22 Oct 2010 06:06:43 -0000
***************
*** 1,10 ****
  #readelf: -g
  #name: automatic section group
  #source: groupauto.s
  
  #...
! COMDAT group section \[    1\] `some_group' \[some_group\] contains 2 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.text
  [ 	]+\[.*\][ 	]+.note.bar
  #pass
--- 1,10 ----
  #readelf: -g
  #name: automatic section group
  #source: groupauto.s
  
  #...
! COMDAT group section \[    1\] `\.group' \[some_group\] contains 2 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.text
  [ 	]+\[.*\][ 	]+.note.bar
  #pass
Index: gas/testsuite/gas/elf/section4.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/section4.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 section4.d
*** gas/testsuite/gas/elf/section4.d	25 Nov 2004 00:56:00 -0000	1.2
--- gas/testsuite/gas/elf/section4.d	22 Oct 2010 06:06:43 -0000
***************
*** 1,10 ****
  #readelf: --sections
  #name: label arithmetic with multiple same-name sections
  
  #...
! [ 	]*\[.*\][ 	]+foo[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*
  #...
  [ 	]*\[.*\][ 	]+\.data[ 	]+PROGBITS.*
  #...
--- 1,10 ----
  #readelf: --sections
  #name: label arithmetic with multiple same-name sections
  
  #...
! [ 	]*\[.*\][ 	]+\.group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*
  #...
  [ 	]*\[.*\][ 	]+\.data[ 	]+PROGBITS.*
  #...
popd

Re: RFC: COMDAT group names become anonymouse local symbols

[Alan Modra]

On Thu, Oct 21, 2010 at 11:10:47PM -0700, Mark Mitchell wrote:
> 2010-10-21  Mark Mitchell  <mark@codesourcery.com>
> 
> 	* config/obj-elf.c (elf_adjust_symtab): New.  Move group section
> 	processing here from elf_frob_file.  Ensure that group signature
> 	symbols have the name of the group.
> 	(elf_frob_file): Move group section processing to
> 	elf_adjust_symtab.
> 	* config/obj-elf.h (elf_adjust_symtab): Declare.
> 	(obj_adjust_symtab): Define.
> 
> 2010-10-21  Mark Mitchell  <mark@codesourcery.com>
> 
> 	* gas/elf/elf.exp: Add group0c test.
> 	* gas/elf/group0c.d: New.
> 	* gas/elf/group0a.d: Expect ".group" for the name of group
> 	sections.
> 	* gas/elf/group0b.d: Likewise.
> 	* gas/elf/group1a.d: Likewise.
> 	* gas/elf/group1b.d: Likewise.
> 	* gas/elf/groupautoa.d: Likewise.
> 	* gas/elf/groupautob.d: Likewise.
> 	* gas/elf/section4.d: Likewise.

Looking good but needs a few more patches, and you need to run the
full binutils testsuite.

grep obj_adjust_symtab says you have a tweak to make to config/tc-arm.c
egrep '(group section|GROUP)' will hit a few more testcases in
gas/testsuite/gas/ia64/, binutils/testsuite/binutils-all/, and
ld/testsuite/ld-elf/

-- 
Alan Modra
Australia Development Lab, IBM

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

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

On 10/22/2010 12:15 AM, Alan Modra wrote:

> grep obj_adjust_symtab says you have a tweak to make to config/tc-arm.c
> egrep '(group section|GROUP)' will hit a few more testcases in
> gas/testsuite/gas/ia64/, binutils/testsuite/binutils-all/, and
> ld/testsuite/ld-elf/

Good catches.  I've now run the full testsuite on
x86_64-unknown-linux-gnu, arm-eabi, and ia64-elf.  How about this version?

Thank you,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

[-- Attachment #2: gas.patch --]
[-- Type: text/plain, Size: 25711 bytes --]

2010-10-21  Mark Mitchell  <mark@codesourcery.com>

	* config/obj-elf.c (elf_adjust_symtab): New.  Move group section
	processing here from elf_frob_file.  Ensure that group signature
	symbols have the name of the group.
	(elf_frob_file): Move group section processing to
	elf_adjust_symtab.
	* config/obj-elf.h (elf_adjust_symtab): Declare.
	(obj_adjust_symtab): Define.
	* config/tc-arm.c (arm_adjust_symtab): Call elf_adjust_symtab.

2010-10-22  Mark Mitchell  <mark@codesourcery.com>

	* binutils-all/group-5.d: Expect ".group" for the name of group
	sections.
	* binutils-all/strip-2.d: Likewise.

2010-10-21  Mark Mitchell  <mark@codesourcery.com>

	* gas/elf/elf.exp: Add group0c test.
	* gas/elf/group0c.d: New.
	* gas/elf/group0a.d: Expect ".group" for the name of group
	sections.
	* gas/elf/group0b.d: Likewise.
	* gas/elf/group1a.d: Likewise.
	* gas/elf/group1b.d: Likewise.
	* gas/elf/groupautoa.d: Likewise.
	* gas/elf/groupautob.d: Likewise.
	* gas/elf/section4.d: Likewise.
	* gas/ia64/group-1.d: Likewise.  Adjust hard-coded constants.

2010-10-22  Mark Mitchell  <mark@codesourcery.com>

	* ld-elf/group10.d: Expect ".group" for the name of group
	sections.
	* ld-elf/group2.d: Likewise.
	* ld-elf/group7.d: Likewise.

Index: gas/config/obj-elf.c
===================================================================
RCS file: /cvs/src/src/gas/config/obj-elf.c,v
retrieving revision 1.131
diff -c -5 -p -r1.131 obj-elf.c
*** gas/config/obj-elf.c	16 Sep 2010 23:55:09 -0000	1.131
--- gas/config/obj-elf.c	22 Oct 2010 15:25:19 -0000
*************** static void free_section_idx (const char
*** 2079,2114 ****
  {
    free ((unsigned int *) val);
  }
  
  void
! elf_frob_file (void)
  {
    struct group_list list;
    unsigned int i;
  
-   bfd_map_over_sections (stdoutput, adjust_stab_sections, NULL);
- 
    /* Go find section groups.  */
    list.num_group = 0;
    list.head = NULL;
    list.elt_count = NULL;
!   list.indexes  = hash_new ();
    bfd_map_over_sections (stdoutput, build_group_lists, &list);
! 
    /* Make the SHT_GROUP sections that describe each section group.  We
       can't set up the section contents here yet, because elf section
       indices have yet to be calculated.  elf.c:set_group_contents does
       the rest of the work.  */
!   for (i = 0; i < list.num_group; i++)
      {
        const char *group_name = elf_group_name (list.head[i]);
        const char *sec_name;
        asection *s;
        flagword flags;
        struct symbol *sy;
-       int has_sym;
        bfd_size_type size;
  
        flags = SEC_READONLY | SEC_HAS_CONTENTS | SEC_IN_MEMORY | SEC_GROUP;
        for (s = list.head[i]; s != NULL; s = elf_next_in_group (s))
  	if ((s->flags ^ flags) & SEC_LINK_ONCE)
--- 2079,2111 ----
  {
    free ((unsigned int *) val);
  }
  
  void
! elf_adjust_symtab (void)
  {
    struct group_list list;
    unsigned int i;
  
    /* Go find section groups.  */
    list.num_group = 0;
    list.head = NULL;
    list.elt_count = NULL;
!   list.indexes = hash_new ();
    bfd_map_over_sections (stdoutput, build_group_lists, &list);
!   
    /* Make the SHT_GROUP sections that describe each section group.  We
       can't set up the section contents here yet, because elf section
       indices have yet to be calculated.  elf.c:set_group_contents does
       the rest of the work.  */
!  for (i = 0; i < list.num_group; i++)
      {
        const char *group_name = elf_group_name (list.head[i]);
        const char *sec_name;
        asection *s;
        flagword flags;
        struct symbol *sy;
        bfd_size_type size;
  
        flags = SEC_READONLY | SEC_HAS_CONTENTS | SEC_IN_MEMORY | SEC_GROUP;
        for (s = list.head[i]; s != NULL; s = elf_next_in_group (s))
  	if ((s->flags ^ flags) & SEC_LINK_ONCE)
*************** elf_frob_file (void)
*** 2120,2140 ****
  			 group_name);
  		break;
  	      }
  	  }
  
!       sec_name = group_name;
!       sy = symbol_find_exact (group_name);
!       has_sym = 0;
!       if (sy != NULL
! 	  && (sy == symbol_lastP
! 	      || (sy->sy_next != NULL
! 		  && sy->sy_next->sy_previous == sy)))
! 	{
! 	  has_sym = 1;
! 	  sec_name = ".group";
! 	}
        s = subseg_force_new (sec_name, 0);
        if (s == NULL
  	  || !bfd_set_section_flags (stdoutput, s, flags)
  	  || !bfd_set_section_alignment (stdoutput, s, 2))
  	{
--- 2117,2127 ----
  			 group_name);
  		break;
  	      }
  	  }
  
!       sec_name = ".group";
        s = subseg_force_new (sec_name, 0);
        if (s == NULL
  	  || !bfd_set_section_flags (stdoutput, s, flags)
  	  || !bfd_set_section_alignment (stdoutput, s, 2))
  	{
*************** elf_frob_file (void)
*** 2143,2171 ****
  	}
        elf_section_type (s) = SHT_GROUP;
  
        /* Pass a pointer to the first section in this group.  */
        elf_next_in_group (s) = list.head[i];
!       if (has_sym)
! 	elf_group_id (s) = sy->bsym;
  
        size = 4 * (list.elt_count[i] + 1);
        bfd_set_section_size (stdoutput, s, size);
        s->contents = (unsigned char *) frag_more (size);
        frag_now->fr_fix = frag_now_fix_octets ();
        frag_wane (frag_now);
      }
  
- #ifdef elf_tc_final_processing
-   elf_tc_final_processing ();
- #endif
- 
    /* Cleanup hash.  */
    hash_traverse (list.indexes, free_section_idx);
    hash_die (list.indexes);
  }
  
  /* It removes any unneeded versioned symbols from the symbol table.  */
  
  void
  elf_frob_file_before_adjust (void)
  {
--- 2130,2176 ----
  	}
        elf_section_type (s) = SHT_GROUP;
  
        /* Pass a pointer to the first section in this group.  */
        elf_next_in_group (s) = list.head[i];
!       /* Make sure that the signature symbol for the group has the
! 	 name of the group.  */
!       sy = symbol_find_exact (group_name);
!       if (!sy
! 	  || (sy != symbol_lastP
! 	      && (sy->sy_next == NULL
! 		  || sy->sy_next->sy_previous != sy)))
! 	{
! 	  /* Create the symbol now.  */
! 	  sy = symbol_new (group_name, now_seg, (valueT) 0, frag_now);
! 	  symbol_get_obj (sy)->local = 1;
! 	  symbol_table_insert (sy);
! 	}
!       elf_group_id (s) = symbol_get_bfdsym (sy);
  
        size = 4 * (list.elt_count[i] + 1);
        bfd_set_section_size (stdoutput, s, size);
        s->contents = (unsigned char *) frag_more (size);
        frag_now->fr_fix = frag_now_fix_octets ();
        frag_wane (frag_now);
      }
  
    /* Cleanup hash.  */
    hash_traverse (list.indexes, free_section_idx);
    hash_die (list.indexes);
  }
  
+ void
+ elf_frob_file (void)
+ {
+   bfd_map_over_sections (stdoutput, adjust_stab_sections, NULL);
+ 
+ #ifdef elf_tc_final_processing
+   elf_tc_final_processing ();
+ #endif
+ }
+ 
  /* It removes any unneeded versioned symbols from the symbol table.  */
  
  void
  elf_frob_file_before_adjust (void)
  {
Index: gas/config/obj-elf.h
===================================================================
RCS file: /cvs/src/src/gas/config/obj-elf.h,v
retrieving revision 1.37
diff -c -5 -p -r1.37 obj-elf.h
*** gas/config/obj-elf.h	13 Jan 2010 14:08:52 -0000	1.37
--- gas/config/obj-elf.h	22 Oct 2010 15:25:19 -0000
*************** void elf_copy_symbol_attributes (symbolS
*** 195,204 ****
--- 195,209 ----
  #ifndef OBJ_COPY_SYMBOL_ATTRIBUTES
  #define OBJ_COPY_SYMBOL_ATTRIBUTES(DEST, SRC) \
    (elf_copy_symbol_attributes (DEST, SRC))
  #endif
  
+ void elf_adjust_symtab (void);
+ #ifndef obj_adjust_symtab
+ #define obj_adjust_symtab	elf_adjust_symtab
+ #endif
+ 
  #ifndef SEPARATE_STAB_SECTIONS
  /* Avoid ifndef each separate macro setting by wrapping the whole of the
     stab group on the assumption that whoever sets SEPARATE_STAB_SECTIONS
     caters to ECOFF_DEBUGGING and the right setting of INIT_STAB_SECTIONS
     and OBJ_PROCESS_STAB too, without needing the tweaks below.  */
Index: gas/config/tc-arm.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-arm.c,v
retrieving revision 1.470
diff -c -5 -p -r1.470 tc-arm.c
*** gas/config/tc-arm.c	6 Oct 2010 08:22:21 -0000	1.470
--- gas/config/tc-arm.c	22 Oct 2010 15:25:19 -0000
*************** arm_adjust_symtab (void)
*** 21955,21964 ****
--- 21955,21966 ----
  	}
      }
  
    /* Remove any overlapping mapping symbols generated by alignment frags.  */
    bfd_map_over_sections (stdoutput, check_mapping_symbols, (char *) 0);
+   /* Now do generic ELF adjustments.  */
+   elf_adjust_symtab ();
  #endif
  }
  
  /* MD interface: Initialization.  */
  
Index: binutils/testsuite/binutils-all/group-5.d
===================================================================
RCS file: /cvs/src/src/binutils/testsuite/binutils-all/group-5.d,v
retrieving revision 1.1
diff -c -5 -p -r1.1 group-5.d
*** binutils/testsuite/binutils-all/group-5.d	18 Feb 2010 00:13:30 -0000	1.1
--- binutils/testsuite/binutils-all/group-5.d	22 Oct 2010 15:25:19 -0000
***************
*** 4,19 ****
  #name: copy removing group member
  
  #readelf: -Sg --wide
  
  #...
!   \[[ 0-9]+\] foo_group[ \t]+GROUP[ \t]+.*
  #...
    \[[ 0-9]+\] \.text.*[ \t]+PROGBITS[ \t0-9a-f]+AXG.*
  #...
    \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG.*
  #...
! COMDAT group section \[[ 0-9]+\] `foo_group' \[foo_group\] contains 2 sections:
     \[Index\]    Name
     \[[ 0-9]+\]   .text.*
     \[[ 0-9]+\]   .data.*
  #pass
--- 4,19 ----
  #name: copy removing group member
  
  #readelf: -Sg --wide
  
  #...
!   \[[ 0-9]+\] \.group[ \t]+GROUP[ \t]+.*
  #...
    \[[ 0-9]+\] \.text.*[ \t]+PROGBITS[ \t0-9a-f]+AXG.*
  #...
    \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG.*
  #...
! COMDAT group section \[[ 0-9]+\] `\.group' \[foo_group\] contains 2 sections:
     \[Index\]    Name
     \[[ 0-9]+\]   .text.*
     \[[ 0-9]+\]   .data.*
  #pass
Index: binutils/testsuite/binutils-all/strip-2.d
===================================================================
RCS file: /cvs/src/src/binutils/testsuite/binutils-all/strip-2.d,v
retrieving revision 1.1
diff -c -5 -p -r1.1 strip-2.d
*** binutils/testsuite/binutils-all/strip-2.d	14 Sep 2006 23:37:35 -0000	1.1
--- binutils/testsuite/binutils-all/strip-2.d	22 Oct 2010 15:25:19 -0000
***************
*** 3,18 ****
  #strip: --strip-unneeded
  #readelf: -Sg --wide
  #name: strip with section group 2
  
  #...
!   \[[ 0-9]+\] foo_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]+\] `foo_group' \[foo_group\] contains 2 sections:
     \[Index\]    Name
     \[[ 0-9]+\]   .text.*
     \[[ 0-9]+\]   .data.*
  #pass
--- 3,18 ----
  #strip: --strip-unneeded
  #readelf: -Sg --wide
  #name: strip with section group 2
  
  #...
!   \[[ 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_group\] contains 2 sections:
     \[Index\]    Name
     \[[ 0-9]+\]   .text.*
     \[[ 0-9]+\]   .data.*
  #pass
Index: gas/testsuite/gas/elf/elf.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/elf.exp,v
retrieving revision 1.67
diff -c -5 -p -r1.67 elf.exp
*** gas/testsuite/gas/elf/elf.exp	20 Sep 2010 16:07:27 -0000	1.67
--- gas/testsuite/gas/elf/elf.exp	22 Oct 2010 15:25:20 -0000
*************** if { ([istarget "*-*-*elf*"]
*** 102,111 ****
--- 102,112 ----
  	    run_dump_test "file"
  	}
      }
      run_dump_test "group0a"
      run_dump_test "group0b"
+     run_dump_test "group0c"
      run_dump_test "group1a"
      run_dump_test "group1b"
      run_dump_test "groupautoa"
      run_dump_test "groupautob"
      case $target_triplet in {
Index: gas/testsuite/gas/elf/group0a.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/group0a.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 group0a.d
*** gas/testsuite/gas/elf/group0a.d	25 Nov 2004 00:56:00 -0000	1.2
--- gas/testsuite/gas/elf/group0a.d	22 Oct 2010 15:25:20 -0000
***************
*** 1,10 ****
  #readelf: -SW
  #name: group section
  #source: group0.s
  
  #...
! [ 	]*\[.*\][ 	]+\.foo_group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.foo[ 	]+PROGBITS.*[ 	]+AXG[ 	]+.*
  [ 	]*\[.*\][ 	]+\.bar[ 	]+PROGBITS.*[ 	]+AG[ 	]+.*
  #pass
--- 1,10 ----
  #readelf: -SW
  #name: group section
  #source: group0.s
  
  #...
! [ 	]*\[.*\][ 	]+\.group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.foo[ 	]+PROGBITS.*[ 	]+AXG[ 	]+.*
  [ 	]*\[.*\][ 	]+\.bar[ 	]+PROGBITS.*[ 	]+AG[ 	]+.*
  #pass
Index: gas/testsuite/gas/elf/group0b.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/group0b.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 group0b.d
*** gas/testsuite/gas/elf/group0b.d	25 May 2005 06:30:25 -0000	1.2
--- gas/testsuite/gas/elf/group0b.d	22 Oct 2010 15:25:20 -0000
***************
*** 1,10 ****
  #readelf: -g
  #name: group section
  #source: group0.s
  
  #...
! COMDAT group section \[    1\] `.foo_group' \[.foo_group\] contains 2 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.foo
  [ 	]+\[.*\][ 	]+.bar
  #pass
--- 1,10 ----
  #readelf: -g
  #name: group section
  #source: group0.s
  
  #...
! COMDAT group section \[    1\] `\.group' \[.foo_group\] contains 2 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.foo
  [ 	]+\[.*\][ 	]+.bar
  #pass
Index: gas/testsuite/gas/elf/group0c.d
===================================================================
RCS file: gas/testsuite/gas/elf/group0c.d
diff -N gas/testsuite/gas/elf/group0c.d
*** /dev/null	1 Jan 1970 00:00:00 -0000
--- gas/testsuite/gas/elf/group0c.d	22 Oct 2010 15:25:20 -0000
***************
*** 0 ****
--- 1,7 ----
+ #readelf: -sW
+ #name: group section name
+ #source: group0.s
+ 
+ #...
+ .*NOTYPE[ 	]+LOCAL[ 	]+DEFAULT[ 	]+[0-9]+[ 	]+\.foo_group
+ #pass
Index: gas/testsuite/gas/elf/group1a.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/group1a.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 group1a.d
*** gas/testsuite/gas/elf/group1a.d	25 Nov 2004 00:56:00 -0000	1.2
--- gas/testsuite/gas/elf/group1a.d	22 Oct 2010 15:25:20 -0000
***************
*** 1,11 ****
  #readelf: -SW
  #name: group section with multiple sections of same name
  #source: group1.s
  
  #...
! [ 	]*\[.*\][ 	]+\.foo_group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AX[ 	]+.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AXG[ 	]+.*
  #pass
--- 1,11 ----
  #readelf: -SW
  #name: group section with multiple sections of same name
  #source: group1.s
  
  #...
! [ 	]*\[.*\][ 	]+\.group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AX[ 	]+.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AXG[ 	]+.*
  #pass
Index: gas/testsuite/gas/elf/group1b.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/group1b.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 group1b.d
*** gas/testsuite/gas/elf/group1b.d	25 May 2005 06:30:25 -0000	1.2
--- gas/testsuite/gas/elf/group1b.d	22 Oct 2010 15:25:20 -0000
***************
*** 1,9 ****
  #readelf: -g
  #name: group section with multiple sections of same name
  #source: group1.s
  
  #...
! COMDAT group section \[    1\] `.foo_group' \[.foo_group\] contains 1 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.text
  #pass
--- 1,9 ----
  #readelf: -g
  #name: group section with multiple sections of same name
  #source: group1.s
  
  #...
! COMDAT group section \[    1\] `\.group' \[.foo_group\] contains 1 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.text
  #pass
Index: gas/testsuite/gas/elf/groupautoa.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/groupautoa.d,v
retrieving revision 1.1
diff -c -5 -p -r1.1 groupautoa.d
*** gas/testsuite/gas/elf/groupautoa.d	18 Aug 2010 00:43:46 -0000	1.1
--- gas/testsuite/gas/elf/groupautoa.d	22 Oct 2010 15:25:20 -0000
***************
*** 1,11 ****
  #readelf: -SW
  #name: automatic section group
  #source: groupauto.s
  
  #...
! [ 	]*\[.*\][ 	]+some_group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AX[ 	]+.*
  #...
  [ 	]*\[.*\][ 	]+\.foo[ 	]+PROGBITS.*[ 	]+A[ 	]+.*
  #...
--- 1,11 ----
  #readelf: -SW
  #name: automatic section group
  #source: groupauto.s
  
  #...
! [ 	]*\[.*\][ 	]+\.group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*[ 	]+AX[ 	]+.*
  #...
  [ 	]*\[.*\][ 	]+\.foo[ 	]+PROGBITS.*[ 	]+A[ 	]+.*
  #...
Index: gas/testsuite/gas/elf/groupautob.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/groupautob.d,v
retrieving revision 1.1
diff -c -5 -p -r1.1 groupautob.d
*** gas/testsuite/gas/elf/groupautob.d	18 Aug 2010 00:43:46 -0000	1.1
--- gas/testsuite/gas/elf/groupautob.d	22 Oct 2010 15:25:20 -0000
***************
*** 1,10 ****
  #readelf: -g
  #name: automatic section group
  #source: groupauto.s
  
  #...
! COMDAT group section \[    1\] `some_group' \[some_group\] contains 2 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.text
  [ 	]+\[.*\][ 	]+.note.bar
  #pass
--- 1,10 ----
  #readelf: -g
  #name: automatic section group
  #source: groupauto.s
  
  #...
! COMDAT group section \[    1\] `\.group' \[some_group\] contains 2 sections:
  [ 	]+\[Index\][ 	]+Name
  [ 	]+\[.*\][ 	]+.text
  [ 	]+\[.*\][ 	]+.note.bar
  #pass
Index: gas/testsuite/gas/elf/section4.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/elf/section4.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 section4.d
*** gas/testsuite/gas/elf/section4.d	25 Nov 2004 00:56:00 -0000	1.2
--- gas/testsuite/gas/elf/section4.d	22 Oct 2010 15:25:20 -0000
***************
*** 1,10 ****
  #readelf: --sections
  #name: label arithmetic with multiple same-name sections
  
  #...
! [ 	]*\[.*\][ 	]+foo[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*
  #...
  [ 	]*\[.*\][ 	]+\.data[ 	]+PROGBITS.*
  #...
--- 1,10 ----
  #readelf: --sections
  #name: label arithmetic with multiple same-name sections
  
  #...
! [ 	]*\[.*\][ 	]+\.group[ 	]+GROUP.*
  #...
  [ 	]*\[.*\][ 	]+\.text[ 	]+PROGBITS.*
  #...
  [ 	]*\[.*\][ 	]+\.data[ 	]+PROGBITS.*
  #...
Index: gas/testsuite/gas/ia64/group-1.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/ia64/group-1.d,v
retrieving revision 1.5
diff -c -5 -p -r1.5 group-1.d
*** gas/testsuite/gas/ia64/group-1.d	19 Jul 2010 15:11:03 -0000	1.5
--- gas/testsuite/gas/ia64/group-1.d	22 Oct 2010 15:25:20 -0000
*************** There are 9 section headers, starting at
*** 6,32 ****
  Section Headers:
    \[Nr\] Name              Type             Address           Offset
         Size              EntSize          Flags  Link  Info  Align
    \[ 0\]                   NULL             0000000000000000  00000000
         0000000000000000  0000000000000000           0     0     0
!   \[ 1\] \._foo             GROUP            0000000000000000  00000040
         0000000000000008  0000000000000004           7     6     4
    \[ 2\] \.text             PROGBITS         0000000000000000  00000050
         0000000000000000  0000000000000000  AX       0     0     16
    \[ 3\] \.data             PROGBITS         0000000000000000  00000050
         0000000000000000  0000000000000000  WA       0     0     1
    \[ 4\] \.bss              NOBITS           0000000000000000  00000050
         0000000000000000  0000000000000000  WA       0     0     1
    \[ 5\] \.text             PROGBITS         0000000000000000  00000050
         0000000000000010  0000000000000000 AXG       0     0     16
    \[ 6\] \.shstrtab         STRTAB           0000000000000000  00000060
!        0000000000000032  0000000000000000           0     0     1
    \[ 7\] \.symtab           SYMTAB           0000000000000000  000002d8
!        00000000000000a8  0000000000000018           8     7     8
!   \[ 8\] \.strtab           STRTAB           0000000000000000  00000380
!        0000000000000006  0000000000000000           0     0     1
  Key to Flags:
  #...
  
! COMDAT group section \[    1\] `\._foo' \[\._foo\] contains 1 sections:
     \[Index\]    Name
     \[    5\]   \.text
--- 6,32 ----
  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
         0000000000000008  0000000000000004           7     6     4
    \[ 2\] \.text             PROGBITS         0000000000000000  00000050
         0000000000000000  0000000000000000  AX       0     0     16
    \[ 3\] \.data             PROGBITS         0000000000000000  00000050
         0000000000000000  0000000000000000  WA       0     0     1
    \[ 4\] \.bss              NOBITS           0000000000000000  00000050
         0000000000000000  0000000000000000  WA       0     0     1
    \[ 5\] \.text             PROGBITS         0000000000000000  00000050
         0000000000000010  0000000000000000 AXG       0     0     16
    \[ 6\] \.shstrtab         STRTAB           0000000000000000  00000060
!        0000000000000033  0000000000000000           0     0     1
    \[ 7\] \.symtab           SYMTAB           0000000000000000  000002d8
!        00000000000000c0  0000000000000018           8     8     8
!   \[ 8\] \.strtab           STRTAB           0000000000000000  00000398
!        000000000000000c  0000000000000000           0     0     1
  Key to Flags:
  #...
  
! COMDAT group section \[    1\] `\.group' \[\._foo\] contains 1 sections:
     \[Index\]    Name
     \[    5\]   \.text
Index: ld/testsuite/ld-elf/group10.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-elf/group10.d,v
retrieving revision 1.1
diff -c -5 -p -r1.1 group10.d
*** ld/testsuite/ld-elf/group10.d	19 Feb 2010 01:47:16 -0000	1.1
--- ld/testsuite/ld-elf/group10.d	22 Oct 2010 15:25:23 -0000
***************
*** 1,11 ****
  #source: group10.s
  #ld: -r -T group.ld
  #readelf: -Sg --wide
  
  #...
! group section \[[ 0-9]+\] `foo_group' \[foo_group\] contains 4 sections:
     \[Index\]    Name
     \[[ 0-9]+\]   \.text.*
     \[[ 0-9]+\]   \.rodata\.str.*
     \[[ 0-9]+\]   \.data.*
     \[[ 0-9]+\]   \.keepme.*
--- 1,11 ----
  #source: group10.s
  #ld: -r -T group.ld
  #readelf: -Sg --wide
  
  #...
! group section \[[ 0-9]+\] `\.group' \[foo_group\] contains 4 sections:
     \[Index\]    Name
     \[[ 0-9]+\]   \.text.*
     \[[ 0-9]+\]   \.rodata\.str.*
     \[[ 0-9]+\]   \.data.*
     \[[ 0-9]+\]   \.keepme.*
Index: ld/testsuite/ld-elf/group2.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-elf/group2.d,v
retrieving revision 1.3
diff -c -5 -p -r1.3 group2.d
*** ld/testsuite/ld-elf/group2.d	23 Sep 2010 12:24:41 -0000	1.3
--- ld/testsuite/ld-elf/group2.d	22 Oct 2010 15:25:23 -0000
***************
*** 5,20 ****
  # cr16 and crx use non-standard scripts with memory regions, which don't play
  # well with unique group sections under ld -r.
  # xstormy also uses a non-standard script, putting .data before .text.
  
  #...
!   \[[ 0-9]+\] foo_group[ \t]+GROUP[ \t]+.*
  #...
    \[[ 0-9]+\] \.text.*[ \t]+PROGBITS[ \t0-9a-f]+AXG.*
  #...
    \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG.*
  #...
! COMDAT group section \[[ 0-9]+\] `foo_group' \[foo_group\] contains 2 sections:
     \[Index\]    Name
     \[[ 0-9]+\]   .text.*
     \[[ 0-9]+\]   .data.*
  #pass
--- 5,20 ----
  # cr16 and crx use non-standard scripts with memory regions, which don't play
  # well with unique group sections under ld -r.
  # xstormy also uses a non-standard script, putting .data before .text.
  
  #...
!   \[[ 0-9]+\] \.group[ \t]+GROUP[ \t]+.*
  #...
    \[[ 0-9]+\] \.text.*[ \t]+PROGBITS[ \t0-9a-f]+AXG.*
  #...
    \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAG.*
  #...
! COMDAT group section \[[ 0-9]+\] `\.group' \[foo_group\] contains 2 sections:
     \[Index\]    Name
     \[[ 0-9]+\]   .text.*
     \[[ 0-9]+\]   .data.*
  #pass
Index: ld/testsuite/ld-elf/group7.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-elf/group7.d,v
retrieving revision 1.2
diff -c -5 -p -r1.2 group7.d
*** ld/testsuite/ld-elf/group7.d	18 Sep 2010 02:30:41 -0000	1.2
--- ld/testsuite/ld-elf/group7.d	22 Oct 2010 15:25:23 -0000
***************
*** 7,17 ****
  #xfail: cr16-*-* crx-*-*
  # cr16 and crx use non-standard scripts with memory regions, which don't play
  # well with unique group sections under ld -r.
  
  #...
! 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:
--- 7,17 ----
  #xfail: cr16-*-* crx-*-*
  # cr16 and crx use non-standard scripts with memory regions, which don't play
  # well with unique group sections under ld -r.
  
  #...
! COMDAT group section \[[ 0-9]+\] `\.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:

Re: RFC: COMDAT group names become anonymouse local symbols

[Alan Modra]

On Fri, Oct 22, 2010 at 08:34:40AM -0700, Mark Mitchell wrote:
> 2010-10-21  Mark Mitchell  <mark@codesourcery.com>
> 
> 	* config/obj-elf.c (elf_adjust_symtab): New.  Move group section
> 	processing here from elf_frob_file.  Ensure that group signature
> 	symbols have the name of the group.
> 	(elf_frob_file): Move group section processing to
> 	elf_adjust_symtab.
> 	* config/obj-elf.h (elf_adjust_symtab): Declare.
> 	(obj_adjust_symtab): Define.
> 	* config/tc-arm.c (arm_adjust_symtab): Call elf_adjust_symtab.
> 
> 2010-10-22  Mark Mitchell  <mark@codesourcery.com>
> 
> 	* binutils-all/group-5.d: Expect ".group" for the name of group
> 	sections.
> 	* binutils-all/strip-2.d: Likewise.
> 
> 2010-10-21  Mark Mitchell  <mark@codesourcery.com>
> 
> 	* gas/elf/elf.exp: Add group0c test.
> 	* gas/elf/group0c.d: New.
> 	* gas/elf/group0a.d: Expect ".group" for the name of group
> 	sections.
> 	* gas/elf/group0b.d: Likewise.
> 	* gas/elf/group1a.d: Likewise.
> 	* gas/elf/group1b.d: Likewise.
> 	* gas/elf/groupautoa.d: Likewise.
> 	* gas/elf/groupautob.d: Likewise.
> 	* gas/elf/section4.d: Likewise.
> 	* gas/ia64/group-1.d: Likewise.  Adjust hard-coded constants.
> 
> 2010-10-22  Mark Mitchell  <mark@codesourcery.com>
> 
> 	* ld-elf/group10.d: Expect ".group" for the name of group
> 	sections.
> 	* ld-elf/group2.d: Likewise.
> 	* ld-elf/group7.d: Likewise.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

On 10/23/2010 1:15 AM, Alan Modra wrote:

>> 2010-10-21  Mark Mitchell  <mark@codesourcery.com>
>>
>> 	* config/obj-elf.c (elf_adjust_symtab): New.  Move group section
>> 	processing here from elf_frob_file.  Ensure that group signature
>> 	symbols have the name of the group.
>> 	(elf_frob_file): Move group section processing to
>> 	elf_adjust_symtab.
>> 	* config/obj-elf.h (elf_adjust_symtab): Declare.
>> 	(obj_adjust_symtab): Define.
>> 	* config/tc-arm.c (arm_adjust_symtab): Call elf_adjust_symtab.

> OK.

Committed, thanks.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFC: COMDAT group names become anonymouse local symbols

[Hans-Peter Nilsson]

> Date: Sat, 23 Oct 2010 11:05:49 -0700
> From: Mark Mitchell <mark@codesourcery.com>

> >> 2010-10-21  Mark Mitchell  <mark@codesourcery.com>
> >>
> >> 	* config/obj-elf.c (elf_adjust_symtab): New.  Move group section
> >> 	processing here from elf_frob_file.  Ensure that group signature
> >> 	symbols have the name of the group.
> >> 	(elf_frob_file): Move group section processing to
> >> 	elf_adjust_symtab.
> >> 	* config/obj-elf.h (elf_adjust_symtab): Declare.
> >> 	(obj_adjust_symtab): Define.
> >> 	* config/tc-arm.c (arm_adjust_symtab): Call elf_adjust_symtab.
> Committed, thanks.

It looks like this patch caused my cris-elf autotester to
regress.  For the ld tests, after your commit:

Running /tmp/hpautotest-binutils/bsrc/src/ld/testsuite/ld-cris/cris.exp ...
FAIL: ld-cris/def2
FAIL: ld-cris/undef2
FAIL: ld-cris/warn1
FAIL: ld-cris/warn3

From  the ld.log:
Executing on host: sh -c {/tmp/hpautotest-binutils/cris-axis-elf/ld/../gas/as-new  --em=crisaout  -o tmpdir/dump0.o /tmp/hpautotest-binutils/bsrc/src/ld/testsuite/ld-cris/start1.s 2>&1}  /dev/null ld.tmp (timeout = 300)
sh: line 1:  6009 Segmentation fault      /tmp/hpautotest-binutils/cris-axis-elf/ld/../gas/as-new --em=crisaout -o tmpdir/dump0.o /tmp/hpautotest-binutils/bsrc/src/ld/testsuite/ld-cris/start1.s 2>&1

Same for the other FAILs.

I guess your new code is missing an adjustment to
gas/config/obj-multi.[hc] to add obj_adjust_symtab there to
handle (not break for) multi-objformat targets: when obj-elf.c
is compiled as not *the* object-format but one of several, and
when another format is selected.

(I'm open to just deprecating and removing the whole
multi-format machinery too, and cris-aout too, as long as you
make the --em=... option an assert.  Patches welcome; I'm sure
global maintainers agree. :)

brgds, H-P

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

On 10/23/2010 6:02 PM, Hans-Peter Nilsson wrote:

> It looks like this patch caused my cris-elf autotester to
> regress.

Sorry!

> I guess your new code is missing an adjustment to
> gas/config/obj-multi.[hc] to add obj_adjust_symtab there to
> handle (not break for) multi-objformat targets: when obj-elf.c
> is compiled as not *the* object-format but one of several, and
> when another format is selected.

Does that mean adding a new entry to struct format_ops?  I'm not
familiar with this code, but it looks like struct format_ops is vtable
for the format, but right now it doesn't have a adjust_symtab entry.
And, how is this supposed to work for something like tc-arm.c which
itself overrides the default ELF obj_adjust_symtab?

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFC: COMDAT group names become anonymouse local symbols

[Hans-Peter Nilsson]

> Date: Sat, 23 Oct 2010 18:43:38 -0700
> From: Mark Mitchell <mark@codesourcery.com>

> > I guess your new code is missing an adjustment to
> > gas/config/obj-multi.[hc] to add obj_adjust_symtab there to
> > handle (not break for) multi-objformat targets: when obj-elf.c
> > is compiled as not *the* object-format but one of several, and
> > when another format is selected.
> 
> Does that mean adding a new entry to struct format_ops?

That's it.  I didn't say, because I'm not familiar too with this
code. :)  I only discovered it and fixed a few bugs eons ago.

>  I'm not
> familiar with this code, but it looks like struct format_ops is vtable
> for the format, but right now it doesn't have a adjust_symtab entry.
> And, how is this supposed to work for something like tc-arm.c which
> itself overrides the default ELF obj_adjust_symtab?

Dunno, it might not, but there's no arm multi-object target
anyway.

(Looking...eek!  All the other multi-object targets are gone!
There used to be mips and i386 targets...)

brgds, H-P

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

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

On 10/23/2010 8:34 PM, Hans-Peter Nilsson wrote:

>> Does that mean adding a new entry to struct format_ops?
> 
> That's it.  I didn't say, because I'm not familiar too with this
> code. :)  I only discovered it and fixed a few bugs eons ago.

I'm traveling tomorrow, so might not get a chance to finish this up.  (I
need to test a few different configurations to convince myself I've
filled in the various blanks correctly.)  Here's a preliminary patch
which seems to pass the LD testsuite for cris-elf.  Does it work for you?

Thank you,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

[-- Attachment #2: gas.patch --]
[-- Type: text/plain, Size: 4736 bytes --]

Index: gas/obj.h
===================================================================
RCS file: /cvs/src/src/gas/obj.h,v
retrieving revision 1.13
diff -c -5 -p -r1.13 obj.h
*** gas/obj.h	2 Nov 2009 11:59:14 -0000	1.13
--- gas/obj.h	24 Oct 2010 04:31:40 -0000
*************** struct format_ops {
*** 70,79 ****
--- 70,80 ----
    void (*ecoff_set_ext) (symbolS *, struct ecoff_extr *);
  
    void (*read_begin_hook) (void);
    void (*symbol_new_hook) (symbolS *);
    void (*symbol_clone_hook) (symbolS *, symbolS *);
+   void (*adjust_symtab) (void);
  };
  
  extern const struct format_ops elf_format_ops;
  extern const struct format_ops ecoff_format_ops;
  extern const struct format_ops coff_format_ops;
Index: gas/config/obj-aout.c
===================================================================
RCS file: /cvs/src/src/gas/config/obj-aout.c,v
retrieving revision 1.29
diff -c -5 -p -r1.29 obj-aout.c
*** gas/config/obj-aout.c	28 Jun 2010 14:06:56 -0000	1.29
--- gas/config/obj-aout.c	24 Oct 2010 04:31:40 -0000
*************** const struct format_ops aout_format_ops 
*** 308,318 ****
    obj_aout_sec_sym_ok_for_reloc,
    aout_pop_insert,
    0,	/* ecoff_set_ext.  */
    0,	/* read_begin_hook.  */
    0, 	/* symbol_new_hook.  */
!   0 	/* symbol_clone_hook.  */
  };
  
  const pseudo_typeS aout_pseudo_table[] =
  {
    {"line", obj_aout_line, 0},	/* Source code line number.  */
--- 308,319 ----
    obj_aout_sec_sym_ok_for_reloc,
    aout_pop_insert,
    0,	/* ecoff_set_ext.  */
    0,	/* read_begin_hook.  */
    0, 	/* symbol_new_hook.  */
!   0, 	/* symbol_clone_hook.  */
!   0     /* adjust_symtab.  */
  };
  
  const pseudo_typeS aout_pseudo_table[] =
  {
    {"line", obj_aout_line, 0},	/* Source code line number.  */
Index: gas/config/obj-coff.c
===================================================================
RCS file: /cvs/src/src/gas/config/obj-coff.c,v
retrieving revision 1.107
diff -c -5 -p -r1.107 obj-coff.c
*** gas/config/obj-coff.c	8 Oct 2010 14:00:50 -0000	1.107
--- gas/config/obj-coff.c	24 Oct 2010 04:31:40 -0000
*************** const struct format_ops coff_format_ops 
*** 1941,1947 ****
    0,	/* sec_sym_ok_for_reloc */
    coff_pop_insert,
    0,	/* ecoff_set_ext */
    coff_obj_read_begin_hook,
    coff_obj_symbol_new_hook,
!   coff_obj_symbol_clone_hook
  };
--- 1941,1948 ----
    0,	/* sec_sym_ok_for_reloc */
    coff_pop_insert,
    0,	/* ecoff_set_ext */
    coff_obj_read_begin_hook,
    coff_obj_symbol_new_hook,
!   coff_obj_symbol_clone_hook,
!   coff_obj_adjust_symtab
  };
Index: gas/config/obj-ecoff.c
===================================================================
RCS file: /cvs/src/src/gas/config/obj-ecoff.c,v
retrieving revision 1.20
diff -c -5 -p -r1.20 obj-ecoff.c
*** gas/config/obj-ecoff.c	2 Nov 2009 11:49:48 -0000	1.20
--- gas/config/obj-ecoff.c	24 Oct 2010 04:31:40 -0000
*************** const struct format_ops ecoff_format_ops
*** 313,319 ****
    ecoff_sec_sym_ok_for_reloc,
    ecoff_pop_insert,
    ecoff_set_ext,
    ecoff_read_begin_hook,
    ecoff_symbol_new_hook,
!   ecoff_symbol_clone_hook
  };
--- 313,320 ----
    ecoff_sec_sym_ok_for_reloc,
    ecoff_pop_insert,
    ecoff_set_ext,
    ecoff_read_begin_hook,
    ecoff_symbol_new_hook,
!   ecoff_symbol_clone_hook,
!   0     /* adjust_symtab.  */
  };
Index: gas/config/obj-elf.c
===================================================================
RCS file: /cvs/src/src/gas/config/obj-elf.c,v
retrieving revision 1.132
diff -c -5 -p -r1.132 obj-elf.c
*** gas/config/obj-elf.c	23 Oct 2010 18:05:08 -0000	1.132
--- gas/config/obj-elf.c	24 Oct 2010 04:31:41 -0000
*************** const struct format_ops elf_format_ops =
*** 2444,2450 ****
  #else
    0,	/* ecoff_set_ext */
  #endif
    elf_obj_read_begin_hook,
    elf_obj_symbol_new_hook,
!   0
  };
--- 2444,2451 ----
  #else
    0,	/* ecoff_set_ext */
  #endif
    elf_obj_read_begin_hook,
    elf_obj_symbol_new_hook,
!   0,
!   elf_adjust_symtab
  };
Index: gas/config/obj-multi.h
===================================================================
RCS file: /cvs/src/src/gas/config/obj-multi.h,v
retrieving revision 1.15
diff -c -5 -p -r1.15 obj-multi.h
*** gas/config/obj-multi.h	2 Nov 2009 11:49:48 -0000	1.15
--- gas/config/obj-multi.h	24 Oct 2010 04:31:41 -0000
***************
*** 89,98 ****
--- 89,103 ----
  #define obj_sec_sym_ok_for_reloc(A)			\
  	(this_format->sec_sym_ok_for_reloc		\
  	 ? (*this_format->sec_sym_ok_for_reloc) (A)	\
  	 : 0)
  
+ #define obj_adjust_symtab()				\
+         (this_format->adjust_symtab			\
+          ? (*this_format->adjust_symtab) ()		\
+          : (void) 0)
+ 
  #define S_GET_SIZE					\
  	(*this_format->s_get_size)
  
  #define S_SET_SIZE(S, N)				\
  	(this_format->s_set_size			\

Re: RFC: COMDAT group names become anonymouse local symbols

[Hans-Peter Nilsson]

> Date: Sat, 23 Oct 2010 21:34:21 -0700
> From: Mark Mitchell <mark@codesourcery.com>

> I'm traveling tomorrow,

If to the gcc summit, see you there!

> so might not get a chance to finish this up.

It looks pretty finished, save for a minor spacing issue and
missing ChangeLog entry.

>  (I
> need to test a few different configurations to convince myself I've
> filled in the various blanks correctly.)  Here's a preliminary patch
> which seems to pass the LD testsuite for cris-elf.  Does it work for you?

With that patch, results are ok again.  Thanks.

brgds, H-P

Re: RFC: COMDAT group names become anonymouse local symbols

[Alan Modra]

On Sat, Oct 23, 2010 at 09:34:21PM -0700, Mark Mitchell wrote:
> On 10/23/2010 8:34 PM, Hans-Peter Nilsson wrote:
> 
> >> Does that mean adding a new entry to struct format_ops?
> > 
> > That's it.  I didn't say, because I'm not familiar too with this
> > code. :)  I only discovered it and fixed a few bugs eons ago.
> 
> I'm traveling tomorrow, so might not get a chance to finish this up.

Applied.

-- 
Alan Modra
Australia Development Lab, IBM

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

On 10/25/2010 8:39 AM, Alan Modra wrote:

>> I'm traveling tomorrow, so might not get a chance to finish this up.
> 
> Applied.

Alan, that was very kind of you!  I should have been clear that I would
have gotten to this later in the week -- but, in any case, I very much
appreciate your help.

Thank you,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFC: COMDAT group names become anonymouse local symbols

[Alan Modra]

Committed.

	* config/obj-coff.c (coff_format_ops): Fix typo.

Index: gas/config/obj-coff.c
===================================================================
RCS file: /cvs/src/src/gas/config/obj-coff.c,v
retrieving revision 1.108
diff -u -p -r1.108 obj-coff.c
--- gas/config/obj-coff.c	25 Oct 2010 12:38:42 -0000	1.108
+++ gas/config/obj-coff.c	26 Oct 2010 03:37:30 -0000
@@ -1944,5 +1944,5 @@ const struct format_ops coff_format_ops 
   coff_obj_read_begin_hook,
   coff_obj_symbol_new_hook,
   coff_obj_symbol_clone_hook,
-  coff_obj_adjust_symtab
+  coff_adjust_symtab
 };

-- 
Alan Modra
Australia Development Lab, IBM

Re: RFC: COMDAT group names become anonymouse local symbols

[Dave Korn]

On 24/10/2010 05:34, Mark Mitchell wrote:
> On 10/23/2010 8:34 PM, Hans-Peter Nilsson wrote:
> 
>>> Does that mean adding a new entry to struct format_ops?
>> That's it.  I didn't say, because I'm not familiar too with this
>> code. :)  I only discovered it and fixed a few bugs eons ago.
> 
> I'm traveling tomorrow, so might not get a chance to finish this up.  (I
> need to test a few different configurations to convince myself I've
> filled in the various blanks correctly.)  Here's a preliminary patch
> which seems to pass the LD testsuite for cris-elf.  Does it work for you?

  Hi, I haven't been following this thread so far, but this hunk:

> --- 1941,1948 ----
>     0,	/* sec_sym_ok_for_reloc */
>     coff_pop_insert,
>     0,	/* ecoff_set_ext */
>     coff_obj_read_begin_hook,
>     coff_obj_symbol_new_hook,
> !   coff_obj_symbol_clone_hook,
> !   coff_obj_adjust_symtab
>   };

... breaks i686-pc-cygwin, and presumably all other COFF targets:

> /gnu/binutils/src/gas/config/obj-coff.c:1947: error: 'coff_obj_adjust_symtab'
> undeclared here (not in a function)
> cc1: warnings being treated as errors
> /gnu/binutils/src/gas/config/obj-coff.c:1948: error: missing initializer
> /gnu/binutils/src/gas/config/obj-coff.c:1948: error: (near initialization for 
> 'coff_format_ops.adjust_symtab')
> make[4]: *** [obj-coff.o] Error 1

  I tried changing the struct entry to 'coff_adjust_symtab' and the build
completes, but so would putting a null there, like in the ecoff version...
which is right?

    cheers,
      DaveK

Re: RFC: COMDAT group names become anonymouse local symbols

[Mark Mitchell]

On 10/26/2010 12:11 PM, Dave Korn wrote:

> ... breaks i686-pc-cygwin, and presumably all other COFF targets:

That was the kind of testing I wanted to do, but didn't have a chance to
do before I got on the plane.

>   I tried changing the struct entry to 'coff_adjust_symtab' and the build
> completes, but so would putting a null there, like in the ecoff version...
> which is right?

The former (coff_adjust_symtab) is correct.  We're adding a new
"adjust_symtab" entry to the "vtable" for object formats; the
obj_adjust_smtab hook (as defined in the .h file for the object format)
needs to go into the vtable.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: RFC: COMDAT group names become anonymouse local symbols

[Dave Korn]

On 26/10/2010 16:49, Mark Mitchell wrote:
> On 10/26/2010 12:11 PM, Dave Korn wrote:
> 
>> ... breaks i686-pc-cygwin, and presumably all other COFF targets:
> 
> That was the kind of testing I wanted to do, but didn't have a chance to
> do before I got on the plane.

  NP :)

>>   I tried changing the struct entry to 'coff_adjust_symtab' and the build
>> completes, but so would putting a null there, like in the ecoff version...
>> which is right?
> 
> The former (coff_adjust_symtab) is correct.  We're adding a new
> "adjust_symtab" entry to the "vtable" for object formats; the
> obj_adjust_smtab hook (as defined in the .h file for the object format)
> needs to go into the vtable.

  I'll check it in shortly, thanks for getting back to me.

    cheers,
      DaveK

Re: RFC: COMDAT group names become anonymouse local symbols

[Dave Korn]

On 26/10/2010 17:22, Dave Korn wrote:
> On 26/10/2010 16:49, Mark Mitchell wrote:

>> The former (coff_adjust_symtab) is correct.  We're adding a new
>> "adjust_symtab" entry to the "vtable" for object formats; the
>> obj_adjust_smtab hook (as defined in the .h file for the object format)
>> needs to go into the vtable.
> 
>   I'll check it in shortly, 

  Oh, I see someone already did.  NM.

    cheers,
      DaveK