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

Message ID 20191125084333.30248-1-ceggers@gmx.de
State New
Headers show
Series
  • Fix several mix up between octets and bytes in ELF program headers
Related show

Commit Message

Christian Eggers Nov. 25, 2019, 8:43 a.m.
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(-)

--
2.16.4

Comments

Christian Eggers Dec. 2, 2019, 8:15 p.m. | #1
Ping?

Is there anyone available to review these patches?

regards
Christian


Am Montag, 25. November 2019, 09:43:33 CET schrieb Christian Eggers:
> 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
Nick Clifton Dec. 4, 2019, 5:59 p.m. | #2
Hi Christian,

> Ping?

> 

> Is there anyone available to review these patches?


Yes - me.  Sorry - I have been neck deep in CVEs this last week.

Anyway, your patch is approved - well the binutils parts anyway.

Cheers
  Nick
Alan Modra Dec. 4, 2019, 11:20 p.m. | #3
On Mon, Nov 25, 2019 at 09:43:33AM +0100, Christian Eggers wrote:
> 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 */

>  };


I'm wondering if you would be better off leaving all the values as
octets when written to file.  There are things in the ELF spec, like
p_offset mod pagesize == p_vaddr mod pagesize, that assume p_offset
and p_vaddr have the same units.

I'm not saying you shouldn't make this change.  The ELF spec says it
"supports various processors with 8-bit bytes and either 32-bit or
64-bit architectures", so every place in the spec that says "byte"
needs some translation for your architecture anyway.  Just, have you
considered leaving all the ELF file values as octets?

-- 
Alan Modra
Australia Development Lab, IBM
Christian Eggers Dec. 5, 2019, 5:40 a.m. | #4
Hi Alan,

Am Donnerstag, 5. Dezember 2019, 00:20:34 CET schrieb Alan Modra:
> I'm wondering if you would be better off leaving all the values as

> octets when written to file.  There are things in the ELF spec, like

> p_offset mod pagesize == p_vaddr mod pagesize, that assume p_offset

> and p_vaddr have the same units.


as in my previous patches, I tried to keep as much as it is and reduce the
changes to a minimum. But this is not necessarily the "correctest" way to do
it. When analyzing the existing code, I came to the conclusion, that sizes are
currently handled in octets while addresses are handled in bytes.

Doing everything in the same unit would also look more consistent for me.
Using "bytes" (as mentioned in the ELF spec) would introduce the risk of
running into trouble for things like DWARF debug information (even if these
sections are not part of the program headers). Using octets should be more
future proof.

>

> I'm not saying you shouldn't make this change.  The ELF spec says it

> "supports various processors with 8-bit bytes and either 32-bit or

> 64-bit architectures", so every place in the spec that says "byte"

> needs some translation for your architecture anyway.  Just, have you

> considered leaving all the ELF file values as octets?


I will try to do everything in octets and provide a v2 version if this patch.
This may take a few days... After that you can decide, which version fits
better.

regards
Christian
Alan Modra Dec. 5, 2019, 6:24 a.m. | #5
On Thu, Dec 05, 2019 at 06:40:14AM +0100, Christian Eggers wrote:
> as in my previous patches, I tried to keep as much as it is and reduce the

> changes to a minimum. But this is not necessarily the "correctest" way to do

> it. When analyzing the existing code, I came to the conclusion, that sizes are

> currently handled in octets while addresses are handled in bytes.


Yes, that is how the existing ld code works.

> Doing everything in the same unit would also look more consistent for me.

> Using "bytes" (as mentioned in the ELF spec) would introduce the risk of

> running into trouble for things like DWARF debug information (even if these

> sections are not part of the program headers). Using octets should be more

> future proof.


Please note that I'm fine with ld and bfd internally recording
addresses in (non-octet) bytes.  I think that will probably be easiest
given that much of ld works that way already.

I was only questioning whether it made sense to use octets everywhere
in the ELF file headers.

-- 
Alan Modra
Australia Development Lab, IBM

Patch

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;