This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]