This is the mail archive of the binutils@sourceware.org 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: Fix for PR ld/20815 doesn't allow to build a working kernel


Hi Matthias,

> https://bugs.debian.org/845690 reports that trunk 20161124 doesn't allow to
> build a working kernel on at least x86_64.  I verified that reverting the fix
> for PR ld/20815 allows to build a working kernel again.

I think that Alan is probably right - it is likely to be the code that sorts
the program headers into ascending order of p_vaddr that is causing the problem.
(It would be nice if the kernel could actually tell us what is wrong however).

Rather than revert the whole patch, since doing so does allow the linker to
silently create broken binaries, would it be possible for you to try out the 
attached patch instead ?  (Actually there are two attachments: pr20815.elf.c.patch
is the change that you need to revert the sorting of PT_LOAD segments, and which
should allow you to test to see if it is sufficient to produce working kernels.
pr20815.rest.patch is a fix to the linker testsuite to adjust the tests that would
fail with the elf.c patch applied, and a patch to the readelf program to stop it
from complaining about binaries with out of order PT_LOAD segments).

Cheers
  Nick

diff --git a/bfd/elf.c b/bfd/elf.c
index 936255e..bd1c1e1 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -5854,52 +5854,6 @@ find_section_in_list (unsigned int i, elf_section_list * list)
   return list;
 }
 
-/* Compare function used when sorting the program header table.
-   The ELF standard requires that a PT_PHDR segment, if present,
-   must appear before any PT_LOAD segments.  It also requires
-   that all PT_LOAD segments are sorted into order of increasing
-   p_vaddr.  */
-
-static signed int
-phdr_sorter (const void * a, const void * b)
-{
-  Elf_Internal_Phdr * ahdr = (Elf_Internal_Phdr *) a;
-  Elf_Internal_Phdr * bhdr = (Elf_Internal_Phdr *) b;
-
-  switch (ahdr->p_type)
-    {
-    case PT_LOAD:
-      switch (bhdr->p_type)
-	{
-	case PT_PHDR:
-	  return 1;
-	case PT_LOAD:
-	  if (ahdr->p_vaddr < bhdr->p_vaddr)
-	    return -1;
-	  if (ahdr->p_vaddr > bhdr->p_vaddr)
-	    return 1;
-	  return 0;
-	default:
-	  return 0;
-	}
-      break;
-    case PT_PHDR:
-      switch (bhdr->p_type)
-	{
-	case PT_PHDR:
-	  _bfd_error_handler (_("error: multiple PHDR segments detecetd"));
-	  return 0;
-	case PT_LOAD:
-	  return -1;
-	default:
-	  return 0;
-	}
-      break;
-    default:
-      return 0;
-    }
-}
-
 /* Work out the file positions of all the sections.  This is called by
    _bfd_elf_compute_section_file_positions.  All the section sizes and
    VMAs must be known before this is called.
@@ -5963,7 +5917,6 @@ assign_file_positions_except_relocs (bfd *abfd,
     }
   else
     {
-      Elf_Internal_Phdr * map;
       unsigned int alloc;
 
       /* Assign file positions for the loaded sections based on the
@@ -6003,14 +5956,6 @@ assign_file_positions_except_relocs (bfd *abfd,
       /* Write out the program headers.  */
       alloc = elf_program_header_size (abfd) / bed->s->sizeof_phdr;
 
-      /* Sort the program headers into the ordering required by the ELF standard.  */
-      if (alloc == 0)
-	return TRUE;
-
-      map = (Elf_Internal_Phdr *) xmalloc (alloc * sizeof (* tdata->phdr));
-      memcpy (map, tdata->phdr, alloc * sizeof (* tdata->phdr));
-      qsort (map, alloc, sizeof (* tdata->phdr), phdr_sorter);
-	  
       /* PR ld/20815 - Check that the program header segment, if present, will
 	 be loaded into memory.  FIXME: The check below is not sufficient as
 	 really all PT_LOAD segments should be checked before issuing an error
@@ -6018,11 +5963,12 @@ assign_file_positions_except_relocs (bfd *abfd,
 	 in the program header table.  But this version of the check should
 	 catch all real world use cases.  */
       if (alloc > 1
-	  && map[0].p_type == PT_PHDR
-	  && ! bed->elf_backend_allow_non_load_phdr (abfd, map, alloc)
-	  && map[1].p_type == PT_LOAD
-	  && (map[1].p_vaddr > map[0].p_vaddr
-	      || (map[1].p_vaddr + map[1].p_memsz) < (map[0].p_vaddr + map[0].p_memsz)))
+	  && tdata->phdr[0].p_type == PT_PHDR
+	  && ! bed->elf_backend_allow_non_load_phdr (abfd, tdata->phdr, alloc)
+	  && tdata->phdr[1].p_type == PT_LOAD
+	  && (tdata->phdr[1].p_vaddr > tdata->phdr[0].p_vaddr
+	      || (tdata->phdr[1].p_vaddr + tdata->phdr[1].p_memsz)
+	      <  (tdata->phdr[0].p_vaddr + tdata->phdr[0].p_memsz)))
 	{
 	  /* The fix for this error is usually to edit the linker script being
 	     used and set up the program headers manually.  Either that or
@@ -6030,18 +5976,12 @@ assign_file_positions_except_relocs (bfd *abfd,
 	  _bfd_error_handler (_("\
 %B: error: PHDR segment not covered by LOAD segment"),
 			      abfd);
-	  free (map);
 	  return FALSE;
 	}
 
       if (bfd_seek (abfd, (bfd_signed_vma) bed->s->sizeof_ehdr, SEEK_SET) != 0
-	  || bed->s->write_out_phdrs (abfd, map, alloc) != 0)
-	{
-	  free (map);
-	  return FALSE;
-	}
-
-      free (map);
+	  || bed->s->write_out_phdrs (abfd, tdata->phdr, alloc) != 0)
+	return FALSE;
     }
 
   return TRUE;
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 347b6b9..854e99f 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -4909,9 +4909,13 @@ process_program_headers (FILE * file)
       switch (segment->p_type)
 	{
 	case PT_LOAD:
+#if 0 /* Do not warn about out of order PT_LOAD segments.  Although officially
+	 required by the ELF standard, several programs, including the Linux
+	 kernel, make use of non-ordered segments.  */
 	  if (previous_load
 	      && previous_load->p_vaddr > segment->p_vaddr)
 	    error (_("LOAD segments must be sorted in order of increasing VirtAddr\n"));
+#endif
 	  if (segment->p_memsz < segment->p_filesz)
 	    error (_("the segment's file size is larger than its memory size\n"));
 	  previous_load = segment;
diff --git a/ld/testsuite/ld-elf/loadaddr1.d b/ld/testsuite/ld-elf/loadaddr1.d
index 0aa372c..0fd96a7 100644
--- a/ld/testsuite/ld-elf/loadaddr1.d
+++ b/ld/testsuite/ld-elf/loadaddr1.d
@@ -5,6 +5,6 @@
 
 #...
   LOAD +0x000000 0xf*80000000 0xf*80000000 0x100050 0x100050 RWE 0x200000
-  LOAD +0x302000 0xf*80102000 0xf*80102000 0x0*10 0x0*10 RW  0x200000
   LOAD +0x200000 0xf*ff600000 0xf*80101000 0x0*10 0x0*10 R E 0x200000
+  LOAD +0x302000 0xf*80102000 0xf*80102000 0x0*10 0x0*10 RW  0x200000
 #pass
diff --git a/ld/testsuite/ld-powerpc/vle-multiseg-5.d b/ld/testsuite/ld-powerpc/vle-multiseg-5.d
index 97de87d..c223876 100644
--- a/ld/testsuite/ld-powerpc/vle-multiseg-5.d
+++ b/ld/testsuite/ld-powerpc/vle-multiseg-5.d
@@ -6,11 +6,11 @@ There are 3 program headers, starting at offset [0-9]+
 Program Headers:
   Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
   LOAD ( +0x[0-9a-f]+){5} R E 0x[0-9a-f]+
-  LOAD ( +0x[0-9a-f]+){5} R E 0x[0-9a-f]+
   LOAD ( +0x[0-9a-f]+){5} RW  0x[0-9a-f]+
+  LOAD ( +0x[0-9a-f]+){5} R E 0x[0-9a-f]+
 
  Section to Segment mapping:
   Segment Sections...
    00     .text_vle .text_iv 
-   01     .iv_handlers 
-   02     .data 
+   01     .data 
+   02     .iv_handlers 
diff --git a/ld/testsuite/ld-scripts/phdrs3a.d b/ld/testsuite/ld-scripts/phdrs3a.d
index d96bd13..80bde71 100644
--- a/ld/testsuite/ld-scripts/phdrs3a.d
+++ b/ld/testsuite/ld-scripts/phdrs3a.d
@@ -4,6 +4,6 @@
 #readelf: -l --wide
 
 #...
-[ \t]+LOAD[ x0-9a-f]+ E [ x0-9a-f]+
 [ \t]+LOAD[ x0-9a-f]+ R [ x0-9a-f]+
+[ \t]+LOAD[ x0-9a-f]+ E [ x0-9a-f]+
 #pass

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