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: Patch to the way BFD reads overlaid ELF sections


Alan Modra <amodra@bigpond.net.au> writes:
> On Wed, May 01, 2002 at 07:27:15PM +0100, Richard Sandiford wrote:
> > 	* elf.c (_bfd_elf_make_section_from_shdr): Base the LMA on the
> > 	smallest matching segment.
> 
> OK, but please put a comment in the code describing why we ought to
> look through all the segments.

On second thoughts, if I can't get it right for like-sized overlays
too, it's probably best to leave things be.

If .bss1 and .bss2 are the same size in:

    OVERLAY
      {
        .bss1 { ... }
        .bss2 { ... }
      }

then we end up with two segments that have the same file offset,
file size (0), VMA, and memory size.  Is it reasonable to expect
BFD to get the mapping right in this case?

The only way I can think of making it work is to have the
file offsets for zeroed sections and segments point to the
segment structure itself.  Would that go against the ELF spec?
I notice (for the section case) it says:

    One section type, SHT_NOBITS described below, occupies no
    space in the file, and its sh_offset member locates the
    conceptual placement in the file

but I don't really understand what "conceptual placement" is,
or how it affects things.

If we made this change, the segments for the overlay-size test
would look like:

  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  [...]
  LOAD           0x000074 0x00020000 0x00020000 0x00000 0x00010 RW  0x1000
  LOAD           0x000094 0x00020000 0x00020010 0x00000 0x00030 RW  0x1000
  LOAD           0x0000b4 0x00020000 0x00020040 0x00000 0x00020 RW  0x1000
  LOAD           0x0000d4 0x00020030 0x00020060 0x00000 0x00230 RW  0x1000
  LOAD           0x003000 0x00010000 0x00030000 0x000a0 0x000a0 R E 0x1000
  LOAD           0x004020 0x00010020 0x000300a0 0x00040 0x00040 R E 0x1000
  LOAD           0x005020 0x00010020 0x000300e0 0x00020 0x00020 R E 0x1000
  LOAD           0x005260 0x00020260 0x00030100 0x00030 0x00030 RW  0x1000
  LOAD           0x006260 0x00020260 0x00030130 0x00040 0x00040 RW  0x1000
  LOAD           0x007260 0x00020260 0x00030170 0x00050 0x00050 RW  0x1000

where the offsets for the first four point to the segments themselves.
The section table would be:

  [Nr] Name          Type         Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]               NULL         00000000 000000 000000 00      0   0  0
  [ 1] .bss1         NOBITS       00020000 000074 000010 00  WA  0   0  1
  [ 2] .bss2         NOBITS       00020000 000094 000030 00  WA  0   0  1
  [ 3] .bss3         NOBITS       00020000 0000b4 000020 00  WA  0   0  1
  [ 4] .mtext        PROGBITS     00010000 003000 000020 00  AX  0   0  1
  [ 5] .mbss         NOBITS       00020030 0000d4 000230 00  WA  0   0  1
  [ 6] .text1        PROGBITS     00010020 003020 000080 00  AX  0   0  1
  [ 7] .text2        PROGBITS     00010020 004020 000040 00  AX  0   0  1
  [ 8] .text3        PROGBITS     00010020 005020 000020 00  AX  0   0  1
  [ 9] .data1        PROGBITS     00020260 005260 000030 00  WA  0   0  1
  [10] .data2        PROGBITS     00020260 006260 000040 00  WA  0   0  1
  [11] .data3        PROGBITS     00020260 007260 000050 00  WA  0   0  1
  [...]

and the current mapping code will hande it fine.

Although, while I'm here, there seems to be some dead code in
_bfd_elf_make_section_from_shdr():

	      if (phdr->p_type == PT_LOAD
		  && (bfd_vma) hdr->sh_offset >= phdr->p_offset
 		  && (hdr->sh_offset + hdr->sh_size
            	      <= phdr->p_offset + phdr->p_memsz)
		  && ((flags & SEC_LOAD) == 0
		      || (hdr->sh_offset + hdr->sh_size
			  <= phdr->p_offset + phdr->p_filesz)))
		{
                  ...
                }

Checking

    hdr->sh_offset + hdr->sh_size     against both
    phdr->p_offset + phdr->p_memsz    and
    phdr->p_offset + phdr->p_filesz

seems to be redundant, since p_memsz can't be smaller than p_filesz.
OK to rework as follows?  Just a clean-up (I hope).

Both patches checked on i686-pc-linux-gnu, mips-elf, mipsel-elf,
mips64-elf, arm-coff, arm-elf and powerpc-eabi.

Richard

[bfd/]
	* elf.c (assign_file_positions_for_segments): If a loadable segment
	has no file contents, set its offset to the position of the segment
	header itself.
	
[bfd/]
	* elf.c (_bfd_elf_make_section_from_shdr): Simplify check for
	file contents.  Update comment.

Index: elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.137
diff -c -p -d -r1.137 elf.c
*** elf.c	12 Apr 2002 03:30:56 -0000	1.137
--- elf.c	2 May 2002 14:32:01 -0000
*************** Error: First section in segment (%s) sta
*** 3618,3623 ****
--- 3619,3643 ----
  	      if ((flags & SEC_READONLY) == 0)
  		p->p_flags |= PF_W;
  	    }
+ 	}
+ 
+       /* If a file contains several overlaid NOBITS sections, there
+ 	 may be several segments with the same VMA, memory size and
+ 	 file size (0).  Using the code above, they would also get
+ 	 the same file offset, and it would be very difficult to
+ 	 figure out which NOBITS section belongs to which segment.
+ 
+ 	 Since the choice of file offset is somewhat arbitrary in this
+ 	 case, we can use it to make the mapping easier.  Set it to the
+ 	 offset of the segment entry, and alter the positions of each
+ 	 member section to match.  */
+       if (p->p_type == PT_LOAD && p->p_filesz == 0)
+ 	{
+ 	  p->p_offset = (bed->s->sizeof_ehdr
+ 			 + (p - phdrs) * bed->s->sizeof_phdr);
+ 
+ 	  for (i = 0, secpp = m->sections; i < m->count; i++, secpp++)
+ 	    (**secpp).filepos = p->p_offset;
  	}
      }
  
Index: elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.137
diff -c -p -d -r1.137 elf.c
*** elf.c	12 Apr 2002 03:30:56 -0000	1.137
--- elf.c	2 May 2002 14:32:01 -0000
*************** _bfd_elf_make_section_from_shdr (abfd, h
*** 631,639 ****
--- 631,643 ----
  
    if ((flags & SEC_ALLOC) != 0)
      {
+       bfd_vma file_size;
        Elf_Internal_Phdr *phdr;
        unsigned int i;
  
+       /* The size of the file contents at SH_OFFSET.  */
+       file_size = (flags & SEC_LOAD) == 0 ? 0 : hdr->sh_size;
+ 
        /* Look through the phdrs to see if we need to adjust the lma.
           If all the p_paddr fields are zero, we ignore them, since
           some ELF linkers produce such output.  */
*************** _bfd_elf_make_section_from_shdr (abfd, h
*** 648,657 ****
  	  phdr = elf_tdata (abfd)->phdr;
  	  for (i = 0; i < elf_elfheader (abfd)->e_phnum; i++, phdr++)
  	    {
! 	      /* This section is part of this segment if its file
! 		 offset plus size lies within the segment's memory
! 		 span and, if the section is loaded, the extent of the
! 		 loaded data lies within the extent of the segment.  
  
  		 Note - we used to check the p_paddr field as well, and
  		 refuse to set the LMA if it was 0.  This is wrong
--- 652,661 ----
  	  phdr = elf_tdata (abfd)->phdr;
  	  for (i = 0; i < elf_elfheader (abfd)->e_phnum; i++, phdr++)
  	    {
! 	      /* Look for segments that contain this section's file
! 		 data.  This comparison is meaningful even for NOBITS
! 		 sections, since SH_OFFSET gives the section's
! 		 "conceptual placement" in the file.
  
  		 Note - we used to check the p_paddr field as well, and
  		 refuse to set the LMA if it was 0.  This is wrong
*************** _bfd_elf_make_section_from_shdr (abfd, h
*** 661,672 ****
  	         executables need to be able to have a segment which
  	         covers this address.  */
  	      if (phdr->p_type == PT_LOAD
! 		  && (bfd_vma) hdr->sh_offset >= phdr->p_offset
! 		  && (hdr->sh_offset + hdr->sh_size
! 		      <= phdr->p_offset + phdr->p_memsz)
! 		  && ((flags & SEC_LOAD) == 0
! 		      || (hdr->sh_offset + hdr->sh_size
! 			  <= phdr->p_offset + phdr->p_filesz)))
  		{
  		  if ((flags & SEC_LOAD) == 0)
  		    newsect->lma = (phdr->p_paddr
--- 665,673 ----
  	         executables need to be able to have a segment which
  	         covers this address.  */
  	      if (phdr->p_type == PT_LOAD
!  		  && (bfd_vma) hdr->sh_offset >= phdr->p_offset
!  		  && (hdr->sh_offset + file_size
!  		      <= phdr->p_offset + phdr->p_filesz))
  		{
  		  if ((flags & SEC_LOAD) == 0)
  		    newsect->lma = (phdr->p_paddr

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