[binutils-gdb] Rewrite SHT_GROUP handling
Alan Modra
amodra@sourceware.org
Wed Jun 26 23:36:01 GMT 2024
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f67619822044ab69b5225038d4e741a8ccdfc52c
commit f67619822044ab69b5225038d4e741a8ccdfc52c
Author: Alan Modra <amodra@gmail.com>
Date: Wed Jun 26 17:47:21 2024 +0930
Rewrite SHT_GROUP handling
This patch delays setting up elf_next_in_group, elf_sec_group and
elf_group_name when reading ELF object files until after all ELF
sections have been processed by bfd_section_from_shdr. This is simpler
and more robust than the current scheme of driving the whole process
on detecting a section with SHF_GROUP set.
* elf-bfd.h (struct elf_obj_tdata): Delete group_sect_ptr,
num_group and group_search_offset.
* elf.c (Elf_Internal_Group): Delete.
(setup_group): Delete function.
(IS_VALID_GROUP_SECTION_HEADER): Delete macro.
(is_valid_group_section_header),
(process_sht_group_entries): New functions.
(_bfd_elf_setup_sections): Handle group sections here..
(_bfd_elf_make_section_from_shdr): ..rather than here.
(bfd_section_from_shdr): Don't check SHT_GROUP validity here.
Diff:
---
bfd/elf-bfd.h | 7 --
bfd/elf.c | 388 ++++++++++++++++------------------------------------------
2 files changed, 108 insertions(+), 287 deletions(-)
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 45d36a78976..b89d3dda05d 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2156,13 +2156,6 @@ struct elf_obj_tdata
in the list. */
struct sdt_note *sdt_note_head;
- Elf_Internal_Shdr **group_sect_ptr;
- unsigned int num_group;
-
- /* Index into group_sect_ptr, updated by setup_group when finding a
- section's group. Used to optimize subsequent group searches. */
- unsigned int group_search_offset;
-
unsigned int symtab_section, dynsymtab_section;
unsigned int dynversym_section, dynverdef_section, dynverref_section;
diff --git a/bfd/elf.c b/bfd/elf.c
index b0b1e94d16c..d7c42273aff 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -559,15 +559,6 @@ bfd_elf_sym_name (bfd *abfd,
return name;
}
-/* Elf_Internal_Shdr->contents is an array of these for SHT_GROUP
- sections. The first element is the flags, the rest are section
- pointers. */
-
-typedef union elf_internal_group {
- Elf_Internal_Shdr *shdr;
- unsigned int flags;
-} Elf_Internal_Group;
-
/* Return the name of the group signature symbol. Why isn't the
signature just a string? */
@@ -597,249 +588,110 @@ group_signature (bfd *abfd, Elf_Internal_Shdr *ghdr)
return bfd_elf_sym_name (abfd, hdr, &isym, NULL);
}
-/* Set next_in_group list pointer, and group name for NEWSECT. */
+static bool
+is_valid_group_section_header (Elf_Internal_Shdr *shdr, size_t minsize)
+{
+ return (shdr->sh_size >= minsize
+ && shdr->sh_entsize == GRP_ENTRY_SIZE
+ && shdr->sh_size % GRP_ENTRY_SIZE == 0
+ && shdr->bfd_section != NULL);
+}
+
+
+/* Set next_in_group, sec_group list pointers, and group names. */
static bool
-setup_group (bfd *abfd, Elf_Internal_Shdr *hdr, asection *newsect)
+process_sht_group_entries (bfd *abfd,
+ Elf_Internal_Shdr *ghdr, unsigned int gidx)
{
- unsigned int num_group = elf_tdata (abfd)->num_group;
+ unsigned char *contents;
- /* If num_group is zero, read in all SHT_GROUP sections. The count
- is set to -1 if there are no SHT_GROUP sections. */
- if (num_group == 0)
+ /* Read the raw contents. */
+ if (!bfd_malloc_and_get_section (abfd, ghdr->bfd_section, &contents))
{
- unsigned int i, shnum;
-
- /* First count the number of groups. If we have a SHT_GROUP
- section with just a flag word (ie. sh_size is 4), ignore it. */
- shnum = elf_numsections (abfd);
- num_group = 0;
+ _bfd_error_handler
+ /* xgettext:c-format */
+ (_("%pB: could not read contents of group [%u]"), abfd, gidx);
+ return false;
+ }
-#define IS_VALID_GROUP_SECTION_HEADER(shdr, minsize) \
- ( (shdr)->sh_type == SHT_GROUP \
- && (shdr)->sh_size >= minsize \
- && (shdr)->sh_entsize == GRP_ENTRY_SIZE \
- && ((shdr)->sh_size % GRP_ENTRY_SIZE) == 0)
+ asection *last_elt = NULL;
+ const char *gname = NULL;
+ unsigned char *p = contents + ghdr->sh_size;
+ while (1)
+ {
+ unsigned int idx;
+ Elf_Internal_Shdr *shdr;
- for (i = 0; i < shnum; i++)
+ p -= 4;
+ idx = H_GET_32 (abfd, p);
+ if (p == contents)
{
- Elf_Internal_Shdr *shdr = elf_elfsections (abfd)[i];
-
- if (IS_VALID_GROUP_SECTION_HEADER (shdr, 2 * GRP_ENTRY_SIZE))
- num_group += 1;
+ if ((idx & GRP_COMDAT) != 0)
+ ghdr->bfd_section->flags
+ |= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
+ break;
}
+ if (idx < elf_numsections (abfd))
+ shdr = elf_elfsections (abfd)[idx];
- if (num_group == 0)
+ if (idx >= elf_numsections (abfd)
+ || shdr->sh_type == SHT_GROUP)
{
- num_group = (unsigned) -1;
- elf_tdata (abfd)->num_group = num_group;
- elf_tdata (abfd)->group_sect_ptr = NULL;
+ _bfd_error_handler
+ (_("%pB: invalid entry in SHT_GROUP section [%u]"), abfd, gidx);
}
else
{
- /* We keep a list of elf section headers for group sections,
- so we can find them quickly. */
- size_t amt;
-
- elf_tdata (abfd)->num_group = num_group;
- amt = num_group * sizeof (Elf_Internal_Shdr *);
- elf_tdata (abfd)->group_sect_ptr
- = (Elf_Internal_Shdr **) bfd_zalloc (abfd, amt);
- if (elf_tdata (abfd)->group_sect_ptr == NULL)
- return false;
- num_group = 0;
+ /* PR binutils/23199: All sections in a section group should
+ be marked with SHF_GROUP. But some tools generate broken
+ objects without SHF_GROUP. Fix them up here. */
+ shdr->sh_flags |= SHF_GROUP;
- for (i = 0; i < shnum; i++)
+ asection *elt = shdr->bfd_section;
+ if (elt != NULL)
{
- Elf_Internal_Shdr *shdr = elf_elfsections (abfd)[i];
-
- if (IS_VALID_GROUP_SECTION_HEADER (shdr, 2 * GRP_ENTRY_SIZE))
+ if (last_elt == NULL)
{
- unsigned char *src;
- Elf_Internal_Group *dest;
-
- /* Make sure the group section has a BFD section
- attached to it. */
- if (!bfd_section_from_shdr (abfd, i))
- return false;
-
- /* Add to list of sections. */
- elf_tdata (abfd)->group_sect_ptr[num_group] = shdr;
- num_group += 1;
-
- /* Read the raw contents. */
- BFD_ASSERT (sizeof (*dest) >= 4 && sizeof (*dest) % 4 == 0);
- shdr->contents = NULL;
- if (_bfd_mul_overflow (shdr->sh_size,
- sizeof (*dest) / 4, &amt)
- || bfd_seek (abfd, shdr->sh_offset, SEEK_SET) != 0
- || !(shdr->contents
- = _bfd_alloc_and_read (abfd, amt, shdr->sh_size)))
- {
- _bfd_error_handler
- /* xgettext:c-format */
- (_("%pB: invalid size field in group section"
- " header: %#" PRIx64 ""),
- abfd, (uint64_t) shdr->sh_size);
- bfd_set_error (bfd_error_bad_value);
- -- num_group;
- continue;
- }
-
- /* Translate raw contents, a flag word followed by an
- array of elf section indices all in target byte order,
- to the flag word followed by an array of elf section
- pointers. */
- src = shdr->contents + shdr->sh_size;
- dest = (Elf_Internal_Group *) (shdr->contents + amt);
-
- while (1)
+ /* Start a circular list with one element.
+ It will be in reverse order to match what gas does. */
+ elf_next_in_group (elt) = elt;
+ /* Point the group section to it. */
+ elf_next_in_group (ghdr->bfd_section) = elt;
+ gname = group_signature (abfd, ghdr);
+ if (gname == NULL)
{
- unsigned int idx;
-
- src -= 4;
- --dest;
- idx = H_GET_32 (abfd, src);
- if (src == shdr->contents)
- {
- dest->shdr = NULL;
- 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)
- {
- dest->shdr = elf_elfsections (abfd)[idx];
- /* PR binutils/23199: All sections in a
- section group should be marked with
- SHF_GROUP. But some tools generate
- broken objects without SHF_GROUP. Fix
- them up here. */
- dest->shdr->sh_flags |= SHF_GROUP;
- }
- if (idx >= shnum
- || dest->shdr->sh_type == SHT_GROUP)
- {
- _bfd_error_handler
- (_("%pB: invalid entry in SHT_GROUP section [%u]"),
- abfd, i);
- dest->shdr = NULL;
- }
+ free (contents);
+ return false;
}
}
- }
-
- /* PR 17510: Corrupt binaries might contain invalid groups. */
- if (num_group != (unsigned) elf_tdata (abfd)->num_group)
- {
- elf_tdata (abfd)->num_group = num_group;
-
- /* If all groups are invalid then fail. */
- if (num_group == 0)
+ else
{
- elf_tdata (abfd)->group_sect_ptr = NULL;
- elf_tdata (abfd)->num_group = num_group = -1;
- _bfd_error_handler
- (_("%pB: no valid group sections found"), abfd);
- bfd_set_error (bfd_error_bad_value);
+ elf_next_in_group (elt) = elf_next_in_group (last_elt);
+ elf_next_in_group (last_elt) = elt;
}
+ last_elt = elt;
+ elf_group_name (elt) = gname;
+ elf_sec_group (elt) = ghdr->bfd_section;
}
- }
- }
-
- if (num_group != (unsigned) -1)
- {
- unsigned int search_offset = elf_tdata (abfd)->group_search_offset;
- unsigned int j;
-
- for (j = 0; j < num_group; j++)
- {
- /* Begin search from previous found group. */
- unsigned i = (j + search_offset) % num_group;
-
- Elf_Internal_Shdr *shdr = elf_tdata (abfd)->group_sect_ptr[i];
- Elf_Internal_Group *idx;
- bfd_size_type n_elt;
-
- if (shdr == NULL)
- continue;
-
- idx = (Elf_Internal_Group *) shdr->contents;
- if (idx == NULL || shdr->sh_size < 4)
+ else if (shdr->sh_type != SHT_RELA
+ && shdr->sh_type != SHT_REL)
{
- /* See PR 21957 for a reproducer. */
- /* xgettext:c-format */
- _bfd_error_handler (_("%pB: group section '%pA' has no contents"),
- abfd, shdr->bfd_section);
- elf_tdata (abfd)->group_sect_ptr[i] = NULL;
- bfd_set_error (bfd_error_bad_value);
- return false;
+ /* There are some unknown sections in the group. */
+ _bfd_error_handler
+ /* xgettext:c-format */
+ (_("%pB: unknown type [%#x] section `%s' in group [%u]"),
+ abfd,
+ shdr->sh_type,
+ bfd_elf_string_from_elf_section (abfd,
+ (elf_elfheader (abfd)
+ ->e_shstrndx),
+ shdr->sh_name),
+ gidx);
}
- n_elt = shdr->sh_size / 4;
-
- /* Look through this group's sections to see if current
- section is a member. */
- while (--n_elt != 0)
- if ((++idx)->shdr == hdr)
- {
- asection *s = NULL;
-
- /* We are a member of this group. Go looking through
- other members to see if any others are linked via
- next_in_group. */
- idx = (Elf_Internal_Group *) shdr->contents;
- n_elt = shdr->sh_size / 4;
- while (--n_elt != 0)
- if ((++idx)->shdr != NULL
- && (s = idx->shdr->bfd_section) != NULL
- && elf_next_in_group (s) != NULL)
- break;
- if (n_elt != 0)
- {
- /* Snarf the group name from other member, and
- insert current section in circular list. */
- elf_group_name (newsect) = elf_group_name (s);
- elf_next_in_group (newsect) = elf_next_in_group (s);
- elf_next_in_group (s) = newsect;
- }
- else
- {
- const char *gname;
-
- gname = group_signature (abfd, shdr);
- if (gname == NULL)
- return false;
- elf_group_name (newsect) = gname;
-
- /* Start a circular list with one element. */
- elf_next_in_group (newsect) = newsect;
- }
-
- /* If the group section has been created, point to the
- new member. */
- if (shdr->bfd_section != NULL)
- elf_next_in_group (shdr->bfd_section) = newsect;
-
- elf_tdata (abfd)->group_search_offset = i;
- j = num_group - 1;
- break;
- }
}
}
-
- if (elf_group_name (newsect) == NULL)
- {
- /* xgettext:c-format */
- _bfd_error_handler (_("%pB: no group info for section '%pA'"),
- abfd, newsect);
- /* PR 29532: Return true here, even though the group info has not been
- read. Separate debug info files can have empty group sections, but
- we do not want this to prevent them from being loaded as otherwise
- GDB will not be able to use them. */
- return true;
- }
+ free (contents);
return true;
}
@@ -847,7 +699,6 @@ bool
_bfd_elf_setup_sections (bfd *abfd)
{
unsigned int i;
- unsigned int num_group = elf_tdata (abfd)->num_group;
bool result = true;
asection *s;
@@ -892,66 +743,49 @@ _bfd_elf_setup_sections (bfd *abfd)
elf_linked_to_section (s) = linksec;
}
}
- else if (this_hdr->sh_type == SHT_GROUP
- && elf_next_in_group (s) == NULL)
- {
- _bfd_error_handler
- /* xgettext:c-format */
- (_("%pB: SHT_GROUP section [index %d] has no SHF_GROUP sections"),
- abfd, elf_section_data (s)->this_idx);
- result = false;
- }
}
/* Process section groups. */
- if (num_group == (unsigned) -1)
- return result;
- for (i = 0; i < num_group; i++)
+ /* First count the number of groups. If we have a SHT_GROUP
+ section with just a flag word (ie. sh_size is 4), ignore it. */
+ unsigned int num_group = 0;
+ for (i = 1; i < elf_numsections (abfd); i++)
{
- Elf_Internal_Shdr *shdr = elf_tdata (abfd)->group_sect_ptr[i];
- Elf_Internal_Group *idx;
- unsigned int n_elt;
+ Elf_Internal_Shdr *shdr = elf_elfsections (abfd)[i];
- /* PR binutils/18758: Beware of corrupt binaries with invalid
- group data. */
- if (shdr == NULL || shdr->bfd_section == NULL || shdr->contents == NULL)
+ if (shdr && shdr->sh_type == SHT_GROUP)
{
- _bfd_error_handler
- /* xgettext:c-format */
- (_("%pB: section group entry number %u is corrupt"),
- abfd, i);
- result = false;
- continue;
- }
-
- idx = (Elf_Internal_Group *) shdr->contents;
- n_elt = shdr->sh_size / 4;
-
- while (--n_elt != 0)
- {
- ++ idx;
-
- if (idx->shdr == NULL)
- continue;
- else if (idx->shdr->bfd_section)
- elf_sec_group (idx->shdr->bfd_section) = shdr->bfd_section;
- else if (idx->shdr->sh_type != SHT_RELA
- && idx->shdr->sh_type != SHT_REL)
+ if (!is_valid_group_section_header (shdr, GRP_ENTRY_SIZE))
{
- /* There are some unknown sections in the group. */
+ /* PR binutils/18758: Beware of corrupt binaries with invalid
+ group data. */
_bfd_error_handler
/* xgettext:c-format */
- (_("%pB: unknown type [%#x] section `%s' in group [%pA]"),
- abfd,
- idx->shdr->sh_type,
- bfd_elf_string_from_elf_section (abfd,
- (elf_elfheader (abfd)
- ->e_shstrndx),
- idx->shdr->sh_name),
- shdr->bfd_section);
+ (_("%pB: section group entry number %u is corrupt"),
+ abfd, i);
result = false;
+ continue;
}
+ if (shdr->sh_size >= 2 * GRP_ENTRY_SIZE)
+ ++num_group;
+ }
+ }
+
+ if (num_group == 0)
+ return result;
+
+ for (i = 1; i < elf_numsections (abfd); i++)
+ {
+ Elf_Internal_Shdr *shdr = elf_elfsections (abfd)[i];
+
+ if (shdr && shdr->sh_type == SHT_GROUP
+ && is_valid_group_section_header (shdr, 2 * GRP_ENTRY_SIZE))
+ {
+ if (!process_sht_group_entries (abfd, shdr, i))
+ result = false;
+ if (--num_group == 0)
+ break;
}
}
@@ -1027,9 +861,6 @@ _bfd_elf_make_section_from_shdr (bfd *abfd,
}
if ((hdr->sh_flags & SHF_STRINGS) != 0)
flags |= SEC_STRINGS;
- if (hdr->sh_flags & SHF_GROUP)
- if (!setup_group (abfd, hdr, newsect))
- return false;
if ((hdr->sh_flags & SHF_TLS) != 0)
flags |= SEC_THREAD_LOCAL;
if ((hdr->sh_flags & SHF_EXCLUDE) != 0)
@@ -3033,9 +2864,6 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex)
goto success;
case SHT_GROUP:
- if (! IS_VALID_GROUP_SECTION_HEADER (hdr, GRP_ENTRY_SIZE))
- goto fail;
-
if (!_bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex))
goto fail;
More information about the Binutils-cvs
mailing list