Re: RFA: Support ARM BKPT instruction without an argument.

[Nick Clifton <nickc at redhat dot com>]

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