More signed overflow fixes
authorAlan Modra <amodra@gmail.com>
Wed, 18 Dec 2019 05:07:44 +0000 (15:37 +1030)
committerAlan Modra <amodra@gmail.com>
Wed, 18 Dec 2019 08:08:13 +0000 (18:38 +1030)
The arc fix in create_map avoiding signed overflow by casting an
unsigned char to unsigned int before shifting, shows one of the
dangers of blinding doing that.  The problem in this case was that the
variable storing the value, newAuxRegister->address, was a long.
Using the unsigned cast meant that the 32-bit value was zero extended
when long is 64 bits.  Previously we had a sign extension.  Net result
was that comparisons in arcExtMap_auxRegName didn't match.  Of course,
I could have cast the 32-bit unsigned value back to signed before
storing in a long, but it's neater to just use an unsigned int for the
address.

opcodes/
* alpha-opc.c (OP): Avoid signed overflow.
* arm-dis.c (print_insn): Likewise.
* mcore-dis.c (print_insn_mcore): Likewise.
* pj-dis.c (get_int): Likewise.
* ppc-opc.c (EBD15, EBD15BI): Likewise.
* score7-dis.c (s7_print_insn): Likewise.
* tic30-dis.c (print_insn_tic30): Likewise.
* v850-opc.c (insert_SELID): Likewise.
* vax-dis.c (print_insn_vax): Likewise.
* arc-ext.c (create_map): Likewise.
(struct ExtAuxRegister): Make "address" field unsigned int.
(arcExtMap_auxRegName): Pass unsigned address.
(dump_ARC_extmap): Adjust.
* arc-ext.h (arcExtMap_auxRegName): Update prototype.

12 files changed:
opcodes/ChangeLog
opcodes/alpha-opc.c
opcodes/arc-ext.c
opcodes/arc-ext.h
opcodes/arm-dis.c
opcodes/mcore-dis.c
opcodes/pj-dis.c
opcodes/ppc-opc.c
opcodes/score7-dis.c
opcodes/tic30-dis.c
opcodes/v850-opc.c
opcodes/vax-dis.c

index aac14ea6a086ba8812b7f81f1e5ec6b14b81661d..96993a3ee68c40d03d6feb387549c8febec784ee 100644 (file)
@@ -1,3 +1,20 @@
+2019-12-18  Alan Modra  <amodra@gmail.com>
+
+       * alpha-opc.c (OP): Avoid signed overflow.
+       * arm-dis.c (print_insn): Likewise.
+       * mcore-dis.c (print_insn_mcore): Likewise.
+       * pj-dis.c (get_int): Likewise.
+       * ppc-opc.c (EBD15, EBD15BI): Likewise.
+       * score7-dis.c (s7_print_insn): Likewise.
+       * tic30-dis.c (print_insn_tic30): Likewise.
+       * v850-opc.c (insert_SELID): Likewise.
+       * vax-dis.c (print_insn_vax): Likewise.
+       * arc-ext.c (create_map): Likewise.
+       (struct ExtAuxRegister): Make "address" field unsigned int.
+       (arcExtMap_auxRegName): Pass unsigned address.
+       (dump_ARC_extmap): Adjust.
+       * arc-ext.h (arcExtMap_auxRegName): Update prototype.
+
 2019-12-17  Alan Modra  <amodra@gmail.com>
 
        * visium-dis.c (print_insn_visium): Avoid signed overflow.
index 02b92235860d682571f85674d01b83dd140f986e..94a023cdfe86522e76e7d31dd5b76d1753558204 100644 (file)
@@ -332,7 +332,7 @@ const unsigned alpha_num_operands = sizeof(alpha_operands)/sizeof(*alpha_operand
 /* Macros used to form opcodes.  */
 
 /* The main opcode.  */
-#define OP(x)          (((x) & 0x3F) << 26)
+#define OP(x)          (((x) & 0x3Fu) << 26)
 #define OP_MASK                0xFC000000
 
 /* Branch format instructions.  */
index cfb13aa0e1d3a2b8e234f5bc1d1f28984784dd11..bc676687914ab2f4833167458e67aa57d44114c5 100644 (file)
@@ -53,7 +53,7 @@
 
 struct ExtAuxRegister
 {
-  long                   address;
+  unsigned               address;
   char *                 name;
   struct ExtAuxRegister * next;
 };
@@ -191,8 +191,8 @@ create_map (unsigned char *block,
            char *aux_name = xstrdup ((char *) (p + 6));
 
            newAuxRegister->name = aux_name;
-           newAuxRegister->address = (p[2] << 24) | (p[3] << 16)
-             | (p[4] << 8) | p[5];
+           newAuxRegister->address = (((unsigned) p[2] << 24) | (p[3] << 16)
+                                      | (p[4] << 8) | p[5]);
            newAuxRegister->next = arc_extension_map.auxRegisters;
            arc_extension_map.auxRegisters = newAuxRegister;
            break;
@@ -406,7 +406,7 @@ arcExtMap_condCodeName (int code)
 /* Get the name of an extension auxiliary register.  */
 
 const char *
-arcExtMap_auxRegName (long address)
+arcExtMap_auxRegName (unsigned address)
 {
   /* Walk the list of auxiliary register names and find the name.  */
   struct ExtAuxRegister *r;
@@ -463,7 +463,7 @@ dump_ARC_extmap (void)
 
     while (r)
     {
-       printf ("AUX : %s %ld\n", r->name, r->address);
+       printf ("AUX : %s %u\n", r->name, r->address);
        r = r->next;
     }
 
index e18e568c7a2942fb1c394e93afa00fecf8e2694d..4127d1873611a09bcd874e694ab40bb2922aa14a 100644 (file)
@@ -125,7 +125,7 @@ extern void build_ARC_extmap (bfd *);
 /* Accessor functions.  */
 extern enum ExtReadWrite arcExtMap_coreReadWrite (int);
 extern const char * arcExtMap_coreRegName (int);
-extern const char * arcExtMap_auxRegName (long);
+extern const char * arcExtMap_auxRegName (unsigned);
 extern const char * arcExtMap_condCodeName (int);
 extern const extInstruction_t *arcExtMap_insn (int, unsigned long long);
 extern struct arc_opcode *arcExtMap_genOpcode (const extInstruction_t *,
index f6937584e8bc57501e5cd0dca147f05b8e53429e..12eae61bb5957e67375b52b32d10de927289a08c 100644 (file)
@@ -11705,7 +11705,7 @@ static int
 print_insn (bfd_vma pc, struct disassemble_info *info, bfd_boolean little)
 {
   unsigned char b[4];
-  long         given;
+  unsigned long given;
   int           status;
   int           is_thumb = FALSE;
   int           is_data = FALSE;
@@ -11885,9 +11885,9 @@ print_insn (bfd_vma pc, struct disassemble_info *info, bfd_boolean little)
 
       status = info->read_memory_func (pc, (bfd_byte *) b, 4, info);
       if (little_code)
-       given = (b[0]) | (b[1] << 8) | (b[2] << 16) | (b[3] << 24);
+       given = (b[0]) | (b[1] << 8) | (b[2] << 16) | ((unsigned) b[3] << 24);
       else
-       given = (b[3]) | (b[2] << 8) | (b[1] << 16) | (b[0] << 24);
+       given = (b[3]) | (b[2] << 8) | (b[1] << 16) | ((unsigned) b[0] << 24);
     }
   else
     {
index 5c0eb084f79f3d59dc76f6757e45dd4b0d88774f..5b3acb80e2804c18417af78d71c593afd534e0c7 100644 (file)
@@ -196,18 +196,14 @@ print_insn_mcore (bfd_vma memaddr,
 
        case BR:
          {
-           long val = inst & 0x3FF;
+           uint32_t val = ((inst & 0x3FF) ^ 0x400) - 0x400;
 
-           if (inst & 0x400)
-             val |= 0xFFFFFC00;
-
-           (*print_func) (stream, "\t0x%lx", (long)(memaddr + 2 + (val << 1)));
+           val = memaddr + 2 + (val << 1);
+           (*print_func) (stream, "\t0x%x", val);
 
            if (strcmp (mcore_table[i].name, "bsr") == 0)
              {
                /* For bsr, we'll try to get a symbol for the target.  */
-               val = memaddr + 2 + (val << 1);
-
                if (info->print_address_func && val != 0)
                  {
                    (*print_func) (stream, "\t// ");
@@ -219,19 +215,18 @@ print_insn_mcore (bfd_vma memaddr,
 
        case BL:
          {
-           long val;
-           val = (inst & 0x000F);
-           (*print_func) (stream, "\t%s, 0x%lx",
+           uint32_t val = inst & 0x000F;
+           (*print_func) (stream, "\t%s, 0x%x",
                           grname[(inst >> 4) & 0xF],
-                          (long) (memaddr - (val << 1)));
+                          (uint32_t) (memaddr - (val << 1)));
          }
          break;
 
        case LR:
          {
-           unsigned long val;
+           uint32_t val;
 
-           val = (memaddr + 2 + ((inst & 0xFF) << 2)) & 0xFFFFFFFC;
+           val = (memaddr + 2 + ((inst & 0xFF) << 2)) & ~3;
 
            /* We are not reading an instruction, so allow
               reads to extend beyond the next symbol.  */
@@ -244,27 +239,27 @@ print_insn_mcore (bfd_vma memaddr,
              }
 
            if (info->endian == BFD_ENDIAN_LITTLE)
-             val = (ibytes[3] << 24) | (ibytes[2] << 16)
-               | (ibytes[1] << 8) | (ibytes[0]);
+             val = (((unsigned) ibytes[3] << 24) | (ibytes[2] << 16)
+                    | (ibytes[1] << 8) | (ibytes[0]));
            else
-             val = (ibytes[0] << 24) | (ibytes[1] << 16)
-               | (ibytes[2] << 8) | (ibytes[3]);
+             val = (((unsigned) ibytes[0] << 24) | (ibytes[1] << 16)
+                    | (ibytes[2] << 8) | (ibytes[3]));
 
            /* Removed [] around literal value to match ABI syntax 12/95.  */
-           (*print_func) (stream, "\t%s, 0x%lX", grname[(inst >> 8) & 0xF], val);
+           (*print_func) (stream, "\t%s, 0x%X", grname[(inst >> 8) & 0xF], val);
 
            if (val == 0)
-             (*print_func) (stream, "\t// from address pool at 0x%lx",
-                            (long) (memaddr + 2
-                                    + ((inst & 0xFF) << 2)) & 0xFFFFFFFC);
+             (*print_func) (stream, "\t// from address pool at 0x%x",
+                            (uint32_t) (memaddr + 2
+                                        + ((inst & 0xFF) << 2)) & ~3);
          }
          break;
 
        case LJ:
          {
-           unsigned long val;
+           uint32_t val;
 
-           val = (memaddr + 2 + ((inst & 0xFF) << 2)) & 0xFFFFFFFC;
+           val = (memaddr + 2 + ((inst & 0xFF) << 2)) & ~3;
 
            /* We are not reading an instruction, so allow
               reads to extend beyond the next symbol.  */
@@ -277,14 +272,14 @@ print_insn_mcore (bfd_vma memaddr,
              }
 
            if (info->endian == BFD_ENDIAN_LITTLE)
-             val = (ibytes[3] << 24) | (ibytes[2] << 16)
-               | (ibytes[1] << 8) | (ibytes[0]);
+             val = (((unsigned) ibytes[3] << 24) | (ibytes[2] << 16)
+                    | (ibytes[1] << 8) | (ibytes[0]));
            else
-             val = (ibytes[0] << 24) | (ibytes[1] << 16)
-               | (ibytes[2] << 8) | (ibytes[3]);
+             val = (((unsigned) ibytes[0] << 24) | (ibytes[1] << 16)
+                    | (ibytes[2] << 8) | (ibytes[3]));
 
            /* Removed [] around literal value to match ABI syntax 12/95.  */
-           (*print_func) (stream, "\t0x%lX", val);
+           (*print_func) (stream, "\t0x%X", val);
            /* For jmpi/jsri, we'll try to get a symbol for the target.  */
            if (info->print_address_func && val != 0)
              {
@@ -293,9 +288,9 @@ print_insn_mcore (bfd_vma memaddr,
              }
            else
              {
-               (*print_func) (stream, "\t// from address pool at 0x%lx",
-                              (long) (memaddr + 2
-                                      + ((inst & 0xFF) << 2)) & 0xFFFFFFFC);
+               (*print_func) (stream, "\t// from address pool at 0x%x",
+                              (uint32_t) (memaddr + 2
+                                          + ((inst & 0xFF) << 2)) & ~3);
              }
          }
          break;
index eb8cdf93c1d275f1c071dd70fee9477024d03958..62f2a2f670c955a51ef19578e9cab7b1151c4e2a 100644 (file)
@@ -32,10 +32,10 @@ get_int (bfd_vma memaddr, int *iptr, struct disassemble_info *info)
   unsigned char ival[4];
   int status = info->read_memory_func (memaddr, ival, 4, info);
 
-  *iptr = (ival[0] << 24)
-    | (ival[1] << 16)
-    | (ival[2] << 8)
-    | (ival[3] << 0);
+  *iptr = (((unsigned) ival[0] << 24)
+          | (ival[1] << 16)
+          | (ival[2] << 8)
+          | (ival[3] << 0));
 
   return status;
 }
index b56fe3e21a9c28d409477a63b7f76139368a42b0..403c9daff53bcfa33bf8ebffcf0afe8bd4f1439a 100644 (file)
@@ -2967,7 +2967,7 @@ const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
 
 /* A BD15 form instruction for extended conditional branch mnemonics.  */
 #define EBD15(op, aa, bo, lk)                  \
-  (((op) & 0x3f) << 26)                                \
+  (((op) & 0x3fu) << 26)                       \
   | (((aa) & 0xf) << 22)                       \
   | (((bo) & 0x3) << 20)                       \
   | ((lk) & 1)
@@ -2976,7 +2976,7 @@ const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
 /* A BD15 form instruction for extended conditional branch mnemonics
    with BI.  */
 #define EBD15BI(op, aa, bo, bi, lk)            \
-  ((((op) & 0x3f) << 26)                       \
+  ((((op) & 0x3fu) << 26)                      \
    | (((aa) & 0xf) << 22)                      \
    | (((bo) & 0x3) << 20)                      \
    | (((bi) & 0x3) << 16)                      \
index 5d74c8bf051dcef4ea3e864b573ad148e90713f1..2dbc6674cc407c4356dfdac27b738d5024081fd2 100644 (file)
@@ -871,7 +871,7 @@ int
 s7_print_insn (bfd_vma pc, struct disassemble_info *info, bfd_boolean little)
 {
   unsigned char b[4];
-  long given;
+  unsigned long given;
   long ridparity;
   int status;
   bfd_boolean insn_pce_p = FALSE;
@@ -907,11 +907,11 @@ s7_print_insn (bfd_vma pc, struct disassemble_info *info, bfd_boolean little)
 
   if (little)
     {
-      given = (b[0]) | (b[1] << 8) | (b[2] << 16) | (b[3] << 24);
+      given = (b[0]) | (b[1] << 8) | (b[2] << 16) | ((unsigned) b[3] << 24);
     }
   else
     {
-      given = (b[0] << 24) | (b[1] << 16) | (b[2] << 8) | (b[3]);
+      given = ((unsigned) b[0] << 24) | (b[1] << 16) | (b[2] << 8) | (b[3]);
     }
 
   if ((given & 0x80008000) == 0x80008000)
index 29948f40196f029dccc62e2668a4718f01d06023..a695159d77c5142dfa68cde83c923c21f1677fa1 100644 (file)
@@ -696,8 +696,10 @@ print_insn_tic30 (bfd_vma pc, disassemble_info *info)
   bfd_vma bufaddr = pc - info->buffer_vma;
 
   /* Obtain the current instruction word from the buffer.  */
-  insn_word = (*(info->buffer + bufaddr) << 24) | (*(info->buffer + bufaddr + 1) << 16) |
-    (*(info->buffer + bufaddr + 2) << 8) | *(info->buffer + bufaddr + 3);
+  insn_word = (((unsigned) *(info->buffer + bufaddr) << 24)
+              | (*(info->buffer + bufaddr + 1) << 16)
+              | (*(info->buffer + bufaddr + 2) << 8)
+              | *(info->buffer + bufaddr + 3));
   _pc = pc / 4;
   /* Get the instruction refered to by the current instruction word
      and print it out based on its type.  */
index b02e22aa9e358b93b335b9e0a8ceb1dbb5ee2f78..dcb4a3bbedbea70978c4bf73f972d0c97a912506 100644 (file)
@@ -693,14 +693,10 @@ extract_WIDTH_L (unsigned long insn, int * invalid)
 static unsigned long
 insert_SELID (unsigned long insn, long selid, const char ** errmsg)
 {
-  unsigned long ret;
-
-  if (selid > 0x1f || selid < 0)
+  if ((unsigned long) selid > 0x1f)
     * errmsg = _(selid_out_of_range);
 
-  ret = (insn | ((selid & 0x1f) << 27));
-
-  return ret;
+  return insn | ((selid & 0x1fUL) << 27);
 }
 
 static unsigned long
index 3bdfa151920434131b2dcc0abe241ef943212ee9..0b331412d39952c590f8c8c8a62e28c62a73714f 100644 (file)
@@ -440,7 +440,8 @@ print_insn_vax (bfd_vma memaddr, disassemble_info *info)
       int offset;
 
       FETCH_DATA (info, buffer + 4);
-      offset = buffer[3] << 24 | buffer[2] << 16 | buffer[1] << 8 | buffer[0];
+      offset = ((unsigned) buffer[3] << 24 | buffer[2] << 16
+               | buffer[1] << 8 | buffer[0]);
       (*info->fprintf_func) (info->stream, ".long 0x%08x", offset);
 
       return 4;