Home Original page

[PATCH] Fix undefined behavior breaking clang-compiled as

Richard Earnshaw rearnsha@arm.com
Thu Nov 20 17:06:00 GMT 2014 
On 19/11/14 23:11, Joerg Sonnenberger wrote:
> Hi all,
> can someone please commit one of the attached patchies. The macro
> currently triggers undefined behavior for n == 0, since the right shift
> by 32 is not defined. Clang does effectively remove the first iteration
> based on that and as a result, "cmp r0, #99" fails to assemble.
> 

Hmm, yes.  It's amazing how long some bugs can persist without being
noticed.

Anyway, I've pushed a fix, but for perversity's sake it's slightly
different from the two you proposed :-)

2014-11-20  Richard Earnshaw  <rearnsha@arm.com>

	* config/tc-arm.c (rotate_left): Avoid undefined behaviour when
	N = 0.

R.

> Regards,
> Joerg
> 
> 
> tc-arm.c.diff
> 
> 
> Index: tc-arm.c
> ===================================================================
> RCS file: /home/joerg/repo/netbsd/src/external/gpl3/binutils/dist/gas/config/tc-arm.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 tc-arm.c
> --- tc-arm.c	29 Sep 2013 14:03:30 -0000	1.6
> +++ tc-arm.c	19 Nov 2014 22:33:03 -0000
> @@ -6936,7 +6936,7 @@ parse_operands (char *str, const unsigne
>  
>  /* Functions for operand encoding.  ARM, then Thumb.  */
>  
> -#define rotate_left(v, n) (v << n | v >> (32 - n))
> +#define rotate_left(v, n) (v << (n % 32) | v >> ((32 - n) % 32))
>  
>  /* If VAL can be encoded in the immediate field of an ARM instruction,
>     return the encoded form.  Otherwise, return FAIL.  */
> 
> 
> tc-arm.c-2.diff
> 
> 
> Index: tc-arm.c
> ===================================================================
> RCS file: /home/joerg/repo/netbsd/src/external/gpl3/binutils/dist/gas/config/tc-arm.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 tc-arm.c
> --- tc-arm.c	29 Sep 2013 14:03:30 -0000	1.6
> +++ tc-arm.c	19 Nov 2014 23:09:33 -0000
> @@ -6936,7 +6936,7 @@ parse_operands (char *str, const unsigne
>  
>  /* Functions for operand encoding.  ARM, then Thumb.  */
>  
> -#define rotate_left(v, n) (v << n | v >> (32 - n))
> +#define rotate_left(v, n) (n == 0 ? v : (v << n | v >> (32 - n)))
>  
>  /* If VAL can be encoded in the immediate field of an ARM instruction,
>     return the encoded form.  Otherwise, return FAIL.  */
> 
-------------- next part --------------
diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 5077f87..9100fb2 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -7251,7 +7251,7 @@ parse_operands (char *str, const unsigned int *pattern, bfd_boolean thumb)
 
 /* Functions for operand encoding.  ARM, then Thumb.  */
 
-#define rotate_left(v, n) (v << n | v >> (32 - n))
+#define rotate_left(v, n) (v << (n & 31) | v >> ((32 - n) & 31))
 
 /* If VAL can be encoded in the immediate field of an ARM instruction,
    return the encoded form.  Otherwise, return FAIL.  */
More information about the Binutils mailing list