This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]