From: Jens Remus <jremus@linux.ibm.com>
To: binutils@sourceware.org
Cc: Jens Remus <jremus@linux.ibm.com>,
Andreas Krebbel <krebbel@linux.ibm.com>
Subject: [PATCH 03/14] s390: Do not erroneously use base operand value for length operand
Date: Thu, 15 Feb 2024 16:58:10 +0100 [thread overview]
Message-ID: <20240215155821.4065623-4-jremus@linux.ibm.com> (raw)
In-Reply-To: <20240215155821.4065623-1-jremus@linux.ibm.com>
The base register operand B may optionally be omitted in D(B) by coding
D and in D(L,B) by coding D(L). The index register operand X may
optionally be omitted in D(X,B) by coding D(,B) or D(B). Both base and
index register operands may optionally be omitted in D(X,B) by coding D.
In any case the omitted base and/or index register operand value
defaults to zero.
When parsing an erroneously omitted length L operand in D(L,B) by coding
D(,B) the base register operand B was erroneously consumed as length
operand. When using a register name for the base register operand this
was detected and reported as error. But when not using a register name
the base register operand value was erroneously used as length operand
value.
Correct the parsing of an omitted optional base or index register to not
erroneously use the base register operand value as length, when
erroneously omitting the length operand.
While at it rename the variable used to remember whether the base or
index register operand was omitted to enhance code readability.
Additionally add test cases for the optional omission of base and/or
index register operands.
Example assembler source:
mvc 16(1,%r1),32(%r2)
mvc 16(1),32(%r2)
mvc 16(,1),32(%r2) # undetected syntax error
Disassembly of bad assembly without commit shows the base register
operand value was erroneously used as length operand value:
0: d2 00 10 10 20 20 mvc 16(1,%r1),32(%r2)
6: d2 00 00 10 20 20 mvc 16(1,%r0),32(%r2)
c: d2 00 00 10 20 20 mvc 16(1,%r0),32(%r2)
Assembler messages with commit:
3: Error: operand 1: missing operand
gas/
* config/tc-s390.c: Correct parsing of omitted base register.
* testsuite/gas/s390/s390.exp: Add test cases for omitted base
and/or index register.
* testsuite/gas/s390/zarch-omitted-base-index.s: Test cases for
omitted optional base or index register.
* testsuite/gas/s390/zarch-omitted-base-index.d: Likewise.
* testsuite/gas/s390/zarch-omitted-base-index-err.s: Test cases
for omitted base and/or index register.
* testsuite/gas/s390/zarch-omitted-base-index-err.l: Likewise.
Reviewed-by: Andreas Krebbel <krebbel@linux.ibm.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
gas/config/tc-s390.c | 35 ++++++++++---------
gas/testsuite/gas/s390/s390.exp | 2 ++
.../gas/s390/zarch-omitted-base-index-err.l | 24 +++++++++++++
.../gas/s390/zarch-omitted-base-index-err.s | 20 +++++++++++
.../gas/s390/zarch-omitted-base-index.d | 22 ++++++++++++
.../gas/s390/zarch-omitted-base-index.s | 24 +++++++++++++
6 files changed, 110 insertions(+), 17 deletions(-)
create mode 100644 gas/testsuite/gas/s390/zarch-omitted-base-index-err.l
create mode 100644 gas/testsuite/gas/s390/zarch-omitted-base-index-err.s
create mode 100644 gas/testsuite/gas/s390/zarch-omitted-base-index.d
create mode 100644 gas/testsuite/gas/s390/zarch-omitted-base-index.s
diff --git a/gas/config/tc-s390.c b/gas/config/tc-s390.c
index 019f26b2f4ab..cfe98b5e94be 100644
--- a/gas/config/tc-s390.c
+++ b/gas/config/tc-s390.c
@@ -1268,16 +1268,15 @@ md_gather_operands (char *str,
expressionS ex;
elf_suffix_type suffix;
bfd_reloc_code_real_type reloc;
- int skip_optional;
+ int omitted_base_or_index;
char *f;
int fc, i;
while (ISSPACE (*str))
str++;
- skip_optional = 0;
-
/* Gather the operands. */
+ omitted_base_or_index = 0; /* Whether B in D(L,B) or X in D(X,B) were omitted. */
fc = 0;
for (opindex_ptr = opcode->operands; *opindex_ptr != 0; opindex_ptr++)
{
@@ -1294,13 +1293,13 @@ md_gather_operands (char *str,
break;
}
- if (skip_optional && (operand->flags & S390_OPERAND_INDEX))
+ if (omitted_base_or_index && (operand->flags & S390_OPERAND_INDEX))
{
- /* We do an early skip. For D(X,B) constructions the index
- register is skipped (X is optional). For D(L,B) the base
- register will be the skipped operand, because L is NOT
- optional. */
- skip_optional = 0;
+ /* Skip omitted optional index register operand in D(X,B) due to
+ D(,B) or D(B). Skip comma, if D(,B). */
+ if (*str == ',')
+ str++;
+ omitted_base_or_index = 0;
continue;
}
@@ -1510,17 +1509,19 @@ md_gather_operands (char *str,
for (f = str; *f != '\0'; f++)
if (*f == ',' || *f == ')')
break;
- /* If there is no comma until the closing parentheses OR
- there is a comma right after the opening parentheses,
- we have to skip optional operands. */
+ /* If there is no comma until the closing parenthesis ')' or
+ there is a comma right after the opening parenthesis '(',
+ we have to skip the omitted optional index or base register
+ operand:
+ - Index X in D(X,B), when D(,B) or D(B)
+ - Base B in D(L,B), when D(L) */
if (*f == ',' && f == str)
{
- /* comma directly after '(' ? */
- skip_optional = 1;
- str++;
+ /* Comma directly after opening parenthesis '(' ? */
+ omitted_base_or_index = 1;
}
else
- skip_optional = (*f != ',');
+ omitted_base_or_index = (*f != ',');
}
}
else if (operand->flags & S390_OPERAND_BASE)
@@ -1530,7 +1531,7 @@ md_gather_operands (char *str,
as_bad (_("syntax error; missing ')' after base register"));
else
str++;
- skip_optional = 0;
+ omitted_base_or_index = 0;
if (*str == '\0' && skip_optargs_p (opcode->flags, &opindex_ptr[1]))
continue;
diff --git a/gas/testsuite/gas/s390/s390.exp b/gas/testsuite/gas/s390/s390.exp
index 86e2dd492cd1..7fbc7f8a7515 100644
--- a/gas/testsuite/gas/s390/s390.exp
+++ b/gas/testsuite/gas/s390/s390.exp
@@ -44,4 +44,6 @@ if [expr [istarget "s390-*-*"] || [istarget "s390x-*-*"]] then {
run_list_test "machine-parsing-4" ""
run_list_test "machine-parsing-5" ""
run_list_test "machine-parsing-6" ""
+ run_dump_test "zarch-omitted-base-index" "{as -m64}"
+ run_list_test "zarch-omitted-base-index-err" ""
}
diff --git a/gas/testsuite/gas/s390/zarch-omitted-base-index-err.l b/gas/testsuite/gas/s390/zarch-omitted-base-index-err.l
new file mode 100644
index 000000000000..fe13f95d3174
--- /dev/null
+++ b/gas/testsuite/gas/s390/zarch-omitted-base-index-err.l
@@ -0,0 +1,24 @@
+.*: Assembler messages:
+.*:5: Error: bad expression
+.*:5: Error: syntax error; missing '\)' after base register
+.*:8: Error: bad expression
+.*:8: Error: syntax error; missing '\)' after base register
+.*:9: Error: bad expression
+.*:9: Error: syntax error; missing '\)' after base register
+.*:12: Error: bad expression
+.*:12: Error: syntax error; missing '\)' after base register
+.*:13: Error: bad expression
+.*:13: Error: syntax error; missing '\)' after base register
+.*:16: Error: missing operand
+.*:17: Error: missing operand
+.*:18: Error: invalid length field specified
+.*:19: Error: bad expression
+.*:19: Error: operand out of range \(0 is not between 1 and 256\)
+.*:19: Error: operand out of range \(32 is not between 0 and 15\)
+.*:19: Error: syntax error; missing '\)' after base register
+.*:19: Error: syntax error; expected ','
+.*:19: Error: bad expression
+.*:19: Error: found 'r', expected: '\)'
+.*:19: Error: syntax error; missing '\)' after base register
+.*:19: Error: junk at end of line: `r2\)'
+.*:20: Error: syntax error; missing '\(' after displacement
diff --git a/gas/testsuite/gas/s390/zarch-omitted-base-index-err.s b/gas/testsuite/gas/s390/zarch-omitted-base-index-err.s
new file mode 100644
index 000000000000..65ad739d02c6
--- /dev/null
+++ b/gas/testsuite/gas/s390/zarch-omitted-base-index-err.s
@@ -0,0 +1,20 @@
+.text
+foo:
+
+# R1,D2(B2)
+ clm %r1,0b1000,16()
+
+# R1,D2(X2,B2)
+ a %r1,16(%r2,)
+ a %r1,16()
+
+# V1,D2(VX2,B2),M3
+ vgef %v1,16(%v2,),0
+ vgef %v1,16(),0
+
+# D1(L1,B1),D2(B2)
+ mvc 16(,%r1),32(%r2)
+ mvc 16(,1),32(%r2)
+ mvc 16(%r1),32(%r2)
+ mvc 16(),32(%r2)
+ mvc 16,32(%r2)
diff --git a/gas/testsuite/gas/s390/zarch-omitted-base-index.d b/gas/testsuite/gas/s390/zarch-omitted-base-index.d
new file mode 100644
index 000000000000..b2ff292628b1
--- /dev/null
+++ b/gas/testsuite/gas/s390/zarch-omitted-base-index.d
@@ -0,0 +1,22 @@
+#name: s390x omit base/index register
+#objdump: -dr
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+.* <foo>:
+.*: bd 18 20 10 [ ]*clm %r1,8,16\(%r2\)
+.*: bd 18 00 10 [ ]*clm %r1,8,16
+.*: 5a 12 30 10 [ ]*a %r1,16\(%r2,%r3\)
+.*: 5a 10 30 10 [ ]*a %r1,16\(%r3\)
+.*: 5a 10 30 10 [ ]*a %r1,16\(%r3\)
+.*: 5a 10 00 10 [ ]*a %r1,16
+.*: e7 12 30 10 00 13 [ ]*vgef %v1,16\(%v2,%r3\),0
+.*: e7 10 30 10 00 13 [ ]*vgef %v1,16\(%r3\),0
+.*: e7 10 30 10 00 13 [ ]*vgef %v1,16\(%r3\),0
+.*: e7 10 00 10 00 13 [ ]*vgef %v1,16,0
+.*: d2 00 10 10 20 20 [ ]*mvc 16\(1,%r1\),32\(%r2\)
+.*: d2 00 10 10 00 20 [ ]*mvc 16\(1,%r1\),32
+.*: d2 00 00 10 20 20 [ ]*mvc 16\(1,%r0\),32\(%r2\)
+.*: d2 00 00 10 00 20 [ ]*mvc 16\(1,%r0\),32
diff --git a/gas/testsuite/gas/s390/zarch-omitted-base-index.s b/gas/testsuite/gas/s390/zarch-omitted-base-index.s
new file mode 100644
index 000000000000..8381319068bc
--- /dev/null
+++ b/gas/testsuite/gas/s390/zarch-omitted-base-index.s
@@ -0,0 +1,24 @@
+.text
+foo:
+
+# R1,D2(B2)
+ clm %r1,0b1000,16(%r2)
+ clm %r1,0b1000,16
+
+# R1,D1(X2,B2)
+ a %r1,16(%r2,%r3)
+ a %r1,16(,%r3)
+ a %r1,16(%r3)
+ a %r1,16
+
+# V1,D2(VX2,B2),M3
+ vgef %v1,16(%v2,%r3),0
+ vgef %v1,16(,%r3),0
+ vgef %v1,16(%r3),0
+ vgef %v1,16,0
+
+# D1(L1,B1),D2(B2)
+ mvc 16(1,%r1),32(%r2)
+ mvc 16(1,%r1),32
+ mvc 16(1),32(%r2)
+ mvc 16(1),32
--
2.40.1
next prev parent reply other threads:[~2024-02-15 15:58 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-15 15:58 [PATCH 00/14] s390: Enhancements to working with addressing operands Jens Remus
2024-02-15 15:58 ` [PATCH 01/14] s390: Lower severity of assembler syntax errors from fatal to error Jens Remus
2024-02-15 15:58 ` [PATCH 02/14] s390: Enhance handling of syntax errors in assembler Jens Remus
2024-02-15 15:58 ` Jens Remus [this message]
2024-02-15 15:58 ` [PATCH 04/14] s390: Correct setting of highgprs flag in ELF output Jens Remus
2024-02-15 15:58 ` [PATCH 05/14] s390: Assemble processor specific test cases for their processor Jens Remus
2024-02-15 15:58 ` [PATCH 06/14] s390: Add comments to assembler operand parsing logic Jens Remus
2024-02-15 15:58 ` [PATCH 07/14] s390: Add test cases for base/index register 0 Jens Remus
2024-02-15 15:58 ` [PATCH 08/14] s390: Add test case for disassembler option warn-areg-zero Jens Remus
2024-02-15 15:58 ` [PATCH 09/14] s390: Revise s390-specific assembler option descriptions Jens Remus
2024-02-15 15:58 ` [PATCH 10/14] s390: Warn when register name type does not match operand Jens Remus
2024-02-15 15:58 ` [PATCH 11/14] s390: Print base register 0 as "0" in disassembly Jens Remus
2024-02-15 15:58 ` [PATCH 12/14] s390: Allow to explicitly omit base register operand in assembly Jens Remus
2024-02-15 15:58 ` [PATCH 13/14] s390: Provide operand number in assembler warning and error messages Jens Remus
2024-02-15 15:58 ` [PATCH 14/14] s390: Be more verbose about missing operand type Jens Remus
2024-03-01 12:24 ` [PATCH 00/14] s390: Enhancements to working with addressing operands Jens Remus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240215155821.4065623-4-jremus@linux.ibm.com \
--to=jremus@linux.ibm.com \
--cc=binutils@sourceware.org \
--cc=krebbel@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).