[binutils-gdb] s390: Do not erroneously use base operand value for length operand

Jens Remus jremus@sourceware.org
Fri Mar 1 12:21:44 GMT 2024


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=dd327181e96b8bc4d9bd97aa8d3d7fe01bf29628

commit dd327181e96b8bc4d9bd97aa8d3d7fe01bf29628
Author: Jens Remus <jremus@linux.ibm.com>
Date:   Fri Mar 1 12:45:14 2024 +0100

    s390: Do not erroneously use base operand value for length operand
    
    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>

Diff:
---
 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/testsuite/gas/s390/zarch-omitted-base-index.d  | 22 ++++++++++++++
 gas/testsuite/gas/s390/zarch-omitted-base-index.s  | 24 +++++++++++++++
 6 files changed, 110 insertions(+), 17 deletions(-)

diff --git a/gas/config/tc-s390.c b/gas/config/tc-s390.c
index 019f26b2f4a..cfe98b5e94b 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 86e2dd492cd..7fbc7f8a751 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 00000000000..fe13f95d317
--- /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 00000000000..65ad739d02c
--- /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 00000000000..b2ff292628b
--- /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 00000000000..8381319068b
--- /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


More information about the Binutils-cvs mailing list