This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH, ARM] Fix out-of-range immediate assembly errors on 64-bit hosts
- From: Julian Brown <julian at codesourcery dot com>
- To: Nick Clifton <nickc at redhat dot com>
- Cc: binutils at sources dot redhat dot com, Paul Brook <paul at codesourcery dot com>
- Date: Wed, 06 Sep 2006 14:27:38 +0100
- Subject: Re: [PATCH, ARM] Fix out-of-range immediate assembly errors on 64-bit hosts
- References: <4489AB12.5090102@codesourcery.com> <448D50C2.5060700@redhat.com> <448D650E.7000305@codesourcery.com> <449115D9.4020809@redhat.com>
Hi,
Nick Clifton wrote:
Hi Julian,
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?
I think that this would be the best solution. It makes the meaning of
the arguments passed to parse_immediate more obvious.
[...]
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).
[...] Or just have the comment in the code.
Here's a (rather belated) new version of the patch which adds
bounds-checking and non-bounds-checking forms to parse_immediate,
avoiding the previous nastiness with INT_MIN/INT_MAX.
Tested with "make check" on x86 and x86_64 with cross to arm-none-eabi.
OK? (for mainline? CSL branch?)
Cheers,
Julian
ChangeLog
gas/
* config/tc-arm.c (parse_immediate): Add BOUNDED parameter, rename
to...
(parse_immediate_maybe_bounded): This. Only bounds-check if BOUNDED
is true.
(parse_immediate_bounded): New function, with same arguments and
semantics as previous parse_immediate.
(parse_immediate_unbounded): New function. Parse an unbounded
integer (with sizeof (exp.X_add_number)).
(parse_big_immediate): Allow for 64-bit exp.X_add_number when
parsing 64-bit immediates.
(parse_address_main): Use parse_immediate_bounded not
parse_immediate.
(parse_ror): Likewise.
(parse_operands): Likewise. For Neon immediates, use
parse_immediate_unbounded. Add new local po_imm_unb_or_fail macro.
? gas/config/~tc-arm.c
Index: gas/config/tc-arm.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-arm.c,v
retrieving revision 1.250.2.29
diff -c -p -r1.250.2.29 tc-arm.c
*** gas/config/tc-arm.c 5 Sep 2006 20:23:46 -0000 1.250.2.29
--- gas/config/tc-arm.c 6 Sep 2006 12:59:19 -0000
*************** const pseudo_typeS md_pseudo_table[] =
*** 3943,3955 ****
/* Generic immediate-value read function for use in insn parsing.
STR points to the beginning of the immediate (the leading #);
! VAL receives the value; if the value is outside [MIN, MAX]
! issue an error. PREFIX_OPT is true if the immediate prefix is
optional. */
static int
! parse_immediate (char **str, int *val, int min, int max,
! bfd_boolean prefix_opt)
{
expressionS exp;
my_get_expression (&exp, str, prefix_opt ? GE_OPT_PREFIX : GE_IMM_PREFIX);
--- 3943,3955 ----
/* Generic immediate-value read function for use in insn parsing.
STR points to the beginning of the immediate (the leading #);
! VAL receives the value; if the value is outside [MIN, MAX] and BOUNDED is
! true, it issue an error. PREFIX_OPT is true if the immediate prefix is
optional. */
static int
! parse_immediate_maybe_bounded (char **str, int *val, bfd_boolean bounded,
! int min, int max, bfd_boolean prefix_opt)
{
expressionS exp;
my_get_expression (&exp, str, prefix_opt ? GE_OPT_PREFIX : GE_IMM_PREFIX);
*************** parse_immediate (char **str, int *val, i
*** 3959,3965 ****
return FAIL;
}
! if (exp.X_add_number < min || exp.X_add_number > max)
{
inst.error = _("immediate value out of range");
return FAIL;
--- 3959,3965 ----
return FAIL;
}
! if (bounded && (exp.X_add_number < min || exp.X_add_number > max))
{
inst.error = _("immediate value out of range");
return FAIL;
*************** parse_immediate (char **str, int *val, i
*** 3969,3974 ****
--- 3969,3987 ----
return SUCCESS;
}
+ static int
+ parse_immediate_bounded (char **str, int *val, int min, int max,
+ bfd_boolean prefix_opt)
+ {
+ return parse_immediate_maybe_bounded (str, val, TRUE, min, max, prefix_opt);
+ }
+
+ static int
+ parse_immediate_unbounded (char **str, int *val, bfd_boolean prefix_opt)
+ {
+ return parse_immediate_maybe_bounded (str, val, FALSE, 0, 0, prefix_opt);
+ }
+
/* Less-generic immediate-value read function with the possibility of loading a
big (64-bit) immediate, as required by Neon VMOV and VMVN immediate
instructions. Puts the result directly in inst.operands[i]. */
*************** parse_big_immediate (char **str, int i)
*** 3982,3988 ****
my_get_expression (&exp, &ptr, GE_OPT_PREFIX_BIG);
if (exp.X_op == O_constant)
! inst.operands[i].imm = exp.X_add_number;
else if (exp.X_op == O_big
&& LITTLENUM_NUMBER_OF_BITS * exp.X_add_number > 32
&& LITTLENUM_NUMBER_OF_BITS * exp.X_add_number <= 64)
--- 3995,4012 ----
my_get_expression (&exp, &ptr, GE_OPT_PREFIX_BIG);
if (exp.X_op == O_constant)
! {
! inst.operands[i].imm = exp.X_add_number & 0xffffffff;
! /* 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)
! {
! /* X >> 32 is illegal if sizeof (exp.X_add_number) == 4. */
! inst.operands[i].reg = ((exp.X_add_number >> 16) >> 16) & 0xffffffff;
! inst.operands[i].regisimm = 1;
! }
! }
else if (exp.X_op == O_big
&& LITTLENUM_NUMBER_OF_BITS * exp.X_add_number > 32
&& LITTLENUM_NUMBER_OF_BITS * exp.X_add_number <= 64)
*************** parse_address_main (char **str, int i, i
*** 4700,4707 ****
if (skip_past_char (&p, '{') == SUCCESS)
{
/* [Rn], {expr} - unindexed, with option */
! if (parse_immediate (&p, &inst.operands[i].imm,
! 0, 255, TRUE) == FAIL)
return PARSE_OPERAND_FAIL;
if (skip_past_char (&p, '}') == FAIL)
--- 4724,4731 ----
if (skip_past_char (&p, '{') == SUCCESS)
{
/* [Rn], {expr} - unindexed, with option */
! if (parse_immediate_bounded (&p, &inst.operands[i].imm,
! 0, 255, TRUE) == FAIL)
return PARSE_OPERAND_FAIL;
if (skip_past_char (&p, '}') == FAIL)
*************** parse_ror (char **str)
*** 4972,4978 ****
return FAIL;
}
! if (parse_immediate (&s, &rot, 0, 24, FALSE) == FAIL)
return FAIL;
switch (rot)
--- 4996,5002 ----
return FAIL;
}
! if (parse_immediate_bounded (&s, &rot, 0, 24, FALSE) == FAIL)
return FAIL;
switch (rot)
*************** parse_operands (char *str, const unsigne
*** 5479,5485 ****
} while (0)
#define po_imm_or_fail(min, max, popt) do { \
! if (parse_immediate (&str, &val, min, max, popt) == FAIL) \
goto failure; \
inst.operands[i].imm = val; \
} while (0)
--- 5503,5515 ----
} while (0)
#define po_imm_or_fail(min, max, popt) do { \
! if (parse_immediate_bounded (&str, &val, min, max, popt) == FAIL) \
! goto failure; \
! inst.operands[i].imm = val; \
! } while (0)
!
! #define po_imm_unb_or_fail(popt) do { \
! if (parse_immediate_unbounded (&str, &val, popt) == FAIL) \
goto failure; \
inst.operands[i].imm = val; \
} while (0)
*************** parse_operands (char *str, const unsigne
*** 5578,5584 ****
break;
try_imm:
/* Immediate gets verified properly later, so accept any now. */
! po_imm_or_fail (INT_MIN, INT_MAX, TRUE);
}
break;
--- 5608,5614 ----
break;
try_imm:
/* Immediate gets verified properly later, so accept any now. */
! po_imm_unb_or_fail (TRUE);
}
break;
*************** parse_operands (char *str, const unsigne
*** 6027,6032 ****
--- 6057,6063 ----
#undef po_reg_or_fail
#undef po_reg_or_goto
#undef po_imm_or_fail
+ #undef po_imm_unb_or_fail
#undef po_scalar_or_fail
/* Shorthand macro for instruction encoding functions issuing errors. */