ubsan: bfin: shift exponent is too large

This was the following in fmtconst_val, x is unsigned int.
    x = SIGNEXTEND (x, constant_formats[cf].nbits);
Problem is, the SIGNEXTEND macro assumed its arg was a long and sign
extended by shifting left then shifting right, and didn't cast the
arg.  So don't do the silly shift thing.  It's not guaranteed to work
anyway according to the C standard.  ">>" might do a logical shift
even if its args are signed.

	* bfin-dis.c (HOST_LONG_WORD_SIZE, XFIELD): Delete.
	(SIGNBIT): New.
	(MASKBITS, SIGNEXTEND): Rewrite.
	(fmtconst): Don't use ? expression now that SIGNEXTEND uses
	unsigned arithmetic, instead assign result of SIGNEXTEND back
	to x.
	(fmtconst_val): Use 1u in shift expression.

diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog
index d3f1e69940668d92601a1eeb5fa39accd8457519..faa160a37b54eedfd8da44f3e10bb2200a88a7f2 100644 (file)
--- a/opcodes/ChangeLog
+++ b/opcodes/ChangeLog
@@ -1,3 +1,13 @@
+2019-12-11  Alan Modra  <amodra@gmail.com>
+
+       * bfin-dis.c (HOST_LONG_WORD_SIZE, XFIELD): Delete.
+       (SIGNBIT): New.
+       (MASKBITS, SIGNEXTEND): Rewrite.
+       (fmtconst): Don't use ? expression now that SIGNEXTEND uses
+       unsigned arithmetic, instead assign result of SIGNEXTEND back
+       to x.
+       (fmtconst_val): Use 1u in shift expression.
+
 2019-12-11  Alan Modra  <amodra@gmail.com>
 
        * arc-dis.c (find_format_from_table): Use ull constant when

diff --git a/opcodes/bfin-dis.c b/opcodes/bfin-dis.c
index 811509fa1abb180d5be4cb1bc6192e93b42260be..711f7e1e07a5cd9e61c01005e7a2162d3594676c 100644 (file)
--- a/opcodes/bfin-dis.c
+++ b/opcodes/bfin-dis.c
@@ -33,10 +33,9 @@
 
 typedef long TIword;
 
-#define HOST_LONG_WORD_SIZE (sizeof (long) * 8)
-#define XFIELD(w,p,s)       (((w) & ((1 << (s)) - 1) << (p)) >> (p))
-#define SIGNEXTEND(v, n)    ((v << (HOST_LONG_WORD_SIZE - (n))) >> (HOST_LONG_WORD_SIZE - (n)))
-#define MASKBITS(val, bits) (val & ((1 << bits) - 1))
+#define SIGNBIT(bits)       (1ul << ((bits) - 1))
+#define MASKBITS(val, bits) ((val) & ((1ul << (bits)) - 1))
+#define SIGNEXTEND(v, n)    ((MASKBITS (v, n) ^ SIGNBIT (n)) - SIGNBIT (n))
 
 #include "disassemble.h"
 
@@ -125,8 +124,11 @@ fmtconst (const_forms_t cf, TIword x, bfd_vma pc, disassemble_info *outf)
 
   if (constant_formats[cf].reloc)
     {
-      bfd_vma ea = (((constant_formats[cf].pcrel ? SIGNEXTEND (x, constant_formats[cf].nbits)
-                     : x) + constant_formats[cf].offset) << constant_formats[cf].scale);
+      bfd_vma ea;
+
+      if (constant_formats[cf].pcrel)
+       x = SIGNEXTEND (x, constant_formats[cf].nbits);
+      ea = (x + constant_formats[cf].offset) << constant_formats[cf].scale;
       if (constant_formats[cf].pcrel)
        ea += pc;
 
@@ -153,8 +155,8 @@ fmtconst (const_forms_t cf, TIword x, bfd_vma pc, disassemble_info *outf)
       x = x | (1 << constant_formats[cf].nbits);
       x = SIGNEXTEND (x, nb);
     }
-  else
-    x = constant_formats[cf].issigned ? SIGNEXTEND (x, constant_formats[cf].nbits) : x;
+  else if (constant_formats[cf].issigned)
+    x = SIGNEXTEND (x, constant_formats[cf].nbits);
 
   if (constant_formats[cf].offset)
     x += constant_formats[cf].offset;
@@ -180,10 +182,11 @@ fmtconst_val (const_forms_t cf, unsigned int x, unsigned int pc)
 {
   if (0 && constant_formats[cf].reloc)
     {
-      bu32 ea = (((constant_formats[cf].pcrel
-                  ? SIGNEXTEND (x, constant_formats[cf].nbits)
-                  : x) + constant_formats[cf].offset)
-                << constant_formats[cf].scale);
+      bu32 ea;
+
+      if (constant_formats[cf].pcrel)
+       x = SIGNEXTEND (x, constant_formats[cf].nbits);
+      ea = (x + constant_formats[cf].offset) << constant_formats[cf].scale;
       if (constant_formats[cf].pcrel)
        ea += pc;
 
@@ -194,7 +197,7 @@ fmtconst_val (const_forms_t cf, unsigned int x, unsigned int pc)
   if (constant_formats[cf].negative)
     {
       int nb = constant_formats[cf].nbits + 1;
-      x = x | (1 << constant_formats[cf].nbits);
+      x = x | (1u << constant_formats[cf].nbits);
       x = SIGNEXTEND (x, nb);
     }
   else if (constant_formats[cf].issigned)