This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: PATCH: SHN_XINDEX support is broken
On Fri, Feb 04, 2005 at 04:24:12PM -0800, H. J. Lu wrote:
> On Fri, Feb 04, 2005 at 03:43:28PM -0800, H. J. Lu wrote:
> > Hi Alan,
> >
> > SHN_XINDEX support doesn't work. I have a testcase to show it. The
> > problem is that binutils assumes the full symbol table is available
> > before all sections are processed.
HJ, you really ought to describe in a little more detail why you need to
make changes. Since I know the ELF linker code fairly well, I can guess
that this change is needed to read a group signature symbol. Correct?
If you have done the work to debug the problem, a description like the
following should be easy to write, and would make review of your patches
simpler.
"SHT_GROUP sections are typically ordered before SHT_SYMTAB and
SHT_SYMTAB_SHNDX in the ELF section headers, and thus are loaded by
elf_object_p before the symbol table sections. When loading a group
section, bfd_section_from_shdr calls group_signature to read a symbol
name associated with the group. group_signature ensures that the
SHT_SYMTAB section is loaded, but doesn't load SHT_SYMTAB_SHNDX. This
can lead to an abort if the symbol st_shndx is SHN_XINDEX."
> > It only works without SHN_XINDEX
> > since SHT_REL/SHT_RELA sections will load SHT_SYMTAB. The problem
> > affects both bfd and readelf. I will see what I can do.
> >
>
> 2005-02-04 H.J. Lu <hongjiu.lu@intel.com>
>
> * elfcode.h (elf_object_p): Read in SHT_SYMTAB and
> SHT_SYMTAB_SHNDX sections first.
I don't think this is the best place to fix this problem. One reason is
that elfcode.h is compiled twice, once for 32-bit support and once for
64-bit, so we should avoid adding code to elfcode.h if at all possible.
Another reason is that bfd_section_from_shdr already handles a number of
similar section dependecies, so that's where I would add code to load
SHT_SYMTAB_SHNDX when handling SHT_SYMTAB.
Like this. Plus a few cleanups and minor optimizations. I'll commit in
the morning assuming my overnight tests look good.
* elf-bfd.h (elf_string_from_elf_strtab): Delete macro.
* elf.c (bfd_elf_string_from_elf_section): Expand occurrence of
elf_string_from_elf_strtab.
(_bfd_elf_setup_group_pointers, bfd_section_from_shdr): Likewise.
(bfd_section_from_shdr): For SHT_SYMTAB, load SHT_SYMTAB_SHNDX too
if it exists. Don't do the reverse for SHT_SYMTAB_SHNDX. For
SHT_STRTAB, check whether the strtab is for symtab or dynsymtab by
looking at cached symtab info first, before iterating over headers.
For SHT_REL and SHT_RELA, load dynsymtab if needed.
* elfcode.h (elf_object_p): Don't load section header stringtab
specially.
Index: bfd/elf-bfd.h
===================================================================
RCS file: /cvs/src/src/bfd/elf-bfd.h,v
retrieving revision 1.170
diff -u -p -r1.170 elf-bfd.h
--- bfd/elf-bfd.h 31 Jan 2005 22:53:24 -0000 1.170
+++ bfd/elf-bfd.h 6 Feb 2005 13:16:46 -0000
@@ -1378,10 +1378,6 @@ extern bfd_boolean _bfd_elf_print_privat
extern void bfd_elf_print_symbol
(bfd *, void *, asymbol *, bfd_print_symbol_type);
-#define elf_string_from_elf_strtab(abfd, strindex) \
- bfd_elf_string_from_elf_section (abfd, elf_elfheader(abfd)->e_shstrndx, \
- strindex)
-
extern void _bfd_elf_sprintf_vma
(bfd *, char *, bfd_vma);
extern void _bfd_elf_fprintf_vma
Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.263
diff -u -p -r1.263 elf.c
--- bfd/elf.c 31 Jan 2005 23:13:18 -0000 1.263
+++ bfd/elf.c 6 Feb 2005 13:16:47 -0000
@@ -291,13 +291,13 @@ bfd_elf_string_from_elf_section (bfd *ab
if (strindex >= hdr->sh_size)
{
+ unsigned int shstrndx = elf_elfheader(abfd)->e_shstrndx;
(*_bfd_error_handler)
(_("%B: invalid string offset %u >= %lu for section `%s'"),
abfd, strindex, (unsigned long) hdr->sh_size,
- ((shindex == elf_elfheader(abfd)->e_shstrndx
- && strindex == hdr->sh_name)
+ (shindex == shstrndx && strindex == hdr->sh_name
? ".shstrtab"
- : elf_string_from_elf_strtab (abfd, hdr->sh_name)));
+ : bfd_elf_string_from_elf_section (abfd, shstrndx, hdr->sh_name)));
return "";
}
@@ -650,7 +650,10 @@ _bfd_elf_setup_group_pointers (bfd *abfd
(_("%B: unknown [%d] section `%s' in group [%s]"),
abfd,
(unsigned int) idx->shdr->sh_type,
- elf_string_from_elf_strtab (abfd, idx->shdr->sh_name),
+ bfd_elf_string_from_elf_section (abfd,
+ (elf_elfheader (abfd)
+ ->e_shstrndx),
+ idx->shdr->sh_name),
shdr->bfd_section->name);
result = FALSE;
}
@@ -1706,7 +1709,9 @@ bfd_section_from_shdr (bfd *abfd, unsign
const struct elf_backend_data *bed = get_elf_backend_data (abfd);
const char *name;
- name = elf_string_from_elf_strtab (abfd, hdr->sh_name);
+ name = bfd_elf_string_from_elf_section (abfd,
+ elf_elfheader (abfd)->e_shstrndx,
+ hdr->sh_name);
switch (hdr->sh_type)
{
@@ -1779,6 +1784,32 @@ bfd_section_from_shdr (bfd *abfd, unsign
&& ! _bfd_elf_make_section_from_shdr (abfd, hdr, name))
return FALSE;
+ /* Go looking for SHT_SYMTAB_SHNDX too, since if there is one we
+ can't read symbols without that section loaded as well. It
+ is most likely specified by the next section header. */
+ if (elf_elfsections (abfd)[elf_symtab_shndx (abfd)]->sh_link != shindex)
+ {
+ unsigned int i, num_sec;
+
+ num_sec = elf_numsections (abfd);
+ for (i = shindex + 1; i < num_sec; i++)
+ {
+ Elf_Internal_Shdr *hdr2 = elf_elfsections (abfd)[i];
+ if (hdr2->sh_type == SHT_SYMTAB_SHNDX
+ && hdr2->sh_link == shindex)
+ break;
+ }
+ if (i == num_sec)
+ for (i = 1; i < shindex; i++)
+ {
+ Elf_Internal_Shdr *hdr2 = elf_elfsections (abfd)[i];
+ if (hdr2->sh_type == SHT_SYMTAB_SHNDX
+ && hdr2->sh_link == shindex)
+ break;
+ }
+ if (i != shindex)
+ return bfd_section_from_shdr (abfd, i);
+ }
return TRUE;
case SHT_DYNSYM: /* A dynamic symbol table */
@@ -1800,11 +1831,7 @@ bfd_section_from_shdr (bfd *abfd, unsign
if (elf_symtab_shndx (abfd) == shindex)
return TRUE;
- /* Get the associated symbol table. */
- if (! bfd_section_from_shdr (abfd, hdr->sh_link)
- || hdr->sh_link != elf_onesymtab (abfd))
- return FALSE;
-
+ BFD_ASSERT (elf_symtab_shndx (abfd) == 0);
elf_symtab_shndx (abfd) = shindex;
elf_tdata (abfd)->symtab_shndx_hdr = *hdr;
elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->symtab_shndx_hdr;
@@ -1819,37 +1846,46 @@ bfd_section_from_shdr (bfd *abfd, unsign
elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->shstrtab_hdr;
return TRUE;
}
- {
- unsigned int i, num_sec;
+ if (elf_elfsections (abfd)[elf_onesymtab (abfd)]->sh_link == shindex)
+ {
+ symtab_strtab:
+ elf_tdata (abfd)->strtab_hdr = *hdr;
+ elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->strtab_hdr;
+ return TRUE;
+ }
+ if (elf_elfsections (abfd)[elf_dynsymtab (abfd)]->sh_link == shindex)
+ {
+ dynsymtab_strtab:
+ elf_tdata (abfd)->dynstrtab_hdr = *hdr;
+ hdr = &elf_tdata (abfd)->dynstrtab_hdr;
+ elf_elfsections (abfd)[shindex] = hdr;
+ /* We also treat this as a regular section, so that objcopy
+ can handle it. */
+ return _bfd_elf_make_section_from_shdr (abfd, hdr, name);
+ }
- num_sec = elf_numsections (abfd);
- for (i = 1; i < num_sec; i++)
- {
- Elf_Internal_Shdr *hdr2 = elf_elfsections (abfd)[i];
- if (hdr2->sh_link == shindex)
- {
- if (! bfd_section_from_shdr (abfd, i))
- return FALSE;
- if (elf_onesymtab (abfd) == i)
- {
- elf_tdata (abfd)->strtab_hdr = *hdr;
- elf_elfsections (abfd)[shindex] =
- &elf_tdata (abfd)->strtab_hdr;
- return TRUE;
- }
- if (elf_dynsymtab (abfd) == i)
- {
- elf_tdata (abfd)->dynstrtab_hdr = *hdr;
- elf_elfsections (abfd)[shindex] = hdr =
- &elf_tdata (abfd)->dynstrtab_hdr;
- /* We also treat this as a regular section, so
- that objcopy can handle it. */
- break;
- }
- }
- }
- }
+ /* If the string table isn't one of the above, then treat it as a
+ regular section. We need to scan all the headers to be sure,
+ just in case this strtab section appeared before the above. */
+ if (elf_onesymtab (abfd) == 0 || elf_dynsymtab (abfd) == 0)
+ {
+ unsigned int i, num_sec;
+ num_sec = elf_numsections (abfd);
+ for (i = 1; i < num_sec; i++)
+ {
+ Elf_Internal_Shdr *hdr2 = elf_elfsections (abfd)[i];
+ if (hdr2->sh_link == shindex)
+ {
+ if (! bfd_section_from_shdr (abfd, i))
+ return FALSE;
+ if (elf_onesymtab (abfd) == i)
+ goto symtab_strtab;
+ if (elf_dynsymtab (abfd) == i)
+ goto dynsymtab_strtab;
+ }
+ }
+ }
return _bfd_elf_make_section_from_shdr (abfd, hdr, name);
case SHT_REL:
@@ -1902,7 +1938,8 @@ bfd_section_from_shdr (bfd *abfd, unsign
}
/* Get the symbol table. */
- if (elf_elfsections (abfd)[hdr->sh_link]->sh_type == SHT_SYMTAB
+ if ((elf_elfsections (abfd)[hdr->sh_link]->sh_type == SHT_SYMTAB
+ || elf_elfsections (abfd)[hdr->sh_link]->sh_type == SHT_DYNSYM)
&& ! bfd_section_from_shdr (abfd, hdr->sh_link))
return FALSE;
@@ -2001,10 +2038,8 @@ bfd_section_from_shdr (bfd *abfd, unsign
default:
/* Check for any processor-specific section types. */
- {
- if (bed->elf_backend_section_from_shdr)
- (*bed->elf_backend_section_from_shdr) (abfd, hdr, name);
- }
+ if (bed->elf_backend_section_from_shdr)
+ (*bed->elf_backend_section_from_shdr) (abfd, hdr, name);
break;
}
Index: bfd/elfcode.h
===================================================================
RCS file: /cvs/src/src/bfd/elfcode.h,v
retrieving revision 1.62
diff -u -p -r1.62 elfcode.h
--- bfd/elfcode.h 28 Jan 2005 17:58:24 -0000 1.62
+++ bfd/elfcode.h 6 Feb 2005 13:16:48 -0000
@@ -475,7 +475,6 @@ elf_object_p (bfd *abfd)
Elf_Internal_Shdr i_shdr;
Elf_Internal_Shdr *i_shdrp; /* Section header table, internal form */
unsigned int shindex;
- char *shstrtab; /* Internal copy of section header stringtab */
const struct elf_backend_data *ebd;
struct bfd_preserve preserve;
asection *s;
@@ -686,12 +685,6 @@ elf_object_p (bfd *abfd)
}
}
- if (i_ehdrp->e_shstrndx && i_ehdrp->e_shoff)
- {
- if (! bfd_section_from_shdr (abfd, i_ehdrp->e_shstrndx))
- goto got_no_match;
- }
-
/* Read in the program headers. */
if (i_ehdrp->e_phnum == 0)
elf_tdata (abfd)->phdr = NULL;
@@ -717,20 +710,10 @@ elf_object_p (bfd *abfd)
}
}
- /* Read in the string table containing the names of the sections. We
- will need the base pointer to this table later. */
- /* We read this inline now, so that we don't have to go through
- bfd_section_from_shdr with it (since this particular strtab is
- used to find all of the ELF section names.) */
-
- if (i_ehdrp->e_shstrndx != 0 && i_ehdrp->e_shoff)
+ if (i_ehdrp->e_shstrndx != 0 && i_ehdrp->e_shoff != 0)
{
unsigned int num_sec;
- shstrtab = bfd_elf_get_str_section (abfd, i_ehdrp->e_shstrndx);
- if (!shstrtab)
- goto got_no_match;
-
/* Once all of the section headers have been read and converted, we
can start processing them. Note that the first section header is
a dummy placeholder entry, so we ignore it. */
--
Alan Modra
IBM OzLabs - Linux Technology Centre