[PATCH, ARM] Fix out-of-range immediate assembly errors on 64-bit hosts

[Julian Brown]

Nick Clifton wrote:
> Hi Julian,
> 
> 
>> This patch fixes some immediate-out-of-range errors for VBIC, VORR 
>> etc. on 64-bit hosts. On such hosts, the X_add_number field of 
>> expressions will be 64 bits wide. INT_MIN and INT_MAX are used to 
>> signify that any immediate may be accepted for parse_immediate: 
>> unfortunately on a 64-bit host, 0xff000000 for instance is then 
>> interpreted as a positive integer outside that range.
> 
> 
> Sorry - can you explain that last part again please ?  I get how INT_MIN 
> and INT_MAX are being used to indicate "any integer value is acceptable" 
> to parse_immediate(), but where does this 0xffff0000 value come from and 
> why would it cause problems for parse_immediate() ?

0xffff0000 is parsed as a positive integer when 
sizeof(exp.X_add_number)==8, and a negative integer when it equals 4. 
For VORR, VBIC, etc. a large range of values is acceptable for the 
immediate, so validation is done properly later on in assembling the 
instruction rather than here.

>> I've just made passing INT_MIN/INT_MAX disable the check instead, 
>> though I'm not very fond of that solution.
> 
> 
> There appears to be only once place where INT_MAX/INT_MIN are used, so 
> maybe it would be cleaner to change that code to pass realistic minimum 
> and maximum values ?

There's a design flaw in here somewhere with respect to binutils mostly 
using the word size of the host not the target, and this patch probably 
exascerbates the situation by not really being well thought-out enough, 
sorry. :-/

I don't know why it didn't occur to me to use LONG_MIN, LONG_MAX at the 
caller instead, but that seems a bit wrong too somehow. I think it'd 
probably work, but maybe it'd be better to add a 
bounds-checked/non-bounds-checked argument to parse_immediate() or write 
a non-bounds-checking version of the function instead?

>> !       /* 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)
>> !     {
>> !           inst.operands[i].reg = ((exp.X_add_number >> 16) >> 16) & 
>> 0xffffffff;
>> !       inst.operands[i].regisimm = 1;
>> !     }
> 
> 
> I am a little bit confused here.  How is ((x >> 16) >> 16) different 
> from (x >> 32) ?  If not, then why express it that way ?

If sizeof(exp.X_add_number)==4 (i.e. on 32-bit hosts), then I think that 
writing (x >> 32) is invalid C (which isn't very nice, even if that code 
would never be executed). Is there a less weird way of writing the same 
thing?

Cheers,

Julian