This is the mail archive of the
binutils-cvs@sourceware.org
mailing list for the binutils project.
[binutils-gdb/binutils-2_25-branch] PPC sync instruction accepts invalid and incompatible operands
- From: Alan Modra <amodra at sourceware dot org>
- To: bfd-cvs at sourceware dot org
- Date: 10 Jul 2015 10:43:02 -0000
- Subject: [binutils-gdb/binutils-2_25-branch] PPC sync instruction accepts invalid and incompatible operands
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=4e8fee9042c5d47dbaec08b2c0c36bca0ba8545b
commit 4e8fee9042c5d47dbaec08b2c0c36bca0ba8545b
Author: Peter Bergner <bergner@vnet.ibm.com>
Date: Mon Jun 22 14:55:24 2015 -0500
PPC sync instruction accepts invalid and incompatible operands
ISA 2.07 added a new category called Elemental Memory Barriers that modifies
the sync instruction to accept an additional operand ESYNC. Edmar added
support for this insruction varient here:
https://sourceware.org/ml/binutils/2012-02/msg00221.html
Looking at this closer, I see that the insert_ls() function is misnamed
(since it's attached to the ESYNC operand, not the LS operand) but more
importantly, it is silently modifying the LS operand value behind the
users back when the LS operand is either invalid or is incompatible with
the new ESYNC operand. The ISA 2.07 doc has an Assembler Note that clearly
states that assemblers that support the ESYNC operand should report all
invalid uses of LS and ESYNC. This patch changes the assembler to
error out on invalid and incompatible operand usage.
opcodes/
* ppc-opc.c (insert_ls): Test for invalid LS operands.
(insert_esync): New function.
(LS, WC): Use insert_ls.
(ESYNC): Use insert_esync.
gas/testsuite/
* gas/ppc/e6500.s <sync>: Fix invalid test.
* gas/ppc/e6500.d: Likewise.
Diff:
---
gas/testsuite/ChangeLog | 4 ++++
gas/testsuite/gas/ppc/e6500.d | 2 +-
gas/testsuite/gas/ppc/e6500.s | 6 ++---
opcodes/ChangeLog | 6 +++++
opcodes/ppc-opc.c | 54 ++++++++++++++++++++++++++++++++-----------
5 files changed, 55 insertions(+), 17 deletions(-)
diff --git a/gas/testsuite/ChangeLog b/gas/testsuite/ChangeLog
index 4af0bb0..c6d27eb 100644
--- a/gas/testsuite/ChangeLog
+++ b/gas/testsuite/ChangeLog
@@ -1,6 +1,10 @@
2015-07-10 Alan Modra <amodra@gmail.com>
Apply from master
+ 2015-06-22 Peter Bergner <bergner@vnet.ibm.com>
+ * gas/ppc/e6500.s <sync>: Fix invalid test.
+ * gas/ppc/e6500.d: Likewise.
+
2015-06-19 Peter Bergner <bergner@vnet.ibm.com>
* gas/ppc/power8.d: Fixup rfebb test results.
* gas/ppc/a2.s: Fix invalid mfcr test.
diff --git a/gas/testsuite/gas/ppc/e6500.d b/gas/testsuite/gas/ppc/e6500.d
index 48b0001..c8d8f57 100644
--- a/gas/testsuite/gas/ppc/e6500.d
+++ b/gas/testsuite/gas/ppc/e6500.d
@@ -62,7 +62,7 @@ Disassembly of section \.text:
d0: (7c 00 04 ac|ac 04 00 7c) sync
d4: (7c 00 04 ac|ac 04 00 7c) sync
d8: (7c 20 04 ac|ac 04 20 7c) lwsync
- dc: (7c 00 04 ac|ac 04 00 7c) sync
+ dc: (7c 21 04 ac|ac 04 21 7c) sync 1,1
e0: (7c 07 04 ac|ac 04 07 7c) sync 0,7
e4: (7c 28 04 ac|ac 04 28 7c) sync 1,8
e8: (7c 00 00 c3|c3 00 00 7c) dni 0,0
diff --git a/gas/testsuite/gas/ppc/e6500.s b/gas/testsuite/gas/ppc/e6500.s
index ceee777..2167cc6 100644
--- a/gas/testsuite/gas/ppc/e6500.s
+++ b/gas/testsuite/gas/ppc/e6500.s
@@ -56,9 +56,9 @@ start:
sync
sync 0,0
sync 1,0
- sync 2,0
- sync 3,7
- sync 3,8
+ sync 1,1
+ sync 0,7
+ sync 1,8
dni 0,0
dni 31,31
dcblq. 2,0,1
diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog
index 64b28e2..d3a168f 100644
--- a/opcodes/ChangeLog
+++ b/opcodes/ChangeLog
@@ -1,6 +1,12 @@
2015-07-10 Alan Modra <amodra@gmail.com>
Apply from master
+ 2015-06-22 Peter Bergner <bergner@vnet.ibm.com>
+ * ppc-opc.c (insert_ls): Test for invalid LS operands.
+ (insert_esync): New function.
+ (LS, WC): Use insert_ls.
+ (ESYNC): Use insert_esync.
+
2015-06-19 Peter Bergner <bergner@vnet.ibm.com>
* ppc-dis.h (skip_optional_operands): Use ppc_optional_operand_value.
* ppc-opc.c (FXM4): Add non-zero optional value.
diff --git a/opcodes/ppc-opc.c b/opcodes/ppc-opc.c
index 40c73ad..a6e2c14 100644
--- a/opcodes/ppc-opc.c
+++ b/opcodes/ppc-opc.c
@@ -53,6 +53,7 @@ static unsigned long insert_bo (unsigned long, long, ppc_cpu_t, const char **);
static long extract_bo (unsigned long, ppc_cpu_t, int *);
static unsigned long insert_boe (unsigned long, long, ppc_cpu_t, const char **);
static long extract_boe (unsigned long, ppc_cpu_t, int *);
+static unsigned long insert_esync (unsigned long, long, ppc_cpu_t, const char **);
static unsigned long insert_fxm (unsigned long, long, ppc_cpu_t, const char **);
static long extract_fxm (unsigned long, ppc_cpu_t, int *);
static unsigned long insert_li20 (unsigned long, long, ppc_cpu_t, const char **);
@@ -417,7 +418,7 @@ const struct powerpc_operand powerpc_operands[] =
/* The LS or WC field in an X (sync or wait) form instruction. */
#define LS LIA + 1
#define WC LS
- { 0x3, 21, NULL, NULL, PPC_OPERAND_OPTIONAL },
+ { 0x3, 21, insert_ls, NULL, PPC_OPERAND_OPTIONAL },
/* The ME field in an M form instruction. */
#define ME LS + 1
@@ -635,7 +636,7 @@ const struct powerpc_operand powerpc_operands[] =
/* The ESYNC field in an X (sync) form instruction. */
#define ESYNC STRM + 1
- { 0xf, 16, insert_ls, NULL, PPC_OPERAND_OPTIONAL },
+ { 0xf, 16, insert_esync, NULL, PPC_OPERAND_OPTIONAL },
/* The SV field in a POWER SC form instruction. */
#define SV ESYNC + 1
@@ -1365,17 +1366,40 @@ extract_li20 (unsigned long insn,
| (insn & 0x7ff);
}
-/* The LS field in a sync instruction that accepts 2 operands
- Values 2 and 3 are reserved,
- must be treated as 0 for future compatibility
- Values 0 and 1 can be accepted, if field ESYNC is zero
- Otherwise L = complement of ESYNC-bit2 (1<<18) */
+/* The 2-bit L field in a SYNC or WC field in a WAIT instruction.
+ For SYNC, some L values are reserved:
+ * Value 3 is reserved on newer server cpus.
+ * Values 2 and 3 are reserved on all other cpus. */
static unsigned long
insert_ls (unsigned long insn,
long value,
- ppc_cpu_t dialect ATTRIBUTE_UNUSED,
- const char **errmsg ATTRIBUTE_UNUSED)
+ ppc_cpu_t dialect,
+ const char **errmsg)
+{
+ /* For SYNC, some L values are illegal. */
+ if (((insn >> 1) & 0x3ff) == 598)
+ {
+ long max_lvalue = (dialect & PPC_OPCODE_POWER4) ? 2 : 1;
+ if (value > max_lvalue)
+ {
+ *errmsg = _("illegal L operand value");
+ return insn;
+ }
+ }
+
+ return insn | ((value & 0x3) << 21);
+}
+
+/* The 4-bit E field in a sync instruction that accepts 2 operands.
+ If ESYNC is non-zero, then the L field must be either 0 or 1 and
+ the complement of ESYNC-bit2. */
+
+static unsigned long
+insert_esync (unsigned long insn,
+ long value,
+ ppc_cpu_t dialect ATTRIBUTE_UNUSED,
+ const char **errmsg)
{
unsigned long ls;
@@ -1383,12 +1407,15 @@ insert_ls (unsigned long insn,
if (value == 0)
{
if (ls > 1)
- return insn & ~(0x3 << 21);
+ *errmsg = _("illegal L operand value");
return insn;
}
- if ((value & 0x2) != 0)
- return (insn & ~(0x3 << 21)) | ((value & 0xf) << 16);
- return (insn & ~(0x3 << 21)) | (0x1 << 21) | ((value & 0xf) << 16);
+
+ if ((ls & ~0x1)
+ || (((value >> 1) & 0x1) ^ ls) == 0)
+ *errmsg = _("incompatible L operand value");
+
+ return insn | ((value & 0xf) << 16);
}
/* The MB and ME fields in an M form instruction expressed as a single
@@ -2024,6 +2051,7 @@ extract_dm (unsigned long insn,
*invalid = 1;
return (value) ? 1 : 0;
}
+
/* The VLESIMM field in an I16A form instruction. This is split. */
static unsigned long