Fix for PR ld/20815 doesn't allow to build a working kernel

Message ID c28b3e65-afbb-ba6a-ed76-bbce282adb0c@redhat.com
State New
Headers show

Commit Message

Nick Clifton Nov. 28, 2016, 12:45 p.m.
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

Comments

Matthias Klose Nov. 28, 2016, 4:44 p.m. | #1
On 28.11.2016 13:45, Nick Clifton wrote:
> 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).


yes, according to Sven's comment in the bug report, that works.
Nick Clifton Nov. 28, 2016, 5:53 p.m. | #2
Hi Matthias,

> yes, according to Sven's comment in the bug report, that works.


Great!  I have checked the patch in (both parts).

Cheers
  Nick
Christophe Lyon Nov. 28, 2016, 7:55 p.m. | #3
On 28 November 2016 at 18:53, Nick Clifton <nickc@redhat.com> wrote:
> Hi Matthias,

>

>> yes, according to Sven's comment in the bug report, that works.

>

> Great!  I have checked the patch in (both parts).

>

> Cheers

>   Nick

>


Hi Nick,
Since you committed these two patches, I'm seeing errors on aarch64/arm:
./ld/ld.sum:FAIL: ld-elf/loadaddr1
./ld/ld.sum:FAIL: PHDRS headers 3a

Christophe
Nick Clifton Nov. 30, 2016, 11:09 a.m. | #4
Hi Christophe,

> Since you committed these two patches, I'm seeing errors on aarch64/arm:

> ./ld/ld.sum:FAIL: ld-elf/loadaddr1

> ./ld/ld.sum:FAIL: PHDRS headers 3a


Sorry - this should be fixed now.

Cheers
  Nick
Christophe Lyon Nov. 30, 2016, 1:06 p.m. | #5
On 30 November 2016 at 12:09, Nick Clifton <nickc@redhat.com> wrote:
> Hi Christophe,

>

>> Since you committed these two patches, I'm seeing errors on aarch64/arm:

>> ./ld/ld.sum:FAIL: ld-elf/loadaddr1

>> ./ld/ld.sum:FAIL: PHDRS headers 3a

>

> Sorry - this should be fixed now.

>

Indeed, thanks.
Christophe

> Cheers

>   Nick

>

Patch

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