* Re: Rewrite SHT_GROUP handling
@ 2024-06-30 22:18 Alan Modra
2024-07-06 3:53 ` Fangrui Song
0 siblings, 1 reply; 4+ messages in thread
From: Alan Modra @ 2024-06-30 22:18 UTC (permalink / raw)
To: binutils
Some more error tweaks. Report a zero entry as "invalid entry.."
rather than "unknown type..", and allow a section to be mentioned
twice in a group.
* elf.c (process_sht_group_entries): Tweak error messages, and
allow a duplicate index in a group without reporting an error.
diff --git a/bfd/elf.c b/bfd/elf.c
index 8bb296f9637..f85f79e1e35 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -622,6 +622,7 @@ process_sht_group_entries (bfd *abfd,
{
unsigned int idx;
Elf_Internal_Shdr *shdr;
+ asection *elt;
p -= 4;
idx = H_GET_32 (abfd, p);
@@ -632,67 +633,68 @@ process_sht_group_entries (bfd *abfd,
|= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
break;
}
- if (idx < elf_numsections (abfd))
- shdr = elf_elfsections (abfd)[idx];
- if (idx >= elf_numsections (abfd)
- || shdr->sh_type == SHT_GROUP
- || (shdr->bfd_section != NULL
- && elf_next_in_group (shdr->bfd_section) != NULL))
+ if (idx == 0
+ || idx >= elf_numsections (abfd)
+ || (shdr = elf_elfsections (abfd)[idx])->sh_type == SHT_GROUP
+ || ((elt = shdr->bfd_section) != NULL
+ && elf_sec_group (elt) != NULL
+ && elf_sec_group (elt) != ghdr->bfd_section))
{
_bfd_error_handler
- (_("%pB: invalid entry in SHT_GROUP section [%u]"), abfd, gidx);
+ (_("%pB: invalid entry (%#x) in group [%u]"),
+ abfd, idx, gidx);
+ continue;
}
- else
- {
- /* 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;
- asection *elt = shdr->bfd_section;
- if (elt != NULL)
- {
- if (last_elt == NULL)
- {
- /* 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)
- {
- free (contents);
- return false;
- }
- }
- else
- {
- 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;
- }
- else if (shdr->sh_type != SHT_RELA
- && shdr->sh_type != SHT_REL)
+ /* PR binutils/23199: According to the ELF gABI all sections in
+ a group must be marked with SHF_GROUP, but some tools
+ generate broken objects. Fix them up here. */
+ shdr->sh_flags |= SHF_GROUP;
+
+ if (elt == NULL)
+ {
+ if (shdr->sh_type != SHT_RELA && shdr->sh_type != SHT_REL)
{
- /* There are some unknown sections in the group. */
+ const char *name = bfd_elf_string_from_elf_section
+ (abfd, elf_elfheader (abfd)->e_shstrndx, shdr->sh_name);
+
_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);
+ (_("%pB: unexpected type (%#x) section `%s' in group [%u]"),
+ abfd, shdr->sh_type, name, gidx);
+ }
+ continue;
+ }
+
+ /* Don't try to add a section to elf_next_in_group list twice. */
+ if (elf_sec_group (elt) != NULL)
+ continue;
+
+ if (last_elt == NULL)
+ {
+ /* 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)
+ {
+ free (contents);
+ return false;
}
}
+ else
+ {
+ 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;
}
+
free (contents);
return true;
}
--
Alan Modra
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Rewrite SHT_GROUP handling
2024-06-30 22:18 Rewrite SHT_GROUP handling Alan Modra
@ 2024-07-06 3:53 ` Fangrui Song
0 siblings, 0 replies; 4+ messages in thread
From: Fangrui Song @ 2024-07-06 3:53 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils
On Sun, Jun 30, 2024 at 3:18 PM Alan Modra <amodra@gmail.com> wrote:
>
> Some more error tweaks. Report a zero entry as "invalid entry.."
> rather than "unknown type..", and allow a section to be mentioned
> twice in a group.
>
> * elf.c (process_sht_group_entries): Tweak error messages, and
> allow a duplicate index in a group without reporting an error.
>
> diff --git a/bfd/elf.c b/bfd/elf.c
> index 8bb296f9637..f85f79e1e35 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -622,6 +622,7 @@ process_sht_group_entries (bfd *abfd,
> {
> unsigned int idx;
> Elf_Internal_Shdr *shdr;
> + asection *elt;
>
> p -= 4;
> idx = H_GET_32 (abfd, p);
> @@ -632,67 +633,68 @@ process_sht_group_entries (bfd *abfd,
> |= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
> break;
> }
> - if (idx < elf_numsections (abfd))
> - shdr = elf_elfsections (abfd)[idx];
>
> - if (idx >= elf_numsections (abfd)
> - || shdr->sh_type == SHT_GROUP
> - || (shdr->bfd_section != NULL
> - && elf_next_in_group (shdr->bfd_section) != NULL))
> + if (idx == 0
> + || idx >= elf_numsections (abfd)
> + || (shdr = elf_elfsections (abfd)[idx])->sh_type == SHT_GROUP
> + || ((elt = shdr->bfd_section) != NULL
> + && elf_sec_group (elt) != NULL
> + && elf_sec_group (elt) != ghdr->bfd_section))
> {
> _bfd_error_handler
> - (_("%pB: invalid entry in SHT_GROUP section [%u]"), abfd, gidx);
> + (_("%pB: invalid entry (%#x) in group [%u]"),
> + abfd, idx, gidx);
> + continue;
> }
> - else
> - {
> - /* 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;
>
> - asection *elt = shdr->bfd_section;
> - if (elt != NULL)
> - {
> - if (last_elt == NULL)
> - {
> - /* 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)
> - {
> - free (contents);
> - return false;
> - }
> - }
> - else
> - {
> - 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;
> - }
> - else if (shdr->sh_type != SHT_RELA
> - && shdr->sh_type != SHT_REL)
> + /* PR binutils/23199: According to the ELF gABI all sections in
> + a group must be marked with SHF_GROUP, but some tools
> + generate broken objects. Fix them up here. */
> + shdr->sh_flags |= SHF_GROUP;
> +
> + if (elt == NULL)
> + {
> + if (shdr->sh_type != SHT_RELA && shdr->sh_type != SHT_REL)
> {
> - /* There are some unknown sections in the group. */
> + const char *name = bfd_elf_string_from_elf_section
> + (abfd, elf_elfheader (abfd)->e_shstrndx, shdr->sh_name);
> +
> _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);
> + (_("%pB: unexpected type (%#x) section `%s' in group [%u]"),
> + abfd, shdr->sh_type, name, gidx);
> + }
> + continue;
> + }
> +
> + /* Don't try to add a section to elf_next_in_group list twice. */
> + if (elf_sec_group (elt) != NULL)
> + continue;
> +
> + if (last_elt == NULL)
> + {
> + /* 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)
> + {
> + free (contents);
> + return false;
> }
> }
> + else
> + {
> + 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;
> }
> +
> free (contents);
> return true;
> }
>
> --
> Alan Modra
The new code is much shorter. Nice! Also thanks for allowing empty
section groups.
https://sourceware.org/bugzilla/show_bug.cgi?id=20520
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Rewrite SHT_GROUP handling
2024-06-26 23:34 Alan Modra
@ 2024-06-27 5:06 ` Alan Modra
0 siblings, 0 replies; 4+ messages in thread
From: Alan Modra @ 2024-06-27 5:06 UTC (permalink / raw)
To: binutils
There is no need to loop over the headers twice. Remove that leftover
from the previous scheme. Also, the previous scheme silently ignored
a section being mentioned in two or more SHT_GROUP sections.
* elf.c (process_sht_group_entries): Prevent sections from
belonging to two groups.
(_bfd_elf_setup_sections): Process groups in a single loop
over headers.
diff --git a/bfd/elf.c b/bfd/elf.c
index d7c42273aff..8bb296f9637 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -636,7 +636,9 @@ process_sht_group_entries (bfd *abfd,
shdr = elf_elfsections (abfd)[idx];
if (idx >= elf_numsections (abfd)
- || shdr->sh_type == SHT_GROUP)
+ || shdr->sh_type == SHT_GROUP
+ || (shdr->bfd_section != NULL
+ && elf_next_in_group (shdr->bfd_section) != NULL))
{
_bfd_error_handler
(_("%pB: invalid entry in SHT_GROUP section [%u]"), abfd, gidx);
@@ -698,12 +700,10 @@ process_sht_group_entries (bfd *abfd,
bool
_bfd_elf_setup_sections (bfd *abfd)
{
- unsigned int i;
bool result = true;
- asection *s;
/* Process SHF_LINK_ORDER. */
- for (s = abfd->sections; s != NULL; s = s->next)
+ for (asection *s = abfd->sections; s != NULL; s = s->next)
{
Elf_Internal_Shdr *this_hdr = &elf_section_data (s)->this_hdr;
if ((this_hdr->sh_flags & SHF_LINK_ORDER) != 0)
@@ -746,46 +746,27 @@ _bfd_elf_setup_sections (bfd *abfd)
}
/* Process section groups. */
-
- /* 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++)
+ for (unsigned int i = 1; i < elf_numsections (abfd); i++)
{
Elf_Internal_Shdr *shdr = elf_elfsections (abfd)[i];
if (shdr && shdr->sh_type == SHT_GROUP)
{
- if (!is_valid_group_section_header (shdr, GRP_ENTRY_SIZE))
+ if (is_valid_group_section_header (shdr, GRP_ENTRY_SIZE))
{
- /* PR binutils/18758: Beware of corrupt binaries with invalid
- group data. */
+ if (shdr->sh_size >= 2 * GRP_ENTRY_SIZE
+ && !process_sht_group_entries (abfd, shdr, i))
+ result = false;
+ }
+ else
+ {
+ /* PR binutils/18758: Beware of corrupt binaries with
+ invalid group data. */
_bfd_error_handler
/* xgettext:c-format */
- (_("%pB: section group entry number %u is corrupt"),
- abfd, i);
+ (_("%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;
}
}
--
Alan Modra
^ permalink raw reply [flat|nested] 4+ messages in thread
* Rewrite SHT_GROUP handling
@ 2024-06-26 23:34 Alan Modra
2024-06-27 5:06 ` Alan Modra
0 siblings, 1 reply; 4+ messages in thread
From: Alan Modra @ 2024-06-26 23:34 UTC (permalink / raw)
To: binutils
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 --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;
--
Alan Modra
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-07-06 3:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-30 22:18 Rewrite SHT_GROUP handling Alan Modra
2024-07-06 3:53 ` Fangrui Song
-- strict thread matches above, loose matches on Subject: below --
2024-06-26 23:34 Alan Modra
2024-06-27 5:06 ` Alan Modra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).