[PATCH] Handle OP_THIS in tracepoints

[PATCH] Handle OP_THIS in tracepoints

[Stan Shebs]

This one comes up pretty quickly when doing tracepoints in C++; members 
in expressions use OP_THIS, which amounts to shorthand for a reference 
to the implicit argument "this".  Handling is uncomplicated, mimics 
expression evaluation.

Stan

2009-12-23  Stan Shebs  <stan@codesourcery.com>

    * ax-gdb.c (gen_expr): Handle OP_THIS.

Index: ax-gdb.c
===================================================================
RCS file: /cvs/src/src/gdb/ax-gdb.c,v
retrieving revision 1.54
diff -p -r1.54 ax-gdb.c
*** ax-gdb.c    24 Dec 2009 00:40:49 -0000    1.54
--- ax-gdb.c    24 Dec 2009 01:21:21 -0000
***************
*** 35,40 ****
--- 35,41 ----
  #include "regcache.h"
  #include "user-regs.h"
  #include "language.h"
+ #include "dictionary.h"
 
  /* To make sense of this file, you should read doc/agentexpr.texi.
     Then look at the types and enums in ax-gdb.h.  For the code itself,
*************** gen_expr (struct expression *exp, union
*** 1759,1764 ****
--- 1760,1797 ----
        }
        break;
 
+     case OP_THIS:
+       {
+     char *name;
+     struct frame_info *frame;
+     struct symbol *func, *sym;
+     struct block *b;
+
+     name = current_language->la_name_of_this;
+     if (!name)
+       error (_("no `this' in current language"));
+
+     frame = get_selected_frame (_("no frame selected"));
+
+     func = get_frame_function (frame);
+     if (!func)
+       error (_("no `%s' in nameless context"), name);
+
+     b = SYMBOL_BLOCK_VALUE (func);
+     if (dict_empty (BLOCK_DICT (b)))
+       error (_("no args, no `%s' in block"), name);
+
+     /* Calling lookup_block_symbol is necessary to get the LOC_REGISTER
+        symbol instead of the LOC_ARG one (if both exist).  */
+     sym = lookup_block_symbol (b, name, NULL, VAR_DOMAIN);
+     if (!sym)
+       error (_("no `%s' found"), name);
+
+     gen_var_ref (exp->gdbarch, ax, value, sym);
+     (*pc) += 2;
+       }
+       break;
+
      case OP_TYPE:
        error (_("Attempt to use a type name as an expression."));
 

Re: [PATCH] Handle OP_THIS in tracepoints

[Pedro Alves]

On Thursday 24 December 2009 01:33:33, Stan Shebs wrote:
> This one comes up pretty quickly when doing tracepoints in C++; members 
> in expressions use OP_THIS, which amounts to shorthand for a reference 
> to the implicit argument "this".  Handling is uncomplicated, mimics 
> expression evaluation.

> 
> +     case OP_THIS:
> +       {
> +     char *name;
> +     struct frame_info *frame;
> +     struct symbol *func, *sym;
> +     struct block *b;
> +
> +     name = current_language->la_name_of_this;
> +     if (!name)
> +       error (_("no `this' in current language"));
> +
> +     frame = get_selected_frame (_("no frame selected"));
> +
> +     func = get_frame_function (frame);
> +     if (!func)
> +       error (_("no `%s' in nameless context"), name);
> +
> +     b = SYMBOL_BLOCK_VALUE (func);
> +     if (dict_empty (BLOCK_DICT (b)))
> +       error (_("no args, no `%s' in block"), name);
> +
> +     /* Calling lookup_block_symbol is necessary to get the LOC_REGISTER
> +        symbol instead of the LOC_ARG one (if both exist).  */
> +     sym = lookup_block_symbol (b, name, NULL, VAR_DOMAIN);
> +     if (!sym)
> +       error (_("no `%s' found"), name);
> +
> +     gen_var_ref (exp->gdbarch, ax, value, sym);
> +     (*pc) += 2;
> +       }
> +       break;
> +
>       case OP_TYPE:
>         error (_("Attempt to use a type name as an expression."));
>  

This is busted, and was later fixed in our internal
tree.  :-)  The selected frame, and the current language
don't have anything to do with the context in which
the tracepoint runs.  E.g.:

(gdb) trace 'Foo::Bar()'
(gdb) actions 
(gdb) collect foo_field

Here, the `this' for foo_field needs to be
looked up in the context of Foo::Bar, not of whatever
context/frame the user had selected when she issued
the 'actions' command.

(Note that `maint agent(-eval)' always works in the context
of the selected frame, so testing with that alone can
mask out these issues.)

-- 
Pedro Alves

Re: [PATCH] Handle OP_THIS in tracepoints

[Stan Shebs]

Pedro Alves wrote:
> This is busted, and was later fixed in our internal
> tree.  :-)  The selected frame, and the current language
> don't have anything to do with the context in which
> the tracepoint runs.  E.g.:
>   
Darn, I forgot about that one.

Oh, but wait a minute!  Issue #6302 says "submission required", and it's 
assigned to... you!  And now that the base code is in, it should be no 
problem to prepare the patch! :-)

Stan

Re: [PATCH] Handle OP_THIS in tracepoints

[Pedro Alves]

On Thursday 24 December 2009 17:57:39, Stan Shebs wrote:
> Pedro Alves wrote:
> > This is busted, and was later fixed in our internal
> > tree.  :-)  The selected frame, and the current language
> > don't have anything to do with the context in which
> > the tracepoint runs.  E.g.:
> >   
> Darn, I forgot about that one.
> 
> Oh, but wait a minute!  Issue #6302 says "submission required", and it's 
> assigned to... you!  And now that the base code is in, 

Eh, oops, I didn't realize from your post that the
patch was applied already.

> it should be no  
> problem to prepare the patch! :-)

Will do.

-- 
Pedro Alves

Re: [PATCH] Handle OP_THIS in tracepoints

[Pedro Alves]

On Thursday 24 December 2009 16:34:54, Pedro Alves wrote:
> This is busted, and was later fixed in our internal
> tree.  :-)  The selected frame, and the current language
> don't have anything to do with the context in which
> the tracepoint runs.  E.g.:
> 
> (gdb) trace 'Foo::Bar()'
> (gdb) actions 
> (gdb) collect foo_field
> 
> Here, the `this' for foo_field needs to be
> looked up in the context of Foo::Bar, not of whatever
> context/frame the user had selected when she issued
> the 'actions' command.

On Thursday 24 December 2009 18:08:01, Pedro Alves wrote:
> On Thursday 24 December 2009 17:57:39, Stan Shebs wrote:
> > it should be no  
> > problem to prepare the patch! :-)
> 
> Will do.

Here it is.

2009-12-24  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* ax-gdb.c (gen_expr) <OP_THIS>: Lookup `this' in the context of
	the tracepoint, not of the selected frame and language.

-- 
Pedro Alves

---
 gdb/ax-gdb.c |   21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

Index: src/gdb/ax-gdb.c
===================================================================
--- src.orig/gdb/ax-gdb.c	2009-12-24 17:08:57.000000000 +0000
+++ src/gdb/ax-gdb.c	2009-12-24 18:12:32.000000000 +0000
@@ -1762,30 +1762,19 @@ gen_expr (struct expression *exp, union 
 
     case OP_THIS:
       {
-	char *name;
-	struct frame_info *frame;
+	char *this_name;
 	struct symbol *func, *sym;
 	struct block *b;
 
-	name = current_language->la_name_of_this;
-	if (!name)
-	  error (_("no `this' in current language"));
-
-	frame = get_selected_frame (_("no frame selected"));
-
-	func = get_frame_function (frame);
-	if (!func)
-	  error (_("no `%s' in nameless context"), name);
-
+	func = block_linkage_function (block_for_pc (ax->scope));
+	this_name = language_def (SYMBOL_LANGUAGE (func))->la_name_of_this;
 	b = SYMBOL_BLOCK_VALUE (func);
-	if (dict_empty (BLOCK_DICT (b)))
-	  error (_("no args, no `%s' in block"), name);
 
 	/* Calling lookup_block_symbol is necessary to get the LOC_REGISTER
 	   symbol instead of the LOC_ARG one (if both exist).  */
-	sym = lookup_block_symbol (b, name, NULL, VAR_DOMAIN);
+	sym = lookup_block_symbol (b, this_name, NULL, VAR_DOMAIN);
 	if (!sym)
-	  error (_("no `%s' found"), name);
+	  error (_("no `%s' found"), this_name);
 

 	gen_var_ref (exp->gdbarch, ax, value, sym);
 	(*pc) += 2;

Re: [PATCH] Handle OP_THIS in tracepoints

[Pedro Alves]

On Thursday 24 December 2009 18:18:59, Pedro Alves wrote:

> 2009-12-24  Pedro Alves  <pedro@codesourcery.com>
> 
> 	gdb/
> 	* ax-gdb.c (gen_expr) <OP_THIS>: Lookup `this' in the context of
> 	the tracepoint, not of the selected frame and language.

I've committed this.

-- 
Pedro Alves

Re: [PATCH] Handle OP_THIS in tracepoints

[Stan Shebs]

Pedro Alves wrote:
> On Thursday 24 December 2009 17:57:39, Stan Shebs wrote:
>   
>> Pedro Alves wrote:
>>     
>>> This is busted, and was later fixed in our internal
>>> tree.  :-)  The selected frame, and the current language
>>> don't have anything to do with the context in which
>>> the tracepoint runs.  E.g.:
>>>   
>>>       
>> Darn, I forgot about that one.
>>
>> Oh, but wait a minute!  Issue #6302 says "submission required", and it's 
>> assigned to... you!  And now that the base code is in, 
>>     
>
> Eh, oops, I didn't realize from your post that the
> patch was applied already.
>   
If truth be known, my semi-sorted patch pile is missing the most recent 
changes, otherwise I would have simply folded yours in.  No value in 
sending in every one of our 150-odd local tracepoint changes individually.

Stan