[binutils-gdb] aarch64: Disallow invalid SVE addressing modes

Alice Carlotti acarlotti@sourceware.org
Fri May 9 19:36:57 GMT 2025


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

commit 4250bea744bdd373ba82b237ef7b1e77570f3fb3
Author: Alice Carlotti <alice.carlotti@arm.com>
Date:   Tue Apr 8 17:30:39 2025 +0100

    aarch64: Disallow invalid SVE addressing modes
    
    The fix for PR22988 in 2018 added a new operand AARCH64_OPND_SVE_ADDR_R
    to support implicit XZR offsets, but this fix had several flaws that
    meant it accepted several invalid addressing modes:
    
    1. The base register type wasn't properly checked when the optional
    register offset was omitted.  This meant that
      ldff1b {z1.s}, p1/z,[z1.d]
    was parsed as if it were
      ldff1b z1.d, p1/z, [x1.d, xzr].
    
    2. The explicit offset parsing didn't include a shift type, so the new
    operand would incorrectly parse
      ldff1h{z0.s}, p0/z, [x0, x0]
    as if it were
      ldff1h{z0.s}, p0/z, [x0, x0, lsl #1].
    
    3. Regardless of the above correctness issues, support for implicit
    offsets should have been added by amending the operands in the existing
    opcode table entries, instead of adding new duplicate table entires.
    
    Issue 1 can be fixed by using an "if" instead of an "else if" in
    parse_operands, while issue 2 can be fixed by failing when the first
    condition is false.  This patch applies just these two fixes, leaving
    issue 3 to be addressed in a subsequent more invasive patch.
    
    The instructions removed from the test sme-5.d are architecturally
    invalid. The new tests cover all of the affected ldff1 variants; the
    issue also affected SME ZA ld1*/st1* instructions using the same operand
    type.

Diff:
---
 gas/config/tc-aarch64.c                       | 10 ++++-
 gas/testsuite/gas/aarch64/sme-5.d             |  4 --
 gas/testsuite/gas/aarch64/sme-5.s             |  4 --
 gas/testsuite/gas/aarch64/sve-ldff1-invalid.d |  4 ++
 gas/testsuite/gas/aarch64/sve-ldff1-invalid.l | 58 +++++++++++++++++++++++++++
 gas/testsuite/gas/aarch64/sve-ldff1-invalid.s | 35 ++++++++++++++++
 6 files changed, 106 insertions(+), 9 deletions(-)

diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index acb56044fb5..10cbf00a8af 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -7817,7 +7817,15 @@ parse_operands (char *str, const aarch64_opcode *opcode)
 	      info->addr.offset.is_reg = 1;
 	      info->addr.offset.regno = 31;
 	    }
-	  else if (base_qualifier != AARCH64_OPND_QLF_X
+	  else
+	    {
+	      /* This operand is used for different shift types, so we have to
+		 reject explicit offsets.  */
+	      set_syntax_error (_("invalid addressing mode"));
+	      goto failure;
+	    }
+
+	  if (base_qualifier != AARCH64_OPND_QLF_X
 	      || offset_qualifier != AARCH64_OPND_QLF_X)
 	    {
 	      set_syntax_error (_("invalid addressing mode"));
diff --git a/gas/testsuite/gas/aarch64/sme-5.d b/gas/testsuite/gas/aarch64/sme-5.d
index 66675349227..f6ef16d74ac 100644
--- a/gas/testsuite/gas/aarch64/sme-5.d
+++ b/gas/testsuite/gas/aarch64/sme-5.d
@@ -87,7 +87,3 @@ Disassembly of section \.text:
  134:	e1dffe2f 	ld1q	{za15v.q\[w15, 0\]}, p7/z, \[x17, xzr, lsl #4\]
  138:	e000ffef 	ld1b	{za0v.b\[w15, 15\]}, p7/z, \[sp, x0\]
  13c:	e0010000 	ld1b	{za0h.b\[w12, 0\]}, p0/z, \[x0, x1\]
- 140:	e0410000 	ld1h	{za0h.h\[w12, 0\]}, p0/z, \[x0, x1, lsl #1\]
- 144:	e0819c0f 	ld1w	{za3v.s\[w12, 3\]}, p7/z, \[x0, x1, lsl #2\]
- 148:	e0c10000 	ld1d	{za0h.d\[w12, 0\]}, p0/z, \[x0, x1, lsl #3\]
- 14c:	e1c18000 	ld1q	{za0v.q\[w12, 0\]}, p0/z, \[x0, x1, lsl #4\]
diff --git a/gas/testsuite/gas/aarch64/sme-5.s b/gas/testsuite/gas/aarch64/sme-5.s
index e0d79f6d5b2..398153a073c 100644
--- a/gas/testsuite/gas/aarch64/sme-5.s
+++ b/gas/testsuite/gas/aarch64/sme-5.s
@@ -95,7 +95,3 @@ ld1q {za15v.q[bar, #0]}, p7/z, [x17]
 /* Optional LSL operator.  */
 ld1b {za0v.b[w15, 15]}, p7/z, [sp, x0, lsl #0]
 ld1b {za0h.b[w12, 0]}, p0/z, [x0, x1]
-ld1h {za0h.h[w12, 0]}, p0/z, [x0, x1]
-ld1w {za3v.s[w12, 3]}, p7/z, [x0, x1]
-ld1d {za0h.d[w12, 0]}, p0/z, [x0, x1]
-ld1q {za0v.q[w12, 0]}, p0/z, [x0, x1]
diff --git a/gas/testsuite/gas/aarch64/sve-ldff1-invalid.d b/gas/testsuite/gas/aarch64/sve-ldff1-invalid.d
new file mode 100644
index 00000000000..07fac45cc6a
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/sve-ldff1-invalid.d
@@ -0,0 +1,4 @@
+#as: -march=armv8-a+sve
+#source: sve-ldff1-invalid.s
+#error_output: sve-ldff1-invalid.l
+
diff --git a/gas/testsuite/gas/aarch64/sve-ldff1-invalid.l b/gas/testsuite/gas/aarch64/sve-ldff1-invalid.l
new file mode 100644
index 00000000000..3f101f107d6
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/sve-ldff1-invalid.l
@@ -0,0 +1,58 @@
+[^:]*: Assembler messages:
+.*: Error: operand mismatch -- `ldff1b {z1\.b},p1/z,\[z1\.d\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1b {z1\.d}, p1/z, \[z1\.d\]
+.*: Error: operand mismatch -- `ldff1b {z1\.h},p1/z,\[z1\.s\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1b {z1\.s}, p1/z, \[z1\.s\]
+.*: Error: operand mismatch -- `ldff1b {z1\.s},p1/z,\[z1\.d\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1b {z1\.s}, p1/z, \[z1\.s\]
+.*: Error: operand mismatch -- `ldff1b {z1\.d},p1/z,\[z1\.s\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1b {z1\.s}, p1/z, \[z1\.s\]
+.*: Error: operand mismatch -- `ldff1d {z1\.d},p1/z,\[z1\.s\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1d {z1\.d}, p1/z, \[z1\.d\]
+.*: Error: operand mismatch -- `ldff1h {z1\.h},p1/z,\[z1\.s\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1h {z1\.s}, p1/z, \[z1\.s\]
+.*: Error: operand mismatch -- `ldff1h {z1\.s},p1/z,\[z1\.d\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1h {z1\.s}, p1/z, \[z1\.s\]
+.*: Error: operand mismatch -- `ldff1h {z1\.d},p1/z,\[z1\.s\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1h {z1\.s}, p1/z, \[z1\.s\]
+.*: Error: operand mismatch -- `ldff1sb {z1\.h},p1/z,\[z1\.d\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1sb {z1\.d}, p1/z, \[z1\.d\]
+.*: Error: operand mismatch -- `ldff1sb {z1\.s},p1/z,\[z1\.d\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1sb {z1\.s}, p1/z, \[z1\.s\]
+.*: Error: operand mismatch -- `ldff1sb {z1\.d},p1/z,\[z1\.s\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1sb {z1\.s}, p1/z, \[z1\.s\]
+.*: Error: operand mismatch -- `ldff1sh {z1\.s},p1/z,\[z1\.d\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1sh {z1\.s}, p1/z, \[z1\.s\]
+.*: Error: operand mismatch -- `ldff1sh {z1\.d},p1/z,\[z1\.s\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1sh {z1\.s}, p1/z, \[z1\.s\]
+.*: Error: operand mismatch -- `ldff1sw {z1\.d},p1/z,\[z1\.s\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1sw {z1\.d}, p1/z, \[z1\.d\]
+.*: Error: operand mismatch -- `ldff1w {z1\.s},p1/z,\[z1\.d\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1w {z1\.s}, p1/z, \[z1\.s\]
+.*: Error: operand mismatch -- `ldff1w {z1\.d},p1/z,\[z1\.s\]'
+.*: Info:    did you mean this\?
+.*: Info:    	ldff1w {z1\.s}, p1/z, \[z1\.s\]
+.*: Error: invalid addressing mode at operand 3 -- `ldff1d {z1\.d},p1/z,\[x1,x1\]'
+.*: Error: invalid addressing mode at operand 3 -- `ldff1h {z1\.h},p1/z,\[x1,x1\]'
+.*: Error: invalid addressing mode at operand 3 -- `ldff1h {z1\.s},p1/z,\[x1,x1\]'
+.*: Error: invalid addressing mode at operand 3 -- `ldff1h {z1\.d},p1/z,\[x1,x1\]'
+.*: Error: invalid addressing mode at operand 3 -- `ldff1sh {z1\.s},p1/z,\[x1,x1\]'
+.*: Error: invalid addressing mode at operand 3 -- `ldff1sh {z1\.d},p1/z,\[x1,x1\]'
+.*: Error: invalid addressing mode at operand 3 -- `ldff1sw {z1\.d},p1/z,\[x1,x1\]'
+.*: Error: invalid addressing mode at operand 3 -- `ldff1w {z1\.s},p1/z,\[x1,x1\]'
+.*: Error: invalid addressing mode at operand 3 -- `ldff1w {z1\.d},p1/z,\[x1,x1\]'
diff --git a/gas/testsuite/gas/aarch64/sve-ldff1-invalid.s b/gas/testsuite/gas/aarch64/sve-ldff1-invalid.s
new file mode 100644
index 00000000000..ba56f03a2b8
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/sve-ldff1-invalid.s
@@ -0,0 +1,35 @@
+// The fix for PR22988 accidentally allowed invalid addressing mode
+//   ldff1b z1.d, p1/z, [z1.d]
+// which was parsed equivalently to the valid instruction:
+//   ldff1b z1.d, p1/z, [x1.d, xzr];
+// and 
+//   ldff1h{z1.s}, p1/z, [x1, x1]
+// which was parsed as:
+//   ldff1h{z1.s}, p1/z, [x1, x1, lsl #1]
+
+	ldff1b {z1.b}, p1/z, [z1.d]
+	ldff1b {z1.h}, p1/z, [z1.s]
+	ldff1b {z1.s}, p1/z, [z1.d]
+	ldff1b {z1.d}, p1/z, [z1.s]
+	ldff1d {z1.d}, p1/z, [z1.s]
+	ldff1h {z1.h}, p1/z, [z1.s]
+	ldff1h {z1.s}, p1/z, [z1.d]
+	ldff1h {z1.d}, p1/z, [z1.s]
+	ldff1sb {z1.h}, p1/z, [z1.d]
+	ldff1sb {z1.s}, p1/z, [z1.d]
+	ldff1sb {z1.d}, p1/z, [z1.s]
+	ldff1sh {z1.s}, p1/z, [z1.d]
+	ldff1sh {z1.d}, p1/z, [z1.s]
+	ldff1sw {z1.d}, p1/z, [z1.s]
+	ldff1w {z1.s}, p1/z, [z1.d]
+	ldff1w {z1.d}, p1/z, [z1.s]
+
+	ldff1d {z1.d}, p1/z, [x1, x1]
+	ldff1h {z1.h}, p1/z, [x1, x1]
+	ldff1h {z1.s}, p1/z, [x1, x1]
+	ldff1h {z1.d}, p1/z, [x1, x1]
+	ldff1sh {z1.s}, p1/z, [x1, x1]
+	ldff1sh {z1.d}, p1/z, [x1, x1]
+	ldff1sw {z1.d}, p1/z, [x1, x1]
+	ldff1w {z1.s}, p1/z, [x1, x1]
+	ldff1w {z1.d}, p1/z, [x1, x1]


More information about the Binutils-cvs mailing list