RFA: Patch for PR gas/12715 incorrect encoding of 64-bit immediates for ARM targets

RFA: Patch for PR gas/12715 incorrect encoding of 64-bit immediates for ARM targets

[Matthew Gretton-Dann]

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

All,

PR gas/12715 shows a bug in gas when assembling 64-bit immediates on an
ARM target when gas has been configured for a 32-bit host, and with
support multiple targets at least one of which is a 64-bit target.

The issue is here in gas/config/tc-arm.c, parse_big_immediate:

      /* If we're on a 64-bit host, then a 64-bit number can be returned
using
        O_constant.  We have to be careful not to break compilation for
        32-bit X_add_number, though.  */
      if ((exp.X_add_number & ~0xffffffffl) != 0)
        ...

On a 32-bit host with a 64-bit target exp.X_add_number is (ultimately)
of type BFD_HOST_64_BIT.  In this parser has stored the whole 64-bit
immediate in exp.X_add_number - not (as in the case when on a 32-bit
host with a 32-bit target) as a bignum.

In this case the if test above always fails, as 0xffffffffl is treated
as a 32-bit integer, so the bitwise inverse is 0, and so the test can
never be true.  And so the 64-bit immediate gets its top 32-bits set to
zero.

The fix is to make sure the constant is of the correct type before doing
the bitwise inverse.

I have tested this on an x86-64 host with the following configuration 
options:
 * --target=arm-none-eabi
 * --target=arm-none-eabi --enable-targets=all
 * --target=arm-none-eabi CFLAGS=-m32
 * --target=arm-none-eabi --enable-targets=all CFLAGS=-m32

Is this okay for trunk and the 2.21 branch?

Thanks,

Matt

gas/ChangeLog:
2011-05-11  Matthew Gretton-Dann  <matthew.gretton-dann@arm.com>

	PR gas/12715
	* config/tc-arm.c (parse_big_immediate):  Fix parsing of 64-bit
	immediates on 32-bit hosts.

gas/testsuite/ChangeLog:
2011-05-11  Matthew Gretton-Dann  <matthew.gretton-dann@arm.com>

	PR gas/12715
	* gas/arm/neon-const.s: Add testcase for 64-bit Neon constants.
	* gas/arm/neon-const.d: Likewise.

-- 
Matthew Gretton-Dann
Principal Engineer - PDSW Tools
ARM Ltd

[-- Attachment #2: 1105-vmov-immediate.patch --]
[-- Type: text/x-patch, Size: 1537 bytes --]

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index af8c4aa..33c5deb 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -4450,7 +4450,7 @@ parse_big_immediate (char **str, int i)
       /* If we're on a 64-bit host, then a 64-bit number can be returned using
 	 O_constant.  We have to be careful not to break compilation for
 	 32-bit X_add_number, though.  */
-      if ((exp.X_add_number & ~0xffffffffl) != 0)
+      if ((exp.X_add_number & ~(offsetT)(0xffffffffU)) != 0)
 	{
           /* X >> 32 is illegal if sizeof (exp.X_add_number) == 4.  */
 	  inst.operands[i].reg = ((exp.X_add_number >> 16) >> 16) & 0xffffffff;
diff --git a/gas/testsuite/gas/arm/neon-const.d b/gas/testsuite/gas/arm/neon-const.d
index a1bc97c..6c46930 100644
--- a/gas/testsuite/gas/arm/neon-const.d
+++ b/gas/testsuite/gas/arm/neon-const.d
@@ -263,3 +263,4 @@ Disassembly of section .text:
 0[0-9a-f]+ <[^>]+> f3850f5f 	vmov\.f32	q0, #-0\.484375	; 0xbef80000
 0[0-9a-f]+ <[^>]+> f3860f5f 	vmov\.f32	q0, #-0\.96875	; 0xbf780000
 0[0-9a-f]+ <[^>]+> f3870f5f 	vmov\.f32	q0, #-1\.9375	; 0xbff80000
+0[0-9a-f]+ <[^>]+> f3879e3f 	vmov\.i64	d9, #0xffffffffffffffff
diff --git a/gas/testsuite/gas/arm/neon-const.s b/gas/testsuite/gas/arm/neon-const.s
index a6fb550..aaaf144 100644
--- a/gas/testsuite/gas/arm/neon-const.s
+++ b/gas/testsuite/gas/arm/neon-const.s
@@ -295,3 +295,5 @@
         vmov.f32 q0, -0.484375
         vmov.f32 q0, -0.96875
         vmov.f32 q0, -1.9375
+
+	vmov.i64 d9, #0xffffffffffffffff

Re: RFA: Patch for PR gas/12715 incorrect encoding of 64-bit immediates for ARM targets

[Alan Modra]

On Wed, May 11, 2011 at 05:29:27PM +0100, Matthew Gretton-Dann wrote:
> gas/ChangeLog:
> 2011-05-11  Matthew Gretton-Dann  <matthew.gretton-dann@arm.com>
> 
> 	PR gas/12715
> 	* config/tc-arm.c (parse_big_immediate):  Fix parsing of 64-bit
> 	immediates on 32-bit hosts.
> 
> gas/testsuite/ChangeLog:
> 2011-05-11  Matthew Gretton-Dann  <matthew.gretton-dann@arm.com>
> 
> 	PR gas/12715
> 	* gas/arm/neon-const.s: Add testcase for 64-bit Neon constants.
> 	* gas/arm/neon-const.d: Likewise.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

Re: RFA: Patch for PR gas/12715 incorrect encoding of 64-bit immediates for ARM targets

[Tristan Gingold]

On May 12, 2011, at 3:35 AM, Alan Modra wrote:

> On Wed, May 11, 2011 at 05:29:27PM +0100, Matthew Gretton-Dann wrote:
>> gas/ChangeLog:
>> 2011-05-11  Matthew Gretton-Dann  <matthew.gretton-dann@arm.com>
>> 
>> 	PR gas/12715
>> 	* config/tc-arm.c (parse_big_immediate):  Fix parsing of 64-bit
>> 	immediates on 32-bit hosts.
>> 
>> gas/testsuite/ChangeLog:
>> 2011-05-11  Matthew Gretton-Dann  <matthew.gretton-dann@arm.com>
>> 
>> 	PR gas/12715
>> 	* gas/arm/neon-const.s: Add testcase for 64-bit Neon constants.
>> 	* gas/arm/neon-const.d: Likewise.
> 
> OK.

Ditto for the branch.

Re: RFA: Patch for PR gas/12715 incorrect encoding of 64-bit immediates for ARM targets

[Matthew Gretton-Dann]

On Thu, 2011-05-12 at 11:05 +0930, Alan Modra wrote:
> On Wed, May 11, 2011 at 05:29:27PM +0100, Matthew Gretton-Dann wrote:
> > gas/ChangeLog:
> > 2011-05-11  Matthew Gretton-Dann  <matthew.gretton-dann@arm.com>
> > 
> > 	PR gas/12715
> > 	* config/tc-arm.c (parse_big_immediate):  Fix parsing of 64-bit
> > 	immediates on 32-bit hosts.
> > 
> > gas/testsuite/ChangeLog:
> > 2011-05-11  Matthew Gretton-Dann  <matthew.gretton-dann@arm.com>
> > 
> > 	PR gas/12715
> > 	* gas/arm/neon-const.s: Add testcase for 64-bit Neon constants.
> > 	* gas/arm/neon-const.d: Likewise.
> 
> OK.

I'm assuming that was just an okay for committing to trunk (which I have
now done)?

Tristan - is this okay for the 2.21 branch as well?

Thanks,

Matt

-- 
Matthew Gretton-Dann
Principal Engineer - PDSW Tools
ARM Ltd