This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: RFA: Support ARM BKPT instruction without an argument.
Hi John,
> On Thu, Nov 28, 2002 at 01:15:23PM +0000, Nick Clifton wrote:
> >> Why does this want to return early from do_t_bkpt()?
> >
> > [...] Plus with my way, the needless testing and
> > insertion of the number returned in exp.X_add_number is avoided.
>
> Well, as a mere peon I don't much want to kick up a fuss and I'm sure
> in these wee functions it won't much matter, but you did ask for "any
> objections" :-)
>
> At present the only early returns from these functions are for error
> handling. Avoiding the "needless" usual bkpt processing is IMHO a bug
> waiting to happen. What happens next year when something extra is
> needed that's not trivial for BKPT 0, and someone adds some new code
> lower down in the function? Either they don't notice this early return
> for valid input, leading to a bug, or they have to change it to something
> like what I'm suggesting anyway.
>
> Perhaps I drank too much of that functional kool-aid...
>
> John "nevermind -- I'm the peon, you're the maintainer :-)"
Hey - I'm not complaining. I'm glad that you responded, and I am
quite willing to change my code if it seems reasonable.
Of course, if we were going to pursue the functional approach to its
logical extreme then we ought to create a new helper function to be
called from both do_bkpt() and do_t_bkpt(), which would then eliminate
the code duplicated in those two functions.
How about this compromise version ? It removes the shortcut function
exit for an empty breakpoint statement, but it also separates out the
if statement to make it, IMHO, more readable.
Cheers
Nick
Index: gas/config/tc-arm.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-arm.c,v
retrieving revision 1.130
diff -c -3 -p -w -r1.130 tc-arm.c
*** gas/config/tc-arm.c 23 Oct 2002 10:34:18 -0000 1.130
--- gas/config/tc-arm.c 28 Nov 2002 15:08:28 -0000
*************** do_t_bkpt (str)
*** 3929,3937 ****
str ++;
memset (& expr, '\0', sizeof (expr));
! if (my_get_expression (& expr, & str) || (expr.X_op != O_constant))
{
! inst.error = _("bad or missing expression");
return;
}
--- 3929,3940 ----
str ++;
memset (& expr, '\0', sizeof (expr));
! if (my_get_expression (& expr, & str) ||
! (expr.X_op != O_constant
! /* As a convenience we allow 'bkpt' without an operand. */
! && expr.X_op != O_absent))
{
! inst.error = _("bad expression");
return;
}
*************** do_bkpt (str)
*** 4111,4119 ****
memset (& expr, '\0', sizeof (expr));
! if (my_get_expression (& expr, & str) || (expr.X_op != O_constant))
{
! inst.error = _("bad or missing expression");
return;
}
--- 4114,4125 ----
memset (& expr, '\0', sizeof (expr));
! if (my_get_expression (& expr, & str) ||
! (expr.X_op != O_constant
! /* As a convenience we allow 'bkpt' without an operand. */
! && expr.X_op != O_absent))
{
! inst.error = _("bad expression");
return;
}
Index: gas/testsuite/gas/arm/arch5tej.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/arm/arch5tej.s,v
retrieving revision 1.2
diff -c -3 -p -w -r1.2 arch5tej.s
*** gas/testsuite/gas/arm/arch5tej.s 22 Aug 2002 16:10:04 -0000 1.2
--- gas/testsuite/gas/arm/arch5tej.s 28 Nov 2002 15:08:28 -0000
*************** label:
*** 8,14 ****
bxjmi r0
bxjpl r7
! # Add two nop instructions to ensure that the output
! # is aligned as will automatically be done for arm-aout.
! nop
! nop
--- 8,12 ----
bxjmi r0
bxjpl r7
! bkpt @ Support for a breakpoint without an argument
! bkpt 10 @ is a feature added to GAS.
Index: gas/testsuite/gas/arm/arch5tej.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/arm/arch5tej.d,v
retrieving revision 1.2
diff -c -3 -p -w -r1.2 arch5tej.d
*** gas/testsuite/gas/arm/arch5tej.d 22 Aug 2002 16:10:04 -0000 1.2
--- gas/testsuite/gas/arm/arch5tej.d 28 Nov 2002 15:08:28 -0000
*************** Disassembly of section .text:
*** 13,17 ****
0+0c <[^>]*> 012fff20 ? bxjeq r0
0+10 <[^>]*> 412fff20 ? bxjmi r0
0+14 <[^>]*> 512fff27 ? bxjpl r7
! 0+18 <[^>]*> e1a00000 ? nop[ ]+\(mov r0,r0\)
! 0+1c <[^>]*> e1a00000 ? nop[ ]+\(mov r0,r0\)
--- 13,17 ----
0+0c <[^>]*> 012fff20 ? bxjeq r0
0+10 <[^>]*> 412fff20 ? bxjmi r0
0+14 <[^>]*> 512fff27 ? bxjpl r7
! 0+18 <[^>]*> e1200070 ? bkpt 0x0000
! 0+1c <[^>]*> e120007a ? bkpt 0x000a