* [PATCH v2] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands
@ 2022-05-04 10:36 Ilya Leoshkevich
2022-05-09 5:44 ` Andreas Krebbel
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Ilya Leoshkevich @ 2022-05-04 10:36 UTC (permalink / raw)
To: Andreas Krebbel; +Cc: binutils, Ilya Leoshkevich
as does not accept instructions like brasl %r0,.-0x100000000, because
of two problems with the generic 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 both by disabling the generic overflow check - it's not needed,
because s390_insert_operand () performs its own.
gas/ChangeLog:
* config/tc-s390.c (md_gather_operands): Set fx_no_overflow.
* 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.
---
v1: https://sourceware.org/pipermail/binutils/2022-May/120634.html
v1 -> v2: Use fx_no_overflow.
gas/config/tc-s390.c | 14 +++++++++-----
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 | 3 +++
gas/testsuite/gas/s390/zarch-z900-err.s | 4 ++++
6 files changed, 26 insertions(+), 6 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..4fd4f1f0693 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,14 @@ 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)));
+ /* s390_insert_operand () does the range checking. */
+ if (operand->flags & S390_OPERAND_PCREL)
+ fixP->fx_no_overflow = 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..72994192095
--- /dev/null
+++ b/gas/testsuite/gas/s390/zarch-z900-err.l
@@ -0,0 +1,3 @@
+.*: Assembler messages:
+.*:3: Error: operand out of range \(fffffffefffffffe not between 0 and 4294967294\)
+.*:4: Error: operand out of range \(0000000100000000 not between 0 and 4294967294\)
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
--
2.35.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands
2022-05-04 10:36 [PATCH v2] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands Ilya Leoshkevich
@ 2022-05-09 5:44 ` Andreas Krebbel
2022-05-09 20:01 ` Andreas Krebbel
2022-05-12 2:55 ` Alan Modra
2 siblings, 0 replies; 5+ messages in thread
From: Andreas Krebbel @ 2022-05-09 5:44 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: binutils
On 5/4/22 12:36, Ilya Leoshkevich wrote:
> as does not accept instructions like brasl %r0,.-0x100000000, because
> of two problems with the generic 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 both by disabling the generic overflow check - it's not needed,
> because s390_insert_operand () performs its own.
>
> gas/ChangeLog:
>
> * config/tc-s390.c (md_gather_operands): Set fx_no_overflow.
> * 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.
Ok. Thanks!
Andreas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands
2022-05-04 10:36 [PATCH v2] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands Ilya Leoshkevich
2022-05-09 5:44 ` Andreas Krebbel
@ 2022-05-09 20:01 ` Andreas Krebbel
2022-05-12 2:55 ` Alan Modra
2 siblings, 0 replies; 5+ messages in thread
From: Andreas Krebbel @ 2022-05-09 20:01 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: binutils
On 5/4/22 12:36, Ilya Leoshkevich wrote:
> as does not accept instructions like brasl %r0,.-0x100000000, because
> of two problems with the generic 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 both by disabling the generic overflow check - it's not needed,
> because s390_insert_operand () performs its own.
>
> gas/ChangeLog:
>
> * config/tc-s390.c (md_gather_operands): Set fx_no_overflow.
> * 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.
Committed. Thanks!
Andreas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands
2022-05-04 10:36 [PATCH v2] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands Ilya Leoshkevich
2022-05-09 5:44 ` Andreas Krebbel
2022-05-09 20:01 ` Andreas Krebbel
@ 2022-05-12 2:55 ` Alan Modra
2022-05-12 12:33 ` Ilya Leoshkevich
2 siblings, 1 reply; 5+ messages in thread
From: Alan Modra @ 2022-05-12 2:55 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: binutils, Andreas Krebbel
The new test failed on s390-linux due to bfd_sprintf_vma trimming
output to 32 bits for 32-bit targets. The test was faulty anyway,
expecting zero as the min end of the range is plainly wrong, but
that's what you get if you cast min to int.
I'm applying the following to fix this. You could also use
as_bad_value_out_of_range here, and you'll get hex values for the
test. The code would become
if (val < min || val > max)
{
if (operand->flags & S390_OPERAND_PCREL)
{
val <<= 1;
min <<= 1;
max <<= 1;
}
as_bad_value_out_of_range (_("operand"), val, min, max, file, line);
return;
}
* config/tc-s390.c (s390_insert_operand): Print range error using
PRId64.
* testsuite/gas/s390/zarch-z900-err.l: Correct expected output.
diff --git a/gas/config/tc-s390.c b/gas/config/tc-s390.c
index 4fd4f1f0693..fb452f8a986 100644
--- a/gas/config/tc-s390.c
+++ b/gas/config/tc-s390.c
@@ -617,8 +617,8 @@ s390_insert_operand (unsigned char *insn,
if (val < min || val > max)
{
const char *err =
- _("operand out of range (%s not between %ld and %ld)");
- char buf[100];
+ _("operand out of range (%" PRId64 " not between %" PRId64
+ " and %" PRId64 ")");
if (operand->flags & S390_OPERAND_PCREL)
{
@@ -626,11 +626,11 @@ s390_insert_operand (unsigned char *insn,
min <<= 1;
max <<= 1;
}
- bfd_sprintf_vma (stdoutput, buf, val);
if (file == (char *) NULL)
- as_bad (err, buf, (int) min, (int) max);
+ as_bad (err, (int64_t) val, (int64_t) min, (int64_t) max);
else
- as_bad_where (file, line, err, buf, (int) min, (int) max);
+ as_bad_where (file, line,
+ err, (int64_t) val, (int64_t) min, (int64_t) max);
return;
}
/* val is ok, now restrict it to operand->bits bits. */
diff --git a/gas/testsuite/gas/s390/zarch-z900-err.l b/gas/testsuite/gas/s390/zarch-z900-err.l
index 72994192095..cf8e9c2cefc 100644
--- a/gas/testsuite/gas/s390/zarch-z900-err.l
+++ b/gas/testsuite/gas/s390/zarch-z900-err.l
@@ -1,3 +1,3 @@
.*: Assembler messages:
-.*:3: Error: operand out of range \(fffffffefffffffe not between 0 and 4294967294\)
-.*:4: Error: operand out of range \(0000000100000000 not between 0 and 4294967294\)
+.*:3: Error: operand out of range \(-4294967298 not between -4294967296 and 4294967294\)
+.*:4: Error: operand out of range \(4294967296 not between -4294967296 and 4294967294\)
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands
2022-05-12 2:55 ` Alan Modra
@ 2022-05-12 12:33 ` Ilya Leoshkevich
0 siblings, 0 replies; 5+ messages in thread
From: Ilya Leoshkevich @ 2022-05-12 12:33 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils, Andreas Krebbel
On Thu, 2022-05-12 at 12:25 +0930, Alan Modra wrote:
> The new test failed on s390-linux due to bfd_sprintf_vma trimming
> output to 32 bits for 32-bit targets. The test was faulty anyway,
> expecting zero as the min end of the range is plainly wrong, but
> that's what you get if you cast min to int.
>
> I'm applying the following to fix this. You could also use
> as_bad_value_out_of_range here, and you'll get hex values for the
> test. The code would become
>
> if (val < min || val > max)
> {
> if (operand->flags & S390_OPERAND_PCREL)
> {
> val <<= 1;
> min <<= 1;
> max <<= 1;
> }
> as_bad_value_out_of_range (_("operand"), val, min, max,
> file, line);
> return;
> }
>
>
> * config/tc-s390.c (s390_insert_operand): Print range error
> using
> PRId64.
> * testsuite/gas/s390/zarch-z900-err.l: Correct expected
> output.
>
> diff --git a/gas/config/tc-s390.c b/gas/config/tc-s390.c
> index 4fd4f1f0693..fb452f8a986 100644
> --- a/gas/config/tc-s390.c
> +++ b/gas/config/tc-s390.c
> @@ -617,8 +617,8 @@ s390_insert_operand (unsigned char *insn,
> if (val < min || val > max)
> {
> const char *err =
> - _("operand out of range (%s not between %ld and %ld)");
> - char buf[100];
> + _("operand out of range (%" PRId64 " not between %"
> PRId64
> + " and %" PRId64 ")");
>
> if (operand->flags & S390_OPERAND_PCREL)
> {
> @@ -626,11 +626,11 @@ s390_insert_operand (unsigned char *insn,
> min <<= 1;
> max <<= 1;
> }
> - bfd_sprintf_vma (stdoutput, buf, val);
> if (file == (char *) NULL)
> - as_bad (err, buf, (int) min, (int) max);
> + as_bad (err, (int64_t) val, (int64_t) min, (int64_t)
> max);
> else
> - as_bad_where (file, line, err, buf, (int) min, (int)
> max);
> + as_bad_where (file, line,
> + err, (int64_t) val, (int64_t) min,
> (int64_t) max);
> return;
> }
> /* val is ok, now restrict it to operand->bits bits. */
> diff --git a/gas/testsuite/gas/s390/zarch-z900-err.l
> b/gas/testsuite/gas/s390/zarch-z900-err.l
> index 72994192095..cf8e9c2cefc 100644
> --- a/gas/testsuite/gas/s390/zarch-z900-err.l
> +++ b/gas/testsuite/gas/s390/zarch-z900-err.l
> @@ -1,3 +1,3 @@
> .*: Assembler messages:
> -.*:3: Error: operand out of range \(fffffffefffffffe not between 0
> and 4294967294\)
> -.*:4: Error: operand out of range \(0000000100000000 not between 0
> and 4294967294\)
> +.*:3: Error: operand out of range \(-4294967298 not between -
> 4294967296 and 4294967294\)
> +.*:4: Error: operand out of range \(4294967296 not between -
> 4294967296 and 4294967294\)
>
Sorry for the breakage and thanks for fixing it.
Not sure how I missed it - I just tried rebuilding my local commit, and
it's visible there as well.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-12 12:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-04 10:36 [PATCH v2] IBM zSystems: Accept (. - 0x100000000) PCRel32 operands Ilya Leoshkevich
2022-05-09 5:44 ` Andreas Krebbel
2022-05-09 20:01 ` Andreas Krebbel
2022-05-12 2:55 ` Alan Modra
2022-05-12 12:33 ` Ilya Leoshkevich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).