[RFA] dwarf2expr.c: Prepare for eventual DW_OP_piece support

[RFA] dwarf2expr.c: Prepare for eventual DW_OP_piece support

[Kevin Buettner]

The patch below prepares execute_stack_op() for eventual DW_OP_piece
support.

The relatively obvious part of the patch is the revision of the tests
which force DW_OP_reg operations to be used alone.  These tests have
been revised to permit a DW_OP_piece operator to follow a DW_OP_reg*
operator.

The not so obvious part concerns the initialization of ctx->in_reg.
When a DW_OP_piece case is eventually added, it will need to know
whether the thing on the stack is a register or some other kind of
location expression.  At the moment, the only indication we have of
what this thing is is being cleared each time through the loop.  Moving
it out of the loop will allow a DW_OP_piece case to use this information
for determining how to proceed.

Okay?

	* dwarf2expr.c (execute_stack_op): Move ``ctx->in_reg'' initialization
	out of loop.  Allow DW_OP_reg0 ... DW_OP_reg31 and DW_OP_regx to
	be used in conjuction with DW_OP_piece.  Revise error message
	accordingly.

Index: dwarf2expr.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.c,v
retrieving revision 1.6
diff -u -p -r1.6 dwarf2expr.c
--- dwarf2expr.c	13 Apr 2003 15:53:44 -0000	1.6
+++ dwarf2expr.c	24 Apr 2003 00:36:43 -0000
@@ -228,6 +227,8 @@ static void
 execute_stack_op (struct dwarf_expr_context *ctx, unsigned char *op_ptr,
 		  unsigned char *op_end)
 {
+  ctx->in_reg = 0;
+
   while (op_ptr < op_end)
     {
       enum dwarf_location_atom op = *op_ptr++;
@@ -236,8 +237,6 @@ execute_stack_op (struct dwarf_expr_cont
       LONGEST offset;
       int bytes_read;
 
-      ctx->in_reg = 0;
-
       switch (op)
 	{
 	case DW_OP_lit0:
@@ -355,10 +354,9 @@ execute_stack_op (struct dwarf_expr_cont
 	case DW_OP_reg29:
 	case DW_OP_reg30:
 	case DW_OP_reg31:
-	  /* NOTE: in the presence of DW_OP_piece this check is incorrect.  */
-	  if (op_ptr != op_end)
+	  if (op_ptr != op_end && *op_ptr != DW_OP_piece)
 	    error ("DWARF-2 expression error: DW_OP_reg operations must be "
-		   "used alone.");
+		   "used either alone or in conjuction with DW_OP_piece.");
 
 	  result = op - DW_OP_reg0;
 	  ctx->in_reg = 1;
@@ -367,9 +365,9 @@ execute_stack_op (struct dwarf_expr_cont
 
 	case DW_OP_regx:
 	  op_ptr = read_uleb128 (op_ptr, op_end, &reg);
-	  if (op_ptr != op_end)
+	  if (op_ptr != op_end && *op_ptr != DW_OP_piece)
 	    error ("DWARF-2 expression error: DW_OP_reg operations must be "
-		   "used alone.");
+		   "used either alone or in conjuction with DW_OP_piece.");
 
 	  result = reg;
 	  ctx->in_reg = 1;

Re: [RFA] dwarf2expr.c: Prepare for eventual DW_OP_piece support

[Kevin Buettner]

On Apr 23,  6:05pm, Kevin Buettner wrote:

> ... prepares execute_stack_op() for eventual DW_OP_piece
> support.

I have a patch which adds _limited_ DW_OP_piece support.  It finds the
various "pieces" (all of which must be registers) in the location
expression and then calls an architecture specific method to determine
whether, given a single register number, gdb can reliably fetch the
necessary pieces.  The architecture dependent method simply checks to
make sure that the register numbers (the pieces) are in the correct
order and it returns the register number that must be used in order to
properly fetch the pieces (or -1 if it can't be done).

This support doesn't handle the interesting case of an object being
split between a register and memory or even of the slightly less
interesting case of non-contiguous registers, but it will be adequate
for most DW_OP_piece expressions that gcc will emit in the near
future.  Anyway... given the limitations outlined above, is there
interest in having me submit this patch? 

Kevin

Re: [RFA] dwarf2expr.c: Prepare for eventual DW_OP_piece support

[Daniel Jacobowitz]

On Wed, Apr 23, 2003 at 06:13:52PM -0700, Kevin Buettner wrote:
> On Apr 23,  6:05pm, Kevin Buettner wrote:
> 
> > ... prepares execute_stack_op() for eventual DW_OP_piece
> > support.
> 
> I have a patch which adds _limited_ DW_OP_piece support.  It finds the
> various "pieces" (all of which must be registers) in the location
> expression and then calls an architecture specific method to determine
> whether, given a single register number, gdb can reliably fetch the
> necessary pieces.  The architecture dependent method simply checks to
> make sure that the register numbers (the pieces) are in the correct
> order and it returns the register number that must be used in order to
> properly fetch the pieces (or -1 if it can't be done).
> 
> This support doesn't handle the interesting case of an object being
> split between a register and memory or even of the slightly less
> interesting case of non-contiguous registers, but it will be adequate
> for most DW_OP_piece expressions that gcc will emit in the near
> future.  Anyway... given the limitations outlined above, is there
> interest in having me submit this patch? 

Ah, sounds like our friend the e500.  I think that you should submit
the patch, personally.

Note that we can make GCC emit more DW_OP_piece information in,
probably, a heartbeat or so.  Once GDB's ready for it.  The movement of
ctx->in_reg out of the loop is appropriate only for the limited
DW_OP_piece support you describe above; but it's in the loop for
exactly the reason of values split between registers and memory.  In
order to support this locations need to become more complete objects. 
I have a plan of attack, but no time.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] dwarf2expr.c: Prepare for eventual DW_OP_piece support

[Kevin Buettner]

Ping!

--- Forwarded mail from Kevin Buettner <kevinb@redhat.com>

Date: Wed, 23 Apr 2003 18:05:36 -0700
From: Kevin Buettner <kevinb@redhat.com>
To: gdb-patches@sources.redhat.com
Subject: [RFA] dwarf2expr.c: Prepare for eventual DW_OP_piece support

The patch below prepares execute_stack_op() for eventual DW_OP_piece
support.

The relatively obvious part of the patch is the revision of the tests
which force DW_OP_reg operations to be used alone.  These tests have
been revised to permit a DW_OP_piece operator to follow a DW_OP_reg*
operator.

The not so obvious part concerns the initialization of ctx->in_reg.
When a DW_OP_piece case is eventually added, it will need to know
whether the thing on the stack is a register or some other kind of
location expression.  At the moment, the only indication we have of
what this thing is is being cleared each time through the loop.  Moving
it out of the loop will allow a DW_OP_piece case to use this information
for determining how to proceed.

Okay?

	* dwarf2expr.c (execute_stack_op): Move ``ctx->in_reg'' initialization
	out of loop.  Allow DW_OP_reg0 ... DW_OP_reg31 and DW_OP_regx to
	be used in conjuction with DW_OP_piece.  Revise error message
	accordingly.

Index: dwarf2expr.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.c,v
retrieving revision 1.6
diff -u -p -r1.6 dwarf2expr.c
--- dwarf2expr.c	13 Apr 2003 15:53:44 -0000	1.6
+++ dwarf2expr.c	24 Apr 2003 00:36:43 -0000
@@ -228,6 +227,8 @@ static void
 execute_stack_op (struct dwarf_expr_context *ctx, unsigned char *op_ptr,
 		  unsigned char *op_end)
 {
+  ctx->in_reg = 0;
+
   while (op_ptr < op_end)
     {
       enum dwarf_location_atom op = *op_ptr++;
@@ -236,8 +237,6 @@ execute_stack_op (struct dwarf_expr_cont
       LONGEST offset;
       int bytes_read;
 
-      ctx->in_reg = 0;
-
       switch (op)
 	{
 	case DW_OP_lit0:
@@ -355,10 +354,9 @@ execute_stack_op (struct dwarf_expr_cont
 	case DW_OP_reg29:
 	case DW_OP_reg30:
 	case DW_OP_reg31:
-	  /* NOTE: in the presence of DW_OP_piece this check is incorrect.  */
-	  if (op_ptr != op_end)
+	  if (op_ptr != op_end && *op_ptr != DW_OP_piece)
 	    error ("DWARF-2 expression error: DW_OP_reg operations must be "
-		   "used alone.");
+		   "used either alone or in conjuction with DW_OP_piece.");
 
 	  result = op - DW_OP_reg0;
 	  ctx->in_reg = 1;
@@ -367,9 +365,9 @@ execute_stack_op (struct dwarf_expr_cont
 
 	case DW_OP_regx:
 	  op_ptr = read_uleb128 (op_ptr, op_end, &reg);
-	  if (op_ptr != op_end)
+	  if (op_ptr != op_end && *op_ptr != DW_OP_piece)
 	    error ("DWARF-2 expression error: DW_OP_reg operations must be "
-		   "used alone.");
+		   "used either alone or in conjuction with DW_OP_piece.");
 
 	  result = reg;
 	  ctx->in_reg = 1;


--- End of forwarded mail from Kevin Buettner <kevinb@redhat.com>

Re: [RFA] dwarf2expr.c: Prepare for eventual DW_OP_piece support

[Elena Zannoni]

Kevin Buettner writes:
 > Ping!
 > 
 > --- Forwarded mail from Kevin Buettner <kevinb@redhat.com>
 > 
 > Date: Wed, 23 Apr 2003 18:05:36 -0700
 > From: Kevin Buettner <kevinb@redhat.com>
 > To: gdb-patches@sources.redhat.com
 > Subject: [RFA] dwarf2expr.c: Prepare for eventual DW_OP_piece support
 > 
 > The patch below prepares execute_stack_op() for eventual DW_OP_piece
 > support.
 > 
 > The relatively obvious part of the patch is the revision of the tests
 > which force DW_OP_reg operations to be used alone.  These tests have
 > been revised to permit a DW_OP_piece operator to follow a DW_OP_reg*
 > operator.
 > 
 > The not so obvious part concerns the initialization of ctx->in_reg.
 > When a DW_OP_piece case is eventually added, it will need to know
 > whether the thing on the stack is a register or some other kind of
 > location expression.  At the moment, the only indication we have of
 > what this thing is is being cleared each time through the loop.  Moving
 > it out of the loop will allow a DW_OP_piece case to use this information
 > for determining how to proceed.
 > 
 > Okay?
 > 

 ok.

elena

 > 	* dwarf2expr.c (execute_stack_op): Move ``ctx->in_reg'' initialization
 > 	out of loop.  Allow DW_OP_reg0 ... DW_OP_reg31 and DW_OP_regx to
 > 	be used in conjuction with DW_OP_piece.  Revise error message
 > 	accordingly.
 > 
 > Index: dwarf2expr.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/dwarf2expr.c,v
 > retrieving revision 1.6
 > diff -u -p -r1.6 dwarf2expr.c
 > --- dwarf2expr.c	13 Apr 2003 15:53:44 -0000	1.6
 > +++ dwarf2expr.c	24 Apr 2003 00:36:43 -0000
 > @@ -228,6 +227,8 @@ static void
 >  execute_stack_op (struct dwarf_expr_context *ctx, unsigned char *op_ptr,
 >  		  unsigned char *op_end)
 >  {
 > +  ctx->in_reg = 0;
 > +
 >    while (op_ptr < op_end)
 >      {
 >        enum dwarf_location_atom op = *op_ptr++;
 > @@ -236,8 +237,6 @@ execute_stack_op (struct dwarf_expr_cont
 >        LONGEST offset;
 >        int bytes_read;
 >  
 > -      ctx->in_reg = 0;
 > -
 >        switch (op)
 >  	{
 >  	case DW_OP_lit0:
 > @@ -355,10 +354,9 @@ execute_stack_op (struct dwarf_expr_cont
 >  	case DW_OP_reg29:
 >  	case DW_OP_reg30:
 >  	case DW_OP_reg31:
 > -	  /* NOTE: in the presence of DW_OP_piece this check is incorrect.  */
 > -	  if (op_ptr != op_end)
 > +	  if (op_ptr != op_end && *op_ptr != DW_OP_piece)
 >  	    error ("DWARF-2 expression error: DW_OP_reg operations must be "
 > -		   "used alone.");
 > +		   "used either alone or in conjuction with DW_OP_piece.");
 >  
 >  	  result = op - DW_OP_reg0;
 >  	  ctx->in_reg = 1;
 > @@ -367,9 +365,9 @@ execute_stack_op (struct dwarf_expr_cont
 >  
 >  	case DW_OP_regx:
 >  	  op_ptr = read_uleb128 (op_ptr, op_end, &reg);
 > -	  if (op_ptr != op_end)
 > +	  if (op_ptr != op_end && *op_ptr != DW_OP_piece)
 >  	    error ("DWARF-2 expression error: DW_OP_reg operations must be "
 > -		   "used alone.");
 > +		   "used either alone or in conjuction with DW_OP_piece.");
 >  
 >  	  result = reg;
 >  	  ctx->in_reg = 1;
 > 
 > 
 > --- End of forwarded mail from Kevin Buettner <kevinb@redhat.com>

Re: [RFA] dwarf2expr.c: Prepare for eventual DW_OP_piece support

[Kevin Buettner]

On May 7,  7:06pm, Elena Zannoni wrote:

>  > Okay?
> 
>  ok.
> 
>  > 	* dwarf2expr.c (execute_stack_op): Move ``ctx->in_reg'' initialization
>  > 	out of loop.  Allow DW_OP_reg0 ... DW_OP_reg31 and DW_OP_regx to
>  > 	be used in conjuction with DW_OP_piece.  Revise error message
>  > 	accordingly.

Committed.

Kevin