This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
RE: Group identifier of a comdat group
- From: "Jessica Han" <jessica at cup dot hp dot com>
- To: "'Alan Modra'" <amodra at bigpond dot net dot au>,<binutils at sources dot redhat dot com>, <reva at cup dot hp dot com>
- Date: Tue, 4 Jun 2002 11:25:20 -0700
- Subject: RE: Group identifier of a comdat group
- Reply-to: <jessica at cup dot hp dot com>
Alan,
I tried your patch yesterday and I am able to generate the COMDAT
sections. I still have a couple of questions:
1. group_signature uses the name field of the symtab section to get the
group signature, but that field is empty. Could you tell me which utility
makes use of setup_group, so I can verify this?
2. for a text section gnu.linkonce.t.foo, gas generates
rela.gnu.linkeonce.t.foo, gnu.linkonce.ia64unwi.foo,
gnu.linkonce.ia64unw.foo, etc. Are they supposed to be in the COMDAT group
as well?
Thanks.
>-----Original Message-----
>From: Alan Modra [mailto:amodra@bigpond.net.au]
>Sent: Monday, June 03, 2002 6:01 PM
>To: Jessica Han; binutils@sources.redhat.com; reva@cup.hp.com
>Subject: Re: Group identifier of a comdat group
>
>
>I'm committing this one. Differs from the last patch in that the BFD
>section for a SHT_GROUP ELF section takes it's name from the group
>signature symbol. Gas makes the name of the group section the same as
>the signature, but the spec says the ELF section name is irrelevant.
>Also, gas now makes a group GRP_COMDAT if any component section is
>named .gnu.linkonce*.
>
>bfd/ChangeLog
> * elf.c (setup_group): Set SEC_LINK_ONCE on GRP_COMDAT groups.
> (bfd_section_from_shdr): Likewise. Set section name of group
> sections from signature.
> (group_signature): Split out from setup_group. Ensure
>symbol table
> is available.
> (bfd_elf_discard_group): New function.
> (_bfd_elf_make_section_from_shdr): Don't set SEC_LINK_ONCE on
> .gnu.linkonce* sections if they are members of a group.
> (set_group_contents): Set GRP_COMDAT flag.
> * section.c (bfd_discard_group): New function.
> * bfd-in.h (bfd_elf_discard_group): Declare.
> * bfd-in2.h: Regenerate.
> * elf-bfd.h (struct bfd_elf_section_data): Add linkonce_p field.
> (elf_linkonce_p): Define.
>
>gas/ChangeLog
> * config/obj-elf.c (obj_elf_change_section): Set and check elf
> linkonce flag. Print all warnings.
> (obj_elf_section): Parse ",comdat" for groups.
> (elf_frob_file): Set SEC_LINK_ONCE on COMDAT groups. Check
> consistency of comdat flag.
>
>ld/ChangeLog
> * ldlang.c (section_already_linked): Call
>bfd_discard_group. Typo fix.
>
>Index: bfd/bfd-in.h
>===================================================================
>RCS file: /cvs/src/src/bfd/bfd-in.h,v
>retrieving revision 1.46
>diff -u -p -r1.46 bfd-in.h
>--- bfd/bfd-in.h 29 May 2002 16:03:04 -0000 1.46
>+++ bfd/bfd-in.h 4 Jun 2002 00:06:55 -0000
>@@ -657,6 +657,8 @@ extern boolean bfd_elf32_discard_info
> PARAMS ((bfd *, struct bfd_link_info *));
> extern boolean bfd_elf64_discard_info
> PARAMS ((bfd *, struct bfd_link_info *));
>+extern void bfd_elf_discard_group
>+ PARAMS ((bfd *, struct sec *));
>
> /* Return an upper bound on the number of bytes required to store a
> copy of ABFD's program header table entries. Return -1 if an error
>Index: bfd/elf-bfd.h
>===================================================================
>RCS file: /cvs/src/src/bfd/elf-bfd.h,v
>retrieving revision 1.73
>diff -u -p -r1.73 elf-bfd.h
>--- bfd/elf-bfd.h 31 May 2002 02:59:47 -0000 1.73
>+++ bfd/elf-bfd.h 4 Jun 2002 00:07:05 -0000
>@@ -926,11 +926,15 @@ struct bfd_elf_section_data
>
> /* Nonzero if this section uses RELA relocations, rather
>than REL. */
> unsigned int use_rela_p:1;
>+
>+ /* Nonzero when a group is COMDAT. */
>+ unsigned int linkonce_p:1;
> };
>
> #define elf_section_data(sec) ((struct
>bfd_elf_section_data*)sec->used_by_bfd)
> #define elf_group_name(sec) (elf_section_data(sec)->group_name)
> #define elf_next_in_group(sec) (elf_section_data(sec)->next_in_group)
>+#define elf_linkonce_p(sec) (elf_section_data(sec)->linkonce_p)
>
> /* Return true if section has been discarded. */
> #define elf_discarded_section(sec)
> \
>Index: bfd/elf.c
>===================================================================
>RCS file: /cvs/src/src/bfd/elf.c,v
>retrieving revision 1.140
>diff -u -p -r1.140 elf.c
>--- bfd/elf.c 31 May 2002 02:59:47 -0000 1.140
>+++ bfd/elf.c 4 Jun 2002 00:07:10 -0000
>@@ -50,6 +50,7 @@ static boolean prep_headers PARAMS ((bfd
> static boolean swap_out_syms PARAMS ((bfd *, struct
>bfd_strtab_hash **, int));
> static boolean copy_private_bfd_data PARAMS ((bfd *, bfd *));
> static char *elf_read PARAMS ((bfd *, file_ptr, bfd_size_type));
>+static const char *group_signature PARAMS ((bfd *,
>Elf_Internal_Shdr *));
> static boolean setup_group PARAMS ((bfd *, Elf_Internal_Shdr
>*, asection *));
> static void merge_sections_remove_hook PARAMS ((bfd *, asection *));
> static void elf_fake_sections PARAMS ((bfd *, asection *, PTR));
>@@ -361,6 +362,35 @@ typedef union elf_internal_group {
> unsigned int flags;
> } Elf_Internal_Group;
>
>+/* Return the name of the group signature symbol. Why isn't the
>+ signature just a string? */
>+
>+static const char *
>+group_signature (abfd, ghdr)
>+ bfd *abfd;
>+ Elf_Internal_Shdr *ghdr;
>+{
>+ struct elf_backend_data *bed;
>+ file_ptr pos;
>+ unsigned char ename[4];
>+ unsigned long iname;
>+
>+ /* First we need to ensure the symbol table is available. */
>+ if (! bfd_section_from_shdr (abfd, ghdr->sh_link))
>+ return NULL;
>+
>+ /* Fortunately, the name index is at the same place in the external
>+ symbol for both 32 and 64 bit ELF. */
>+ bed = get_elf_backend_data (abfd);
>+ pos = elf_tdata (abfd)->symtab_hdr.sh_offset;
>+ pos += ghdr->sh_info * bed->s->sizeof_sym;
>+ if (bfd_seek (abfd, pos, SEEK_SET) != 0
>+ || bfd_bread (ename, (bfd_size_type) 4, abfd) != 4)
>+ return NULL;
>+ iname = H_GET_32 (abfd, ename);
>+ return elf_string_from_elf_strtab (abfd, iname);
>+}
>+
> /* Set next_in_group list pointer, and group name for NEWSECT. */
>
> static boolean
>@@ -440,6 +470,9 @@ setup_group (abfd, hdr, newsect)
> if (src == shdr->contents)
> {
> dest->flags = idx;
>+ if (shdr->bfd_section != NULL && (idx
>& GRP_COMDAT))
>+ shdr->bfd_section->flags
>+ |= SEC_LINK_ONCE |
>SEC_LINK_DUPLICATES_DISCARD;
> break;
> }
> if (idx >= shnum)
>@@ -492,32 +525,20 @@ setup_group (abfd, hdr, newsect)
> }
> else
> {
>- struct elf_backend_data *bed;
>- file_ptr pos;
>- unsigned char ename[4];
>- unsigned long iname;
> const char *gname;
>
>- /* Humbug. Get the name from the group signature
>- symbol. Why isn't the signature just a string?
>- Fortunately, the name index is at the same
>- place in the external symbol for both 32 and 64
>- bit ELF. */
>- bed = get_elf_backend_data (abfd);
>- pos = elf_tdata (abfd)->symtab_hdr.sh_offset;
>- pos += shdr->sh_info * bed->s->sizeof_sym;
>- if (bfd_seek (abfd, pos, SEEK_SET) != 0
>- || bfd_bread (ename, (bfd_size_type) 4,
>abfd) != 4)
>+ gname = group_signature (abfd, shdr);
>+ if (gname == NULL)
> return false;
>- iname = H_GET_32 (abfd, ename);
>- gname = elf_string_from_elf_strtab (abfd, iname);
> elf_group_name (newsect) = gname;
>
> /* Start a circular list with one element. */
> elf_next_in_group (newsect) = newsect;
> }
>+
> if (shdr->bfd_section != NULL)
> elf_next_in_group (shdr->bfd_section) = newsect;
>+
> i = num_group - 1;
> break;
> }
>@@ -532,6 +553,24 @@ setup_group (abfd, hdr, newsect)
> return true;
> }
>
>+void
>+bfd_elf_discard_group (abfd, group)
>+ bfd *abfd ATTRIBUTE_UNUSED;
>+ asection *group;
>+{
>+ asection *first = elf_next_in_group (group);
>+ asection *s = first;
>+
>+ while (s != NULL)
>+ {
>+ s->output_section = bfd_abs_section_ptr;
>+ s = elf_next_in_group (s);
>+ /* These lists are circular. */
>+ if (s == first)
>+ break;
>+ }
>+}
>+
> /* Make a BFD section from an ELF section. We store a pointer to the
> BFD section in the bfd_section field of the header. */
>
>@@ -620,7 +659,8 @@ _bfd_elf_make_section_from_shdr (abfd, h
> The symbols will be defined as weak, so that multiple definitions
> are permitted. The GNU linker extension is to actually discard
> all but one of the sections. */
>- if (strncmp (name, ".gnu.linkonce", sizeof ".gnu.linkonce"
>- 1) == 0)
>+ if (strncmp (name, ".gnu.linkonce", sizeof ".gnu.linkonce" - 1) == 0
>+ && elf_next_in_group (newsect) == NULL)
> flags |= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
>
> bed = get_elf_backend_data (abfd);
>@@ -1576,7 +1616,7 @@ bfd_section_from_shdr (abfd, shindex)
> Elf_Internal_Shdr *hdr = elf_elfsections (abfd)[shindex];
> Elf_Internal_Ehdr *ehdr = elf_elfheader (abfd);
> struct elf_backend_data *bed = get_elf_backend_data (abfd);
>- char *name;
>+ const char *name;
>
> name = elf_string_from_elf_strtab (abfd, hdr->sh_name);
>
>@@ -1820,7 +1860,12 @@ bfd_section_from_shdr (abfd, shindex)
> return true;
>
> case SHT_GROUP:
>- /* Make a section for objcopy and relocatable links. */
>+ /* We need a BFD section for objcopy and relocatable linking,
>+ and it's handy to have the signature available as the section
>+ name. */
>+ name = group_signature (abfd, hdr);
>+ if (name == NULL)
>+ return false;
> if (!_bfd_elf_make_section_from_shdr (abfd, hdr, name))
> return false;
> if (hdr->contents != NULL)
>@@ -1829,6 +1874,10 @@ bfd_section_from_shdr (abfd, shindex)
> unsigned int n_elt = hdr->sh_size / 4;
> asection *s;
>
>+ if (idx->flags & GRP_COMDAT)
>+ hdr->bfd_section->flags
>+ |= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
>+
> while (--n_elt != 0)
> if ((s = (++idx)->shdr->bfd_section) != NULL
> && elf_next_in_group (s) != NULL)
>@@ -2369,7 +2418,7 @@ set_group_contents (abfd, sec, failedptr
> while (elt != elf_next_in_group (l->u.indirect.section));
>
> loc -= 4;
>- H_PUT_32 (abfd, 0, loc);
>+ H_PUT_32 (abfd, sec->flags & SEC_LINK_ONCE ? GRP_COMDAT : 0, loc);
>
> BFD_ASSERT (loc == sec->contents);
> }
>Index: bfd/section.c
>===================================================================
>RCS file: /cvs/src/src/bfd/section.c,v
>retrieving revision 1.45
>diff -u -p -r1.45 section.c
>--- bfd/section.c 23 May 2002 13:12:46 -0000 1.45
>+++ bfd/section.c 4 Jun 2002 00:07:49 -0000
>@@ -1375,3 +1375,24 @@ _bfd_strip_section_from_output (info, s)
>
> s->flags |= SEC_EXCLUDE;
> }
>+
>+/*
>+FUNCTION
>+ bfd_discard_group
>+
>+SYNOPSIS
>+ void bfd_discard_group (bfd *abfd, asection *group);
>+
>+DESCRIPTION
>+ Remove all members of @var{group} from the output.
>+*/
>+
>+void
>+bfd_discard_group (abfd, group)
>+ bfd *abfd;
>+ asection *group;
>+{
>+ if ((group->flags & SEC_GROUP) != 0
>+ && abfd->xvec->flavour == bfd_target_elf_flavour)
>+ bfd_elf_discard_group (abfd, group);
>+}
>Index: gas/config/obj-elf.c
>===================================================================
>RCS file: /cvs/src/src/gas/config/obj-elf.c,v
>retrieving revision 1.50
>diff -u -p -r1.50 obj-elf.c
>--- gas/config/obj-elf.c 23 May 2002 13:12:47 -0000 1.50
>+++ gas/config/obj-elf.c 3 Jun 2002 23:53:29 -0000
>@@ -77,7 +77,7 @@ static void obj_elf_weak PARAMS ((int));
> static void obj_elf_local PARAMS ((int));
> static void obj_elf_visibility PARAMS ((int));
> static void obj_elf_change_section
>- PARAMS ((const char *, int, int, int, const char *, int));
>+ PARAMS ((const char *, int, int, int, const char *, int, int));
> static int obj_elf_parse_section_letters PARAMS ((char *, size_t));
> static int obj_elf_section_word PARAMS ((char *, size_t));
> static char *obj_elf_section_name PARAMS ((void));
>@@ -664,12 +664,13 @@ static struct special_section const spec
> };
>
> static void
>-obj_elf_change_section (name, type, attr, entsize, group_name, push)
>+obj_elf_change_section (name, type, attr, entsize,
>group_name, linkonce, push)
> const char *name;
> int type;
> int attr;
> int entsize;
> const char *group_name;
>+ int linkonce;
> int push;
> {
> asection *old_sec;
>@@ -758,6 +759,7 @@ obj_elf_change_section (name, type, attr
> if (flags & SEC_MERGE)
> sec->entsize = entsize;
> elf_group_name (sec) = group_name;
>+ elf_linkonce_p (sec) = linkonce;
>
> /* Add a symbol for this section to the symbol table. */
> secsym = symbol_find (name);
>@@ -771,15 +773,16 @@ obj_elf_change_section (name, type, attr
> /* If section attributes are specified the second time we see a
> particular section, then check that they are the same as we
> saw the first time. */
>- if ((old_sec->flags ^ flags)
>- & (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE
>- | SEC_EXCLUDE | SEC_SORT_ENTRIES | SEC_MERGE | SEC_STRINGS
>- | SEC_THREAD_LOCAL))
>+ if (((old_sec->flags ^ flags)
>+ & (SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_CODE
>+ | SEC_EXCLUDE | SEC_SORT_ENTRIES | SEC_MERGE | SEC_STRINGS
>+ | SEC_THREAD_LOCAL))
>+ || linkonce != elf_linkonce_p (sec))
> as_warn (_("ignoring changed section attributes for %s"), name);
>- else if ((flags & SEC_MERGE) && old_sec->entsize !=
>(unsigned) entsize)
>+ if ((flags & SEC_MERGE) && old_sec->entsize !=
>(unsigned) entsize)
> as_warn (_("ignoring changed section entity size for
>%s"), name);
>- else if ((attr & SHF_GROUP) != 0
>- && strcmp (elf_group_name (old_sec), group_name) != 0)
>+ if ((attr & SHF_GROUP) != 0
>+ && strcmp (elf_group_name (old_sec), group_name) != 0)
> as_warn (_("ignoring new section group for %s"), name);
> }
>
>@@ -947,6 +950,7 @@ obj_elf_section (push)
> char *name, *group_name, *beg;
> int type, attr, dummy;
> int entsize;
>+ int linkonce;
>
> #ifndef TC_I370
> if (flag_mri)
>@@ -977,6 +981,7 @@ obj_elf_section (push)
> attr = 0;
> group_name = NULL;
> entsize = 0;
>+ linkonce = 0;
>
> if (*input_line_pointer == ',')
> {
>@@ -1050,6 +1055,13 @@ obj_elf_section (push)
> group_name = obj_elf_section_name ();
> if (group_name == NULL)
> attr &= ~SHF_GROUP;
>+ else if (strncmp (input_line_pointer, ",comdat", 7) == 0)
>+ {
>+ input_line_pointer += 7;
>+ linkonce = 1;
>+ }
>+ else if (strncmp (name, ".gnu.linkonce", 13) == 0)
>+ linkonce = 1;
> }
> else if ((attr & SHF_GROUP) != 0)
> {
>@@ -1085,7 +1097,7 @@ obj_elf_section (push)
>
> demand_empty_rest_of_line ();
>
>- obj_elf_change_section (name, type, attr, entsize,
>group_name, push);
>+ obj_elf_change_section (name, type, attr, entsize,
>group_name, linkonce, push);
> }
>
> /* Change to the .data section. */
>@@ -2015,8 +2027,20 @@ elf_frob_file ()
> asection *s;
> flagword flags;
>
>- s = subseg_force_new (group_name, 0);
> 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 (elf_linkonce_p (s) != ((flags & SEC_LINK_ONCE) != 0))
>+ {
>+ flags |= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
>+ if (s != list.head[i])
>+ {
>+ as_warn (_("assuming all members of group `%s'
>are COMDAT"),
>+ group_name);
>+ break;
>+ }
>+ }
>+
>+ s = subseg_force_new (group_name, 0);
> if (s == NULL
> || !bfd_set_section_flags (stdoutput, s, flags)
> || !bfd_set_section_alignment (stdoutput, s, 2))
>Index: ld/ldlang.c
>===================================================================
>RCS file: /cvs/src/src/ld/ldlang.c,v
>retrieving revision 1.88
>diff -u -p -r1.88 ldlang.c
>--- ld/ldlang.c 27 May 2002 08:22:07 -0000 1.88
>+++ ld/ldlang.c 3 Jun 2002 23:54:02 -0000
>@@ -985,7 +985,7 @@ section_already_linked (abfd, sec, data)
> of having link once sections in the first place.
>
> Also, not merging link once sections in a relocatable link
>- causes trouble for MIPS ELF, which relies in link once semantics
>+ causes trouble for MIPS ELF, which relies on link once semantics
> to handle the .reginfo section correctly. */
>
> name = bfd_get_section_name (abfd, sec);
>@@ -1038,6 +1038,9 @@ section_already_linked (abfd, sec, data)
> does not create a lang_input_section structure for this
> section. */
> sec->output_section = bfd_abs_section_ptr;
>+
>+ if (flags & SEC_GROUP)
>+ bfd_discard_group (abfd, sec);
>
> return;
> }
>
>--
>Alan Modra
>IBM OzLabs - Linux Technology Centre
>