[PATCH] Fix several mix up between octets and bytes in ELF program headers

Christian Eggers ceggers@gmx.de
Mon Nov 25 08:43:00 GMT 2019


ELF_SECTION_IN_SEGMENT_1() curently compares Elf_Internal_Shdr::sh_addr
[bytes] and elf_internal_phdr::p_vaddr [bytes] with
Elf_Internal_Shdr::sh_size [octets] and elf_internal_phdr::p_memsz
[octets]. Convert the former ones into octets prior comparison.

In ld, the SIZEOF_HEADERS linker script statement must be resolved to
bytes instead of octets.

Currently I assume that all sections which are related to program
headers have the same value for octets_per_byte.

BTW: I regognized that there are two similar macros:
- include/elf/internal.h:308 ELF_TBSS_SPECIAL()
- bfd/elf.c:4665: IS_TBSS()
Is this by intention?

include/
	* elf/internal.h (struct elf_internal_phdr): Add unit (octets/
	bytes) to several member field comments.
	(Elf_Internal_Shdr): likewise.
	(ELF_SECTION_IN_SEGMENT_1): Add new parameter opb. Convert
	bytes into octets prior comparison.
	(ELF_SECTION_IN_SEGMENT): Add new parameter opb.
	(ELF_SECTION_IN_SEGMENT_STRICT): likewise.

bfd/
	* elf.c (_bfd_elf_make_section_from_shdr): Supply new parameter
	opb to ELF_SECTION_IN_SEGMENT(). Fix calculation of section LMA
	from segment LMA.
	(_bfd_elf_map_sections_to_segments): Don't mix up octets and
	bytes.
	(assign_file_positions_for_load_sections): Don't mix up octets
	and bytes. Supply new parameter opb to
	ELF_SECTION_IN_SEGMENT_1().
	(rewrite_elf_program_header): Don't mix up octets and bytes.
	(copy_elf_program_header): Supply new parameter opb to
	ELF_SECTION_IN_SEGMENT().
	(copy_private_bfd_data): likewise.
	* elf32-spu.c (spu_elf_object_p): likewise.

binutils/
	* readelf.c (process_program_headers): Add new parameter opb to
	call of ELF_SECTION_IN_SEGMENT_STRICT() macro.

ld/
	* ldexp.c (fold_name): Return SIZEOF_HEADERS in bytes.

gdb/
	* elfread.c (elf_symfile_segments): Add new parameter opb to
	call of ELF_SECTION_IN_SEGMENT() macro.

Signed-off-by: Christian Eggers <ceggers@gmx.de>
---
 bfd/elf.c              | 70 +++++++++++++++++++++++++++++---------------------
 bfd/elf32-spu.c        |  3 ++-
 binutils/readelf.c     |  5 +++-
 gdb/elfread.c          |  3 ++-
 include/elf/internal.h | 33 ++++++++++++------------
 ld/ldexp.c             |  3 ++-
 6 files changed, 68 insertions(+), 49 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index a4f26dac71d..10369483de3 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -1153,6 +1153,7 @@ _bfd_elf_make_section_from_shdr (bfd *abfd,
     {
       Elf_Internal_Phdr *phdr;
       unsigned int i, nload;
+      unsigned int opb = bfd_octets_per_byte (abfd, newsect);

       /* Some ELF linkers produce binaries with all the program header
 	 p_paddr fields zero.  If we have such a binary with more than
@@ -1173,7 +1174,7 @@ _bfd_elf_make_section_from_shdr (bfd *abfd,
 	  if (((phdr->p_type == PT_LOAD
 		&& (hdr->sh_flags & SHF_TLS) == 0)
 	       || phdr->p_type == PT_TLS)
-	      && ELF_SECTION_IN_SEGMENT (hdr, phdr))
+	      && ELF_SECTION_IN_SEGMENT (hdr, phdr, opb))
 	    {
 	      if ((flags & SEC_LOAD) == 0)
 		newsect->lma = (phdr->p_paddr
@@ -1187,7 +1188,7 @@ _bfd_elf_make_section_from_shdr (bfd *abfd,
 		   segment will contain sections with contiguous
 		   LMAs, even if the VMAs are not.  */
 		newsect->lma = (phdr->p_paddr
-				+ hdr->sh_offset - phdr->p_offset);
+				+ (hdr->sh_offset - phdr->p_offset) / opb);

 	      /* With contiguous segments, we can't tell from file
 		 offsets whether a section with zero size should
@@ -4685,6 +4686,7 @@ _bfd_elf_map_sections_to_segments (bfd *abfd, struct bfd_link_info *info)
       bfd_size_type amt;
       bfd_vma addr_mask, wrap_to = 0;
       bfd_size_type phdr_size;
+      unsigned int opb = bfd_octets_per_byte (abfd, NULL);

       /* Select the allocated sections, and sort them.  */

@@ -4724,6 +4726,8 @@ _bfd_elf_map_sections_to_segments (bfd *abfd, struct bfd_link_info *info)
       if (phdr_size == (bfd_size_type) -1)
 	phdr_size = get_program_header_size (abfd, info);
       phdr_size += bed->s->sizeof_ehdr;
+      /* phdr_size is compared to LMA values which are in bytes */
+      phdr_size /= opb;
       maxpagesize = bed->maxpagesize;
       if (maxpagesize == 0)
 	maxpagesize = 1;
@@ -4948,7 +4952,7 @@ _bfd_elf_map_sections_to_segments (bfd *abfd, struct bfd_link_info *info)
 		executable = TRUE;
 	      last_hdr = hdr;
 	      /* .tbss sections effectively have zero size.  */
-	      last_size = !IS_TBSS (hdr) ? hdr->size : 0;
+	      last_size = (!IS_TBSS (hdr) ? hdr->size : 0) / opb;
 	      continue;
 	    }

@@ -4974,7 +4978,7 @@ _bfd_elf_map_sections_to_segments (bfd *abfd, struct bfd_link_info *info)

 	  last_hdr = hdr;
 	  /* .tbss sections effectively have zero size.  */
-	  last_size = !IS_TBSS (hdr) ? hdr->size : 0;
+	  last_size = (!IS_TBSS (hdr) ? hdr->size : 0) / opb;
 	  hdr_index = i;
 	  phdr_in_segment = FALSE;
 	}
@@ -5428,11 +5432,12 @@ assign_file_positions_for_load_sections (bfd *abfd,
   struct elf_segment_map *phdr_load_seg;
   Elf_Internal_Phdr *phdrs;
   Elf_Internal_Phdr *p;
-  file_ptr off;
+  file_ptr off;  /* octets */
   bfd_size_type maxpagesize;
   unsigned int alloc, actual;
   unsigned int i, j;
   struct elf_segment_map **sorted_seg_map;
+  unsigned int opb = bfd_octets_per_byte (abfd, NULL);

   if (link_info == NULL
       && !_bfd_elf_map_sections_to_segments (abfd, link_info))
@@ -5540,7 +5545,7 @@ assign_file_positions_for_load_sections (bfd *abfd,
   for (j = 0; j < alloc; j++)
     {
       asection **secpp;
-      bfd_vma off_adjust;
+      bfd_vma off_adjust;  /* bytes */
       bfd_boolean no_contents;

       /* An ELF segment (described by Elf_Internal_Phdr) may contain a
@@ -5600,6 +5605,7 @@ assign_file_positions_for_load_sections (bfd *abfd,
 	{
 	  bfd_size_type align;
 	  unsigned int align_power = 0;
+	  file_ptr off_bytes = off / opb;  /* bytes */

 	  if (m->p_align_valid)
 	    align = p->p_align;
@@ -5635,7 +5641,7 @@ assign_file_positions_for_load_sections (bfd *abfd,
 		break;
 	      }

-	  off_adjust = vma_page_aligned_bias (p->p_vaddr, off, align);
+	  off_adjust = vma_page_aligned_bias (p->p_vaddr, off_bytes, align);

 	  /* Broken hardware and/or kernel require that files do not
 	     map the same page with different permissions on some hppa
@@ -5643,10 +5649,11 @@ assign_file_positions_for_load_sections (bfd *abfd,
 	  if (j != 0
 	      && (abfd->flags & D_PAGED) != 0
 	      && bed->no_page_alias
-	      && (off & (maxpagesize - 1)) != 0
-	      && (off & -maxpagesize) == ((off + off_adjust) & -maxpagesize))
+	      && (off_bytes & (maxpagesize - 1)) != 0
+	      && ((off_bytes & -maxpagesize)
+		  == ((off_bytes + off_adjust) & -maxpagesize)))
 	    off_adjust += maxpagesize;
-	  off += off_adjust;
+	  off += off_adjust * opb;
 	  if (no_contents)
 	    {
 	      /* We shouldn't need to align the segment on disk since
@@ -5689,9 +5696,11 @@ assign_file_positions_for_load_sections (bfd *abfd,
 	    {
 	      if (m->count > 0)
 		{
-		  if (p->p_vaddr < (bfd_vma) off
+		  file_ptr off_bytes = off / opb;
+
+		  if (p->p_vaddr < (bfd_vma) off_bytes
 		      || (!m->p_paddr_valid
-			  && p->p_paddr < (bfd_vma) off))
+			  && p->p_paddr < (bfd_vma) off_bytes))
 		    {
 		      _bfd_error_handler
 			(_("%pB: not enough room for program headers,"
@@ -5700,9 +5709,9 @@ assign_file_positions_for_load_sections (bfd *abfd,
 		      bfd_set_error (bfd_error_bad_value);
 		      return FALSE;
 		    }
-		  p->p_vaddr -= off;
+		  p->p_vaddr -= off_bytes;
 		  if (!m->p_paddr_valid)
-		    p->p_paddr -= off;
+		    p->p_paddr -= off_bytes;
 		}
 	    }
 	  else if (sorted_seg_map[0]->includes_filehdr)
@@ -5727,20 +5736,20 @@ assign_file_positions_for_load_sections (bfd *abfd,
 		  elf_elfheader (abfd)->e_phoff = p->p_offset;
 		  if (m->count > 0)
 		    {
-		      p->p_vaddr -= off - p->p_offset;
+		      p->p_vaddr -= (off - p->p_offset) / opb;
 		      if (!m->p_paddr_valid)
-			p->p_paddr -= off - p->p_offset;
+			p->p_paddr -= (off - p->p_offset) / opb;
 		    }
 		}
 	      else if (phdr_load_seg != NULL)
 		{
 		  Elf_Internal_Phdr *phdr = phdrs + phdr_load_seg->idx;
-		  bfd_vma phdr_off = 0;
+		  bfd_vma phdr_off = 0;  /* octets */
 		  if (phdr_load_seg->includes_filehdr)
 		    phdr_off = bed->s->sizeof_ehdr;
-		  p->p_vaddr = phdr->p_vaddr + phdr_off;
+		  p->p_vaddr = phdr->p_vaddr + phdr_off / opb;
 		  if (!m->p_paddr_valid)
-		    p->p_paddr = phdr->p_paddr + phdr_off;
+		    p->p_paddr = phdr->p_paddr + phdr_off / opb;
 		  p->p_offset = phdr->p_offset + phdr_off;
 		}
 	      else
@@ -5755,7 +5764,7 @@ assign_file_positions_for_load_sections (bfd *abfd,
 	    p->p_offset = off;
 	  else
 	    {
-	      file_ptr adjust;
+	      file_ptr adjust;  /* octets */

 	      adjust = off - (p->p_offset + p->p_filesz);
 	      if (!no_contents)
@@ -5786,10 +5795,10 @@ assign_file_positions_for_load_sections (bfd *abfd,
 		      && ((this_hdr->sh_flags & SHF_TLS) == 0
 			  || p->p_type == PT_TLS))))
 	    {
-	      bfd_vma p_start = p->p_paddr;
-	      bfd_vma p_end = p_start + p->p_memsz;
-	      bfd_vma s_start = sec->lma;
-	      bfd_vma adjust = s_start - p_end;
+	      bfd_vma p_start = p->p_paddr;                /* bytes */
+	      bfd_vma p_end = p_start + p->p_memsz / opb;  /* bytes */
+	      bfd_vma s_start = sec->lma;                  /* bytes */
+	      bfd_vma adjust = (s_start - p_end) * opb;    /* octets */

 	      if (adjust != 0
 		  && (s_start < p_end
@@ -5905,7 +5914,7 @@ assign_file_positions_for_load_sections (bfd *abfd,
 	    }
 	}

-      off -= off_adjust;
+      off -= off_adjust * opb;

       /* PR ld/20815 - Check that the program header segment, if
 	 present, will be loaded into memory.  */
@@ -5948,7 +5957,7 @@ assign_file_positions_for_load_sections (bfd *abfd,

 	      sec = m->sections[i];
 	      this_hdr = &(elf_section_data(sec)->this_hdr);
-	      if (!ELF_SECTION_IN_SEGMENT_1 (this_hdr, p, check_vma, 0)
+	      if (!ELF_SECTION_IN_SEGMENT_1 (this_hdr, p, opb, check_vma, 0)
 		  && !ELF_TBSS_SPECIAL (this_hdr, p))
 		{
 		  _bfd_error_handler
@@ -7223,6 +7232,7 @@ rewrite_elf_program_header (bfd *ibfd, bfd *obfd)

 	      /* Account for padding before the first section in the
 		 segment.  */
+	      hdr_size /= bfd_octets_per_byte (ibfd, NULL);
 	      map->p_vaddr_offset = map->p_paddr + hdr_size - matching_lma->lma;
 	    }

@@ -7445,6 +7455,7 @@ copy_elf_program_header (bfd *ibfd, bfd *obfd)
   unsigned int num_segments;
   bfd_boolean phdr_included = FALSE;
   bfd_boolean p_paddr_valid;
+  unsigned int opb = bfd_octets_per_byte (ibfd, NULL);

   iehdr = elf_elfheader (ibfd);

@@ -7481,7 +7492,7 @@ copy_elf_program_header (bfd *ibfd, bfd *obfd)
 	   section = section->next)
 	{
 	  this_hdr = &(elf_section_data(section)->this_hdr);
-	  if (ELF_SECTION_IN_SEGMENT (this_hdr, segment))
+	  if (ELF_SECTION_IN_SEGMENT (this_hdr, segment, opb))
 	    {
 	      if (first_section == NULL)
 		first_section = section;
@@ -7550,7 +7561,7 @@ copy_elf_program_header (bfd *ibfd, bfd *obfd)
 	       section = section->next)
 	    {
 	      this_hdr = &(elf_section_data(section)->this_hdr);
-	      if (ELF_SECTION_IN_SEGMENT (this_hdr, segment))
+	      if (ELF_SECTION_IN_SEGMENT (this_hdr, segment, opb))
 		{
 		  map->sections[isec++] = section->output_section;
 		  if ((section->flags & SEC_ALLOC) != 0)
@@ -7625,6 +7636,7 @@ copy_private_bfd_data (bfd *ibfd, bfd *obfd)
       unsigned int i, num_segments;
       Elf_Internal_Shdr *this_hdr;
       const struct elf_backend_data *bed;
+      unsigned int opb = bfd_octets_per_byte (ibfd, NULL);

       bed = get_elf_backend_data (ibfd);

@@ -7662,7 +7674,7 @@ copy_private_bfd_data (bfd *ibfd, bfd *obfd)

 	      /* Check if this section is covered by the segment.  */
 	      this_hdr = &(elf_section_data(section)->this_hdr);
-	      if (ELF_SECTION_IN_SEGMENT (this_hdr, segment))
+	      if (ELF_SECTION_IN_SEGMENT (this_hdr, segment, opb))
 		{
 		  /* FIXME: Check if its output section is changed or
 		     removed.  What else do we need to check?  */
diff --git a/bfd/elf32-spu.c b/bfd/elf32-spu.c
index 823628898f7..8429863dcc6 100644
--- a/bfd/elf32-spu.c
+++ b/bfd/elf32-spu.c
@@ -287,7 +287,8 @@ spu_elf_object_p (bfd *abfd)
 		Elf_Internal_Shdr *shdr = elf_elfsections (abfd)[j];

 		if (ELF_SECTION_SIZE (shdr, phdr) != 0
-		    && ELF_SECTION_IN_SEGMENT (shdr, phdr))
+		    && ELF_SECTION_IN_SEGMENT (shdr, phdr,
+					       OCTETS_PER_BYTE (abfd, NULL)))
 		  {
 		    asection *sec = shdr->bfd_section;
 		    spu_elf_section_data (sec)->u.o.ovl_index = num_ovl;
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 52bd71f2177..e84713f1771 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -5367,6 +5367,9 @@ process_program_headers (Filedata * filedata)
       && filedata->section_headers != NULL
       && filedata->string_table != NULL)
     {
+      /* up to now, all ELF targets have one octet per byte */
+      unsigned int opb = 1;
+
       printf (_("\n Section to Segment mapping:\n"));
       printf (_("  Segment Sections...\n"));

@@ -5383,7 +5386,7 @@ process_program_headers (Filedata * filedata)
 	  for (j = 1; j < filedata->file_header.e_shnum; j++, section++)
 	    {
 	      if (!ELF_TBSS_SPECIAL (section, segment)
-		  && ELF_SECTION_IN_SEGMENT_STRICT (section, segment))
+		  && ELF_SECTION_IN_SEGMENT_STRICT (section, segment, opb))
 		printf ("%s ", printable_section_name (filedata, section));
 	    }

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 44b793d8f14..d17fb792520 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -134,7 +134,8 @@ elf_symfile_segments (bfd *abfd)
       Elf_Internal_Shdr *this_hdr = &elf_section_data (sect)->this_hdr;

       for (j = 0; j < num_segments; j++)
-	if (ELF_SECTION_IN_SEGMENT (this_hdr, segments[j]))
+	if (ELF_SECTION_IN_SEGMENT (this_hdr, segments[j],
+				    bfd_octets_per_byte (abfd, sect)))
 	  {
 	    data->segment_info[i] = j + 1;
 	    break;
diff --git a/include/elf/internal.h b/include/elf/internal.h
index 794c16812ee..d78014a0426 100644
--- a/include/elf/internal.h
+++ b/include/elf/internal.h
@@ -86,11 +86,11 @@ typedef struct elf_internal_ehdr {
 struct elf_internal_phdr {
   unsigned long	p_type;			/* Identifies program segment type */
   unsigned long	p_flags;		/* Segment flags */
-  bfd_vma	p_offset;		/* Segment file offset */
-  bfd_vma	p_vaddr;		/* Segment virtual address */
-  bfd_vma	p_paddr;		/* Segment physical address */
-  bfd_vma	p_filesz;		/* Segment size in file */
-  bfd_vma	p_memsz;		/* Segment size in memory */
+  bfd_vma	p_offset;		/* Segment file offset in octets */
+  bfd_vma	p_vaddr;		/* Segment virtual address in bytes */
+  bfd_vma	p_paddr;		/* Segment physical address in bytes */
+  bfd_vma	p_filesz;		/* Segment size in file in octets */
+  bfd_vma	p_memsz;		/* Segment size in memory in octets */
   bfd_vma	p_align;		/* Segment alignment, file & memory */
 };

@@ -102,9 +102,10 @@ typedef struct elf_internal_shdr {
   unsigned int	sh_name;		/* Section name, index in string tbl */
   unsigned int	sh_type;		/* Type of section */
   bfd_vma	sh_flags;		/* Miscellaneous section attributes */
-  bfd_vma	sh_addr;		/* Section virtual addr at execution */
-  file_ptr	sh_offset;		/* Section file offset */
-  bfd_size_type	sh_size;		/* Size of section in bytes */
+  bfd_vma	sh_addr;		/* Section virtual addr at execution in
+					   bytes */
+  file_ptr	sh_offset;		/* Section file offset in octets */
+  bfd_size_type	sh_size;		/* Size of section in octets */
   unsigned int	sh_link;		/* Index of another section */
   unsigned int	sh_info;		/* Additional section information */
   bfd_vma	sh_addralign;		/* Section alignment */
@@ -318,7 +319,7 @@ struct elf_segment_map
    is also zero size.  Regardless of STRICT and CHECK_VMA, zero size
    sections won't match at the start or end of PT_DYNAMIC nor PT_NOTE,
    unless PT_DYNAMIC and PT_NOTE are themselves zero sized.  */
-#define ELF_SECTION_IN_SEGMENT_1(sec_hdr, segment, check_vma, strict)	\
+#define ELF_SECTION_IN_SEGMENT_1(sec_hdr, segment, opb, check_vma, strict) \
   ((/* Only PT_LOAD, PT_GNU_RELRO and PT_TLS segments can contain	\
        SHF_TLS sections.  */						\
     ((((sec_hdr)->sh_flags & SHF_TLS) != 0)				\
@@ -354,9 +355,9 @@ struct elf_segment_map
        || ((sec_hdr)->sh_flags & SHF_ALLOC) == 0			\
        || ((sec_hdr)->sh_addr >= (segment)->p_vaddr			\
 	   && (!(strict)						\
-	       || ((sec_hdr)->sh_addr - (segment)->p_vaddr		\
+	       || (((sec_hdr)->sh_addr - (segment)->p_vaddr) * (opb)	\
 		   <= (segment)->p_memsz - 1))				\
-	   && (((sec_hdr)->sh_addr - (segment)->p_vaddr			\
+	   && ((((sec_hdr)->sh_addr - (segment)->p_vaddr) * (opb)	\
 		+ ELF_SECTION_SIZE(sec_hdr, segment))			\
 	       <= (segment)->p_memsz)))					\
    /* No zero size sections at start or end of PT_DYNAMIC nor		\
@@ -371,13 +372,13 @@ struct elf_segment_map
 		    < (segment)->p_filesz)))				\
 	   && (((sec_hdr)->sh_flags & SHF_ALLOC) == 0			\
 	       || ((sec_hdr)->sh_addr > (segment)->p_vaddr		\
-		   && ((sec_hdr)->sh_addr - (segment)->p_vaddr		\
+		   && (((sec_hdr)->sh_addr - (segment)->p_vaddr) * (opb)\
 		       < (segment)->p_memsz))))))

-#define ELF_SECTION_IN_SEGMENT(sec_hdr, segment)			\
-  (ELF_SECTION_IN_SEGMENT_1 (sec_hdr, segment, 1, 0))
+#define ELF_SECTION_IN_SEGMENT(sec_hdr, segment, opb)			\
+  (ELF_SECTION_IN_SEGMENT_1 (sec_hdr, segment, opb, 1, 0))

-#define ELF_SECTION_IN_SEGMENT_STRICT(sec_hdr, segment)			\
-  (ELF_SECTION_IN_SEGMENT_1 (sec_hdr, segment, 1, 1))
+#define ELF_SECTION_IN_SEGMENT_STRICT(sec_hdr, segment, opb)		\
+  (ELF_SECTION_IN_SEGMENT_1 (sec_hdr, segment, opb, 1, 1))

 #endif /* _ELF_INTERNAL_H */
diff --git a/ld/ldexp.c b/ld/ldexp.c
index b287022f5a1..9c599c2d470 100644
--- a/ld/ldexp.c
+++ b/ld/ldexp.c
@@ -700,7 +700,8 @@ fold_name (etree_type *tree)
 	  /* Don't find the real header size if only marking sections;
 	     The bfd function may cache incorrect data.  */
 	  if (expld.phase != lang_mark_phase_enum)
-	    hdr_size = bfd_sizeof_headers (link_info.output_bfd, &link_info);
+	    hdr_size = (bfd_sizeof_headers (link_info.output_bfd, &link_info)
+			/ bfd_octets_per_byte (link_info.output_bfd, NULL));
 	  new_number (hdr_size);
 	}
       break;
--
2.16.4



More information about the Binutils mailing list