[PATCH] sim: or1k: Eliminate dangerous RWX load segments

[PATCH] sim: or1k: Eliminate dangerous RWX load segments

[Stafford Horne]

This fixes test failures caused by the new linker warning which report:

  ./ld/ld-new: warning: load.S.x has a LOAD segment with RWX permissions

Fix this by splitting the linker MEMORY into ram and rom to avoid
generating RWX sections.  This required tests to be adjusted to fix
issues with the move.  Namely:

  - fpu tests: were incorrectly using l.ori with ha(anchor) which now
    that we pushed the anchor up in memory it exposes the bug.  Update
    to used the correct l.movhi instruction instead.
  - adrp test: the test reports ram offset addresses, now that we have
    moved memory layout around a bit I adjusted the test output.  Some
    padding is added before pi to show that the actual address of pi and
    the adrp page offset are not the same.

Bug: https://sourceware.org/PR29957
---
 sim/testsuite/or1k/adrp.S               | 5 +++--
 sim/testsuite/or1k/fpu-unordered.S      | 2 +-
 sim/testsuite/or1k/fpu64a32-unordered.S | 2 +-
 sim/testsuite/or1k/fpu64a32.S           | 2 +-
 sim/testsuite/or1k/or1k-test.ld         | 7 ++++---
 5 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/sim/testsuite/or1k/adrp.S b/sim/testsuite/or1k/adrp.S
index eaddcb03885..192324c698e 100644
--- a/sim/testsuite/or1k/adrp.S
+++ b/sim/testsuite/or1k/adrp.S
@@ -17,9 +17,9 @@
 
 # mach: or1k
 # output: report(0x00002064);\n
-# output: report(0x00012138);\n
+# output: report(0x0001a008);\n
 # output: report(0x00002000);\n
-# output: report(0x00012000);\n
+# output: report(0x0001a000);\n
 # output: report(0x00002000);\n
 # output: report(0x00014000);\n
 # output: report(0x00000000);\n
@@ -32,6 +32,7 @@
 	.section .data
 	.org 0x10000
 	.align 4
+pad:	.quad 	0
 	.type   pi, @object
 	.size   pi, 4
 pi:
diff --git a/sim/testsuite/or1k/fpu-unordered.S b/sim/testsuite/or1k/fpu-unordered.S
index 624aa0fe05d..a89172e37af 100644
--- a/sim/testsuite/or1k/fpu-unordered.S
+++ b/sim/testsuite/or1k/fpu-unordered.S
@@ -57,7 +57,7 @@ start_tests:
 	 *  r13  e   as float
 	 *  r16  nan as float
 	 */
-	l.ori	r11, r0, ha(anchor)
+	l.movhi	r11, ha(anchor)
 	l.addi	r11, r11, lo(anchor)
 	l.lwz	r12, 0(r11)
 
diff --git a/sim/testsuite/or1k/fpu64a32-unordered.S b/sim/testsuite/or1k/fpu64a32-unordered.S
index e0ae6e770d1..51d915e4e75 100644
--- a/sim/testsuite/or1k/fpu64a32-unordered.S
+++ b/sim/testsuite/or1k/fpu64a32-unordered.S
@@ -58,7 +58,7 @@ start_tests:
 	 *  r14,r15  e   as double
 	 *  r16,r17  nan as double
 	 */
-	l.ori	r11, r0, ha(anchor)
+	l.movhi	r11, ha(anchor)
 	l.addi	r11, r11, lo(anchor)
 	l.lwz	r12, 0(r11)
 	l.lwz	r13, 4(r11)
diff --git a/sim/testsuite/or1k/fpu64a32.S b/sim/testsuite/or1k/fpu64a32.S
index 71b72b7761c..6ea60b28cf2 100644
--- a/sim/testsuite/or1k/fpu64a32.S
+++ b/sim/testsuite/or1k/fpu64a32.S
@@ -98,7 +98,7 @@ start_tests:
 	 *  r14,r15  e  as double
 	 *  r16,r17  a long long
 	 */
-	l.ori	r11, r0, ha(anchor)
+	l.movhi	r11, ha(anchor)
 	l.addi	r11, r11, lo(anchor)
 	l.lwz	r12, 0(r11)
 	l.lwz	r13, 4(r11)
diff --git a/sim/testsuite/or1k/or1k-test.ld b/sim/testsuite/or1k/or1k-test.ld
index f1535daeabd..c26ecaf3f23 100644
--- a/sim/testsuite/or1k/or1k-test.ld
+++ b/sim/testsuite/or1k/or1k-test.ld
@@ -20,8 +20,9 @@ MEMORY
     /* The exception vectors actually start at 0x100, but if you specify
        that address here, the "--output-target binary" step will start from
        address 0 with the contents meant for address 0x100.  */
-    exception_vectors : ORIGIN =  0 , LENGTH = 8K
-    ram               : ORIGIN =  8K, LENGTH = 2M - 8K
+    exception_vectors : ORIGIN =  0 , LENGTH =  8K
+    rom               : ORIGIN =  8K, LENGTH = 40K
+    ram               : ORIGIN = 40K, LENGTH =  2M - 40K
 }
 
 SECTIONS
@@ -37,7 +38,7 @@ SECTIONS
 	*(.text.*)
 	*(.rodata)
 	*(.rodata.*)
-    } > ram
+    } > rom
 
     .data :
     {
-- 
2.39.1

Re: [PATCH] sim: or1k: Eliminate dangerous RWX load segments

[Nick Clifton]

Hi Stafford Horne,

   The simulator code is actually maintained by the GNU GDB project
   rather than the GNU Binutils project, so I am redirecting this
   email to the gdb-patches list.

> This fixes test failures caused by the new linker warning which report:
> 
>    ./ld/ld-new: warning: load.S.x has a LOAD segment with RWX permissions
> 
> Fix this by splitting the linker MEMORY into ram and rom to avoid
> generating RWX sections.  This required tests to be adjusted to fix
> issues with the move.  Namely:
> 
>    - fpu tests: were incorrectly using l.ori with ha(anchor) which now
>      that we pushed the anchor up in memory it exposes the bug.  Update
>      to used the correct l.movhi instruction instead.
>    - adrp test: the test reports ram offset addresses, now that we have
>      moved memory layout around a bit I adjusted the test output.  Some
>      padding is added before pi to show that the actual address of pi and
>      the adrp page offset are not the same.

(It is nice to see that this new linker feature helped to detect these problems. :-)

> Bug: https://sourceware.org/PR29957
> ---
>   sim/testsuite/or1k/adrp.S               | 5 +++--
>   sim/testsuite/or1k/fpu-unordered.S      | 2 +-
>   sim/testsuite/or1k/fpu64a32-unordered.S | 2 +-
>   sim/testsuite/or1k/fpu64a32.S           | 2 +-
>   sim/testsuite/or1k/or1k-test.ld         | 7 ++++---
>   5 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/sim/testsuite/or1k/adrp.S b/sim/testsuite/or1k/adrp.S
> index eaddcb03885..192324c698e 100644
> --- a/sim/testsuite/or1k/adrp.S
> +++ b/sim/testsuite/or1k/adrp.S
> @@ -17,9 +17,9 @@
>   
>   # mach: or1k
>   # output: report(0x00002064);\n
> -# output: report(0x00012138);\n
> +# output: report(0x0001a008);\n
>   # output: report(0x00002000);\n
> -# output: report(0x00012000);\n
> +# output: report(0x0001a000);\n
>   # output: report(0x00002000);\n
>   # output: report(0x00014000);\n
>   # output: report(0x00000000);\n
> @@ -32,6 +32,7 @@
>   	.section .data
>   	.org 0x10000
>   	.align 4
> +pad:	.quad 	0
>   	.type   pi, @object
>   	.size   pi, 4
>   pi:
> diff --git a/sim/testsuite/or1k/fpu-unordered.S b/sim/testsuite/or1k/fpu-unordered.S
> index 624aa0fe05d..a89172e37af 100644
> --- a/sim/testsuite/or1k/fpu-unordered.S
> +++ b/sim/testsuite/or1k/fpu-unordered.S
> @@ -57,7 +57,7 @@ start_tests:
>   	 *  r13  e   as float
>   	 *  r16  nan as float
>   	 */
> -	l.ori	r11, r0, ha(anchor)
> +	l.movhi	r11, ha(anchor)
>   	l.addi	r11, r11, lo(anchor)
>   	l.lwz	r12, 0(r11)
>   
> diff --git a/sim/testsuite/or1k/fpu64a32-unordered.S b/sim/testsuite/or1k/fpu64a32-unordered.S
> index e0ae6e770d1..51d915e4e75 100644
> --- a/sim/testsuite/or1k/fpu64a32-unordered.S
> +++ b/sim/testsuite/or1k/fpu64a32-unordered.S
> @@ -58,7 +58,7 @@ start_tests:
>   	 *  r14,r15  e   as double
>   	 *  r16,r17  nan as double
>   	 */
> -	l.ori	r11, r0, ha(anchor)
> +	l.movhi	r11, ha(anchor)
>   	l.addi	r11, r11, lo(anchor)
>   	l.lwz	r12, 0(r11)
>   	l.lwz	r13, 4(r11)
> diff --git a/sim/testsuite/or1k/fpu64a32.S b/sim/testsuite/or1k/fpu64a32.S
> index 71b72b7761c..6ea60b28cf2 100644
> --- a/sim/testsuite/or1k/fpu64a32.S
> +++ b/sim/testsuite/or1k/fpu64a32.S
> @@ -98,7 +98,7 @@ start_tests:
>   	 *  r14,r15  e  as double
>   	 *  r16,r17  a long long
>   	 */
> -	l.ori	r11, r0, ha(anchor)
> +	l.movhi	r11, ha(anchor)
>   	l.addi	r11, r11, lo(anchor)
>   	l.lwz	r12, 0(r11)
>   	l.lwz	r13, 4(r11)
> diff --git a/sim/testsuite/or1k/or1k-test.ld b/sim/testsuite/or1k/or1k-test.ld
> index f1535daeabd..c26ecaf3f23 100644
> --- a/sim/testsuite/or1k/or1k-test.ld
> +++ b/sim/testsuite/or1k/or1k-test.ld
> @@ -20,8 +20,9 @@ MEMORY
>       /* The exception vectors actually start at 0x100, but if you specify
>          that address here, the "--output-target binary" step will start from
>          address 0 with the contents meant for address 0x100.  */
> -    exception_vectors : ORIGIN =  0 , LENGTH = 8K
> -    ram               : ORIGIN =  8K, LENGTH = 2M - 8K
> +    exception_vectors : ORIGIN =  0 , LENGTH =  8K
> +    rom               : ORIGIN =  8K, LENGTH = 40K
> +    ram               : ORIGIN = 40K, LENGTH =  2M - 40K
>   }
>   
>   SECTIONS
> @@ -37,7 +38,7 @@ SECTIONS
>   	*(.text.*)
>   	*(.rodata)
>   	*(.rodata.*)
> -    } > ram
> +    } > rom
>   
>       .data :
>       {

Re: [PATCH] sim: or1k: Eliminate dangerous RWX load segments

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1009 bytes --]

On 19 Aug 2023 08:45, Stafford Horne wrote:
> This fixes test failures caused by the new linker warning which report:
> 
>   ./ld/ld-new: warning: load.S.x has a LOAD segment with RWX permissions
> 
> Fix this by splitting the linker MEMORY into ram and rom to avoid
> generating RWX sections.  This required tests to be adjusted to fix
> issues with the move.  Namely:
> 
>   - fpu tests: were incorrectly using l.ori with ha(anchor) which now
>     that we pushed the anchor up in memory it exposes the bug.  Update
>     to used the correct l.movhi instruction instead.
>   - adrp test: the test reports ram offset addresses, now that we have
>     moved memory layout around a bit I adjusted the test output.  Some
>     padding is added before pi to show that the actual address of pi and
>     the adrp page offset are not the same.
> 
> Bug: https://sourceware.org/PR29957

nit: put [PR sim/29957] at the end of the git commit summary (first line)

otherwise, lgtm, thanks
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] sim: or1k: Eliminate dangerous RWX load segments

[Stafford Horne]

On Fri, Oct 13, 2023 at 05:20:55PM +0545, Mike Frysinger wrote:
> On 19 Aug 2023 08:45, Stafford Horne wrote:
> > This fixes test failures caused by the new linker warning which report:
> > 
> >   ./ld/ld-new: warning: load.S.x has a LOAD segment with RWX permissions
> > 
> > Fix this by splitting the linker MEMORY into ram and rom to avoid
> > generating RWX sections.  This required tests to be adjusted to fix
> > issues with the move.  Namely:
> > 
> >   - fpu tests: were incorrectly using l.ori with ha(anchor) which now
> >     that we pushed the anchor up in memory it exposes the bug.  Update
> >     to used the correct l.movhi instruction instead.
> >   - adrp test: the test reports ram offset addresses, now that we have
> >     moved memory layout around a bit I adjusted the test output.  Some
> >     padding is added before pi to show that the actual address of pi and
> >     the adrp page offset are not the same.
> > 
> > Bug: https://sourceware.org/PR29957
> 
> nit: put [PR sim/29957] at the end of the git commit summary (first line)
> 
> otherwise, lgtm, thanks
> -mike

Thanks Mike,

I actually pushed this long back without the above update after I read Nick's
comment.  I for some reason missed that he said he was forwarding to the sim
maintainers.

Sorry for that.

-Stafford

Re: [PATCH] sim: or1k: Eliminate dangerous RWX load segments

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1524 bytes --]

On 14 Oct 2023 07:30, Stafford Horne wrote:
> On Fri, Oct 13, 2023 at 05:20:55PM +0545, Mike Frysinger wrote:
> > On 19 Aug 2023 08:45, Stafford Horne wrote:
> > > This fixes test failures caused by the new linker warning which report:
> > > 
> > >   ./ld/ld-new: warning: load.S.x has a LOAD segment with RWX permissions
> > > 
> > > Fix this by splitting the linker MEMORY into ram and rom to avoid
> > > generating RWX sections.  This required tests to be adjusted to fix
> > > issues with the move.  Namely:
> > > 
> > >   - fpu tests: were incorrectly using l.ori with ha(anchor) which now
> > >     that we pushed the anchor up in memory it exposes the bug.  Update
> > >     to used the correct l.movhi instruction instead.
> > >   - adrp test: the test reports ram offset addresses, now that we have
> > >     moved memory layout around a bit I adjusted the test output.  Some
> > >     padding is added before pi to show that the actual address of pi and
> > >     the adrp page offset are not the same.
> > > 
> > > Bug: https://sourceware.org/PR29957
> > 
> > nit: put [PR sim/29957] at the end of the git commit summary (first line)
> > 
> > otherwise, lgtm, thanks
> > -mike
> 
> Thanks Mike,
> 
> I actually pushed this long back without the above update after I read Nick's
> comment.  I for some reason missed that he said he was forwarding to the sim
> maintainers.
> 
> Sorry for that.

np.  had some hardware failures on my main desktop so only just catching up.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]