Re: [PATCH] Fix undefined behavior breaking clang-compiled as

[Richard Earnshaw <rearnsha at arm dot com>]

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.  */
>

Attachment: arm-undef.patch
Description: Text document