[RFA/RFC] Support DW_OP_breg for tracepoints

[RFA/RFC] Support DW_OP_breg for tracepoints

[Randolph Chung]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

Warning: I don't really understand how this works :-)

On hppa-linux, when running gdb.trace/save-trace.exp, the test fails 
because of:

"Unsupported DWARF opcode in the location of q1"

The opcode in question is "DW_OP_breg3". The attached patch attempts to 
add support for DW_OP_breg0..DW_OP_breg31. With this patch hppa-linux 
passes the testcase, although not really understanding DWARF-2 I am not 
certain this is 100% correct. DWARF experts, comments appreciated.

I was going to write the code for DW_OP_bregx too, but I'm not sure how 
that is supposed to work in terms of the "size" argument to that 
function since there are two params to read off the stack. In any case 
since I can't test that case easily I haven't written any code for it.

Comments? ok to check in?

randolph

[-- Attachment #2: d.diff --]
[-- Type: text/x-patch, Size: 1501 bytes --]

2005-11-13  Randolph Chung  <tausq@debian.org>

	* dwarf2loc.c (dwarf2_tracepoint_var_ref): Handle DW_OP_breg0
	through DW_OP_breg31.  Print DWARF opcode for unsupported
	case.

Index: dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.30
diff -u -p -r1.30 dwarf2loc.c
--- dwarf2loc.c	4 Nov 2005 02:42:34 -0000	1.30
+++ dwarf2loc.c	13 Nov 2005 15:29:18 -0000
@@ -479,9 +479,30 @@ dwarf2_tracepoint_var_ref (struct symbol
       ax_simple (ax, aop_add);
       value->kind = axs_lvalue_memory;
     }
+  else if (data[0] >= DW_OP_breg0
+	   && data[0] <= DW_OP_breg31)
+    {
+      unsigned int reg;
+      LONGEST offset;
+      gdb_byte *buf_end;
+
+      reg = data[0] - DW_OP_breg0;
+      buf_end = read_sleb128 (data + 1, data + size, &offset);
+      if (buf_end != data + size)
+	error (_("Unexpected opcode after DW_OP_breg%u for symbol \"%s\"."),
+	       reg, SYMBOL_PRINT_NAME (symbol));
+
+      ax_reg (ax, reg);
+      ax_const_l (ax, offset);
+      ax_simple (ax, aop_add);
+
+      ax_const_l (ax, offset);
+      ax_simple (ax, aop_add);
+      value->kind = axs_lvalue_memory;
+    }
   else
-    error (_("Unsupported DWARF opcode in the location of \"%s\"."),
-	   SYMBOL_PRINT_NAME (symbol));
+    error (_("Unsupported DWARF opcode 0x%x in the location of \"%s\"."),
+	   data[0], SYMBOL_PRINT_NAME (symbol));
 }
 \f
 /* Return the value of SYMBOL in FRAME using the DWARF-2 expression

Re: [RFA/RFC] Support DW_OP_breg for tracepoints

[Daniel Jacobowitz]

On Sun, Nov 13, 2005 at 11:38:03PM +0800, Randolph Chung wrote:
> Warning: I don't really understand how this works :-)

You got it right though.

> 2005-11-13  Randolph Chung  <tausq@debian.org>
> 
> 	* dwarf2loc.c (dwarf2_tracepoint_var_ref): Handle DW_OP_breg0
> 	through DW_OP_breg31.  Print DWARF opcode for unsupported
> 	case.

This is OK.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [RFA/RFC] Support DW_OP_breg for tracepoints

[Jim Blandy]

On 11/13/05, Randolph Chung <randolph@tausq.org> wrote:
> Warning: I don't really understand how this works :-)
>
> On hppa-linux, when running gdb.trace/save-trace.exp, the test fails
> because of:
>
> "Unsupported DWARF opcode in the location of q1"
>
> The opcode in question is "DW_OP_breg3". The attached patch attempts to
> add support for DW_OP_breg0..DW_OP_breg31. With this patch hppa-linux
> passes the testcase, although not really understanding DWARF-2 I am not
> certain this is 100% correct. DWARF experts, comments appreciated.

Why are you adding the offset twice?  Is it just cut-and-paste from
the DW_OP_fbreg code?  That looks wrong, too.

This code should go in ax-gdb.[ch].  That has a translator from GDB
expressions to agent expressions; it would make sense to also have a
translator from Dwarf expressions to agent expressions.

Re: [RFA/RFC] Support DW_OP_breg for tracepoints

[Randolph Chung]

> Why are you adding the offset twice?  Is it just cut-and-paste from
> the DW_OP_fbreg code?  That looks wrong, too.

Yes, copy and paste. I don't understand why it's added twice either :) 
Dan seems to think it's ok, but I'll wait a bit before I commit anything.

> This code should go in ax-gdb.[ch].  That has a translator from GDB
> expressions to agent expressions; it would make sense to also have a
> translator from Dwarf expressions to agent expressions.

I don't feel comfortable enough with the dwarf code to touch this, so 
I'll have to leave this part to one of you experts :)

randolph

Re: [RFA/RFC] Support DW_OP_breg for tracepoints

[Daniel Jacobowitz]

On Mon, Nov 14, 2005 at 07:48:27AM +0800, Randolph Chung wrote:
> >Why are you adding the offset twice?  Is it just cut-and-paste from
> >the DW_OP_fbreg code?  That looks wrong, too.
> 
> Yes, copy and paste. I don't understand why it's added twice either :) 
> Dan seems to think it's ok, but I'll wait a bit before I commit anything.

I assumed it was right by analogy to the existing frame base code. 
Presumably I was just wrong the first time.  Please fix both places.

Thanks, Jim.

> >This code should go in ax-gdb.[ch].  That has a translator from GDB
> >expressions to agent expressions; it would make sense to also have a
> >translator from Dwarf expressions to agent expressions.

Why is it any more logical to have dwarf code in ax-gdb.c than to have
agent code in dwarf2loc.c?  In any case, it is a method of the symbol.
Please see struct symbol_ops.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [RFA/RFC] Support DW_OP_breg for tracepoints

[Jim Blandy]

On 11/13/05, Daniel Jacobowitz <drow@false.org> wrote:
> > >This code should go in ax-gdb.[ch].  That has a translator from GDB
> > >expressions to agent expressions; it would make sense to also have a
> > >translator from Dwarf expressions to agent expressions.
>
> Why is it any more logical to have dwarf code in ax-gdb.c than to have
> agent code in dwarf2loc.c?  In any case, it is a method of the symbol.
> Please see struct symbol_ops.

Yes, I'm not suggesting that we move locexpr_tracepoint_var_ref or
loclist_tracepoint_var_ref.

ax-general.c and ax.h are supposed to be very GDB-independent, and
depend only on the definition of the agent expression bytecode
language for their correctness.  Everyplace else in GDB is supposed to
be as ignorant as possible of the details of agent expressions. 
ax-gdb.c is supposed to be the meeting place where we handle the GDB
expression / agent expression relationship.

dwarf2_tracepoint_var_ref is a similar situation: it is where Dwarf
expressions and agent expressions meet.  Other Dwarf code should stay
independent of the details of agent expressions, and other agent code
should stay independent of Dwarf.  So maybe there should be a file
parallel to ax-gdb.c called ax-dwarf.c; but that seems like overkill. 
My thought was to make ax-gdb.c the overall rendezvous where we get
down and dirty about the correspondence between agent expression
semantics and other little languages' semantics.

Re: [RFA/RFC] Support DW_OP_breg for tracepoints

[Randolph Chung]

[-- Attachment #1: Type: text/plain, Size: 195 bytes --]

> I assumed it was right by analogy to the existing frame base code. 
> Presumably I was just wrong the first time.  Please fix both places.

this is what i finally checked in.

thanks
randolph


[-- Attachment #2: d.diff --]
[-- Type: text/x-patch, Size: 1553 bytes --]

2005-11-19  Randolph Chung  <tausq@debian.org>

	* dwarf2loc.c (dwarf2_tracepoint_var_ref): Remove extra add for
	DW_OP_fbreg.  Handle DW_OP_breg0 through DW_OP_breg31.  Print 
	DWARF opcode for unsupported case.

Index: dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.31
diff -u -p -r1.31 dwarf2loc.c
--- dwarf2loc.c	14 Nov 2005 22:25:16 -0000	1.31
+++ dwarf2loc.c	19 Nov 2005 13:41:37 -0000
@@ -477,13 +477,30 @@ dwarf2_tracepoint_var_ref (struct symbol
       ax_const_l (ax, frame_offset);
       ax_simple (ax, aop_add);
 
-      ax_const_l (ax, frame_offset);
+      value->kind = axs_lvalue_memory;
+    }
+  else if (data[0] >= DW_OP_breg0
+	   && data[0] <= DW_OP_breg31)
+    {
+      unsigned int reg;
+      LONGEST offset;
+      gdb_byte *buf_end;
+
+      reg = data[0] - DW_OP_breg0;
+      buf_end = read_sleb128 (data + 1, data + size, &offset);
+      if (buf_end != data + size)
+	error (_("Unexpected opcode after DW_OP_breg%u for symbol \"%s\"."),
+	       reg, SYMBOL_PRINT_NAME (symbol));
+
+      ax_reg (ax, reg);
+      ax_const_l (ax, offset);
       ax_simple (ax, aop_add);
+
       value->kind = axs_lvalue_memory;
     }
   else
-    error (_("Unsupported DWARF opcode in the location of \"%s\"."),
-	   SYMBOL_PRINT_NAME (symbol));
+    error (_("Unsupported DWARF opcode 0x%x in the location of \"%s\"."),
+	   data[0], SYMBOL_PRINT_NAME (symbol));
 }
 \f
 /* Return the value of SYMBOL in FRAME using the DWARF-2 expression