diff mbox series

[for-2.9,3/6] disas/m68k: Avoid unintended sign extension in get_field()

Message ID 1488556233-31246-4-git-send-email-peter.maydell@linaro.org
State Accepted
Commit 2e3883d03df167b15f2acc5345eb9a7e0150a062
Headers show
Series disas: Fix various coverity nits | expand

Commit Message

Peter Maydell March 3, 2017, 3:50 p.m. UTC
In get_field(), we take an 'unsigned char' value and shift it left,
which implicitly promotes it to 'signed int', before ORing it into an
'unsigned long' type.  If 'unsigned long' is 64 bits then this will
result in a sign extension and the top 32 bits of the result will be
1s.  Add explicit casts to unsigned long before shifting to prevent
this.

(Spotted by Coverity, CID 715697.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 disas/m68k.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.7.4

Comments

Laurent Vivier March 3, 2017, 6:56 p.m. UTC | #1
Le 03/03/2017 à 16:50, Peter Maydell a écrit :
> In get_field(), we take an 'unsigned char' value and shift it left,

> which implicitly promotes it to 'signed int', before ORing it into an

> 'unsigned long' type.  If 'unsigned long' is 64 bits then this will

> result in a sign extension and the top 32 bits of the result will be

> 1s.  Add explicit casts to unsigned long before shifting to prevent

> this.

> 

> (Spotted by Coverity, CID 715697.)

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Reviewed-by: Laurent Vivier <laurent@vivier.eu>


> ---

>  disas/m68k.c | 5 +++--

>  1 file changed, 3 insertions(+), 2 deletions(-)

> 

> diff --git a/disas/m68k.c b/disas/m68k.c

> index 073abb9..61b689e 100644

> --- a/disas/m68k.c

> +++ b/disas/m68k.c

> @@ -4685,10 +4685,11 @@ get_field (const unsigned char *data, enum floatformat_byteorders order,

>  	/* This is the last byte; zero out the bits which are not part of

>  	   this field.  */

>  	result |=

> -	  (*(data + cur_byte) & ((1 << (len - cur_bitshift)) - 1))

> +	  (unsigned long)(*(data + cur_byte)

> +			  & ((1 << (len - cur_bitshift)) - 1))

>  	    << cur_bitshift;

>        else

> -	result |= *(data + cur_byte) << cur_bitshift;

> +	result |= (unsigned long)*(data + cur_byte) << cur_bitshift;

>        cur_bitshift += FLOATFORMAT_CHAR_BIT;

>        if (order == floatformat_little)

>  	++cur_byte;

>
diff mbox series

Patch

diff --git a/disas/m68k.c b/disas/m68k.c
index 073abb9..61b689e 100644
--- a/disas/m68k.c
+++ b/disas/m68k.c
@@ -4685,10 +4685,11 @@  get_field (const unsigned char *data, enum floatformat_byteorders order,
 	/* This is the last byte; zero out the bits which are not part of
 	   this field.  */
 	result |=
-	  (*(data + cur_byte) & ((1 << (len - cur_bitshift)) - 1))
+	  (unsigned long)(*(data + cur_byte)
+			  & ((1 << (len - cur_bitshift)) - 1))
 	    << cur_bitshift;
       else
-	result |= *(data + cur_byte) << cur_bitshift;
+	result |= (unsigned long)*(data + cur_byte) << cur_bitshift;
       cur_bitshift += FLOATFORMAT_CHAR_BIT;
       if (order == floatformat_little)
 	++cur_byte;