Re: [PATCH] Handle LOC_COMPUTED variables in tracepoint actions

[Tom Tromey <tromey@redhat.com>]

>>>>> "Stan" == Stan Shebs <stan@codesourcery.com> writes:

Stan> The last time that anybody used tracepoints much, stabs was still
Stan> the norm and Dwarf 2 the newbie, and so nobody thought much about
Stan> handling variables with computed locations.

Stan> This patch doesn't handle every possible computed location, that's
Stan> a big project of its own (basically translating arbitrary dwarf2
Stan> bytecode sequences into equivalent agent expression bytecodes)

Fun project :-).  Let me suggest a feature request for this in
bugzilla...

Stan> Even the simplified version is a little arcane, so I'll leave it
Stan> up for comments for a few days before committing.

This patch looks pretty good to me.

Does this handle DW_OP_call_frame_cfa?  I didn't see any explicit
support for it, but I could be missing something.  IIUC, recent versions
of gcc tend to emit this for all local variables.

I also have a couple tiny nits to pick.
  
Stan> + struct agent_expr *
Stan> + gen_trace_for_var (CORE_ADDR scope, struct symbol *var)

Intro comment.

Stan> + static void
Stan> + dwarf_expr_frame_base_1 (struct symbol *framefunc, CORE_ADDR pc,
Stan> + 			 gdb_byte **start, size_t * length);

Extra space before "length".  This occurs twice.

Stan> + 	  /* We don't know what to do with the frame base expression,
Stan> + 	     so we can't trace this variable; give up.  */
Stan> + 	  error (_("Cannot generate expression to collect symbol \"%s\"."),
Stan> + 		 SYMBOL_PRINT_NAME (symbol));

As a user I would appreciate it if this error told me more about what
was wrong.  Like, maybe it could mention that the DWARF expression is
too complicated, or maybe ask me to file a bug report.

Tom