This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: Bug in >64k-section ELF handling when linking (with -r)
- From: Hans-Peter Nilsson <hans-peter dot nilsson at axis dot com>
- To: amodra at bigpond dot net dot au
- Cc: binutils at sources dot redhat dot com
- Date: Mon, 4 Nov 2002 12:54:15 +0100
- Subject: Re: Bug in >64k-section ELF handling when linking (with -r)
> Date: Sun, 3 Nov 2002 16:02:52 +0100
> From: Hans-Peter Nilsson <hp@axis.com>
> > Date: Mon, 4 Nov 2002 00:33:15 +1030
> > From: Alan Modra <amodra@bigpond.net.au>
>
> > The following hasn't yet been tested properly as I hit
> > a segfault due to freeing a buffer twice, and I'm not delaying
> > bedtime another half hour for my testcase to link again..
>
> [ChangeLog]
> I tweaked your previous patch and have almost the same patch as
> that one [...] I'll post that one,
> valgrind:ed and dmalloc:ed when it passes tests, if only because
> it zeroes .symtab_shndx.
Not dmalloc-tested yet, and only the -r-test has finished a
valgrind run, but here it is. It handles the posted test-cases
and the original problem case. Alan spotted the reason for the
assert. Valgrind found the uninitialized accesses; usually
zero, in the last loop in the patched function. I myself found
nothing further to complain about. :-) Tweaks for expected
output for the previously posted testcases forthcoming.
It might be that Alan wants to tweak the way allocation of
finfo.symshndxbuf is handled to that of his patch. I would have
no objection; I had already written this, going for the simplest
change that wouldn't have noticeable performance effects
(centered around the realloc, I figure). Plus it zeros the
contents, as the standard says.
Ok? If so, should this go on the 2.13 branch? (Supposedly
triggering a new pre-release tarball?)
2002-11-04 Alan Modra <amodra@bigpond.net.au>
Hans-Peter Nilsson <hp@axis.com>
* elflink.h (elf_bfd_final_link): Use bfd_zmalloc when allocating
symshndxbuf. Don't bother zeroing symtab_hdr fields.
<Emitting section symbols for emitrelocs>: Correct test for
skipping reserved sections. Initialize symtab_shndx_hdr and emit
finfo.symshndxbuf as its contents after the symtab has been output.
Free symshndxbuf, not symbuf twice.
(elf_link_output_sym): Offset destshndx (finfo->symshndxbuf) by
the known size of symtab_shndx.
(elf_link_flush_output_syms): Accumulate symbol extension section
indices, reallocating symshndxbuf rather than writing it out.
* elf.c (assign_section_numbers): When present, clear reserved
entries in i_shdrp.
Index: elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.164
diff -p -c -r1.164 elf.c
*** elf.c 19 Oct 2002 13:52:58 -0000 1.164
--- elf.c 4 Nov 2002 08:30:32 -0000
*************** assign_section_numbers (abfd)
*** 2675,2680 ****
--- 2675,2687 ----
if (i_shdrp == NULL)
return false;
+ /* When the reserved numbers have slots in the internal section headers,
+ clear i_shdrp for them, so iterating over i_shdrp[0..section_number-1]
+ looks at only defined values for reserved but unused section numbers. */
+ if (section_number > SHN_HIRESERVE)
+ memset (i_shdrp + SHN_LORESERVE, 0,
+ (SHN_HIRESERVE - SHN_LORESERVE + 1) * sizeof (*i_shdrp));
+
amt = sizeof (Elf_Internal_Shdr);
i_shdrp[0] = (Elf_Internal_Shdr *) bfd_alloc (abfd, amt);
if (i_shdrp[0] == NULL)
Index: elflink.h
===================================================================
RCS file: /cvs/src/src/bfd/elflink.h,v
retrieving revision 1.192
diff -p -c -r1.192 elflink.h
*** elflink.h 22 Oct 2002 21:00:10 -0000 1.192
--- elflink.h 4 Nov 2002 08:30:35 -0000
*************** elf_bfd_final_link (abfd, info)
*** 4919,4924 ****
--- 4919,4925 ----
Elf_Internal_Sym elfsym;
unsigned int i;
Elf_Internal_Shdr *symtab_hdr;
+ Elf_Internal_Shdr *symtab_shndx_hdr;
Elf_Internal_Shdr *symstrtab_hdr;
struct elf_backend_data *bed = get_elf_backend_data (abfd);
struct elf_outext_info eoinfo;
*************** elf_bfd_final_link (abfd, info)
*** 5192,5200 ****
symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
/* sh_name is set in prep_headers. */
symtab_hdr->sh_type = SHT_SYMTAB;
! symtab_hdr->sh_flags = 0;
! symtab_hdr->sh_addr = 0;
! symtab_hdr->sh_size = 0;
symtab_hdr->sh_entsize = sizeof (Elf_External_Sym);
/* sh_link is set in assign_section_numbers. */
/* sh_info is set below. */
--- 5193,5199 ----
symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
/* sh_name is set in prep_headers. */
symtab_hdr->sh_type = SHT_SYMTAB;
! /* sh_flags, sh_addr and sh_size all start off zero. */
symtab_hdr->sh_entsize = sizeof (Elf_External_Sym);
/* sh_link is set in assign_section_numbers. */
/* sh_info is set below. */
*************** elf_bfd_final_link (abfd, info)
*** 5223,5229 ****
{
amt = finfo.symbuf_size;
amt *= sizeof (Elf_External_Sym_Shndx);
! finfo.symshndxbuf = (Elf_External_Sym_Shndx *) bfd_malloc (amt);
if (finfo.symshndxbuf == NULL)
goto error_return;
}
--- 5222,5230 ----
{
amt = finfo.symbuf_size;
amt *= sizeof (Elf_External_Sym_Shndx);
!
! /* Allocate it zero-initialized, so unused entries contain zero. */
! finfo.symshndxbuf = (Elf_External_Sym_Shndx *) bfd_zmalloc (amt);
if (finfo.symshndxbuf == NULL)
goto error_return;
}
*************** elf_bfd_final_link (abfd, info)
*** 5283,5289 ****
if (! elf_link_output_sym (&finfo, (const char *) NULL,
&elfsym, o))
goto error_return;
! if (i == SHN_LORESERVE)
i += SHN_HIRESERVE + 1 - SHN_LORESERVE;
}
}
--- 5284,5290 ----
if (! elf_link_output_sym (&finfo, (const char *) NULL,
&elfsym, o))
goto error_return;
! if (i == SHN_LORESERVE - 1)
i += SHN_HIRESERVE + 1 - SHN_LORESERVE;
}
}
*************** elf_bfd_final_link (abfd, info)
*** 5558,5563 ****
--- 5559,5583 ----
/* Now we know the size of the symtab section. */
off += symtab_hdr->sh_size;
+ symtab_shndx_hdr = &elf_tdata (abfd)->symtab_shndx_hdr;
+ if (symtab_shndx_hdr->sh_name != 0)
+ {
+ symtab_shndx_hdr->sh_type = SHT_SYMTAB_SHNDX;
+ symtab_shndx_hdr->sh_entsize = sizeof (Elf_External_Sym_Shndx);
+ symtab_shndx_hdr->sh_addralign = sizeof (Elf_External_Sym_Shndx);
+ off = _bfd_elf_assign_file_position_for_section (symtab_shndx_hdr,
+ off, true);
+
+ /* Write out the .symtab_shndx contents, now that it's finished and
+ its position is known. */
+ if (bfd_seek (abfd, symtab_shndx_hdr->sh_offset,
+ SEEK_SET) != 0
+ || (bfd_bwrite ((PTR) finfo.symshndxbuf,
+ symtab_shndx_hdr->sh_size, abfd)
+ != symtab_shndx_hdr->sh_size))
+ return false;
+ }
+
/* Finish up and write out the symbol string table (.strtab)
section. */
symstrtab_hdr = &elf_tdata (abfd)->strtab_hdr;
*************** elf_bfd_final_link (abfd, info)
*** 5866,5872 ****
if (finfo.symbuf != NULL)
free (finfo.symbuf);
if (finfo.symshndxbuf != NULL)
! free (finfo.symbuf);
for (o = abfd->sections; o != NULL; o = o->next)
{
if ((o->flags & SEC_RELOC) != 0
--- 5886,5892 ----
if (finfo.symbuf != NULL)
free (finfo.symbuf);
if (finfo.symshndxbuf != NULL)
! free (finfo.symshndxbuf);
for (o = abfd->sections; o != NULL; o = o->next)
{
if ((o->flags & SEC_RELOC) != 0
*************** elf_bfd_final_link (abfd, info)
*** 5900,5906 ****
if (finfo.symbuf != NULL)
free (finfo.symbuf);
if (finfo.symshndxbuf != NULL)
! free (finfo.symbuf);
for (o = abfd->sections; o != NULL; o = o->next)
{
if ((o->flags & SEC_RELOC) != 0
--- 5920,5926 ----
if (finfo.symbuf != NULL)
free (finfo.symbuf);
if (finfo.symshndxbuf != NULL)
! free (finfo.symshndxbuf);
for (o = abfd->sections; o != NULL; o = o->next)
{
if ((o->flags & SEC_RELOC) != 0
*************** elf_link_output_sym (finfo, name, elfsym
*** 5958,5965 ****
dest = finfo->symbuf + finfo->symbuf_count;
destshndx = finfo->symshndxbuf;
if (destshndx != NULL)
! destshndx += finfo->symbuf_count;
elf_swap_symbol_out (finfo->output_bfd, elfsym, (PTR) dest, (PTR) destshndx);
++finfo->symbuf_count;
--- 5978,5990 ----
dest = finfo->symbuf + finfo->symbuf_count;
destshndx = finfo->symshndxbuf;
+
if (destshndx != NULL)
! destshndx
! += (finfo->symbuf_count
! + (elf_tdata (finfo->output_bfd)->symtab_shndx_hdr.sh_size
! / sizeof (Elf_External_Sym_Shndx)));
!
elf_swap_symbol_out (finfo->output_bfd, elfsym, (PTR) dest, (PTR) destshndx);
++finfo->symbuf_count;
*************** elf_link_flush_output_syms (finfo)
*** 5991,6005 ****
if (finfo->symshndxbuf != NULL)
{
hdr = &elf_tdata (finfo->output_bfd)->symtab_shndx_hdr;
- pos = hdr->sh_offset + hdr->sh_size;
amt = finfo->symbuf_count * sizeof (Elf_External_Sym_Shndx);
- if (bfd_seek (finfo->output_bfd, pos, SEEK_SET) != 0
- || (bfd_bwrite ((PTR) finfo->symshndxbuf, amt, finfo->output_bfd)
- != amt))
- return false;
hdr->sh_size += amt;
}
finfo->symbuf_count = 0;
--- 6016,6045 ----
if (finfo->symshndxbuf != NULL)
{
+ /* We can't flush symshndxbuf to disk as is done with symbuf,
+ because we haven't calculated the position of the
+ .symtab_shndx section yet, which we can't do until we know
+ the size of the .symtab section (which we are in progress of
+ outputting, finding its final size). So we keep symshndxbuf,
+ (the whole .symtab_shndx section contents), in core: four
+ bytes per symbol in the output. We realloc that buffer here
+ and update the .symtab_shndx section size, synchronizing it
+ with the flushing of the symtab buffer. */
hdr = &elf_tdata (finfo->output_bfd)->symtab_shndx_hdr;
amt = finfo->symbuf_count * sizeof (Elf_External_Sym_Shndx);
+ /* Update the known size of the .symtab_shndx section. */
hdr->sh_size += amt;
+
+ /* Reallocate the buffer to make room for additional contents.
+ Clear the new contents to make unused entries hold 0. */
+ finfo->symshndxbuf
+ = (Elf_External_Sym_Shndx *) bfd_realloc ((PTR) finfo->symshndxbuf,
+ hdr->sh_size + amt);
+ if (finfo->symshndxbuf == NULL)
+ return false;
+
+ memset ((bfd_byte *) finfo->symshndxbuf + hdr->sh_size, 0, amt);
}
finfo->symbuf_count = 0;
brgds, H-P