From 2fd2b153a3819d3ab6b9c4cf06943d498187714c Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Tue, 10 Dec 2019 22:32:06 +1030 Subject: [PATCH] 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. --- opcodes/ChangeLog | 10 ++++++++++ opcodes/bfin-dis.c | 29 ++++++++++++++++------------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog index d3f1e699406..faa160a37b5 100644 --- a/opcodes/ChangeLog +++ b/opcodes/ChangeLog @@ -1,3 +1,13 @@ +2019-12-11 Alan Modra + + * 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 * arc-dis.c (find_format_from_table): Use ull constant when diff --git a/opcodes/bfin-dis.c b/opcodes/bfin-dis.c index 811509fa1ab..711f7e1e07a 100644 --- 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) -- 2.30.2