[PATCH 2/6] S/390: Fix disassembler's treatment of signed/unsigned operands

[Andreas Arnez <arnez at linux dot vnet dot ibm dot com>]

Currently the S/390 disassembler relies on implementation-defined
behavior upon integer overflow and sometimes uses the "%i" printf
format for unsigned values.  This patch intends to fix these.

opcodes/
	* s390-dis.c (union operand_value): New.
	(s390_extract_operand): Change return type to union operand_value.
	Also avoid integer overflow in sign-extension.
	(s390_print_insn_with_opcode): Adjust to changed return value from
	s390_extract_operand().  Change "%i" printf format to "%u" for
	unsigned values.
---
 opcodes/s390-dis.c | 55 ++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/opcodes/s390-dis.c b/opcodes/s390-dis.c
index 444e7ae..47c449a 100644
--- a/opcodes/s390-dis.c
+++ b/opcodes/s390-dis.c
@@ -92,16 +92,23 @@ s390_insn_matches_opcode (const bfd_byte *buffer,
     && (buffer[5] & opcode->mask[5]) == opcode->opcode[5];
 }
 
+union operand_value
+{
+  int i;
+  unsigned int u;
+};
+
 /* Extracts an operand value from an instruction.  */
 /* We do not perform the shift operation for larl-type address
    operands here since that would lead to an overflow of the 32 bit
    integer value.  Instead the shift operation is done when printing
    the operand.  */
 
-static inline unsigned int
+static inline union operand_value
 s390_extract_operand (const bfd_byte *insn,
 		      const struct s390_operand *operand)
 {
+  union operand_value ret;
   unsigned int val;
   int bits;
 
@@ -123,15 +130,24 @@ s390_extract_operand (const bfd_byte *insn,
   if (operand->bits == 20 && operand->shift == 20)
     val = (val & 0xff) << 12 | (val & 0xfff00) >> 8;
 
-  /* Sign extend value if the operand is signed or pc relative.  */
-  if ((operand->flags & (S390_OPERAND_SIGNED | S390_OPERAND_PCREL))
-      && (val & (1U << (operand->bits - 1))))
-    val |= (-1U << (operand->bits - 1)) << 1;
+  /* Sign extend value if the operand is signed or pc relative.  Avoid
+     integer overflows.  */
+  if (operand->flags & (S390_OPERAND_SIGNED | S390_OPERAND_PCREL))
+    {
+      unsigned int m = 1U << (operand->bits - 1);
+
+      if (val >= m)
+	ret.i = (int) (val - m) - 1 - (int) (m - 1U);
+      else
+	ret.i = (int) val;
+    }
+  else if (operand->flags & S390_OPERAND_LENGTH)
+    /* Length x in an instruction has real length x + 1.  */
+    ret.u = val + 1;
+  else
+    ret.u = val;
 
-  /* Length x in an instructions has real length x + 1.  */
-  if (operand->flags & S390_OPERAND_LENGTH)
-    val++;
-  return val;
+  return ret;
 }
 
 /* Print the S390 instruction in BUFFER, assuming that it matches the
@@ -156,12 +172,12 @@ s390_print_insn_with_opcode (bfd_vma memaddr,
   for (opindex = opcode->operands; *opindex != 0; opindex++)
     {
       const struct s390_operand *operand = s390_operands + *opindex;
-      unsigned int value = s390_extract_operand (buffer, operand);
+      union operand_value val = s390_extract_operand (buffer, operand);
 
-      if ((operand->flags & S390_OPERAND_INDEX) && value == 0)
+      if ((operand->flags & S390_OPERAND_INDEX) && val.u == 0)
 	continue;
       if ((operand->flags & S390_OPERAND_BASE) &&
-	  value == 0 && separator == '(')
+	  val.u == 0 && separator == '(')
 	{
 	  separator = ',';
 	  continue;
@@ -171,20 +187,19 @@ s390_print_insn_with_opcode (bfd_vma memaddr,
 	(*info->fprintf_func) (info->stream, "%c", separator);
 
       if (operand->flags & S390_OPERAND_GPR)
-	(*info->fprintf_func) (info->stream, "%%r%i", value);
+	(*info->fprintf_func) (info->stream, "%%r%u", val.u);
       else if (operand->flags & S390_OPERAND_FPR)
-	(*info->fprintf_func) (info->stream, "%%f%i", value);
+	(*info->fprintf_func) (info->stream, "%%f%u", val.u);
       else if (operand->flags & S390_OPERAND_AR)
-	(*info->fprintf_func) (info->stream, "%%a%i", value);
+	(*info->fprintf_func) (info->stream, "%%a%u", val.u);
       else if (operand->flags & S390_OPERAND_CR)
-	(*info->fprintf_func) (info->stream, "%%c%i", value);
+	(*info->fprintf_func) (info->stream, "%%c%u", val.u);
       else if (operand->flags & S390_OPERAND_PCREL)
-	(*info->print_address_func) (memaddr + (int)value + (int)value,
-				     info);
+	(*info->print_address_func) (memaddr + val.i + val.i, info);
       else if (operand->flags & S390_OPERAND_SIGNED)
-	(*info->fprintf_func) (info->stream, "%i", (int) value);
+	(*info->fprintf_func) (info->stream, "%i", val.i);
       else
-	(*info->fprintf_func) (info->stream, "%u", value);
+	(*info->fprintf_func) (info->stream, "%u", val.u);
 
       if (operand->flags & S390_OPERAND_DISP)
 	{
-- 
1.8.4.2