[PATCH] tracepoint: add new trace command "printf"[0] gdb

[PATCH] tracepoint: add new trace command "printf"[0] gdb

[Hui Zhu]

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

Hi,

I add a new patch add new trace command "printf".
This printf is same with the simple gdb command, it can do formatted
output.  But it will happen in gdbserver part when it break by a
tracepoint.
Then the user can get the format value that he want in tracepint.  It
will be more easy and clear to handle the bug sometimes.

For example:
(gdb) trace 16
During symbol reading, DW_AT_name missing from DW_TAG_base_type.
Tracepoint 1 at 0x4004c3: file 1.c, line 16.
(gdb) tvariable $c
Trace state variable $c created, with initial value 0.
(gdb) actions
Enter actions for tracepoint 1, one per line.
End with a line saying just "end".
>printf "%d 0x%lx %d\n",$c=$c+1,$rax,argc
>end
(gdb) target remote localhost:1234
(gdb) tstart
(gdb) c

gdbserver/gdbserver  localhost:1234 ./a.out
Process ./a.out created; pid = 25804
Listening on port 1234
Remote debugging from host 127.0.0.1
1 0x7f2cb8711ec8 1
2 0x7f2cb8711ec8 2
3 0x7f2cb8711ec8 3
4 0x7f2cb8711ec8 4
5 0x7f2cb8711ec8 5
6 0x7f2cb8711ec8 6
7 0x7f2cb8711ec8 7


About the command part, I use the "printf" instead add a new commands
because the behavior of  this command is same with printf. They will
use the same function string_printf(update from ui_printf) to parse
the command arg.

To support the printf command, I add a new agent expression 0x31
printf, the format for it is:
0x31(op_printf) + arg(1/0) + format string with end by 0x0.
The arg is the number of argument of printf.  It will only be 1 (one
argument) or 0 (no argument).  I make it cannot have more than one
argument because I cannot found a good way to handle va_list that send
arguments to vprintf.
The format string with end by 0x0 is the simple format string.  It end
by 0x0 then the gdbserver can use it directly.

I didn't make the patch for doc because I am still not sure about
every part of this patch.  When this patch is OK.  I will make doc
patch for it.

Thanks,
Hui

2011-01-04  Hui Zhu  <teawater@gmail.com>

	* ax-gdb.c (gen_printf_expr_callback): New function.
	* ax-general.c (ax_memcpy): New function.
	(aop_map): Add new entry for "printf".
	(ax_print): Handle "printf".
	(ax_reqs): Ditto.
	* ax.h (agent_op): Add aop_printf.
	(ax_memcpy): Forward declare.
	* printcmd.c (printf_callback): New typedef.
	(string_printf): New function from ui_printf.
	(ui_printf): Call string_printf.
	(printf_command): Remove static.
	* tracepoint.c (printf_command, gen_printf_expr_callback,
	printf_callback, string_printf): Forward declares.
	(validate_actionline, encode_actions_1): handle printf_command.

[-- Attachment #2: tp_print.txt --]
[-- Type: text/plain, Size: 10249 bytes --]

---
 ax-gdb.c     |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ax-general.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++-------
 ax.h         |    3 +++
 printcmd.c   |   41 +++++++++++++++++++++++++++++++++++------
 tracepoint.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 189 insertions(+), 13 deletions(-)

--- a/ax-gdb.c
+++ b/ax-gdb.c
@@ -2435,6 +2435,63 @@ gen_eval_for_expr (CORE_ADDR scope, stru
   return ax;
 }
 
+void
+gen_printf_expr_callback (char *fbuf, char **expp, struct bp_location *loc,
+			  struct agent_expr *aexpr)
+{
+  if (expp)
+    {
+      struct cleanup *old_chain = NULL;
+      struct expression *expr = NULL;
+      union exp_element *pc;
+      struct axs_value value;
+
+      expr = parse_exp_1 (expp, block_for_pc (loc->address), 1);
+      old_chain = make_cleanup (free_current_contents, &expr);
+
+      pc = expr->elts;
+      trace_kludge = 0;
+      value.optimized_out = 0;
+      gen_expr (expr, &pc, aexpr, &value);
+
+
+      if (value.optimized_out)
+        error (_("value has been optimized out"));
+      switch (value.kind)
+        {
+	case axs_lvalue_memory:
+	  {
+	    int length = TYPE_LENGTH (check_typedef (value.type));
+	    switch (length)
+	      {
+	      case 4:
+		ax_simple (aexpr, aop_ref32);
+		break;
+	      case 8:
+		ax_simple (aexpr, aop_ref64);
+		break;
+	      default:
+		error (_("size of value is not OK."));
+		break;
+	      }
+	  }
+	  break;
+	case axs_lvalue_register:
+	  ax_reg (aexpr, value.u.reg);
+	  break;
+        }
+
+      do_cleanups (old_chain);
+    }
+
+  ax_simple (aexpr, aop_printf);
+  if (expp)
+    ax_simple (aexpr, 1);
+  else
+    ax_simple (aexpr, 0);
+  ax_memcpy (aexpr, fbuf, strlen (fbuf) + 1);
+}
+
 static void
 agent_command (char *exp, int from_tty)
 {
--- a/ax-general.c
+++ b/ax-general.c
@@ -313,6 +313,14 @@ ax_tsv (struct agent_expr *x, enum agent
   x->buf[x->len + 2] = (num) & 0xff;
   x->len += 3;
 }
+
+void
+ax_memcpy (struct agent_expr *x, const void *src, size_t n)
+{
+  grow_expr (x, n);
+  memcpy (x->buf + x->len, src, n);
+  x->len += n;
+}
 \f
 
 
@@ -370,6 +378,7 @@ struct aop_map aop_map[] =
   {"tracev", 2, 0, 0, 1},	/* 0x2e */
   {0, 0, 0, 0, 0},		/* 0x2f */
   {"trace16", 2, 0, 1, 1},	/* 0x30 */
+  {"printf", 0, 0, 0, 0},	/* 0x31 */
 };
 
 
@@ -395,6 +404,7 @@ ax_print (struct ui_file *f, struct agen
   for (i = 0; i < x->len;)
     {
       enum agent_op op = x->buf[i];
+      int op_size;
 
       if (op >= (sizeof (aop_map) / sizeof (aop_map[0]))
 	  || !aop_map[op].name)
@@ -403,7 +413,19 @@ ax_print (struct ui_file *f, struct agen
 	  i++;
 	  continue;
 	}
-      if (i + 1 + aop_map[op].op_size > x->len)
+      if (op == aop_printf)
+        {
+	  if (i + 2 >= x->len)
+	    {
+	      fprintf_filtered (f, _("%3d  <bad opcode %02x>\n"), i, op);
+	      i++;
+	      continue;
+	    }
+	  op_size = 1 + strlen (x->buf + i + 2) + 1;
+	}
+      else
+	op_size = aop_map[op].op_size;
+      if (i + 1 + op_size > x->len)
 	{
 	  fprintf_filtered (f, _("%3d  <incomplete opcode %s>\n"),
 			    i, aop_map[op].name);
@@ -411,15 +433,15 @@ ax_print (struct ui_file *f, struct agen
 	}
 
       fprintf_filtered (f, "%3d  %s", i, aop_map[op].name);
-      if (aop_map[op].op_size > 0)
+      if (op_size > 0)
 	{
 	  fputs_filtered (" ", f);
 
 	  print_longest (f, 'd', 0,
-			 read_const (x, i + 1, aop_map[op].op_size));
+			 read_const (x, i + 1, op_size));
 	}
       fprintf_filtered (f, "\n");
-      i += 1 + aop_map[op].op_size;
+      i += 1 + op_size;
 
       is_float = (op == aop_float);
     }
@@ -487,6 +509,8 @@ ax_reqs (struct agent_expr *ax)
   /* Pointer to a description of the present op.  */
   struct aop_map *op;
 
+  int op_size = 0, consumed = 0;
+
   memset (targets, 0, ax->len * sizeof (targets[0]));
   memset (boundary, 0, ax->len * sizeof (boundary[0]));
 
@@ -494,7 +518,7 @@ ax_reqs (struct agent_expr *ax)
   ax->flaw = agent_flaw_none;
   ax->max_data_size = 0;
 
-  for (i = 0; i < ax->len; i += 1 + op->op_size)
+  for (i = 0; i < ax->len; i += 1 + op_size)
     {
       if (ax->buf[i] > (sizeof (aop_map) / sizeof (aop_map[0])))
 	{
@@ -510,7 +534,23 @@ ax_reqs (struct agent_expr *ax)
 	  return;
 	}
 
-      if (i + 1 + op->op_size > ax->len)
+      if (ax->buf[i] == aop_printf)
+        {
+	  if (i + 2 >= ax->len)
+	    {
+	      ax->flaw = agent_flaw_incomplete_instruction;
+	      return;
+	    }
+	  consumed = ax->buf[i + 1];
+	  op_size = 1 + strlen (ax->buf + i + 2) + 1;
+	}
+      else
+        {
+	  op_size = op->op_size;
+	  consumed = op->consumed;
+        }
+
+      if (i + 1 + op_size > ax->len)
 	{
 	  ax->flaw = agent_flaw_incomplete_instruction;
 	  return;
@@ -528,7 +568,7 @@ ax_reqs (struct agent_expr *ax)
       boundary[i] = 1;
       heights[i] = height;
 
-      height -= op->consumed;
+      height -= consumed;
       if (height < ax->min_height)
 	ax->min_height = height;
       height += op->produced;
--- a/ax.h
+++ b/ax.h
@@ -204,6 +204,7 @@ enum agent_op
     aop_setv = 0x2d,
     aop_tracev = 0x2e,
     aop_trace16 = 0x30,
+    aop_printf = 0x31,
     aop_last
   };
 \f
@@ -260,6 +261,8 @@ extern void ax_reg_mask (struct agent_ex
 
 /* Assemble code to operate on a trace state variable.  */
 extern void ax_tsv (struct agent_expr *expr, enum agent_op op, int num);
+
+extern void ax_memcpy (struct agent_expr *x, const void *src, size_t n);
 \f
 
 /* Functions for printing out expressions, and otherwise debugging
--- a/printcmd.c
+++ b/printcmd.c
@@ -1963,10 +1963,13 @@ print_variable_and_value (const char *na
   fprintf_filtered (stream, "\n");
 }
 
-/* printf "printf format string" ARG to STREAM.  */
+typedef void (printf_callback) (char *fbuf, char **expp,
+				struct bp_location *loc,
+				struct agent_expr *aexpr);
 
-static void
-ui_printf (char *arg, struct ui_file *stream)
+void
+string_printf (char *arg, struct ui_file *stream, printf_callback callback,
+	       struct bp_location *loc, struct agent_expr *aexpr)
 {
   char *f = NULL;
   char *s = arg;
@@ -2298,26 +2301,42 @@ ui_printf (char *arg, struct ui_file *st
     /* Now, parse all arguments and evaluate them.
        Store the VALUEs in VAL_ARGS.  */
 
+    if (callback)
+      current_substring = substrings;
     while (*s != '\0')
       {
 	char *s1;
 
+	s1 = s;
 	if (nargs == allocated_args)
 	  val_args = (struct value **) xrealloc ((char *) val_args,
 						 (allocated_args *= 2)
 						 * sizeof (struct value *));
-	s1 = s;
-	val_args[nargs] = parse_to_comma_and_eval (&s1);
+	if (callback)
+	  {
+	    if (nargs >= nargs_wanted)
+	      error (_("Wrong number of arguments for specified "
+		       "format-string"));
+	    callback (current_substring, &s1, loc, aexpr);
+	    current_substring += strlen (current_substring) + 1;
+	  }
+	else
+	  val_args[nargs] = parse_to_comma_and_eval (&s1);
 
 	nargs++;
 	s = s1;
 	if (*s == ',')
 	  s++;
       }
+    if (callback)
+      callback (last_arg, NULL, loc, aexpr);
 
     if (nargs != nargs_wanted)
       error (_("Wrong number of arguments for specified format-string"));
 
+    if (!stream)
+      goto after_print;
+
     /* Now actually print them.  */
     current_substring = substrings;
     for (i = 0; i < nargs; i++)
@@ -2672,12 +2691,22 @@ ui_printf (char *arg, struct ui_file *st
        by default, which will warn here if there is no argument.  */
     fprintf_filtered (stream, last_arg, 0);
   }
+
+after_print:
   do_cleanups (old_cleanups);
 }
 
-/* Implement the "printf" command.  */
+/* printf "printf format string" ARG to STREAM.  */
 
 static void
+ui_printf (char *arg, struct ui_file *stream)
+{
+  string_printf (arg, stream, NULL, NULL, NULL);
+}
+
+/* Implement the "printf" command.  */
+
+void
 printf_command (char *arg, int from_tty)
 {
   ui_printf (arg, gdb_stdout);
--- a/tracepoint.c
+++ b/tracepoint.c
@@ -187,6 +187,15 @@ extern void send_disconnected_tracing_va
 static void free_uploaded_tps (struct uploaded_tp **utpp);
 static void free_uploaded_tsvs (struct uploaded_tsv **utsvp);
 
+extern void printf_command (char *arg, int from_tty);
+extern void gen_printf_expr_callback (char **expp, struct bp_location *loc,
+				      struct agent_expr *aexpr);
+typedef void (printf_callback) (char **expp, struct bp_location *loc,
+				struct agent_expr *aexpr);
+extern void string_printf (char *arg, struct ui_file *stream,
+			   printf_callback callback, struct bp_location *loc,
+			   struct agent_expr *aexpr);
+
 
 extern void _initialize_tracepoint (void);
 
@@ -719,6 +728,28 @@ validate_actionline (char **line, struct
 	error (_("while-stepping step count `%s' is malformed."), *line);
     }
 
+  else if (cmd_cfunc_eq (c, printf_command))
+    {
+      char fbuf[101];
+
+      for (loc = t->loc; loc; loc = loc->next)
+	{
+	  int nargs;
+	  aexpr = new_agent_expr (loc->gdbarch, loc->address);
+	  old_chain = make_cleanup_free_agent_expr (aexpr);
+	  string_printf (p, NULL, gen_printf_expr_callback,
+			 loc, aexpr);
+	  ax_simple (aexpr, aop_end);
+	  /* The agent expr include expr for arguments, format string, 1 byte
+	   * for aop_printf, 1 byte for the number of arguments, 1 byte for
+	   * size of format string, 1 byte for blank after format string
+	   * and 1 byte for aop_end.  */
+	  if (aexpr->len > MAX_AGENT_EXPR_LEN)
+	    error (_("Expression is too complicated."));
+	  do_cleanups (old_chain);
+	}
+    }
+
   else if (cmd_cfunc_eq (c, end_actions_pseudocommand))
     ;
 
@@ -1430,6 +1461,22 @@ encode_actions_1 (struct command_line *a
 	  encode_actions_1 (action->body_list[0], t, tloc, frame_reg,
 			    frame_offset, stepping_list, NULL);
 	}
+      else if (cmd_cfunc_eq (cmd, printf_command))
+	{
+          char fbuf[101];
+	  struct cleanup *old_chain = NULL;
+
+	  aexpr = new_agent_expr (tloc->gdbarch, tloc->address);
+	  old_chain = make_cleanup_free_agent_expr (aexpr);
+	  string_printf (action_exp, NULL, gen_printf_expr_callback,
+			 tloc, aexpr);
+	  ax_simple (aexpr, aop_end);
+
+	  ax_reqs (aexpr);
+	  report_agent_reqs_errors (aexpr);
+	  discard_cleanups (old_chain);
+	  add_aexpr (collect, aexpr);
+	}
       else
 	error (_("Invalid tracepoint command '%s'"), action->line);
     }				/* for */

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Doug Evans]

On Mon, Jan 3, 2011 at 8:29 AM, Hui Zhu <teawater@gmail.com> wrote:
> Hi,
>
> I add a new patch add new trace command "printf".
> This printf is same with the simple gdb command, it can do formatted
> output.  But it will happen in gdbserver part when it break by a
> tracepoint.
> Then the user can get the format value that he want in tracepint.  It
> will be more easy and clear to handle the bug sometimes.

One could do this with a breakpoint and attaching commands to the
breakpoint too, right?
Or are they too cumbersome for the intended use case?
[Extending tracepoints like this doesn't seem justified yet, so I'm
just looking for more data.]

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Hui Zhu]

On Tue, Jan 4, 2011 at 03:21, Doug Evans <dje@google.com> wrote:
> On Mon, Jan 3, 2011 at 8:29 AM, Hui Zhu <teawater@gmail.com> wrote:
>> Hi,
>>
>> I add a new patch add new trace command "printf".
>> This printf is same with the simple gdb command, it can do formatted
>> output.  But it will happen in gdbserver part when it break by a
>> tracepoint.
>> Then the user can get the format value that he want in tracepint.  It
>> will be more easy and clear to handle the bug sometimes.
>
> One could do this with a breakpoint and attaching commands to the
> breakpoint too, right?
> Or are they too cumbersome for the intended use case?
> [Extending tracepoints like this doesn't seem justified yet, so I'm
> just looking for more data.]
>

Thanks Doug.

I agree with the tracepoint "printf" will be very close with add a
breakpoint commands "printf" if the gdb and gdbserver in same pc.

But I have some status need the tracepoint "printf" that breakpoint is
not very fit with.
1. The gdb and gdbserver connect through a low speed net.  Sometimes,
the debug target that I use is in the other side of the earth.
The breakpoint commands "printf" is too slow for that issue, because
each time the inferior is break by the breakpoint, gdbserver need send
the rsp package to gdb, and gdb will get the data that "printf" need
though low speed net from gdbsever.  And sometime, it will disconnect.
But if through tracepoint, I will not have this trouble.  I can "set
disconnected-tracing on" to handle the network disconnect issue.  I
still need to get the value from inferior through tfind and other
commands.  It is still be affect by the low speed network.  So I make
the tracepoint "printf" to handle it.

2.  KGTP(https://code.google.com/p/kgtp/) just support the gdb
tracepoint rsp commands.  For not stop the Linux the Kernel.  It
doesn't support the breakpoint.
So if it want directly show the Kernel val value, it need "printf".
This printf will be very powerful that can set most part of Kernel and
we can set condition for it.
And in https://code.google.com/p/kgtp/wiki/HOWTO#Offline_debug,  we
can dump the gdbrsp package to a file and send to Kernel.  Then kernel
can be debug without a local gdb or a remote connect.   But user still
need copy the trace file to pc that have GDB.  But if support
tracepoint "printf", we will not need do that.


Thanks,
Hui

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Doug Evans]

On Mon, Jan 3, 2011 at 8:33 PM, Hui Zhu <teawater@gmail.com> wrote:
> On Tue, Jan 4, 2011 at 03:21, Doug Evans <dje@google.com> wrote:
>> On Mon, Jan 3, 2011 at 8:29 AM, Hui Zhu <teawater@gmail.com> wrote:
>>> Hi,
>>>
>>> I add a new patch add new trace command "printf".
>>> This printf is same with the simple gdb command, it can do formatted
>>> output.  But it will happen in gdbserver part when it break by a
>>> tracepoint.
>>> Then the user can get the format value that he want in tracepint.  It
>>> will be more easy and clear to handle the bug sometimes.
>>
>> One could do this with a breakpoint and attaching commands to the
>> breakpoint too, right?
>> Or are they too cumbersome for the intended use case?
>> [Extending tracepoints like this doesn't seem justified yet, so I'm
>> just looking for more data.]
>>
>
> Thanks Doug.
>
> I agree with the tracepoint "printf" will be very close with add a
> breakpoint commands "printf" if the gdb and gdbserver in same pc.
>
> But I have some status need the tracepoint "printf" that breakpoint is
> not very fit with.
> 1. The gdb and gdbserver connect through a low speed net.  Sometimes,
> the debug target that I use is in the other side of the earth.
> The breakpoint commands "printf" is too slow for that issue, because
> each time the inferior is break by the breakpoint, gdbserver need send
> the rsp package to gdb, and gdb will get the data that "printf" need
> though low speed net from gdbsever.  And sometime, it will disconnect.
> But if through tracepoint, I will not have this trouble.  I can "set
> disconnected-tracing on" to handle the network disconnect issue.  I
> still need to get the value from inferior through tfind and other
> commands.  It is still be affect by the low speed network.  So I make
> the tracepoint "printf" to handle it.
>
> 2.  KGTP(https://code.google.com/p/kgtp/) just support the gdb
> tracepoint rsp commands.  For not stop the Linux the Kernel.  It
> doesn't support the breakpoint.
> So if it want directly show the Kernel val value, it need "printf".
> This printf will be very powerful that can set most part of Kernel and
> we can set condition for it.
> And in https://code.google.com/p/kgtp/wiki/HOWTO#Offline_debug,  we
> can dump the gdbrsp package to a file and send to Kernel.  Then kernel
> can be debug without a local gdb or a remote connect.   But user still
> need copy the trace file to pc that have GDB.  But if support
> tracepoint "printf", we will not need do that.

Thanks for the additional info.
I still think this is kinda orthogonal to tracepoint functionality,
and thus implementing it on top of tracepoints is a hack, but I
understand better why you want it.

[for reference sake]
To me this is a subset of a bigger feature set that is missing:
partitioning of the things that can be accomplished by gdbserver from
the setup that is needed (IOW separate the heavy lifting of parsing
debug info and translating a user query into, for example, an agent
expression (the gdb side) from the processing of that query when the
breakpoint(/tracepoint) is hit (the gdbserver side).
Plus it might be useful to not require a gdb/gdbserver connection to
get things started, e.g., convey the tracepoint info (and anything
else) to gdbserver from a local source.
[I'm using "query" loosely here.  I'm using "gdbserver" loosely too:
anything that looks like gdbserver to gdb will do.]

AIUI, agent expressions are currently only used for tracepoints (I
thought they might be used for fast conditional breakpoints but I
can't find it).  [If I'm wrong and agent exprs are used for other
things, great, we're already down this path.]  Are they useful for
more things?  [I don't know what, but if printf is useful, I can
imagine there being more things that are useful too.]
Thus it might be useful to take a step back before adding printf to tracepoints.
E.g. Another way to go would be to have a new kind of command list
that can be attached to breakpoints, commands that are executed on the
gdbserver side.

[One might think why not just add printf (and whatever else) to
tracepoints and leave it at that.  Tracepoints to me convey a specific
use-case and I'm not sure we should muddy that up.  But for now I
suppose printf could be sufficiently useful, so I'm not opposed to the
patch (pragmatic hacks are sometimes useful enough to justify their
existence).  This is not an approval though.   I can see the patch
needs at least a few changes, but before reviewing it I'd like to make
sure there is general agreement on this approach.  Someone else is
free to review and approve it of course.]

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Hui Zhu]

On Tue, Jan 4, 2011 at 14:18, Doug Evans <dje@google.com> wrote:
> On Mon, Jan 3, 2011 at 8:33 PM, Hui Zhu <teawater@gmail.com> wrote:
>> On Tue, Jan 4, 2011 at 03:21, Doug Evans <dje@google.com> wrote:
>>> On Mon, Jan 3, 2011 at 8:29 AM, Hui Zhu <teawater@gmail.com> wrote:
>>>> Hi,
>>>>
>>>> I add a new patch add new trace command "printf".
>>>> This printf is same with the simple gdb command, it can do formatted
>>>> output.  But it will happen in gdbserver part when it break by a
>>>> tracepoint.
>>>> Then the user can get the format value that he want in tracepint.  It
>>>> will be more easy and clear to handle the bug sometimes.
>>>
>>> One could do this with a breakpoint and attaching commands to the
>>> breakpoint too, right?
>>> Or are they too cumbersome for the intended use case?
>>> [Extending tracepoints like this doesn't seem justified yet, so I'm
>>> just looking for more data.]
>>>
>>
>> Thanks Doug.
>>
>> I agree with the tracepoint "printf" will be very close with add a
>> breakpoint commands "printf" if the gdb and gdbserver in same pc.
>>
>> But I have some status need the tracepoint "printf" that breakpoint is
>> not very fit with.
>> 1. The gdb and gdbserver connect through a low speed net.  Sometimes,
>> the debug target that I use is in the other side of the earth.
>> The breakpoint commands "printf" is too slow for that issue, because
>> each time the inferior is break by the breakpoint, gdbserver need send
>> the rsp package to gdb, and gdb will get the data that "printf" need
>> though low speed net from gdbsever.  And sometime, it will disconnect.
>> But if through tracepoint, I will not have this trouble.  I can "set
>> disconnected-tracing on" to handle the network disconnect issue.  I
>> still need to get the value from inferior through tfind and other
>> commands.  It is still be affect by the low speed network.  So I make
>> the tracepoint "printf" to handle it.
>>
>> 2.  KGTP(https://code.google.com/p/kgtp/) just support the gdb
>> tracepoint rsp commands.  For not stop the Linux the Kernel.  It
>> doesn't support the breakpoint.
>> So if it want directly show the Kernel val value, it need "printf".
>> This printf will be very powerful that can set most part of Kernel and
>> we can set condition for it.
>> And in https://code.google.com/p/kgtp/wiki/HOWTO#Offline_debug,  we
>> can dump the gdbrsp package to a file and send to Kernel.  Then kernel
>> can be debug without a local gdb or a remote connect.   But user still
>> need copy the trace file to pc that have GDB.  But if support
>> tracepoint "printf", we will not need do that.
>
> Thanks for the additional info.
> I still think this is kinda orthogonal to tracepoint functionality,
> and thus implementing it on top of tracepoints is a hack, but I
> understand better why you want it.
>
> [for reference sake]
> To me this is a subset of a bigger feature set that is missing:
> partitioning of the things that can be accomplished by gdbserver from
> the setup that is needed (IOW separate the heavy lifting of parsing
> debug info and translating a user query into, for example, an agent
> expression (the gdb side) from the processing of that query when the
> breakpoint(/tracepoint) is hit (the gdbserver side).
> Plus it might be useful to not require a gdb/gdbserver connection to
> get things started, e.g., convey the tracepoint info (and anything
> else) to gdbserver from a local source.
> [I'm using "query" loosely here.  I'm using "gdbserver" loosely too:
> anything that looks like gdbserver to gdb will do.]
>
> AIUI, agent expressions are currently only used for tracepoints (I
> thought they might be used for fast conditional breakpoints but I
> can't find it).  [If I'm wrong and agent exprs are used for other
> things, great, we're already down this path.]  Are they useful for
> more things?  [I don't know what, but if printf is useful, I can
> imagine there being more things that are useful too.]
> Thus it might be useful to take a step back before adding printf to tracepoints.
> E.g. Another way to go would be to have a new kind of command list
> that can be attached to breakpoints, commands that are executed on the
> gdbserver side.
>
> [One might think why not just add printf (and whatever else) to
> tracepoints and leave it at that.  Tracepoints to me convey a specific
> use-case and I'm not sure we should muddy that up.  But for now I
> suppose printf could be sufficiently useful, so I'm not opposed to the
> patch (pragmatic hacks are sometimes useful enough to justify their
> existence).  This is not an approval though.   I can see the patch
> needs at least a few changes, but before reviewing it I'd like to make
> sure there is general agreement on this approach.  Someone else is
> free to review and approve it of course.]
>

That is great.  Thanks Doug.,

Best,
Hui

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Doug Evans]

On Mon, Jan 3, 2011 at 10:18 PM, Doug Evans <dje@google.com> wrote:
> [One might think why not just add printf (and whatever else) to
> tracepoints and leave it at that.  Tracepoints to me convey a specific
> use-case and I'm not sure we should muddy that up.  But for now I
> suppose printf could be sufficiently useful, so I'm not opposed to the
> patch (pragmatic hacks are sometimes useful enough to justify their
> existence).  This is not an approval though.   I can see the patch
> needs at least a few changes, but before reviewing it I'd like to make
> sure there is general agreement on this approach.  Someone else is
> free to review and approve it of course.]

I haven't heard comments from any other GMs.
Does anyone have a problem with adding some kind of printf to tracepoints?
Or does anyone have a problem with adding a new kind of command list
to breakpoints that is executed on the target?
[P.S. If you respond, IWBN to include your thoughts on why.]
I'm inclined to go with having some kind of printf in tracepoints for now.

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Michael Snyder]

Doug Evans wrote:
> On Mon, Jan 3, 2011 at 10:18 PM, Doug Evans <dje@google.com> wrote:
>> [One might think why not just add printf (and whatever else) to
>> tracepoints and leave it at that.  Tracepoints to me convey a specific
>> use-case and I'm not sure we should muddy that up.  But for now I
>> suppose printf could be sufficiently useful, so I'm not opposed to the
>> patch (pragmatic hacks are sometimes useful enough to justify their
>> existence).  This is not an approval though.   I can see the patch
>> needs at least a few changes, but before reviewing it I'd like to make
>> sure there is general agreement on this approach.  Someone else is
>> free to review and approve it of course.]
> 
> I haven't heard comments from any other GMs.
> Does anyone have a problem with adding some kind of printf to tracepoints?
> Or does anyone have a problem with adding a new kind of command list
> to breakpoints that is executed on the target?
> [P.S. If you respond, IWBN to include your thoughts on why.]
> I'm inclined to go with having some kind of printf in tracepoints for now.

I don't quite understand the "use case" for printf in tracepoints.

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Stan Shebs]

On 1/3/11 10:18 PM, Doug Evans wrote:
> [for reference sake]
> To me this is a subset of a bigger feature set that is missing:
> partitioning of the things that can be accomplished by gdbserver from
> the setup that is needed (IOW separate the heavy lifting of parsing
> debug info and translating a user query into, for example, an agent
> expression (the gdb side) from the processing of that query when the
> breakpoint(/tracepoint) is hit (the gdbserver side).
> Plus it might be useful to not require a gdb/gdbserver connection to
> get things started, e.g., convey the tracepoint info (and anything
> else) to gdbserver from a local source.
> [I'm using "query" loosely here.  I'm using "gdbserver" loosely too:
> anything that looks like gdbserver to gdb will do.]

I'm actually working on a contract proposal to do a bunch of work in 
this area.

One of the specific ideas is to introduce a "dynamic printf" that works 
somewhat like what I think Hui Zhu was wanting; it stops the program at 
a location, runs the printf in gdbserver, and then continues.

Another one of the ideas is to use agent expressions to do target-side 
conditional breakpoints.  This is especially compelling for many-core 
targets, where we don't want 100 threads on 100 cores to be trying to 
get GDB to do 100 conditional expression evaluations all at once.

I didn't specifically propose to go beyond that, into general 
partitioning of command lists between host and target, although it's a 
very interesting direction.  The idea gets me to thinking about whether 
we should keep the command list form, or raise it to the level of a real 
language, or maybe support several - Mentor's EDGE debugger for instance 
uses a C syntax for its "codelets" ( 
http://www.mentor.com/embedded-software/resources/overview/codelets-15b9eaed-8e4b-43a2-aad5-c189cd7f2d68 
) even though they run on the host, and whether the language is C or 
Python, it seems useful to be able to inject real code bits into the 
target system.

Anyway, if we get the contract (fingers crossed!) then I expect we'll be 
putting up some proposals for discussion within the next couple of months.

I'll comment on the patch in a different message.

Stan

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Hui Zhu]

On Thu, Jan 6, 2011 at 02:17, Michael Snyder <msnyder@vmware.com> wrote:
> Doug Evans wrote:
>>
>> On Mon, Jan 3, 2011 at 10:18 PM, Doug Evans <dje@google.com> wrote:
>>>
>>> [One might think why not just add printf (and whatever else) to
>>> tracepoints and leave it at that.  Tracepoints to me convey a specific
>>> use-case and I'm not sure we should muddy that up.  But for now I
>>> suppose printf could be sufficiently useful, so I'm not opposed to the
>>> patch (pragmatic hacks are sometimes useful enough to justify their
>>> existence).  This is not an approval though.   I can see the patch
>>> needs at least a few changes, but before reviewing it I'd like to make
>>> sure there is general agreement on this approach.  Someone else is
>>> free to review and approve it of course.]
>>
>> I haven't heard comments from any other GMs.
>> Does anyone have a problem with adding some kind of printf to tracepoints?
>> Or does anyone have a problem with adding a new kind of command list
>> to breakpoints that is executed on the target?
>> [P.S. If you respond, IWBN to include your thoughts on why.]
>> I'm inclined to go with having some kind of printf in tracepoints for now.
>
> I don't quite understand the "use case" for printf in tracepoints.
>
>

http://sourceware.org/ml/gdb-patches/2011-01/msg00052.html

Wish this link can be helpful.  :)

Thanks,
Hui

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Hui Zhu]

On Thu, Jan 6, 2011 at 04:51, Stan Shebs <stan@codesourcery.com> wrote:
> On 1/3/11 10:18 PM, Doug Evans wrote:
>>
>> [for reference sake]
>> To me this is a subset of a bigger feature set that is missing:
>> partitioning of the things that can be accomplished by gdbserver from
>> the setup that is needed (IOW separate the heavy lifting of parsing
>> debug info and translating a user query into, for example, an agent
>> expression (the gdb side) from the processing of that query when the
>> breakpoint(/tracepoint) is hit (the gdbserver side).
>> Plus it might be useful to not require a gdb/gdbserver connection to
>> get things started, e.g., convey the tracepoint info (and anything
>> else) to gdbserver from a local source.
>> [I'm using "query" loosely here.  I'm using "gdbserver" loosely too:
>> anything that looks like gdbserver to gdb will do.]
>
> I'm actually working on a contract proposal to do a bunch of work in this
> area.
>
> One of the specific ideas is to introduce a "dynamic printf" that works
> somewhat like what I think Hui Zhu was wanting; it stops the program at a
> location, runs the printf in gdbserver, and then continues.
>
> Another one of the ideas is to use agent expressions to do target-side
> conditional breakpoints.  This is especially compelling for many-core
> targets, where we don't want 100 threads on 100 cores to be trying to get
> GDB to do 100 conditional expression evaluations all at once.
>
> I didn't specifically propose to go beyond that, into general partitioning
> of command lists between host and target, although it's a very interesting
> direction.  The idea gets me to thinking about whether we should keep the
> command list form, or raise it to the level of a real language, or maybe
> support several - Mentor's EDGE debugger for instance uses a C syntax for
> its "codelets" (
> http://www.mentor.com/embedded-software/resources/overview/codelets-15b9eaed-8e4b-43a2-aad5-c189cd7f2d68
> ) even though they run on the host, and whether the language is C or Python,
> it seems useful to be able to inject real code bits into the target system.
>
> Anyway, if we get the contract (fingers crossed!) then I expect we'll be
> putting up some proposals for discussion within the next couple of months.
>
> I'll comment on the patch in a different message.
>
> Stan
>
>

That will be great!  Thanks Stan.

Hui

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Hui Zhu]

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

On Thu, Jan 6, 2011 at 14:43, Hui Zhu <teawater@gmail.com> wrote:
> On Thu, Jan 6, 2011 at 04:51, Stan Shebs <stan@codesourcery.com> wrote:
>> On 1/3/11 10:18 PM, Doug Evans wrote:
>>>
>>> [for reference sake]
>>> To me this is a subset of a bigger feature set that is missing:
>>> partitioning of the things that can be accomplished by gdbserver from
>>> the setup that is needed (IOW separate the heavy lifting of parsing
>>> debug info and translating a user query into, for example, an agent
>>> expression (the gdb side) from the processing of that query when the
>>> breakpoint(/tracepoint) is hit (the gdbserver side).
>>> Plus it might be useful to not require a gdb/gdbserver connection to
>>> get things started, e.g., convey the tracepoint info (and anything
>>> else) to gdbserver from a local source.
>>> [I'm using "query" loosely here.  I'm using "gdbserver" loosely too:
>>> anything that looks like gdbserver to gdb will do.]
>>
>> I'm actually working on a contract proposal to do a bunch of work in this
>> area.
>>
>> One of the specific ideas is to introduce a "dynamic printf" that works
>> somewhat like what I think Hui Zhu was wanting; it stops the program at a
>> location, runs the printf in gdbserver, and then continues.
>>
>> Another one of the ideas is to use agent expressions to do target-side
>> conditional breakpoints.  This is especially compelling for many-core
>> targets, where we don't want 100 threads on 100 cores to be trying to get
>> GDB to do 100 conditional expression evaluations all at once.
>>
>> I didn't specifically propose to go beyond that, into general partitioning
>> of command lists between host and target, although it's a very interesting
>> direction.  The idea gets me to thinking about whether we should keep the
>> command list form, or raise it to the level of a real language, or maybe
>> support several - Mentor's EDGE debugger for instance uses a C syntax for
>> its "codelets" (
>> http://www.mentor.com/embedded-software/resources/overview/codelets-15b9eaed-8e4b-43a2-aad5-c189cd7f2d68
>> ) even though they run on the host, and whether the language is C or Python,
>> it seems useful to be able to inject real code bits into the target system.
>>
>> Anyway, if we get the contract (fingers crossed!) then I expect we'll be
>> putting up some proposals for discussion within the next couple of months.
>>
>> I'll comment on the patch in a different message.
>>
>> Stan
>>
>>
>
> That will be great!  Thanks Stan.
>
> Hui
>

Update follow the trunk.

Thanks,
Hui

2011-01-28  Hui Zhu  <teawater@gmail.com>

	* ax-gdb.c (gen_printf_expr_callback): New function.
	* ax-general.c (ax_memcpy): New function.
	(aop_map): Add new entry for "printf".
	(ax_print): Handle "printf".
	(ax_reqs): Ditto.
	* ax.h (agent_op): Add aop_printf.
	(ax_memcpy): Forward declare.
	* printcmd.c (printf_callback): New typedef.
	(string_printf): New function from ui_printf.
	(ui_printf): Call string_printf.
	(printf_command): Remove static.
	* tracepoint.c (printf_command, gen_printf_expr_callback,
	printf_callback, string_printf): Forward declares.
	(validate_actionline, encode_actions_1): handle printf_command.

[-- Attachment #2: tp_print.txt --]
[-- Type: text/plain, Size: 10249 bytes --]

---
 ax-gdb.c     |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ax-general.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++-------
 ax.h         |    3 +++
 printcmd.c   |   41 +++++++++++++++++++++++++++++++++++------
 tracepoint.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 189 insertions(+), 13 deletions(-)

--- a/ax-gdb.c
+++ b/ax-gdb.c
@@ -2446,6 +2446,63 @@ gen_eval_for_expr (CORE_ADDR scope, stru
   return ax;
 }
 
+void
+gen_printf_expr_callback (char *fbuf, char **expp, struct bp_location *loc,
+			  struct agent_expr *aexpr)
+{
+  if (expp)
+    {
+      struct cleanup *old_chain = NULL;
+      struct expression *expr = NULL;
+      union exp_element *pc;
+      struct axs_value value;
+
+      expr = parse_exp_1 (expp, block_for_pc (loc->address), 1);
+      old_chain = make_cleanup (free_current_contents, &expr);
+
+      pc = expr->elts;
+      trace_kludge = 0;
+      value.optimized_out = 0;
+      gen_expr (expr, &pc, aexpr, &value);
+
+
+      if (value.optimized_out)
+        error (_("value has been optimized out"));
+      switch (value.kind)
+        {
+	case axs_lvalue_memory:
+	  {
+	    int length = TYPE_LENGTH (check_typedef (value.type));
+	    switch (length)
+	      {
+	      case 4:
+		ax_simple (aexpr, aop_ref32);
+		break;
+	      case 8:
+		ax_simple (aexpr, aop_ref64);
+		break;
+	      default:
+		error (_("size of value is not OK."));
+		break;
+	      }
+	  }
+	  break;
+	case axs_lvalue_register:
+	  ax_reg (aexpr, value.u.reg);
+	  break;
+        }
+
+      do_cleanups (old_chain);
+    }
+
+  ax_simple (aexpr, aop_printf);
+  if (expp)
+    ax_simple (aexpr, 1);
+  else
+    ax_simple (aexpr, 0);
+  ax_memcpy (aexpr, fbuf, strlen (fbuf) + 1);
+}
+
 static void
 agent_command (char *exp, int from_tty)
 {
--- a/ax-general.c
+++ b/ax-general.c
@@ -319,6 +319,14 @@ ax_tsv (struct agent_expr *x, enum agent
   x->buf[x->len + 2] = (num) & 0xff;
   x->len += 3;
 }
+
+void
+ax_memcpy (struct agent_expr *x, const void *src, size_t n)
+{
+  grow_expr (x, n);
+  memcpy (x->buf + x->len, src, n);
+  x->len += n;
+}
 \f
 
 
@@ -376,6 +384,7 @@ struct aop_map aop_map[] =
   {"tracev", 2, 0, 0, 1},	/* 0x2e */
   {0, 0, 0, 0, 0},		/* 0x2f */
   {"trace16", 2, 0, 1, 1},	/* 0x30 */
+  {"printf", 0, 0, 0, 0},	/* 0x31 */
 };
 
 
@@ -401,6 +410,7 @@ ax_print (struct ui_file *f, struct agen
   for (i = 0; i < x->len;)
     {
       enum agent_op op = x->buf[i];
+      int op_size;
 
       if (op >= (sizeof (aop_map) / sizeof (aop_map[0]))
 	  || !aop_map[op].name)
@@ -409,7 +419,19 @@ ax_print (struct ui_file *f, struct agen
 	  i++;
 	  continue;
 	}
-      if (i + 1 + aop_map[op].op_size > x->len)
+      if (op == aop_printf)
+        {
+	  if (i + 2 >= x->len)
+	    {
+	      fprintf_filtered (f, _("%3d  <bad opcode %02x>\n"), i, op);
+	      i++;
+	      continue;
+	    }
+	  op_size = 1 + strlen (x->buf + i + 2) + 1;
+	}
+      else
+	op_size = aop_map[op].op_size;
+      if (i + 1 + op_size > x->len)
 	{
 	  fprintf_filtered (f, _("%3d  <incomplete opcode %s>\n"),
 			    i, aop_map[op].name);
@@ -417,15 +439,15 @@ ax_print (struct ui_file *f, struct agen
 	}
 
       fprintf_filtered (f, "%3d  %s", i, aop_map[op].name);
-      if (aop_map[op].op_size > 0)
+      if (op_size > 0)
 	{
 	  fputs_filtered (" ", f);
 
 	  print_longest (f, 'd', 0,
-			 read_const (x, i + 1, aop_map[op].op_size));
+			 read_const (x, i + 1, op_size));
 	}
       fprintf_filtered (f, "\n");
-      i += 1 + aop_map[op].op_size;
+      i += 1 + op_size;
 
       is_float = (op == aop_float);
     }
@@ -493,6 +515,8 @@ ax_reqs (struct agent_expr *ax)
   /* Pointer to a description of the present op.  */
   struct aop_map *op;
 
+  int op_size = 0, consumed = 0;
+
   memset (targets, 0, ax->len * sizeof (targets[0]));
   memset (boundary, 0, ax->len * sizeof (boundary[0]));
 
@@ -500,7 +524,7 @@ ax_reqs (struct agent_expr *ax)
   ax->flaw = agent_flaw_none;
   ax->max_data_size = 0;
 
-  for (i = 0; i < ax->len; i += 1 + op->op_size)
+  for (i = 0; i < ax->len; i += 1 + op_size)
     {
       if (ax->buf[i] > (sizeof (aop_map) / sizeof (aop_map[0])))
 	{
@@ -516,7 +540,23 @@ ax_reqs (struct agent_expr *ax)
 	  return;
 	}
 
-      if (i + 1 + op->op_size > ax->len)
+      if (ax->buf[i] == aop_printf)
+        {
+	  if (i + 2 >= ax->len)
+	    {
+	      ax->flaw = agent_flaw_incomplete_instruction;
+	      return;
+	    }
+	  consumed = ax->buf[i + 1];
+	  op_size = 1 + strlen (ax->buf + i + 2) + 1;
+	}
+      else
+        {
+	  op_size = op->op_size;
+	  consumed = op->consumed;
+        }
+
+      if (i + 1 + op_size > ax->len)
 	{
 	  ax->flaw = agent_flaw_incomplete_instruction;
 	  return;
@@ -534,7 +574,7 @@ ax_reqs (struct agent_expr *ax)
       boundary[i] = 1;
       heights[i] = height;
 
-      height -= op->consumed;
+      height -= consumed;
       if (height < ax->min_height)
 	ax->min_height = height;
       height += op->produced;
--- a/ax.h
+++ b/ax.h
@@ -204,6 +204,7 @@ enum agent_op
     aop_setv = 0x2d,
     aop_tracev = 0x2e,
     aop_trace16 = 0x30,
+    aop_printf = 0x31,
     aop_last
   };
 \f
@@ -260,6 +261,8 @@ extern void ax_reg_mask (struct agent_ex
 
 /* Assemble code to operate on a trace state variable.  */
 extern void ax_tsv (struct agent_expr *expr, enum agent_op op, int num);
+
+extern void ax_memcpy (struct agent_expr *x, const void *src, size_t n);
 \f
 
 /* Functions for printing out expressions, and otherwise debugging
--- a/printcmd.c
+++ b/printcmd.c
@@ -1958,10 +1958,13 @@ print_variable_and_value (const char *na
   fprintf_filtered (stream, "\n");
 }
 
-/* printf "printf format string" ARG to STREAM.  */
+typedef void (printf_callback) (char *fbuf, char **expp,
+				struct bp_location *loc,
+				struct agent_expr *aexpr);
 
-static void
-ui_printf (char *arg, struct ui_file *stream)
+void
+string_printf (char *arg, struct ui_file *stream, printf_callback callback,
+	       struct bp_location *loc, struct agent_expr *aexpr)
 {
   char *f = NULL;
   char *s = arg;
@@ -2294,26 +2297,42 @@ ui_printf (char *arg, struct ui_file *st
     /* Now, parse all arguments and evaluate them.
        Store the VALUEs in VAL_ARGS.  */
 
+    if (callback)
+      current_substring = substrings;
     while (*s != '\0')
       {
 	char *s1;
 
+	s1 = s;
 	if (nargs == allocated_args)
 	  val_args = (struct value **) xrealloc ((char *) val_args,
 						 (allocated_args *= 2)
 						 * sizeof (struct value *));
-	s1 = s;
-	val_args[nargs] = parse_to_comma_and_eval (&s1);
+	if (callback)
+	  {
+	    if (nargs >= nargs_wanted)
+	      error (_("Wrong number of arguments for specified "
+		       "format-string"));
+	    callback (current_substring, &s1, loc, aexpr);
+	    current_substring += strlen (current_substring) + 1;
+	  }
+	else
+	  val_args[nargs] = parse_to_comma_and_eval (&s1);
 
 	nargs++;
 	s = s1;
 	if (*s == ',')
 	  s++;
       }
+    if (callback)
+      callback (last_arg, NULL, loc, aexpr);
 
     if (nargs != nargs_wanted)
       error (_("Wrong number of arguments for specified format-string"));
 
+    if (!stream)
+      goto after_print;
+
     /* Now actually print them.  */
     current_substring = substrings;
     for (i = 0; i < nargs; i++)
@@ -2668,12 +2687,22 @@ ui_printf (char *arg, struct ui_file *st
        by default, which will warn here if there is no argument.  */
     fprintf_filtered (stream, last_arg, 0);
   }
+
+after_print:
   do_cleanups (old_cleanups);
 }
 
-/* Implement the "printf" command.  */
+/* printf "printf format string" ARG to STREAM.  */
 
 static void
+ui_printf (char *arg, struct ui_file *stream)
+{
+  string_printf (arg, stream, NULL, NULL, NULL);
+}
+
+/* Implement the "printf" command.  */
+
+void
 printf_command (char *arg, int from_tty)
 {
   ui_printf (arg, gdb_stdout);
--- a/tracepoint.c
+++ b/tracepoint.c
@@ -187,6 +187,15 @@ extern void send_disconnected_tracing_va
 static void free_uploaded_tps (struct uploaded_tp **utpp);
 static void free_uploaded_tsvs (struct uploaded_tsv **utsvp);
 
+extern void printf_command (char *arg, int from_tty);
+extern void gen_printf_expr_callback (char **expp, struct bp_location *loc,
+				      struct agent_expr *aexpr);
+typedef void (printf_callback) (char **expp, struct bp_location *loc,
+				struct agent_expr *aexpr);
+extern void string_printf (char *arg, struct ui_file *stream,
+			   printf_callback callback, struct bp_location *loc,
+			   struct agent_expr *aexpr);
+
 
 extern void _initialize_tracepoint (void);
 
@@ -725,6 +734,28 @@ validate_actionline (char **line, struct
 	error (_("while-stepping step count `%s' is malformed."), *line);
     }
 
+  else if (cmd_cfunc_eq (c, printf_command))
+    {
+      char fbuf[101];
+
+      for (loc = t->loc; loc; loc = loc->next)
+	{
+	  int nargs;
+	  aexpr = new_agent_expr (loc->gdbarch, loc->address);
+	  old_chain = make_cleanup_free_agent_expr (aexpr);
+	  string_printf (p, NULL, gen_printf_expr_callback,
+			 loc, aexpr);
+	  ax_simple (aexpr, aop_end);
+	  /* The agent expr include expr for arguments, format string, 1 byte
+	   * for aop_printf, 1 byte for the number of arguments, 1 byte for
+	   * size of format string, 1 byte for blank after format string
+	   * and 1 byte for aop_end.  */
+	  if (aexpr->len > MAX_AGENT_EXPR_LEN)
+	    error (_("Expression is too complicated."));
+	  do_cleanups (old_chain);
+	}
+    }
+
   else if (cmd_cfunc_eq (c, end_actions_pseudocommand))
     ;
 
@@ -1436,6 +1467,22 @@ encode_actions_1 (struct command_line *a
 	  encode_actions_1 (action->body_list[0], t, tloc, frame_reg,
 			    frame_offset, stepping_list, NULL);
 	}
+      else if (cmd_cfunc_eq (cmd, printf_command))
+	{
+          char fbuf[101];
+	  struct cleanup *old_chain = NULL;
+
+	  aexpr = new_agent_expr (tloc->gdbarch, tloc->address);
+	  old_chain = make_cleanup_free_agent_expr (aexpr);
+	  string_printf (action_exp, NULL, gen_printf_expr_callback,
+			 tloc, aexpr);
+	  ax_simple (aexpr, aop_end);
+
+	  ax_reqs (aexpr);
+	  report_agent_reqs_errors (aexpr);
+	  discard_cleanups (old_chain);
+	  add_aexpr (collect, aexpr);
+	}
       else
 	error (_("Invalid tracepoint command '%s'"), action->line);
     }				/* for */

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Hui Zhu]

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

The prev version cannot support %s.  So make a new version to support it.

And I think current way to handle printf is too much hack way.  Maybe
I need find out a more better way to handle the printf.

Thanks,
Hui

2011-02-04  Hui Zhu  <teawater@gmail.com>

	* ax-gdb.c (gen_printf_expr_callback): New function.
	* ax-general.c (ax_memcpy): New function.
	(aop_map): Add new entry for "printf".
	(ax_print): Handle "printf".
	(ax_reqs): Ditto.
	* ax.h (agent_op): Add aop_printf.
	(ax_memcpy): Forward declare.
	* printcmd.c (printf_callback): New typedef.
	(string_printf): New function from ui_printf.
	(ui_printf): Call string_printf.
	(printf_command): Remove static.
	* tracepoint.c (printf_command, gen_printf_expr_callback,
	printf_callback, string_printf): Forward declares.
	(validate_actionline, encode_actions_1): handle printf_command.

[-- Attachment #2: tp_print.txt --]
[-- Type: text/plain, Size: 10296 bytes --]

---
 ax-gdb.c     |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ax-general.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++-------
 ax.h         |    3 +++
 printcmd.c   |   41 +++++++++++++++++++++++++++++++++++------
 tracepoint.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 190 insertions(+), 13 deletions(-)

--- a/ax-gdb.c
+++ b/ax-gdb.c
@@ -2446,6 +2446,64 @@ gen_eval_for_expr (CORE_ADDR scope, stru
   return ax;
 }
 
+void
+gen_printf_expr_callback (char *fbuf, char **expp, struct bp_location *loc,
+			  struct agent_expr *aexpr)
+{
+  if (expp)
+    {
+      struct cleanup *old_chain = NULL;
+      struct expression *expr = NULL;
+      union exp_element *pc;
+      struct axs_value value;
+
+      expr = parse_exp_1 (expp, block_for_pc (loc->address), 1);
+      old_chain = make_cleanup (free_current_contents, &expr);
+
+      pc = expr->elts;
+      trace_kludge = 0;
+      value.optimized_out = 0;
+      gen_expr (expr, &pc, aexpr, &value);
+
+
+      if (value.optimized_out)
+        error (_("value has been optimized out"));
+      switch (value.kind)
+        {
+	case axs_lvalue_memory:
+	  if (TYPE_CODE (value.type) != TYPE_CODE_ARRAY)
+	    {
+	      int length = TYPE_LENGTH (check_typedef (value.type));
+	      switch (length)
+		{
+		case 4:
+		  ax_simple (aexpr, aop_ref32);
+		  break;
+		case 8:
+		  ax_simple (aexpr, aop_ref64);
+		  break;
+		default:
+		  error (_("Size of value is not OK."));
+		  break;
+		}
+	    }
+	  break;
+	case axs_lvalue_register:
+	  ax_reg (aexpr, value.u.reg);
+	  break;
+        }
+
+      do_cleanups (old_chain);
+    }
+
+  ax_simple (aexpr, aop_printf);
+  if (expp)
+    ax_simple (aexpr, 1);
+  else
+    ax_simple (aexpr, 0);
+  ax_memcpy (aexpr, fbuf, strlen (fbuf) + 1);
+}
+
 static void
 agent_command (char *exp, int from_tty)
 {
--- a/ax-general.c
+++ b/ax-general.c
@@ -319,6 +319,14 @@ ax_tsv (struct agent_expr *x, enum agent
   x->buf[x->len + 2] = (num) & 0xff;
   x->len += 3;
 }
+
+void
+ax_memcpy (struct agent_expr *x, const void *src, size_t n)
+{
+  grow_expr (x, n);
+  memcpy (x->buf + x->len, src, n);
+  x->len += n;
+}
 \f
 
 
@@ -376,6 +384,7 @@ struct aop_map aop_map[] =
   {"tracev", 2, 0, 0, 1},	/* 0x2e */
   {0, 0, 0, 0, 0},		/* 0x2f */
   {"trace16", 2, 0, 1, 1},	/* 0x30 */
+  {"printf", 0, 0, 0, 0},	/* 0x31 */
 };
 
 
@@ -401,6 +410,7 @@ ax_print (struct ui_file *f, struct agen
   for (i = 0; i < x->len;)
     {
       enum agent_op op = x->buf[i];
+      int op_size;
 
       if (op >= (sizeof (aop_map) / sizeof (aop_map[0]))
 	  || !aop_map[op].name)
@@ -409,7 +419,19 @@ ax_print (struct ui_file *f, struct agen
 	  i++;
 	  continue;
 	}
-      if (i + 1 + aop_map[op].op_size > x->len)
+      if (op == aop_printf)
+        {
+	  if (i + 2 >= x->len)
+	    {
+	      fprintf_filtered (f, _("%3d  <bad opcode %02x>\n"), i, op);
+	      i++;
+	      continue;
+	    }
+	  op_size = 1 + strlen (x->buf + i + 2) + 1;
+	}
+      else
+	op_size = aop_map[op].op_size;
+      if (i + 1 + op_size > x->len)
 	{
 	  fprintf_filtered (f, _("%3d  <incomplete opcode %s>\n"),
 			    i, aop_map[op].name);
@@ -417,15 +439,15 @@ ax_print (struct ui_file *f, struct agen
 	}
 
       fprintf_filtered (f, "%3d  %s", i, aop_map[op].name);
-      if (aop_map[op].op_size > 0)
+      if (op_size > 0)
 	{
 	  fputs_filtered (" ", f);
 
 	  print_longest (f, 'd', 0,
-			 read_const (x, i + 1, aop_map[op].op_size));
+			 read_const (x, i + 1, op_size));
 	}
       fprintf_filtered (f, "\n");
-      i += 1 + aop_map[op].op_size;
+      i += 1 + op_size;
 
       is_float = (op == aop_float);
     }
@@ -493,6 +515,8 @@ ax_reqs (struct agent_expr *ax)
   /* Pointer to a description of the present op.  */
   struct aop_map *op;
 
+  int op_size = 0, consumed = 0;
+
   memset (targets, 0, ax->len * sizeof (targets[0]));
   memset (boundary, 0, ax->len * sizeof (boundary[0]));
 
@@ -500,7 +524,7 @@ ax_reqs (struct agent_expr *ax)
   ax->flaw = agent_flaw_none;
   ax->max_data_size = 0;
 
-  for (i = 0; i < ax->len; i += 1 + op->op_size)
+  for (i = 0; i < ax->len; i += 1 + op_size)
     {
       if (ax->buf[i] > (sizeof (aop_map) / sizeof (aop_map[0])))
 	{
@@ -516,7 +540,23 @@ ax_reqs (struct agent_expr *ax)
 	  return;
 	}
 
-      if (i + 1 + op->op_size > ax->len)
+      if (ax->buf[i] == aop_printf)
+        {
+	  if (i + 2 >= ax->len)
+	    {
+	      ax->flaw = agent_flaw_incomplete_instruction;
+	      return;
+	    }
+	  consumed = ax->buf[i + 1];
+	  op_size = 1 + strlen (ax->buf + i + 2) + 1;
+	}
+      else
+        {
+	  op_size = op->op_size;
+	  consumed = op->consumed;
+        }
+
+      if (i + 1 + op_size > ax->len)
 	{
 	  ax->flaw = agent_flaw_incomplete_instruction;
 	  return;
@@ -534,7 +574,7 @@ ax_reqs (struct agent_expr *ax)
       boundary[i] = 1;
       heights[i] = height;
 
-      height -= op->consumed;
+      height -= consumed;
       if (height < ax->min_height)
 	ax->min_height = height;
       height += op->produced;
--- a/ax.h
+++ b/ax.h
@@ -204,6 +204,7 @@ enum agent_op
     aop_setv = 0x2d,
     aop_tracev = 0x2e,
     aop_trace16 = 0x30,
+    aop_printf = 0x31,
     aop_last
   };
 \f
@@ -260,6 +261,8 @@ extern void ax_reg_mask (struct agent_ex
 
 /* Assemble code to operate on a trace state variable.  */
 extern void ax_tsv (struct agent_expr *expr, enum agent_op op, int num);
+
+extern void ax_memcpy (struct agent_expr *x, const void *src, size_t n);
 \f
 
 /* Functions for printing out expressions, and otherwise debugging
--- a/printcmd.c
+++ b/printcmd.c
@@ -1958,10 +1958,13 @@ print_variable_and_value (const char *na
   fprintf_filtered (stream, "\n");
 }
 
-/* printf "printf format string" ARG to STREAM.  */
+typedef void (printf_callback) (char *fbuf, char **expp,
+				struct bp_location *loc,
+				struct agent_expr *aexpr);
 
-static void
-ui_printf (char *arg, struct ui_file *stream)
+void
+string_printf (char *arg, struct ui_file *stream, printf_callback callback,
+	       struct bp_location *loc, struct agent_expr *aexpr)
 {
   char *f = NULL;
   char *s = arg;
@@ -2294,26 +2297,42 @@ ui_printf (char *arg, struct ui_file *st
     /* Now, parse all arguments and evaluate them.
        Store the VALUEs in VAL_ARGS.  */
 
+    if (callback)
+      current_substring = substrings;
     while (*s != '\0')
       {
 	char *s1;
 
+	s1 = s;
 	if (nargs == allocated_args)
 	  val_args = (struct value **) xrealloc ((char *) val_args,
 						 (allocated_args *= 2)
 						 * sizeof (struct value *));
-	s1 = s;
-	val_args[nargs] = parse_to_comma_and_eval (&s1);
+	if (callback)
+	  {
+	    if (nargs >= nargs_wanted)
+	      error (_("Wrong number of arguments for specified "
+		       "format-string"));
+	    callback (current_substring, &s1, loc, aexpr);
+	    current_substring += strlen (current_substring) + 1;
+	  }
+	else
+	  val_args[nargs] = parse_to_comma_and_eval (&s1);
 
 	nargs++;
 	s = s1;
 	if (*s == ',')
 	  s++;
       }
+    if (callback)
+      callback (last_arg, NULL, loc, aexpr);
 
     if (nargs != nargs_wanted)
       error (_("Wrong number of arguments for specified format-string"));
 
+    if (!stream)
+      goto after_print;
+
     /* Now actually print them.  */
     current_substring = substrings;
     for (i = 0; i < nargs; i++)
@@ -2668,12 +2687,22 @@ ui_printf (char *arg, struct ui_file *st
        by default, which will warn here if there is no argument.  */
     fprintf_filtered (stream, last_arg, 0);
   }
+
+after_print:
   do_cleanups (old_cleanups);
 }
 
-/* Implement the "printf" command.  */
+/* printf "printf format string" ARG to STREAM.  */
 
 static void
+ui_printf (char *arg, struct ui_file *stream)
+{
+  string_printf (arg, stream, NULL, NULL, NULL);
+}
+
+/* Implement the "printf" command.  */
+
+void
 printf_command (char *arg, int from_tty)
 {
   ui_printf (arg, gdb_stdout);
--- a/tracepoint.c
+++ b/tracepoint.c
@@ -187,6 +187,15 @@ extern void send_disconnected_tracing_va
 static void free_uploaded_tps (struct uploaded_tp **utpp);
 static void free_uploaded_tsvs (struct uploaded_tsv **utsvp);
 
+extern void printf_command (char *arg, int from_tty);
+extern void gen_printf_expr_callback (char **expp, struct bp_location *loc,
+				      struct agent_expr *aexpr);
+typedef void (printf_callback) (char **expp, struct bp_location *loc,
+				struct agent_expr *aexpr);
+extern void string_printf (char *arg, struct ui_file *stream,
+			   printf_callback callback, struct bp_location *loc,
+			   struct agent_expr *aexpr);
+
 
 extern void _initialize_tracepoint (void);
 
@@ -725,6 +734,28 @@ validate_actionline (char **line, struct
 	error (_("while-stepping step count `%s' is malformed."), *line);
     }
 
+  else if (cmd_cfunc_eq (c, printf_command))
+    {
+      char fbuf[101];
+
+      for (loc = t->loc; loc; loc = loc->next)
+	{
+	  int nargs;
+	  aexpr = new_agent_expr (loc->gdbarch, loc->address);
+	  old_chain = make_cleanup_free_agent_expr (aexpr);
+	  string_printf (p, NULL, gen_printf_expr_callback,
+			 loc, aexpr);
+	  ax_simple (aexpr, aop_end);
+	  /* The agent expr include expr for arguments, format string, 1 byte
+	   * for aop_printf, 1 byte for the number of arguments, 1 byte for
+	   * size of format string, 1 byte for blank after format string
+	   * and 1 byte for aop_end.  */
+	  if (aexpr->len > MAX_AGENT_EXPR_LEN)
+	    error (_("Expression is too complicated."));
+	  do_cleanups (old_chain);
+	}
+    }
+
   else if (cmd_cfunc_eq (c, end_actions_pseudocommand))
     ;
 
@@ -1436,6 +1467,22 @@ encode_actions_1 (struct command_line *a
 	  encode_actions_1 (action->body_list[0], t, tloc, frame_reg,
 			    frame_offset, stepping_list, NULL);
 	}
+      else if (cmd_cfunc_eq (cmd, printf_command))
+	{
+          char fbuf[101];
+	  struct cleanup *old_chain = NULL;
+
+	  aexpr = new_agent_expr (tloc->gdbarch, tloc->address);
+	  old_chain = make_cleanup_free_agent_expr (aexpr);
+	  string_printf (action_exp, NULL, gen_printf_expr_callback,
+			 tloc, aexpr);
+	  ax_simple (aexpr, aop_end);
+
+	  ax_reqs (aexpr);
+	  report_agent_reqs_errors (aexpr);
+	  discard_cleanups (old_chain);
+	  add_aexpr (collect, aexpr);
+	}
       else
 	error (_("Invalid tracepoint command '%s'"), action->line);
     }				/* for */

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Hui Zhu]

Hi all,

I sent the first version patch has been more than a month.  And I have
done with the support in the KGTP for the support.

But I am still not get a code review or something.  Are you reviewing
or have plan to review this patch?

Thanks,
Hui

On Fri, Feb 4, 2011 at 23:59, Hui Zhu <teawater@gmail.com> wrote:
> The prev version cannot support %s.  So make a new version to support it.
>
> And I think current way to handle printf is too much hack way.  Maybe
> I need find out a more better way to handle the printf.
>
> Thanks,
> Hui
>
> 2011-02-04  Hui Zhu  <teawater@gmail.com>
>
>        * ax-gdb.c (gen_printf_expr_callback): New function.
>        * ax-general.c (ax_memcpy): New function.
>        (aop_map): Add new entry for "printf".
>        (ax_print): Handle "printf".
>        (ax_reqs): Ditto.
>        * ax.h (agent_op): Add aop_printf.
>        (ax_memcpy): Forward declare.
>        * printcmd.c (printf_callback): New typedef.
>        (string_printf): New function from ui_printf.
>        (ui_printf): Call string_printf.
>        (printf_command): Remove static.
>        * tracepoint.c (printf_command, gen_printf_expr_callback,
>        printf_callback, string_printf): Forward declares.
>        (validate_actionline, encode_actions_1): handle printf_command.
>

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Tom Tromey]

>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:

>> +      gen_expr (expr, &pc, aexpr, &value);
>> +
>> +
>> +      if (value.optimized_out)

There's no need to have 2 blank lines here.

This function and some other new ones have no documentation.

>> +  {"printf", 0, 0, 0, 0},	/* 0x31 */
[...]
>> +    aop_printf = 0x31,

If you add a new AX expression, you must update agentexpr.texi.

I think any AX addition should also require a corresponding addition to
gdbserver.

>> +typedef void (printf_callback) (char *fbuf, char **expp,
>> +				struct bp_location *loc,
>> +				struct agent_expr *aexpr);

From what I can see, the `loc' and `aexpr' arguments are passed through
string_printf without interpretation.

In a case like this it is customary to just add a single `void *data'
argument and have the caller and callback agree on the type.  Here, that
type would be an instance of a struct wrapping the two values.

This definition should not be here.

>>  static void
>> +ui_printf (char *arg, struct ui_file *stream)
>> +{
>> +  string_printf (arg, stream, NULL, NULL, NULL);
>> +}

There's no reason to keep ui_printf around, just inline this into the 2
callers.

>> +extern void printf_command (char *arg, int from_tty);
>> +typedef void (printf_callback) (char **expp, struct bp_location *loc,
>> +				struct agent_expr *aexpr);
>> +extern void string_printf (char *arg, struct ui_file *stream,
>> +			   printf_callback callback, struct bp_location *loc,
>> +			   struct agent_expr *aexpr);

These should be in some appropriate header, not tracepoint.c.

>> +	  /* The agent expr include expr for arguments, format string, 1 byte
>> +	   * for aop_printf, 1 byte for the number of arguments, 1 byte for
>> +	   * size of format string, 1 byte for blank after format string
>> +	   * and 1 byte for aop_end.  */

Wrong comment format, no leading "*"s.


This new feature needs a documentation patch and probably also a patch
to NEWS.

From what I can tell from the patch, the idea here is to add a printf to
a tracepoint's actions, with the printf evaluated on the remote side.  I
guess that is an ok idea, though I don't use this stuff enough to really
have an opinion.

I think it would be good for other maintainers to speak up now.  If it
is left to me, I will allow this if you fix up the problems and write
the documentation.

Tom

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Hui Zhu]

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

On Sat, Feb 12, 2011 at 02:45, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:
>
>>> +      gen_expr (expr, &pc, aexpr, &value);
>>> +
>>> +
>>> +      if (value.optimized_out)
>
> There's no need to have 2 blank lines here.
>
> This function and some other new ones have no documentation.
>
>>> +  {"printf", 0, 0, 0, 0},   /* 0x31 */
> [...]
>>> +    aop_printf = 0x31,
>
> If you add a new AX expression, you must update agentexpr.texi.
>
> I think any AX addition should also require a corresponding addition to
> gdbserver.
>
>>> +typedef void (printf_callback) (char *fbuf, char **expp,
>>> +                            struct bp_location *loc,
>>> +                            struct agent_expr *aexpr);
>
> From what I can see, the `loc' and `aexpr' arguments are passed through
> string_printf without interpretation.
>
> In a case like this it is customary to just add a single `void *data'
> argument and have the caller and callback agree on the type.  Here, that
> type would be an instance of a struct wrapping the two values.
>
> This definition should not be here.
>
>>>  static void
>>> +ui_printf (char *arg, struct ui_file *stream)
>>> +{
>>> +  string_printf (arg, stream, NULL, NULL, NULL);
>>> +}
>
> There's no reason to keep ui_printf around, just inline this into the 2
> callers.
>
>>> +extern void printf_command (char *arg, int from_tty);
>>> +typedef void (printf_callback) (char **expp, struct bp_location *loc,
>>> +                            struct agent_expr *aexpr);
>>> +extern void string_printf (char *arg, struct ui_file *stream,
>>> +                       printf_callback callback, struct bp_location *loc,
>>> +                       struct agent_expr *aexpr);
>
> These should be in some appropriate header, not tracepoint.c.
>
>>> +      /* The agent expr include expr for arguments, format string, 1 byte
>>> +       * for aop_printf, 1 byte for the number of arguments, 1 byte for
>>> +       * size of format string, 1 byte for blank after format string
>>> +       * and 1 byte for aop_end.  */
>
> Wrong comment format, no leading "*"s.
>
>
> This new feature needs a documentation patch and probably also a patch
> to NEWS.
>
> From what I can tell from the patch, the idea here is to add a printf to
> a tracepoint's actions, with the printf evaluated on the remote side.  I
> guess that is an ok idea, though I don't use this stuff enough to really
> have an opinion.
>
> I think it would be good for other maintainers to speak up now.  If it
> is left to me, I will allow this if you fix up the problems and write
> the documentation.
>
> Tom
>

Thanks for your help Tom.

I make a new patch according to your comments.  And I have send the
patch for doc in prev mail.

Best,
Hui

2011-02-17  Hui Zhu  <teawater@gmail.com>

	* Makefile.in (HFILES_NO_SRCDIR): Add printcmd.h.
	* ax-gdb.c (gen_printf_expr_callback): New function.
	* ax-general.c (ax_memcpy): New function.
	(aop_map): Add new entry for "printf".
	(ax_print): Handle "printf".
	(ax_reqs): Ditto.
	* ax.h (agent_op): Add aop_printf.
	(ax_memcpy): Forward declare.
	* printcmd.c (printf_callback): New typedef.
	(string_printf): New function from ui_printf.
	(ui_printf): Call string_printf.
	(printf_command): Remove static.
	* printcmd.h: New file.
	* tracepoint.c (printf_command, gen_printf_expr_callback,
	printf_callback, string_printf): Forward declares.
	(validate_actionline, encode_actions_1): handle printf_command.

[-- Attachment #2: tp_print.txt --]
[-- Type: text/plain, Size: 12472 bytes --]

---
 Makefile.in  |    2 +-
 ax-gdb.c     |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ax-gdb.h     |    2 ++
 ax-general.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++-------
 ax.h         |    3 +++
 printcmd.c   |   39 +++++++++++++++++++++++++++++++--------
 printcmd.h   |   30 ++++++++++++++++++++++++++++++
 tracepoint.c |   39 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 212 insertions(+), 16 deletions(-)

--- a/Makefile.in
+++ b/Makefile.in
@@ -813,7 +813,7 @@ annotate.h sim-regno.h dictionary.h dfp.
 remote-fileio.h i386-linux-tdep.h vax-tdep.h objc-lang.h \
 sentinel-frame.h bcache.h symfile.h windows-tdep.h linux-tdep.h \
 gdb_usleep.h jit.h xml-syscall.h ada-operator.inc microblaze-tdep.h \
-psymtab.h psympriv.h progspace.h bfin-tdep.h ia64-hpux-tdep.h
+psymtab.h psympriv.h progspace.h bfin-tdep.h ia64-hpux-tdep.h printcmd.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
--- a/ax-gdb.c
+++ b/ax-gdb.c
@@ -2445,6 +2445,65 @@ gen_eval_for_expr (CORE_ADDR scope, stru
   return ax;
 }
 
+void
+gen_printf_expr_callback (char *fbuf, char **expp, void *loc_v, void *aexpr_v)
+{
+  struct bp_location	*loc = loc_v;
+  struct agent_expr	*aexpr = aexpr_v;
+
+  if (expp)
+    {
+      struct cleanup *old_chain = NULL;
+      struct expression *expr = NULL;
+      union exp_element *pc;
+      struct axs_value value;
+
+      expr = parse_exp_1 (expp, block_for_pc (loc->address), 1);
+      old_chain = make_cleanup (free_current_contents, &expr);
+
+      pc = expr->elts;
+      trace_kludge = 0;
+      value.optimized_out = 0;
+      gen_expr (expr, &pc, aexpr, &value);
+
+      if (value.optimized_out)
+        error (_("value has been optimized out"));
+      switch (value.kind)
+        {
+	case axs_lvalue_memory:
+	  if (TYPE_CODE (value.type) != TYPE_CODE_ARRAY)
+	    {
+	      int length = TYPE_LENGTH (check_typedef (value.type));
+	      switch (length)
+		{
+		case 4:
+		  ax_simple (aexpr, aop_ref32);
+		  break;
+		case 8:
+		  ax_simple (aexpr, aop_ref64);
+		  break;
+		default:
+		  error (_("Size of value is not OK."));
+		  break;
+		}
+	    }
+	  break;
+	case axs_lvalue_register:
+	  ax_reg (aexpr, value.u.reg);
+	  break;
+        }
+
+      do_cleanups (old_chain);
+    }
+
+  ax_simple (aexpr, aop_printf);
+  if (expp)
+    ax_simple (aexpr, 1);
+  else
+    ax_simple (aexpr, 0);
+  ax_memcpy (aexpr, fbuf, strlen (fbuf) + 1);
+}
+
 static void
 agent_command (char *exp, int from_tty)
 {
--- a/ax-gdb.h
+++ b/ax-gdb.h
@@ -108,6 +108,8 @@ extern struct agent_expr *gen_trace_for_
 
 extern struct agent_expr *gen_eval_for_expr (CORE_ADDR, struct expression *);
 
+extern void gen_printf_expr_callback (char *, char **, void *, void *);
+
 extern int trace_kludge;
 
 #endif /* AX_GDB_H */
--- a/ax-general.c
+++ b/ax-general.c
@@ -319,6 +319,14 @@ ax_tsv (struct agent_expr *x, enum agent
   x->buf[x->len + 2] = (num) & 0xff;
   x->len += 3;
 }
+
+void
+ax_memcpy (struct agent_expr *x, const void *src, size_t n)
+{
+  grow_expr (x, n);
+  memcpy (x->buf + x->len, src, n);
+  x->len += n;
+}
 \f
 
 
@@ -376,6 +384,7 @@ struct aop_map aop_map[] =
   {"tracev", 2, 0, 0, 1},	/* 0x2e */
   {0, 0, 0, 0, 0},		/* 0x2f */
   {"trace16", 2, 0, 1, 1},	/* 0x30 */
+  {"printf", 0, 0, 0, 0},	/* 0x31 */
 };
 
 
@@ -401,6 +410,7 @@ ax_print (struct ui_file *f, struct agen
   for (i = 0; i < x->len;)
     {
       enum agent_op op = x->buf[i];
+      int op_size;
 
       if (op >= (sizeof (aop_map) / sizeof (aop_map[0]))
 	  || !aop_map[op].name)
@@ -409,7 +419,19 @@ ax_print (struct ui_file *f, struct agen
 	  i++;
 	  continue;
 	}
-      if (i + 1 + aop_map[op].op_size > x->len)
+      if (op == aop_printf)
+        {
+	  if (i + 2 >= x->len)
+	    {
+	      fprintf_filtered (f, _("%3d  <bad opcode %02x>\n"), i, op);
+	      i++;
+	      continue;
+	    }
+	  op_size = 1 + strlen (x->buf + i + 2) + 1;
+	}
+      else
+	op_size = aop_map[op].op_size;
+      if (i + 1 + op_size > x->len)
 	{
 	  fprintf_filtered (f, _("%3d  <incomplete opcode %s>\n"),
 			    i, aop_map[op].name);
@@ -417,15 +439,15 @@ ax_print (struct ui_file *f, struct agen
 	}
 
       fprintf_filtered (f, "%3d  %s", i, aop_map[op].name);
-      if (aop_map[op].op_size > 0)
+      if (op_size > 0)
 	{
 	  fputs_filtered (" ", f);
 
 	  print_longest (f, 'd', 0,
-			 read_const (x, i + 1, aop_map[op].op_size));
+			 read_const (x, i + 1, op_size));
 	}
       fprintf_filtered (f, "\n");
-      i += 1 + aop_map[op].op_size;
+      i += 1 + op_size;
 
       is_float = (op == aop_float);
     }
@@ -493,6 +515,8 @@ ax_reqs (struct agent_expr *ax)
   /* Pointer to a description of the present op.  */
   struct aop_map *op;
 
+  int op_size = 0, consumed = 0;
+
   memset (targets, 0, ax->len * sizeof (targets[0]));
   memset (boundary, 0, ax->len * sizeof (boundary[0]));
 
@@ -500,7 +524,7 @@ ax_reqs (struct agent_expr *ax)
   ax->flaw = agent_flaw_none;
   ax->max_data_size = 0;
 
-  for (i = 0; i < ax->len; i += 1 + op->op_size)
+  for (i = 0; i < ax->len; i += 1 + op_size)
     {
       if (ax->buf[i] > (sizeof (aop_map) / sizeof (aop_map[0])))
 	{
@@ -516,7 +540,23 @@ ax_reqs (struct agent_expr *ax)
 	  return;
 	}
 
-      if (i + 1 + op->op_size > ax->len)
+      if (ax->buf[i] == aop_printf)
+        {
+	  if (i + 2 >= ax->len)
+	    {
+	      ax->flaw = agent_flaw_incomplete_instruction;
+	      return;
+	    }
+	  consumed = ax->buf[i + 1];
+	  op_size = 1 + strlen (ax->buf + i + 2) + 1;
+	}
+      else
+        {
+	  op_size = op->op_size;
+	  consumed = op->consumed;
+        }
+
+      if (i + 1 + op_size > ax->len)
 	{
 	  ax->flaw = agent_flaw_incomplete_instruction;
 	  return;
@@ -534,7 +574,7 @@ ax_reqs (struct agent_expr *ax)
       boundary[i] = 1;
       heights[i] = height;
 
-      height -= op->consumed;
+      height -= consumed;
       if (height < ax->min_height)
 	ax->min_height = height;
       height += op->produced;
--- a/ax.h
+++ b/ax.h
@@ -204,6 +204,7 @@ enum agent_op
     aop_setv = 0x2d,
     aop_tracev = 0x2e,
     aop_trace16 = 0x30,
+    aop_printf = 0x31,
     aop_last
   };
 \f
@@ -260,6 +261,8 @@ extern void ax_reg_mask (struct agent_ex
 
 /* Assemble code to operate on a trace state variable.  */
 extern void ax_tsv (struct agent_expr *expr, enum agent_op op, int num);
+
+extern void ax_memcpy (struct agent_expr *x, const void *src, size_t n);
 \f
 
 /* Functions for printing out expressions, and otherwise debugging
--- a/printcmd.c
+++ b/printcmd.c
@@ -1958,10 +1958,13 @@ print_variable_and_value (const char *na
   fprintf_filtered (stream, "\n");
 }
 
-/* printf "printf format string" ARG to STREAM.  */
+typedef void (printf_callback) (char *fbuf, char **expp,
+				struct bp_location *loc,
+				struct agent_expr *aexpr);
 
-static void
-ui_printf (char *arg, struct ui_file *stream)
+void
+string_printf (char *arg, struct ui_file *stream, printf_callback callback,
+	       void *loc_v, void *aexpr_v)
 {
   char *f = NULL;
   char *s = arg;
@@ -1972,6 +1975,8 @@ ui_printf (char *arg, struct ui_file *st
   int nargs = 0;
   int allocated_args = 20;
   struct cleanup *old_cleanups;
+  struct bp_location *loc = loc_v;
+  struct agent_expr *aexpr = aexpr_v;
 
   val_args = xmalloc (allocated_args * sizeof (struct value *));
   old_cleanups = make_cleanup (free_current_contents, &val_args);
@@ -2294,26 +2299,42 @@ ui_printf (char *arg, struct ui_file *st
     /* Now, parse all arguments and evaluate them.
        Store the VALUEs in VAL_ARGS.  */
 
+    if (callback)
+      current_substring = substrings;
     while (*s != '\0')
       {
 	char *s1;
 
+	s1 = s;
 	if (nargs == allocated_args)
 	  val_args = (struct value **) xrealloc ((char *) val_args,
 						 (allocated_args *= 2)
 						 * sizeof (struct value *));
-	s1 = s;
-	val_args[nargs] = parse_to_comma_and_eval (&s1);
+	if (callback)
+	  {
+	    if (nargs >= nargs_wanted)
+	      error (_("Wrong number of arguments for specified "
+		       "format-string"));
+	    callback (current_substring, &s1, loc, aexpr);
+	    current_substring += strlen (current_substring) + 1;
+	  }
+	else
+	  val_args[nargs] = parse_to_comma_and_eval (&s1);
 
 	nargs++;
 	s = s1;
 	if (*s == ',')
 	  s++;
       }
+    if (callback)
+      callback (last_arg, NULL, loc, aexpr);
 
     if (nargs != nargs_wanted)
       error (_("Wrong number of arguments for specified format-string"));
 
+    if (!stream)
+      goto after_print;
+
     /* Now actually print them.  */
     current_substring = substrings;
     for (i = 0; i < nargs; i++)
@@ -2668,15 +2689,17 @@ ui_printf (char *arg, struct ui_file *st
        by default, which will warn here if there is no argument.  */
     fprintf_filtered (stream, last_arg, 0);
   }
+
+after_print:
   do_cleanups (old_cleanups);
 }
 
 /* Implement the "printf" command.  */
 
-static void
+void
 printf_command (char *arg, int from_tty)
 {
-  ui_printf (arg, gdb_stdout);
+  string_printf (arg, gdb_stdout, NULL, NULL, NULL);
 }
 
 /* Implement the "eval" command.  */
@@ -2688,7 +2711,7 @@ eval_command (char *arg, int from_tty)
   struct cleanup *cleanups = make_cleanup_ui_file_delete (ui_out);
   char *expanded;
 
-  ui_printf (arg, ui_out);
+  string_printf (arg, ui_out, NULL, NULL, NULL);
 
   expanded = ui_file_xstrdup (ui_out, NULL);
   make_cleanup (xfree, expanded);
--- /dev/null
+++ b/printcmd.h
@@ -0,0 +1,30 @@
+/* Print values for GNU debugger GDB.
+
+   Copyright (C) 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef _PRINTCMD_H_
+#define _PRINTCMD_H_
+
+extern void printf_command (char *arg, int from_tty);
+typedef void (printf_callback) (char *fbuf, char **expp, void *loc_v,
+				void *aexpr_v);
+extern void string_printf (char *arg, struct ui_file *stream,
+			   printf_callback callback, void *loc_v,
+			   void *aexpr_v);
+
+#endif /* _PRINTCMD_H_ */
--- a/tracepoint.c
+++ b/tracepoint.c
@@ -51,6 +51,7 @@
 #include "ax.h"
 #include "ax-gdb.h"
 #include "memrange.h"
+#include "printcmd.h"
 
 /* readline include files */
 #include "readline/readline.h"
@@ -763,6 +764,28 @@ validate_actionline (char **line, struct
 	error (_("while-stepping step count `%s' is malformed."), *line);
     }
 
+  else if (cmd_cfunc_eq (c, printf_command))
+    {
+      char fbuf[101];
+
+      for (loc = t->loc; loc; loc = loc->next)
+	{
+	  int nargs;
+	  aexpr = new_agent_expr (loc->gdbarch, loc->address);
+	  old_chain = make_cleanup_free_agent_expr (aexpr);
+	  string_printf (p, NULL, gen_printf_expr_callback,
+			 loc, aexpr);
+	  ax_simple (aexpr, aop_end);
+	  /* The agent expr include expr for arguments, format string, 1 byte
+	     for aop_printf, 1 byte for the number of arguments, 1 byte for
+	     size of format string, 1 byte for blank after format string
+	     and 1 byte for aop_end.  */
+	  if (aexpr->len > MAX_AGENT_EXPR_LEN)
+	    error (_("Expression is too complicated."));
+	  do_cleanups (old_chain);
+	}
+    }
+
   else if (cmd_cfunc_eq (c, end_actions_pseudocommand))
     ;
 
@@ -1474,6 +1497,22 @@ encode_actions_1 (struct command_line *a
 	  encode_actions_1 (action->body_list[0], t, tloc, frame_reg,
 			    frame_offset, stepping_list, NULL);
 	}
+      else if (cmd_cfunc_eq (cmd, printf_command))
+	{
+          char fbuf[101];
+	  struct cleanup *old_chain = NULL;
+
+	  aexpr = new_agent_expr (tloc->gdbarch, tloc->address);
+	  old_chain = make_cleanup_free_agent_expr (aexpr);
+	  string_printf (action_exp, NULL, gen_printf_expr_callback,
+			 tloc, aexpr);
+	  ax_simple (aexpr, aop_end);
+
+	  ax_reqs (aexpr);
+	  report_agent_reqs_errors (aexpr);
+	  discard_cleanups (old_chain);
+	  add_aexpr (collect, aexpr);
+	}
       else
 	error (_("Invalid tracepoint command '%s'"), action->line);
     }				/* for */

Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb

[Hui Zhu]

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

On Thu, Feb 17, 2011 at 16:15, Hui Zhu <teawater@gmail.com> wrote:
> On Sat, Feb 12, 2011 at 02:45, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:
>>
>>>> +      gen_expr (expr, &pc, aexpr, &value);
>>>> +
>>>> +
>>>> +      if (value.optimized_out)
>>
>> There's no need to have 2 blank lines here.
>>
>> This function and some other new ones have no documentation.
>>
>>>> +  {"printf", 0, 0, 0, 0},   /* 0x31 */
>> [...]
>>>> +    aop_printf = 0x31,
>>
>> If you add a new AX expression, you must update agentexpr.texi.
>>
>> I think any AX addition should also require a corresponding addition to
>> gdbserver.
>>
>>>> +typedef void (printf_callback) (char *fbuf, char **expp,
>>>> +                            struct bp_location *loc,
>>>> +                            struct agent_expr *aexpr);
>>
>> From what I can see, the `loc' and `aexpr' arguments are passed through
>> string_printf without interpretation.
>>
>> In a case like this it is customary to just add a single `void *data'
>> argument and have the caller and callback agree on the type.  Here, that
>> type would be an instance of a struct wrapping the two values.
>>
>> This definition should not be here.
>>
>>>>  static void
>>>> +ui_printf (char *arg, struct ui_file *stream)
>>>> +{
>>>> +  string_printf (arg, stream, NULL, NULL, NULL);
>>>> +}
>>
>> There's no reason to keep ui_printf around, just inline this into the 2
>> callers.
>>
>>>> +extern void printf_command (char *arg, int from_tty);
>>>> +typedef void (printf_callback) (char **expp, struct bp_location *loc,
>>>> +                            struct agent_expr *aexpr);
>>>> +extern void string_printf (char *arg, struct ui_file *stream,
>>>> +                       printf_callback callback, struct bp_location *loc,
>>>> +                       struct agent_expr *aexpr);
>>
>> These should be in some appropriate header, not tracepoint.c.
>>
>>>> +      /* The agent expr include expr for arguments, format string, 1 byte
>>>> +       * for aop_printf, 1 byte for the number of arguments, 1 byte for
>>>> +       * size of format string, 1 byte for blank after format string
>>>> +       * and 1 byte for aop_end.  */
>>
>> Wrong comment format, no leading "*"s.
>>
>>
>> This new feature needs a documentation patch and probably also a patch
>> to NEWS.
>>
>> From what I can tell from the patch, the idea here is to add a printf to
>> a tracepoint's actions, with the printf evaluated on the remote side.  I
>> guess that is an ok idea, though I don't use this stuff enough to really
>> have an opinion.
>>
>> I think it would be good for other maintainers to speak up now.  If it
>> is left to me, I will allow this if you fix up the problems and write
>> the documentation.
>>
>> Tom
>>
>
> Thanks for your help Tom.
>
> I make a new patch according to your comments.  And I have send the
> patch for doc in prev mail.
>
> Best,
> Hui
>
> 2011-02-17  Hui Zhu  <teawater@gmail.com>
>
>        * Makefile.in (HFILES_NO_SRCDIR): Add printcmd.h.
>        * ax-gdb.c (gen_printf_expr_callback): New function.
>        * ax-general.c (ax_memcpy): New function.
>        (aop_map): Add new entry for "printf".
>        (ax_print): Handle "printf".
>        (ax_reqs): Ditto.
>        * ax.h (agent_op): Add aop_printf.
>        (ax_memcpy): Forward declare.
>        * printcmd.c (printf_callback): New typedef.
>        (string_printf): New function from ui_printf.
>        (ui_printf): Call string_printf.
>        (printf_command): Remove static.
>        * printcmd.h: New file.
>        * tracepoint.c (printf_command, gen_printf_expr_callback,
>        printf_callback, string_printf): Forward declares.
>        (validate_actionline, encode_actions_1): handle printf_command.
>

Hi guys,

Patch in attachment checked in.

Thanks,
Hui

2011-02-17  Hui Zhu  <teawater@gmail.com>

	* Makefile.in (HFILES_NO_SRCDIR): Add printcmd.h.
	* ax-gdb.c (gen_printf_expr_callback): New function.
	* ax-gdb.h (gen_printf_expr_callback): Forward declare.
	* ax-general.c (ax_memcpy): New function.
	(aop_map): Add new entry for "printf".
	(ax_print): Handle "printf".
	(ax_reqs): Ditto.
	* ax.h (ax_memcpy): Forward declare.
	* common/ax.def (invalid2): Removed.
	(printf): New entry.
	* printcmd.c (printcmd.h): New include.
	(string_printf): New function.
	(ui_printf): Removed.
	(printf_command): Remove static.  Call string_printf.
	(eval_command): Call string_printf.
	* printcmd.h: New file.
	* tracepoint.c (printf_command, gen_printf_expr_callback,
	printf_callback, string_printf): Forward declares.
	(validate_actionline, encode_actions_1): handle printf_command.

[-- Attachment #2: tp_print.txt --]
[-- Type: text/plain, Size: 12813 bytes --]

---
 Makefile.in   |    2 -
 ax-gdb.c      |   59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ax-gdb.h      |    2 +
 ax-general.c  |   53 +++++++++++++++++++++++++++++++++++++++++++++-------
 ax.h          |    2 +
 common/ax.def |    6 -----
 printcmd.c    |   38 ++++++++++++++++++++++++++++---------
 printcmd.h    |   30 +++++++++++++++++++++++++++++
 tracepoint.c  |   39 ++++++++++++++++++++++++++++++++++++++
 9 files changed, 209 insertions(+), 22 deletions(-)

--- a/Makefile.in
+++ b/Makefile.in
@@ -813,7 +813,7 @@ annotate.h sim-regno.h dictionary.h dfp.
 remote-fileio.h i386-linux-tdep.h vax-tdep.h objc-lang.h \
 sentinel-frame.h bcache.h symfile.h windows-tdep.h linux-tdep.h \
 gdb_usleep.h jit.h xml-syscall.h microblaze-tdep.h \
-psymtab.h psympriv.h progspace.h bfin-tdep.h ia64-hpux-tdep.h
+psymtab.h psympriv.h progspace.h bfin-tdep.h ia64-hpux-tdep.h printcmd.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
--- a/ax-gdb.c
+++ b/ax-gdb.c
@@ -2445,6 +2445,65 @@ gen_eval_for_expr (CORE_ADDR scope, stru
   return ax;
 }
 
+void
+gen_printf_expr_callback (char *fbuf, char **expp, void *loc_v, void *aexpr_v)
+{
+  struct bp_location	*loc = loc_v;
+  struct agent_expr	*aexpr = aexpr_v;
+
+  if (expp)
+    {
+      struct cleanup *old_chain = NULL;
+      struct expression *expr = NULL;
+      union exp_element *pc;
+      struct axs_value value;
+
+      expr = parse_exp_1 (expp, block_for_pc (loc->address), 1);
+      old_chain = make_cleanup (free_current_contents, &expr);
+
+      pc = expr->elts;
+      trace_kludge = 0;
+      value.optimized_out = 0;
+      gen_expr (expr, &pc, aexpr, &value);
+
+      if (value.optimized_out)
+        error (_("value has been optimized out"));
+      switch (value.kind)
+        {
+	case axs_lvalue_memory:
+	  if (TYPE_CODE (value.type) != TYPE_CODE_ARRAY)
+	    {
+	      int length = TYPE_LENGTH (check_typedef (value.type));
+	      switch (length)
+		{
+		case 4:
+		  ax_simple (aexpr, aop_ref32);
+		  break;
+		case 8:
+		  ax_simple (aexpr, aop_ref64);
+		  break;
+		default:
+		  error (_("Size of value is not OK."));
+		  break;
+		}
+	    }
+	  break;
+	case axs_lvalue_register:
+	  ax_reg (aexpr, value.u.reg);
+	  break;
+        }
+
+      do_cleanups (old_chain);
+    }
+
+  ax_simple (aexpr, aop_printf);
+  if (expp)
+    ax_simple (aexpr, 1);
+  else
+    ax_simple (aexpr, 0);
+  ax_memcpy (aexpr, fbuf, strlen (fbuf) + 1);
+}
+
 static void
 agent_command (char *exp, int from_tty)
 {
--- a/ax-gdb.h
+++ b/ax-gdb.h
@@ -108,6 +108,8 @@ extern struct agent_expr *gen_trace_for_
 
 extern struct agent_expr *gen_eval_for_expr (CORE_ADDR, struct expression *);
 
+extern void gen_printf_expr_callback (char *, char **, void *, void *);
+
 extern int trace_kludge;
 
 #endif /* AX_GDB_H */
--- a/ax-general.c
+++ b/ax-general.c
@@ -330,6 +330,14 @@ ax_tsv (struct agent_expr *x, enum agent
   x->buf[x->len + 2] = (num) & 0xff;
   x->len += 3;
 }
+
+void
+ax_memcpy (struct agent_expr *x, const void *src, size_t n)
+{
+  grow_expr (x, n);
+  memcpy (x->buf + x->len, src, n);
+  x->len += n;
+}
 \f
 
 
@@ -368,6 +376,7 @@ ax_print (struct ui_file *f, struct agen
   for (i = 0; i < x->len;)
     {
       enum agent_op op = x->buf[i];
+      int op_size;
 
       if (op >= (sizeof (aop_map) / sizeof (aop_map[0]))
 	  || !aop_map[op].name)
@@ -376,7 +385,19 @@ ax_print (struct ui_file *f, struct agen
 	  i++;
 	  continue;
 	}
-      if (i + 1 + aop_map[op].op_size > x->len)
+      if (op == aop_printf)
+        {
+	  if (i + 2 >= x->len)
+	    {
+	      fprintf_filtered (f, _("%3d  <bad opcode %02x>\n"), i, op);
+	      i++;
+	      continue;
+	    }
+	  op_size = 1 + strlen (x->buf + i + 2) + 1;
+	}
+      else
+	op_size = aop_map[op].op_size;
+      if (i + 1 + op_size > x->len)
 	{
 	  fprintf_filtered (f, _("%3d  <incomplete opcode %s>\n"),
 			    i, aop_map[op].name);
@@ -384,15 +405,15 @@ ax_print (struct ui_file *f, struct agen
 	}
 
       fprintf_filtered (f, "%3d  %s", i, aop_map[op].name);
-      if (aop_map[op].op_size > 0)
+      if (op_size > 0)
 	{
 	  fputs_filtered (" ", f);
 
 	  print_longest (f, 'd', 0,
-			 read_const (x, i + 1, aop_map[op].op_size));
+			 read_const (x, i + 1, op_size));
 	}
       fprintf_filtered (f, "\n");
-      i += 1 + aop_map[op].op_size;
+      i += 1 + op_size;
 
       is_float = (op == aop_float);
     }
@@ -460,6 +481,8 @@ ax_reqs (struct agent_expr *ax)
   /* Pointer to a description of the present op.  */
   struct aop_map *op;
 
+  int op_size = 0, consumed = 0;
+
   memset (targets, 0, ax->len * sizeof (targets[0]));
   memset (boundary, 0, ax->len * sizeof (boundary[0]));
 
@@ -467,7 +490,7 @@ ax_reqs (struct agent_expr *ax)
   ax->flaw = agent_flaw_none;
   ax->max_data_size = 0;
 
-  for (i = 0; i < ax->len; i += 1 + op->op_size)
+  for (i = 0; i < ax->len; i += 1 + op_size)
     {
       if (ax->buf[i] > (sizeof (aop_map) / sizeof (aop_map[0])))
 	{
@@ -483,7 +506,23 @@ ax_reqs (struct agent_expr *ax)
 	  return;
 	}
 
-      if (i + 1 + op->op_size > ax->len)
+      if (ax->buf[i] == aop_printf)
+        {
+	  if (i + 2 >= ax->len)
+	    {
+	      ax->flaw = agent_flaw_incomplete_instruction;
+	      return;
+	    }
+	  consumed = ax->buf[i + 1];
+	  op_size = 1 + strlen (ax->buf + i + 2) + 1;
+	}
+      else
+        {
+	  op_size = op->op_size;
+	  consumed = op->consumed;
+        }
+
+      if (i + 1 + op_size > ax->len)
 	{
 	  ax->flaw = agent_flaw_incomplete_instruction;
 	  return;
@@ -501,7 +540,7 @@ ax_reqs (struct agent_expr *ax)
       boundary[i] = 1;
       heights[i] = height;
 
-      height -= op->consumed;
+      height -= consumed;
       if (height < ax->min_height)
 	ax->min_height = height;
       height += op->produced;
--- a/ax.h
+++ b/ax.h
@@ -213,6 +213,8 @@ extern void ax_reg_mask (struct agent_ex
 
 /* Assemble code to operate on a trace state variable.  */
 extern void ax_tsv (struct agent_expr *expr, enum agent_op op, int num);
+
+extern void ax_memcpy (struct agent_expr *x, const void *src, size_t n);
 \f
 
 /* Functions for printing out expressions, and otherwise debugging
--- a/common/ax.def
+++ b/common/ax.def
@@ -86,12 +86,8 @@ DEFOP (swap, 0, 0, 2, 2, 0x2b)
 DEFOP (getv, 2, 0, 0, 1, 0x2c)
 DEFOP (setv, 2, 0, 0, 1, 0x2d)
 DEFOP (tracev, 2, 0, 0, 1, 0x2e)
-/* We need something here just to make the tables come out ok.  */
 DEFOP (invalid, 0, 0, 0, 0, 0x2f)
 DEFOP (trace16, 2, 0, 1, 1, 0x30)
-/* We need something here just to make the tables come out ok.  */
-DEFOP (invalid2, 0, 0, 0, 0, 0x2f)
-/* The "consumed" number for pick is wrong, but there's no way to
-   express the right thing.  */
+DEFOP (printf, 0, 0, 0, 0, 0x31)
 DEFOP (pick, 1, 0, 0, 1, 0x32)
 DEFOP (rot, 0, 0, 3, 3, 0x33)
--- a/printcmd.c
+++ b/printcmd.c
@@ -49,6 +49,7 @@
 #include "parser-defs.h"
 #include "charset.h"
 #include "arch-utils.h"
+#include "printcmd.h"
 
 #ifdef TUI
 #include "tui/tui.h"		/* For tui_active et al.   */
@@ -1960,10 +1961,9 @@ print_variable_and_value (const char *na
   fprintf_filtered (stream, "\n");
 }
 
-/* printf "printf format string" ARG to STREAM.  */
-
-static void
-ui_printf (char *arg, struct ui_file *stream)
+void
+string_printf (char *arg, struct ui_file *stream, printf_callback callback,
+	       void *loc_v, void *aexpr_v)
 {
   char *f = NULL;
   char *s = arg;
@@ -1974,6 +1974,8 @@ ui_printf (char *arg, struct ui_file *st
   int nargs = 0;
   int allocated_args = 20;
   struct cleanup *old_cleanups;
+  struct bp_location *loc = loc_v;
+  struct agent_expr *aexpr = aexpr_v;
 
   val_args = xmalloc (allocated_args * sizeof (struct value *));
   old_cleanups = make_cleanup (free_current_contents, &val_args);
@@ -2296,26 +2298,42 @@ ui_printf (char *arg, struct ui_file *st
     /* Now, parse all arguments and evaluate them.
        Store the VALUEs in VAL_ARGS.  */
 
+    if (callback)
+      current_substring = substrings;
     while (*s != '\0')
       {
 	char *s1;
 
+	s1 = s;
 	if (nargs == allocated_args)
 	  val_args = (struct value **) xrealloc ((char *) val_args,
 						 (allocated_args *= 2)
 						 * sizeof (struct value *));
-	s1 = s;
-	val_args[nargs] = parse_to_comma_and_eval (&s1);
+	if (callback)
+	  {
+	    if (nargs >= nargs_wanted)
+	      error (_("Wrong number of arguments for specified "
+		       "format-string"));
+	    callback (current_substring, &s1, loc, aexpr);
+	    current_substring += strlen (current_substring) + 1;
+	  }
+	else
+	  val_args[nargs] = parse_to_comma_and_eval (&s1);
 
 	nargs++;
 	s = s1;
 	if (*s == ',')
 	  s++;
       }
+    if (callback)
+      callback (last_arg, NULL, loc, aexpr);
 
     if (nargs != nargs_wanted)
       error (_("Wrong number of arguments for specified format-string"));
 
+    if (!stream)
+      goto after_print;
+
     /* Now actually print them.  */
     current_substring = substrings;
     for (i = 0; i < nargs; i++)
@@ -2670,15 +2688,17 @@ ui_printf (char *arg, struct ui_file *st
        by default, which will warn here if there is no argument.  */
     fprintf_filtered (stream, last_arg, 0);
   }
+
+after_print:
   do_cleanups (old_cleanups);
 }
 
 /* Implement the "printf" command.  */
 
-static void
+void
 printf_command (char *arg, int from_tty)
 {
-  ui_printf (arg, gdb_stdout);
+  string_printf (arg, gdb_stdout, NULL, NULL, NULL);
 }
 
 /* Implement the "eval" command.  */
@@ -2690,7 +2710,7 @@ eval_command (char *arg, int from_tty)
   struct cleanup *cleanups = make_cleanup_ui_file_delete (ui_out);
   char *expanded;
 
-  ui_printf (arg, ui_out);
+  string_printf (arg, ui_out, NULL, NULL, NULL);
 
   expanded = ui_file_xstrdup (ui_out, NULL);
   make_cleanup (xfree, expanded);
--- /dev/null
+++ b/printcmd.h
@@ -0,0 +1,30 @@
+/* Print values for GNU debugger GDB.
+
+   Copyright (C) 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef _PRINTCMD_H_
+#define _PRINTCMD_H_
+
+extern void printf_command (char *arg, int from_tty);
+typedef void (printf_callback) (char *fbuf, char **expp, void *loc_v,
+				void *aexpr_v);
+extern void string_printf (char *arg, struct ui_file *stream,
+			   printf_callback callback, void *loc_v,
+			   void *aexpr_v);
+
+#endif /* _PRINTCMD_H_ */
--- a/tracepoint.c
+++ b/tracepoint.c
@@ -51,6 +51,7 @@
 #include "ax.h"
 #include "ax-gdb.h"
 #include "memrange.h"
+#include "printcmd.h"
 
 /* readline include files */
 #include "readline/readline.h"
@@ -763,6 +764,28 @@ validate_actionline (char **line, struct
 	error (_("while-stepping step count `%s' is malformed."), *line);
     }
 
+  else if (cmd_cfunc_eq (c, printf_command))
+    {
+      char fbuf[101];
+
+      for (loc = t->loc; loc; loc = loc->next)
+	{
+	  int nargs;
+	  aexpr = new_agent_expr (loc->gdbarch, loc->address);
+	  old_chain = make_cleanup_free_agent_expr (aexpr);
+	  string_printf (p, NULL, gen_printf_expr_callback,
+			 loc, aexpr);
+	  ax_simple (aexpr, aop_end);
+	  /* The agent expr include expr for arguments, format string, 1 byte
+	     for aop_printf, 1 byte for the number of arguments, 1 byte for
+	     size of format string, 1 byte for blank after format string
+	     and 1 byte for aop_end.  */
+	  if (aexpr->len > MAX_AGENT_EXPR_LEN)
+	    error (_("Expression is too complicated."));
+	  do_cleanups (old_chain);
+	}
+    }
+
   else if (cmd_cfunc_eq (c, end_actions_pseudocommand))
     ;
 
@@ -1474,6 +1497,22 @@ encode_actions_1 (struct command_line *a
 	  encode_actions_1 (action->body_list[0], t, tloc, frame_reg,
 			    frame_offset, stepping_list, NULL);
 	}
+      else if (cmd_cfunc_eq (cmd, printf_command))
+	{
+          char fbuf[101];
+	  struct cleanup *old_chain = NULL;
+
+	  aexpr = new_agent_expr (tloc->gdbarch, tloc->address);
+	  old_chain = make_cleanup_free_agent_expr (aexpr);
+	  string_printf (action_exp, NULL, gen_printf_expr_callback,
+			 tloc, aexpr);
+	  ax_simple (aexpr, aop_end);
+
+	  ax_reqs (aexpr);
+	  report_agent_reqs_errors (aexpr);
+	  discard_cleanups (old_chain);
+	  add_aexpr (collect, aexpr);
+	}
       else
 	error (_("Invalid tracepoint command '%s'"), action->line);
     }				/* for */