[PATCH][arm] build fix for gcc-6 -Werror=shift-negative-value

[PATCH][arm] build fix for gcc-6 -Werror=shift-negative-value

[Szabolcs Nagy]

[-- Attachment #1: Type: text/plain, Size: 529 bytes --]

binutils does not compile here when building it for aarch64 or arm
targets with gcc trunk native compiler:

opcodes/arm-dis.c: In function 'print_insn_coprocessor':
opcodes/arm-dis.c:3331:20: error: left shift of negative value [-Werror=shift-negative-value]
         imm |= (-1 << 7);
                    ^

The fix assumes 2's complement signed int representation on the host.

Is this OK?

2015-06-15  Szabolcs Nagy  <szabolcs.nagy@arm.com>

	* opcodes/arm-dis.c (print_insn_coprocessor): Avoid negative shift.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: negshift.diff --]
[-- Type: text/x-patch; name=negshift.diff, Size: 484 bytes --]

diff --git a/opcodes/arm-dis.c b/opcodes/arm-dis.c
index 1585a4f..8e92cc5 100644
--- a/opcodes/arm-dis.c
+++ b/opcodes/arm-dis.c
@@ -3326,9 +3326,9 @@ print_insn_coprocessor (bfd_vma pc,
 
 		    imm = (given & 0xf) | ((given & 0xe0) >> 1);
 
-		    /* Is ``imm'' a negative number?  */
+		    /* Is ``imm'' a negative number?  Then sign-extend it.  */
 		    if (imm & 0x40)
-		      imm |= (-1 << 7);
+		      imm |= ~0x7f;
 
 		    func (stream, "%d", imm);
 		  }

Re: [PATCH][arm] build fix for gcc-6 -Werror=shift-negative-value

[Andreas Schwab]

Szabolcs Nagy <szabolcs.nagy@arm.com> writes:

> -		    /* Is ``imm'' a negative number?  */
> +		    /* Is ``imm'' a negative number?  Then sign-extend it.  */
>  		    if (imm & 0x40)
> -		      imm |= (-1 << 7);
> +		      imm |= ~0x7f;

                    imm = (imm ^ 0x40) - 0x40;

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH][arm] build fix for gcc-6 -Werror=shift-negative-value

[Szabolcs Nagy]

[-- Attachment #1: Type: text/plain, Size: 610 bytes --]



On 15/06/15 13:03, Andreas Schwab wrote:
> Szabolcs Nagy <szabolcs.nagy@arm.com> writes:
> 
>> -		    /* Is ``imm'' a negative number?  */
>> +		    /* Is ``imm'' a negative number?  Then sign-extend it.  */
>>  		    if (imm & 0x40)
>> -		      imm |= (-1 << 7);
>> +		      imm |= ~0x7f;
> 
>                     imm = (imm ^ 0x40) - 0x40;
> 

hm right, then i don't need assumptions about signed int representation.

imm -= 0x80;

is even simpler, is that ok?

2015-06-15  Szabolcs Nagy  <szabolcs.nagy@arm.com>

	* opcodes/arm-dis.c (print_insn_coprocessor): Avoid negative shift.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: negshift2.diff --]
[-- Type: text/x-patch; name=negshift2.diff, Size: 360 bytes --]

diff --git a/opcodes/arm-dis.c b/opcodes/arm-dis.c
index e9f4425..818847d 100644
--- a/opcodes/arm-dis.c
+++ b/opcodes/arm-dis.c
@@ -3352,7 +3352,7 @@ print_insn_coprocessor (bfd_vma pc,
 
 		    /* Is ``imm'' a negative number?  */
 		    if (imm & 0x40)
-		      imm |= (-1 << 7);
+		      imm -= 0x80;
 
 		    func (stream, "%d", imm);
 		  }

Re: [PATCH][arm] build fix for gcc-6 -Werror=shift-negative-value

[Nicholas Clifton]

Hi Szabolcs,

>   Szabolcs Nagy<szabolcs.nagy@arm.com>
>
> 	* opcodes/arm-dis.c (print_insn_coprocessor): Avoid negative shift.

Approved and applied.

Cheers
   Nick