[PATCH] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands

[PATCH] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands

[Ilya Leoshkevich]

as does not accept instructions like brasl %r0,.-0x100000000, because
of two problems with the overflow check:

1. PCRel32 operands are signed, but are treated as unsigned.

2. The allowed range for these operands is [-(1 << 32), (1 << 32) - 1],
   and not [-(1 << 31), (1 << 31) - 1].

Fix the first problem by setting fx_signed in the s390 backend.
Fix the second problem by introducing fx_shift and setting it in the
s390 backend.

gas/ChangeLog:

        * config/tc-s390.c (md_gather_operands): Set fx_signed and
	fx_shift for PCRel operands.
        * write.c (fix_new_internal): Initialize fx_shift.
        (fixup_segment): Handle fx_shift.
        * write.h (struct fix): Add fx_shift.
        * testsuite/gas/s390/s390.exp: Add zarch-z900-err.
        * testsuite/gas/s390/esa-z900.d: New test.
        * testsuite/gas/s390/esa-z900.s: New test.
        * testsuite/gas/s390/zarch-z900-err.l: New test.
        * testsuite/gas/s390/zarch-z900-err.s: New test.
---
 gas/config/tc-s390.c                    | 16 +++++++++++-----
 gas/testsuite/gas/s390/esa-z900.d       |  6 +++++-
 gas/testsuite/gas/s390/esa-z900.s       |  4 ++++
 gas/testsuite/gas/s390/s390.exp         |  1 +
 gas/testsuite/gas/s390/zarch-z900-err.l |  5 +++++
 gas/testsuite/gas/s390/zarch-z900-err.s |  4 ++++
 gas/write.c                             |  4 +++-
 gas/write.h                             |  5 ++++-
 8 files changed, 37 insertions(+), 8 deletions(-)
 create mode 100644 gas/testsuite/gas/s390/zarch-z900-err.l
 create mode 100644 gas/testsuite/gas/s390/zarch-z900-err.s

diff --git a/gas/config/tc-s390.c b/gas/config/tc-s390.c
index 4af635beac3..2cf831a9a21 100644
--- a/gas/config/tc-s390.c
+++ b/gas/config/tc-s390.c
@@ -1619,6 +1619,7 @@ md_gather_operands (char *str,
      md_apply_fix.  */
   for (i = 0; i < fc; i++)
     {
+      fixS *fixP;
 
       if (fixups[i].opindex < 0)
 	{
@@ -1633,7 +1634,6 @@ md_gather_operands (char *str,
       if (fixups[i].reloc != BFD_RELOC_UNUSED)
 	{
 	  reloc_howto_type *reloc_howto;
-	  fixS *fixP;
 	  int size;
 
 	  reloc_howto = bfd_reloc_type_lookup (stdoutput, fixups[i].reloc);
@@ -1661,10 +1661,16 @@ md_gather_operands (char *str,
 	    fixP->fx_pcrel_adjust = operand->shift / 8;
 	}
       else
-	fix_new_exp (frag_now, f - frag_now->fr_literal, 4, &fixups[i].exp,
-		     (operand->flags & S390_OPERAND_PCREL) != 0,
-		     ((bfd_reloc_code_real_type)
-		      (fixups[i].opindex + (int) BFD_RELOC_UNUSED)));
+	fixP = fix_new_exp (frag_now, f - frag_now->fr_literal, 4,
+			    &fixups[i].exp,
+			    (operand->flags & S390_OPERAND_PCREL) != 0,
+			    ((bfd_reloc_code_real_type)
+			     (fixups[i].opindex + (int) BFD_RELOC_UNUSED)));
+      if (operand->flags & S390_OPERAND_PCREL)
+	{
+	  fixP->fx_signed = 1;
+	  fixP->fx_shift = 1;
+	}
     }
   return str;
 }
diff --git a/gas/testsuite/gas/s390/esa-z900.d b/gas/testsuite/gas/s390/esa-z900.d
index 42f408b624f..86db0641e95 100644
--- a/gas/testsuite/gas/s390/esa-z900.d
+++ b/gas/testsuite/gas/s390/esa-z900.d
@@ -52,6 +52,10 @@ Disassembly of section .text:
 .*:	c0 f4 00 00 00 00 [	 ]*jg	102 <foo\+0x102>
 .*:	c0 65 00 00 00 00 [	 ]*brasl	%r6,108 <foo\+0x108>
 .*:	c0 65 00 00 00 00 [	 ]*brasl	%r6,10e <foo\+0x10e>
+.*:	c0 65 80 00 00 00 [	 ]*brasl	%r6,114 <foo\+0x114>
+.*:	c0 65 80 00 00 00 [	 ]*brasl	%r6,11a <foo\+0x11a>
+.*:	c0 65 7f ff ff ff [	 ]*brasl	%r6,11e <foo\+0x11e>
+.*:	c0 65 7f ff ff ff [	 ]*brasl	%r6,124 <foo\+0x124>
 .*:	01 0b [	 ]*tam
 .*:	01 0c [	 ]*sam24
 .*:	01 0d [	 ]*sam31
@@ -62,7 +66,7 @@ Disassembly of section .text:
 .*:	b9 97 00 69 [	 ]*dlr	%r6,%r9
 .*:	b9 98 00 69 [	 ]*alcr	%r6,%r9
 .*:	b9 99 00 69 [	 ]*slbr	%r6,%r9
-.*:	c0 60 00 00 00 00 [	 ]*larl	%r6,136 <foo\+0x136>
+.*:	c0 60 00 00 00 00 [	 ]*larl	%r6,14e <foo\+0x14e>
 .*:	e3 65 af ff 00 1e [	 ]*lrv	%r6,4095\(%r5,%r10\)
 .*:	e3 65 af ff 00 1f [	 ]*lrvh	%r6,4095\(%r5,%r10\)
 .*:	e3 65 af ff 00 3e [	 ]*strv	%r6,4095\(%r5,%r10\)
diff --git a/gas/testsuite/gas/s390/esa-z900.s b/gas/testsuite/gas/s390/esa-z900.s
index 7a006168f9e..74bbf612cd9 100644
--- a/gas/testsuite/gas/s390/esa-z900.s
+++ b/gas/testsuite/gas/s390/esa-z900.s
@@ -46,6 +46,10 @@ foo:
 	brul	.
 	brasl	%r6,.
 	jasl	%r6,.
+	brasl	%r6,.-0x100000000
+	jasl	%r6,.-0x100000000
+	brasl	%r6,.+0xfffffffe
+	jasl	%r6,.+0xfffffffe
 	tam
 	sam24
 	sam31
diff --git a/gas/testsuite/gas/s390/s390.exp b/gas/testsuite/gas/s390/s390.exp
index d03555a7aef..356fba95885 100644
--- a/gas/testsuite/gas/s390/s390.exp
+++ b/gas/testsuite/gas/s390/s390.exp
@@ -20,6 +20,7 @@ if [expr [istarget "s390-*-*"] ||  [istarget "s390x-*-*"]]  then {
 #    }
 
     run_dump_test "zarch-z900" "{as -m64}"
+    run_list_test "zarch-z900-err" "-march=z900"
     run_dump_test "zarch-z990" "{as -m64} {as -march=z990}"
     run_list_test "zarch-z990-symbol-lay" "-m64 -march=z990"
     run_dump_test "zarch-z9-109" "{as -m64} {as -march=z9-109}"
diff --git a/gas/testsuite/gas/s390/zarch-z900-err.l b/gas/testsuite/gas/s390/zarch-z900-err.l
new file mode 100644
index 00000000000..d4f44e24f7c
--- /dev/null
+++ b/gas/testsuite/gas/s390/zarch-z900-err.l
@@ -0,0 +1,5 @@
+.*: Assembler messages:
+.*:3: Error: operand out of range \(fffffffefffffffe not between 0 and 4294967294\)
+.*:3: Error: value of fffffffefffffffe too large for field of 4 bytes at 0000000000000000
+.*:4: Error: operand out of range \(0000000100000000 not between 0 and 4294967294\)
+.*:4: Error: value of 0000000100000000 too large for field of 4 bytes at 0000000000000006
diff --git a/gas/testsuite/gas/s390/zarch-z900-err.s b/gas/testsuite/gas/s390/zarch-z900-err.s
new file mode 100644
index 00000000000..96fef2a487c
--- /dev/null
+++ b/gas/testsuite/gas/s390/zarch-z900-err.s
@@ -0,0 +1,4 @@
+.text
+foo:
+	brasl	%r6,.-0x100000002
+	brasl	%r6,.+0x100000000
diff --git a/gas/write.c b/gas/write.c
index f607562794a..2addc29964b 100644
--- a/gas/write.c
+++ b/gas/write.c
@@ -169,6 +169,7 @@ fix_new_internal (fragS *frag,		/* Which frag?  */
   fixP->fx_addnumber = 0;
   fixP->fx_tcbit = 0;
   fixP->fx_tcbit2 = 0;
+  fixP->fx_shift = 0;
   fixP->fx_done = 0;
   fixP->fx_no_overflow = 0;
   fixP->fx_signed = 0;
@@ -1101,7 +1102,8 @@ fixup_segment (fixS *fixP, segT this_segment)
 
 	      mask = 0;
 	      mask--;		/* Set all bits to one.  */
-	      mask <<= fixP->fx_size * 8 - (fixP->fx_signed ? 1 : 0);
+	      mask <<= fixP->fx_size * 8 + fixP->fx_shift
+		       - (fixP->fx_signed ? 1 : 0);
 	      if ((add_number & mask) != 0
 		  && (fixP->fx_signed
 		      ? (add_number & mask) != mask
diff --git a/gas/write.h b/gas/write.h
index 501bdd828fc..bbd5318dd6f 100644
--- a/gas/write.h
+++ b/gas/write.h
@@ -72,8 +72,11 @@ struct fix
   unsigned fx_tcbit : 1;
   unsigned fx_tcbit2 : 1;
 
+  /* Right shift amount for the overflow check.  */
+  unsigned fx_shift : 1;
+
   /* Spare bits.  */
-  unsigned fx_unused : 10;
+  unsigned fx_unused : 9;
 
   /* pc-relative offset adjust (only used by some CPU specific code) */
   int fx_pcrel_adjust : 8;
-- 
2.35.1

Re: [PATCH] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands

[Andreas Krebbel]

On 5/3/22 12:48, Ilya Leoshkevich wrote:
> as does not accept instructions like brasl %r0,.-0x100000000, because
> of two problems with the overflow check:
> 
> 1. PCRel32 operands are signed, but are treated as unsigned.
> 
> 2. The allowed range for these operands is [-(1 << 32), (1 << 32) - 1],
>    and not [-(1 << 31), (1 << 31) - 1].
> 
> Fix the first problem by setting fx_signed in the s390 backend.
> Fix the second problem by introducing fx_shift and setting it in the
> s390 backend.
> 
> gas/ChangeLog:
> 
>         * config/tc-s390.c (md_gather_operands): Set fx_signed and
> 	fx_shift for PCRel operands.
>         * write.c (fix_new_internal): Initialize fx_shift.
>         (fixup_segment): Handle fx_shift.
>         * write.h (struct fix): Add fx_shift.
>         * testsuite/gas/s390/s390.exp: Add zarch-z900-err.
>         * testsuite/gas/s390/esa-z900.d: New test.
>         * testsuite/gas/s390/esa-z900.s: New test.
>         * testsuite/gas/s390/zarch-z900-err.l: New test.
>         * testsuite/gas/s390/zarch-z900-err.s: New test.

S/390 parts are ok. Thanks!

Andreas

Re: [PATCH] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands

[Alan Modra]

On Tue, May 03, 2022 at 12:48:58PM +0200, Ilya Leoshkevich wrote:
> Fix the second problem by introducing fx_shift and setting it in the
> s390 backend.

I think you could do without fx_shift by arithmetic shifting *valP
right by one for relevant fixups in the s390 md_apply_fix.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands

[Ilya Leoshkevich]

On Wed, 2022-05-04 at 11:59 +0930, Alan Modra wrote:
> On Tue, May 03, 2022 at 12:48:58PM +0200, Ilya Leoshkevich wrote:
> > Fix the second problem by introducing fx_shift and setting it in
> > the
> > s390 backend.
> 
> I think you could do without fx_shift by arithmetic shifting *valP
> right by one for relevant fixups in the s390 md_apply_fix.
> 

Thanks for the hint!
Having looked into that code, I think we can simply set fx_no_overflow,
because s390_insert_operand () does its own range checking.

I'll send a s390-only v2.