* [PATCH] Collecting strings at tracepoints
@ 2011-10-03 15:51 Stan Shebs
2011-10-03 16:39 ` Eli Zaretskii
2011-10-04 19:10 ` Tom Tromey
0 siblings, 2 replies; 5+ messages in thread
From: Stan Shebs @ 2011-10-03 15:51 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]
By default, when you ask to collect a char * at a tracepoint, only the
numeric address is collected, which is a problem if you really want the
string that the char * is pointing to. This patch adds a /s option to
the collect action that will dereference character pointers and collect
the bytes up to the first zero, just as the familiar print command
does. You can optionally add a limit to the collect, so for instance
"collect/s80 mystring" collects a maximum of 80 characters.
While conceptually simple, we need a new agent bytecode to make this
work (writing a loop using existing bytecodes doesn't let us check that
we're running off the edge of valid memory), and a support flag so that
users get informed if the target doesn't support string collection.
If you're having a sense of dejavu about all this :-) , there was a
discussion of syntax last year, starting from
http://sourceware.org/ml/gdb/2010-06/msg00018.html . I ended up
following Michael Snyder's suggested syntax.
Stan
stan@codesourcery.com
2011-10-03 Stan Shebs <stan@codesourcery.com>
String collection for tracepoints.
* common/ax.def (tracenz): New bytecode.
* ax-gdb.h (string_kludge): Declare.
* ax-gdb.c: Include valprint.h and c-lang.h.
(string_kludge): New global.
(gen_traced_pop): Add string case.
(agent_command): Add string case.
* tracepoint.h (decode_agent_options): Declare.
* tracepoint.c (decode_agent_options): New function.
(validate_actionline): Call it.
(encode_actions_1): Ditto.
* target.h (struct target_ops): New method to_supports_string_tracing.
(target_supports_string_tracing): New macro.
* target.c (update_current_target): Add to_supports_string_tracing.
* remote.c (struct remote_state): New field string_tracing.
(remote_string_tracing_feature): New function.
(remote_protocol_features): New feature tracenz.
(remote_supports_string_tracing): New function.
(init_remote_ops): Set to_supports_string_tracing.
* tracepoint.c (agent_mem_read_string): New function.
(eval_agent_expr): Call it for tracenz.
* server.c (handle_query): Report support for tracenz.
* gdb.texinfo (Tracepoint Action Lists): Document collect/s.
* agentexpr.texi: Describe tracenz.
* gdb.trace/collection.c: Add code using strings.
* gdb.trace/collection.exp: Add tests of string collection.
[-- Attachment #2: tstr-patch-1 --]
[-- Type: text/plain, Size: 20844 bytes --]
Index: ax-gdb.c
===================================================================
RCS file: /cvs/src/src/gdb/ax-gdb.c,v
retrieving revision 1.89
diff -p -r1.89 ax-gdb.c
*** ax-gdb.c 27 Sep 2011 13:09:34 -0000 1.89
--- ax-gdb.c 3 Oct 2011 15:34:16 -0000
***************
*** 42,47 ****
--- 42,50 ----
#include "cp-support.h"
#include "arch-utils.h"
+ #include "valprint.h"
+ #include "c-lang.h"
+
/* To make sense of this file, you should read doc/agentexpr.texi.
Then look at the types and enums in ax-gdb.h. For the code itself,
look at gen_expr, towards the bottom; that's the main function that
*************** maybe_const_expr (union exp_element **pc
*** 335,340 ****
--- 338,348 ----
emits the trace bytecodes at the appropriate points. */
int trace_kludge;
+ /* Inspired by trace_kludge, this indicates that pointers to chars
+ should get an added tracenz bytecode to record nonzero bytes, up to
+ a length that is the value of string_kludge. */
+ int string_kludge;
+
/* Scan for all static fields in the given class, including any base
classes, and generate tracing bytecodes for each. */
*************** static void
*** 393,411 ****
gen_traced_pop (struct gdbarch *gdbarch,
struct agent_expr *ax, struct axs_value *value)
{
if (trace_kludge)
switch (value->kind)
{
case axs_rvalue:
! /* We don't trace rvalues, just the lvalues necessary to
! produce them. So just dispose of this value. */
! ax_simple (ax, aop_pop);
break;
case axs_lvalue_memory:
{
int length = TYPE_LENGTH (check_typedef (value->type));
/* There's no point in trying to use a trace_quick bytecode
here, since "trace_quick SIZE pop" is three bytes, whereas
"const8 SIZE trace" is also three bytes, does the same
--- 401,435 ----
gen_traced_pop (struct gdbarch *gdbarch,
struct agent_expr *ax, struct axs_value *value)
{
+ int string_trace = 0;
+ if (string_kludge
+ && TYPE_CODE (value->type) == TYPE_CODE_PTR
+ && c_textual_element_type (check_typedef (TYPE_TARGET_TYPE (value->type)),
+ 's'))
+ string_trace = 1;
+
if (trace_kludge)
switch (value->kind)
{
case axs_rvalue:
! if (string_trace)
! {
! ax_const_l (ax, string_kludge);
! ax_simple (ax, aop_tracenz);
! }
! else
! /* We don't trace rvalues, just the lvalues necessary to
! produce them. So just dispose of this value. */
! ax_simple (ax, aop_pop);
break;
case axs_lvalue_memory:
{
int length = TYPE_LENGTH (check_typedef (value->type));
+ if (string_trace)
+ ax_simple (ax, aop_dup);
+
/* There's no point in trying to use a trace_quick bytecode
here, since "trace_quick SIZE pop" is three bytes, whereas
"const8 SIZE trace" is also three bytes, does the same
*************** gen_traced_pop (struct gdbarch *gdbarch,
*** 413,418 ****
--- 437,449 ----
work correctly for objects with large sizes. */
ax_const_l (ax, length);
ax_simple (ax, aop_trace);
+
+ if (string_trace)
+ {
+ ax_simple (ax, aop_ref32);
+ ax_const_l (ax, string_kludge);
+ ax_simple (ax, aop_tracenz);
+ }
}
break;
*************** gen_traced_pop (struct gdbarch *gdbarch,
*** 422,427 ****
--- 453,467 ----
larger than will fit in a stack, so just mark it for
collection and be done with it. */
ax_reg_mask (ax, value->u.reg);
+
+ /* But if the register points to a string, assume the value
+ will fit on the stack and push it anyway. */
+ if (string_trace)
+ {
+ ax_reg (ax, value->u.reg);
+ ax_const_l (ax, string_kludge);
+ ax_simple (ax, aop_tracenz);
+ }
break;
}
else
*************** agent_command (char *exp, int from_tty)
*** 2489,2494 ****
--- 2529,2538 ----
if (exp == 0)
error_no_arg (_("expression to translate"));
+ string_kludge = 0;
+ if (*exp == '/')
+ exp = decode_agent_options (exp);
+
/* Recognize the return address collection directive specially. Note
that it is not really an expression of any sort. */
if (strcmp (exp, "$_ret") == 0)
Index: ax-gdb.h
===================================================================
RCS file: /cvs/src/src/gdb/ax-gdb.h,v
retrieving revision 1.21
diff -p -r1.21 ax-gdb.h
*** ax-gdb.h 27 Sep 2011 13:09:35 -0000 1.21
--- ax-gdb.h 3 Oct 2011 15:34:16 -0000
*************** extern struct agent_expr *gen_trace_for_
*** 112,116 ****
--- 112,117 ----
extern struct agent_expr *gen_eval_for_expr (CORE_ADDR, struct expression *);
extern int trace_kludge;
+ extern int string_kludge;
#endif /* AX_GDB_H */
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.463
diff -p -r1.463 remote.c
*** remote.c 14 Sep 2011 12:26:29 -0000 1.463
--- remote.c 3 Oct 2011 15:34:16 -0000
*************** struct remote_state
*** 331,336 ****
--- 331,339 ----
tracepoints while a trace experiment is running. */
int enable_disable_tracepoints;
+ /* True if the stub can collect strings using tracenz bytecode. */
+ int string_tracing;
+
/* Nonzero if the user has pressed Ctrl-C, but the target hasn't
responded to that. */
int ctrlc_pending_p;
*************** remote_enable_disable_tracepoint_feature
*** 3711,3716 ****
--- 3714,3729 ----
rs->enable_disable_tracepoints = (support == PACKET_ENABLE);
}
+ static void
+ remote_string_tracing_feature (const struct protocol_feature *feature,
+ enum packet_support support,
+ const char *value)
+ {
+ struct remote_state *rs = get_remote_state ();
+
+ rs->string_tracing = (support == PACKET_ENABLE);
+ }
+
static struct protocol_feature remote_protocol_features[] = {
{ "PacketSize", PACKET_DISABLE, remote_packet_size, -1 },
{ "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet,
*************** static struct protocol_feature remote_pr
*** 3761,3766 ****
--- 3774,3781 ----
remote_enable_disable_tracepoint_feature, -1 },
{ "qXfer:fdpic:read", PACKET_DISABLE, remote_supported_packet,
PACKET_qXfer_fdpic },
+ { "tracenz", PACKET_DISABLE,
+ remote_string_tracing_feature, -1 },
};
static char *remote_support_xml;
*************** remote_supports_enable_disable_tracepoin
*** 9704,9709 ****
--- 9719,9732 ----
return rs->enable_disable_tracepoints;
}
+ static int
+ remote_supports_string_tracing (void)
+ {
+ struct remote_state *rs = get_remote_state ();
+
+ return rs->string_tracing;
+ }
+
static void
remote_trace_init (void)
{
*************** Specify the serial device it is connecte
*** 10421,10426 ****
--- 10444,10450 ----
remote_ops.to_supports_non_stop = remote_supports_non_stop;
remote_ops.to_supports_multi_process = remote_supports_multi_process;
remote_ops.to_supports_enable_disable_tracepoint = remote_supports_enable_disable_tracepoint;
+ remote_ops.to_supports_string_tracing = remote_supports_string_tracing;
remote_ops.to_trace_init = remote_trace_init;
remote_ops.to_download_tracepoint = remote_download_tracepoint;
remote_ops.to_download_trace_state_variable
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.285
diff -p -r1.285 target.c
*** target.c 6 Jun 2011 12:47:07 -0000 1.285
--- target.c 3 Oct 2011 15:34:17 -0000
*************** update_current_target (void)
*** 672,677 ****
--- 672,678 ----
/* Do not inherit to_search_memory. */
INHERIT (to_supports_multi_process, t);
INHERIT (to_supports_enable_disable_tracepoint, t);
+ INHERIT (to_supports_string_tracing, t);
INHERIT (to_trace_init, t);
INHERIT (to_download_tracepoint, t);
INHERIT (to_download_trace_state_variable, t);
*************** update_current_target (void)
*** 840,845 ****
--- 841,849 ----
de_fault (to_supports_enable_disable_tracepoint,
(int (*) (void))
return_zero);
+ de_fault (to_supports_string_tracing,
+ (int (*) (void))
+ return_zero);
de_fault (to_trace_init,
(void (*) (void))
tcomplain);
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.212
diff -p -r1.212 target.h
*** target.h 27 Sep 2011 15:30:18 -0000 1.212
--- target.h 3 Oct 2011 15:34:17 -0000
*************** struct target_ops
*** 653,658 ****
--- 653,661 ----
experiment is running? */
int (*to_supports_enable_disable_tracepoint) (void);
+ /* Does this target support the tracenz bytecode for string collection? */
+ int (*to_supports_string_tracing) (void);
+
/* Determine current architecture of thread PTID.
The target is supposed to determine the architecture of the code where
*************** struct address_space *target_thread_addr
*** 894,899 ****
--- 897,905 ----
#define target_supports_enable_disable_tracepoint() \
(*current_target.to_supports_enable_disable_tracepoint) ()
+ #define target_supports_string_tracing() \
+ (*current_target.to_supports_string_tracing) ()
+
/* Invalidate all target dcaches. */
extern void target_dcache_invalidate (void);
Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.232
diff -p -r1.232 tracepoint.c
*** tracepoint.c 27 Sep 2011 13:09:36 -0000 1.232
--- tracepoint.c 3 Oct 2011 15:34:17 -0000
*************** teval_pseudocommand (char *args, int fro
*** 574,579 ****
--- 574,620 ----
error (_("This command can only be used in a tracepoint actions list."));
}
+ /* Parse any collection options, such as /s for strings. */
+
+ char *
+ decode_agent_options (char *exp)
+ {
+ struct value_print_options opts;
+
+ if (*exp != '/')
+ return exp;
+
+ /* Call this to borrow the print elements default for collection
+ size. */
+ get_user_print_options (&opts);
+
+ exp++;
+ if (*exp == 's')
+ {
+ if (target_supports_string_tracing ())
+ {
+ /* Allow an optional decimal number giving an explicit maximum
+ string length, defaulting it to the "print elements" value;
+ so "collect/s80 mystr" gets at most 80 bytes of string. */
+ string_kludge = opts.print_max;
+ exp++;
+ if (*exp >= '0' && *exp <= '9')
+ string_kludge = atoi (exp);
+ while (*exp >= '0' && *exp <= '9')
+ exp++;
+ }
+ else
+ error (_("Target does not support \"/s\" option for string tracing."));
+ }
+ else
+ error (_("Undefined collection format \"%c\"."), *exp);
+
+ while (*exp == ' ' || *exp == '\t')
+ exp++;
+
+ return exp;
+ }
+
/* Enter a list of actions for a tracepoint. */
static void
trace_actions_command (char *args, int from_tty)
*************** validate_actionline (char **line, struct
*** 656,661 ****
--- 697,706 ----
if (cmd_cfunc_eq (c, collect_pseudocommand))
{
+ string_kludge = 0;
+ if (*p == '/')
+ p = decode_agent_options (p);
+
do
{ /* Repeat over a comma-separated list. */
QUIT; /* Allow user to bail out with ^C. */
*************** encode_actions_1 (struct command_line *a
*** 1313,1318 ****
--- 1358,1367 ----
if (cmd_cfunc_eq (cmd, collect_pseudocommand))
{
+ string_kludge = 0;
+ if (*action_exp == '/')
+ action_exp = decode_agent_options (action_exp);
+
do
{ /* Repeat over a comma-separated list. */
QUIT; /* Allow user to bail out with ^C. */
*************** trace_dump_actions (struct command_line
*** 2581,2586 ****
--- 2630,2638 ----
STEPPING_ACTIONS should be equal. */
if (stepping_frame == stepping_actions)
{
+ if (*action_exp == '/')
+ action_exp = decode_agent_options (action_exp);
+
do
{ /* Repeat over a comma-separated list. */
QUIT; /* Allow user to bail out with ^C. */
Index: tracepoint.h
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.h,v
retrieving revision 1.43
diff -p -r1.43 tracepoint.h
*** tracepoint.h 25 Jul 2011 11:24:44 -0000 1.43
--- tracepoint.h 3 Oct 2011 15:34:17 -0000
*************** struct cleanup *make_cleanup_restore_cur
*** 212,217 ****
--- 212,220 ----
struct cleanup *make_cleanup_restore_traceframe_number (void);
void free_actions (struct breakpoint *);
+
+ extern char *decode_agent_options (char *exp);
+
extern void validate_actionline (char **, struct breakpoint *);
extern void end_actions_pseudocommand (char *args, int from_tty);
Index: gdbserver/server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.148
diff -p -r1.148 server.c
*** gdbserver/server.c 1 Sep 2011 03:14:10 -0000 1.148
--- gdbserver/server.c 3 Oct 2011 15:34:17 -0000
*************** handle_query (char *own_buf, int packet_
*** 1559,1565 ****
strcat (own_buf, ";qXfer:statictrace:read+");
strcat (own_buf, ";qXfer:traceframe-info:read+");
strcat (own_buf, ";EnableDisableTracepoints+");
! }
return;
}
--- 1559,1566 ----
strcat (own_buf, ";qXfer:statictrace:read+");
strcat (own_buf, ";qXfer:traceframe-info:read+");
strcat (own_buf, ";EnableDisableTracepoints+");
! strcat (own_buf, ";tracenz+");
! }
return;
}
Index: gdbserver/tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/tracepoint.c,v
retrieving revision 1.27
diff -p -r1.27 tracepoint.c
*** gdbserver/tracepoint.c 15 Sep 2011 22:54:13 -0000 1.27
--- gdbserver/tracepoint.c 3 Oct 2011 15:34:17 -0000
*************** static enum eval_result_type eval_agent_
*** 1209,1214 ****
--- 1209,1217 ----
static int agent_mem_read (struct traceframe *tframe,
unsigned char *to, CORE_ADDR from, ULONGEST len);
+ static int agent_mem_read_string (struct traceframe *tframe,
+ unsigned char *to, CORE_ADDR from,
+ ULONGEST len);
static int agent_tsv_read (struct traceframe *tframe, int n);
#ifndef IN_PROCESS_AGENT
*************** eval_agent_expr (struct tracepoint_hit_c
*** 4651,4656 ****
--- 4654,4666 ----
agent_tsv_read (tframe, arg);
break;
+ case gdb_agent_op_tracenz:
+ agent_mem_read_string (tframe, NULL, (CORE_ADDR) stack[--sp],
+ (ULONGEST) top);
+ if (--sp >= 0)
+ top = stack[sp];
+ break;
+
/* GDB never (currently) generates any of these ops. */
case gdb_agent_op_float:
case gdb_agent_op_ref_float:
*************** agent_mem_read (struct traceframe *tfram
*** 4734,4739 ****
--- 4744,4809 ----
return 0;
}
+ static int
+ agent_mem_read_string (struct traceframe *tframe,
+ unsigned char *to, CORE_ADDR from, ULONGEST len)
+ {
+ unsigned char *buf, *mspace;
+ ULONGEST remaining = len;
+ unsigned short blocklen, i;
+
+ /* To save a bit of space, block lengths are 16-bit, so break large
+ requests into multiple blocks. Bordering on overkill for strings,
+ but it could happen that someone specifies a large max length. */
+ while (remaining > 0)
+ {
+ size_t sp;
+
+ blocklen = (remaining > 65535 ? 65535 : remaining);
+ /* We want working space to accumulate nonzero bytes, since
+ traceframes must have a predecided size (otherwise it gets
+ harder to wrap correctly for the circular case, etc). */
+ buf = (unsigned char *) xmalloc (blocklen + 1);
+ for (i = 0; i < blocklen; ++i)
+ {
+ /* Read the string one byte at a time, in case the string is
+ at the end of a valid memory area - we don't want a
+ correctly-terminated string to engender segvio
+ complaints. */
+ read_inferior_memory (from + i, buf + i, 1);
+
+ if (buf[i] == '\0')
+ {
+ blocklen = i + 1;
+ /* Make sure outer loop stops now too. */
+ remaining = blocklen;
+ break;
+ }
+ }
+ sp = 1 + sizeof (from) + sizeof (blocklen) + blocklen;
+ mspace = add_traceframe_block (tframe, sp);
+ if (mspace == NULL)
+ {
+ xfree (buf);
+ return 1;
+ }
+ /* Identify block as a memory block. */
+ *mspace = 'M';
+ ++mspace;
+ /* Record address and size. */
+ memcpy ((void *) mspace, (void *) &from, sizeof (from));
+ mspace += sizeof (from);
+ memcpy ((void *) mspace, (void *) &blocklen, sizeof (blocklen));
+ mspace += sizeof (blocklen);
+ /* Copy the string contents. */
+ memcpy ((void *) mspace, (void *) buf, blocklen);
+ remaining -= blocklen;
+ from += blocklen;
+ xfree (buf);
+ }
+ return 0;
+ }
+
/* Record the value of a trace state variable. */
static int
Index: doc/agentexpr.texi
===================================================================
RCS file: /cvs/src/src/gdb/doc/agentexpr.texi,v
retrieving revision 1.15
diff -p -r1.15 agentexpr.texi
*** doc/agentexpr.texi 24 Feb 2011 07:38:00 -0000 1.15
--- doc/agentexpr.texi 3 Oct 2011 15:34:17 -0000
*************** named @code{trace_quick16}, for consiste
*** 489,494 ****
--- 489,499 ----
Record the value of trace state variable number @var{n} in the trace
buffer. The handling of @var{n} is as described for @code{getv}.
+ @item @code{tracenz} (0x2f) @var{addr} @var{size} @result{}
+ Record the bytes at @var{addr} in a trace buffer, for later retrieval
+ by GDB. Stop at either the first zero byte, or when @var{size} bytes
+ have been recorded, whichever occurs first.
+
@item @code{end} (0x27): @result{}
Stop executing bytecode; the result should be the top element of the
stack. If the purpose of the expression was to compute an lvalue or a
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.865
diff -p -r1.865 gdb.texinfo
*** doc/gdb.texinfo 29 Sep 2011 02:25:49 -0000 1.865
--- doc/gdb.texinfo 3 Oct 2011 15:34:18 -0000
*************** end
*** 10268,10274 ****
@end smallexample
@kindex collect @r{(tracepoints)}
! @item collect @var{expr1}, @var{expr2}, @dots{}
Collect values of the given expressions when the tracepoint is hit.
This command accepts a comma-separated list of any valid expressions.
In addition to global, static, or local variables, the following
--- 10268,10274 ----
@end smallexample
@kindex collect @r{(tracepoints)}
! @item collect@r{[}/@var{mods}@r{]} @var{expr1}, @var{expr2}, @dots{}
Collect values of the given expressions when the tracepoint is hit.
This command accepts a comma-separated list of any valid expressions.
In addition to global, static, or local variables, the following
*************** You can give several consecutive @code{c
*** 10314,10319 ****
--- 10314,10328 ----
with a single argument, or one @code{collect} command with several
arguments separated by commas; the effect is the same.
+ The optional @var{mods} changes the usual handling of the arguments.
+ @code{s} requests that pointers to chars be handled as strings, in
+ particular collecting the contents of the memory being pointed at, up
+ to the first zero. The upper bound is by default the value of the
+ @code{print elements} variable; if @code{s} is followed by a decimal
+ number, that is the upper bound instead. So for instance
+ @samp{collect/s25 mystr} collects as many as 25 characters at
+ @samp{mystr}.
+
The command @code{info scope} (@pxref{Symbols, info scope}) is
particularly useful for figuring out what data to collect.
Index: common/ax.def
===================================================================
RCS file: /cvs/src/src/gdb/common/ax.def,v
retrieving revision 1.4
diff -p -r1.4 ax.def
*** common/ax.def 25 Feb 2011 14:21:05 -0000 1.4
--- common/ax.def 3 Oct 2011 15:34:19 -0000
*************** DEFOP (swap, 0, 0, 2, 2, 0x2b)
*** 86,93 ****
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, 0x31)
--- 86,92 ----
DEFOP (getv, 2, 0, 0, 1, 0x2c)
DEFOP (setv, 2, 0, 0, 1, 0x2d)
DEFOP (tracev, 2, 0, 0, 1, 0x2e)
! DEFOP (tracenz, 0, 0, 2, 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, 0x31)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Collecting strings at tracepoints
2011-10-03 15:51 [PATCH] Collecting strings at tracepoints Stan Shebs
@ 2011-10-03 16:39 ` Eli Zaretskii
2011-10-04 19:11 ` Tom Tromey
2011-10-04 19:10 ` Tom Tromey
1 sibling, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2011-10-03 16:39 UTC (permalink / raw)
To: Stan Shebs; +Cc: gdb-patches
> Date: Mon, 03 Oct 2011 08:50:49 -0700
> From: Stan Shebs <stan_shebs@mentor.com>
>
> By default, when you ask to collect a char * at a tracepoint, only the
> numeric address is collected, which is a problem if you really want the
> string that the char * is pointing to. This patch adds a /s option to
> the collect action that will dereference character pointers and collect
> the bytes up to the first zero, just as the familiar print command
> does. You can optionally add a limit to the collect, so for instance
> "collect/s80 mystring" collects a maximum of 80 characters.
Thanks, the documentation part is approved.
Is this NEWS-worthy, btw?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Collecting strings at tracepoints
2011-10-03 15:51 [PATCH] Collecting strings at tracepoints Stan Shebs
2011-10-03 16:39 ` Eli Zaretskii
@ 2011-10-04 19:10 ` Tom Tromey
2011-11-04 22:03 ` Stan Shebs
1 sibling, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2011-10-04 19:10 UTC (permalink / raw)
To: Stan Shebs; +Cc: gdb-patches
>>>>> "Stan" == Stan Shebs <stan_shebs@mentor.com> writes:
Stan> While conceptually simple, we need a new agent bytecode to make this
Stan> work (writing a loop using existing bytecodes doesn't let us check
Stan> that we're running off the edge of valid memory), and a support flag
Stan> so that users get informed if the target doesn't support string
Stan> collection.
Stan> + /* Inspired by trace_kludge, this indicates that pointers to chars
Stan> + should get an added tracenz bytecode to record nonzero bytes, up to
Stan> + a length that is the value of string_kludge. */
Stan> + int string_kludge;
I'd prefer a name that makes it obvious that this is a tracing thing.
Stan> + { "tracenz", PACKET_DISABLE,
Stan> + remote_string_tracing_feature, -1 },
I think this requires documentation in the remote protocol section.
Stan> + while (*exp == ' ' || *exp == '\t')
Stan> + exp++;
We have skip_spaces now.
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Collecting strings at tracepoints
2011-10-03 16:39 ` Eli Zaretskii
@ 2011-10-04 19:11 ` Tom Tromey
0 siblings, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2011-10-04 19:11 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Stan Shebs, gdb-patches
>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
Eli> Is this NEWS-worthy, btw?
I definitely think so.
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Collecting strings at tracepoints
2011-10-04 19:10 ` Tom Tromey
@ 2011-11-04 22:03 ` Stan Shebs
0 siblings, 0 replies; 5+ messages in thread
From: Stan Shebs @ 2011-11-04 22:03 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2654 bytes --]
On 10/4/11 12:10 PM, Tom Tromey wrote:
>>>>>> "Stan" == Stan Shebs<stan_shebs@mentor.com> writes:
> Stan> While conceptually simple, we need a new agent bytecode to make this
> Stan> work (writing a loop using existing bytecodes doesn't let us check
> Stan> that we're running off the edge of valid memory), and a support flag
> Stan> so that users get informed if the target doesn't support string
> Stan> collection.
>
> Stan> + /* Inspired by trace_kludge, this indicates that pointers to chars
> Stan> + should get an added tracenz bytecode to record nonzero bytes, up to
> Stan> + a length that is the value of string_kludge. */
> Stan> + int string_kludge;
>
> I'd prefer a name that makes it obvious that this is a tracing thing.
>
> Stan> + { "tracenz", PACKET_DISABLE,
> Stan> + remote_string_tracing_feature, -1 },
>
> I think this requires documentation in the remote protocol section.
>
> Stan> + while (*exp == ' ' || *exp == '\t')
> Stan> + exp++;
>
> We have skip_spaces now.
>
>
Thanks for the feedback! I took all into account, and committed this:
2011-11-02 Stan Shebs <stan@codesourcery.com>
String collection for tracepoints.
* NEWS: Mention string collection.
* common/ax.def (tracenz): New bytecode.
* ax-gdb.h (trace_string_kludge): Declare.
* ax-gdb.c: Include valprint.h and c-lang.h.
(trace_string_kludge): New global.
(gen_traced_pop): Add string case.
(agent_command): Add string case.
* tracepoint.h (decode_agent_options): Declare.
* tracepoint.c: Include cli-utils.h.
(decode_agent_options): New function.
(validate_actionline): Call it.
(encode_actions_1): Ditto.
* target.h (struct target_ops): New method to_supports_string_tracing.
(target_supports_string_tracing): New macro.
* target.c (update_current_target): Add to_supports_string_tracing.
* remote.c (struct remote_state): New field string_tracing.
(remote_string_tracing_feature): New function.
(remote_protocol_features): New feature tracenz.
(remote_supports_string_tracing): New function.
(init_remote_ops): Set to_supports_string_tracing.
* tracepoint.c (agent_mem_read_string): New function.
(eval_agent_expr): Call it for tracenz.
* server.c (handle_query): Report support for tracenz.
* gdb.texinfo (Tracepoint Action Lists): Document collect/s.
(General Query Packets): Describe tracenz feature.
* agentexpr.texi (Bytecode Descriptions): Describe tracenz.
* gdb.trace/collection.c: Add code using strings.
* gdb.trace/collection.exp: Add tests of string collection.
[-- Attachment #2: tstr-patch-2 --]
[-- Type: text/plain, Size: 13148 bytes --]
Index: ax-gdb.c
===================================================================
RCS file: /cvs/src/src/gdb/ax-gdb.c,v
retrieving revision 1.89
diff -r1.89 ax-gdb.c
44a45,47
> #include "valprint.h"
> #include "c-lang.h"
>
337a341,345
> /* Inspired by trace_kludge, this indicates that pointers to chars
> should get an added tracenz bytecode to record nonzero bytes, up to
> a length that is the value of trace_string_kludge. */
> int trace_string_kludge;
>
395a404,410
> int string_trace = 0;
> if (trace_string_kludge
> && TYPE_CODE (value->type) == TYPE_CODE_PTR
> && c_textual_element_type (check_typedef (TYPE_TARGET_TYPE (value->type)),
> 's'))
> string_trace = 1;
>
400,402c415,423
< /* We don't trace rvalues, just the lvalues necessary to
< produce them. So just dispose of this value. */
< ax_simple (ax, aop_pop);
---
> if (string_trace)
> {
> ax_const_l (ax, trace_string_kludge);
> ax_simple (ax, aop_tracenz);
> }
> else
> /* We don't trace rvalues, just the lvalues necessary to
> produce them. So just dispose of this value. */
> ax_simple (ax, aop_pop);
408a430,432
> if (string_trace)
> ax_simple (ax, aop_dup);
>
415a440,446
>
> if (string_trace)
> {
> ax_simple (ax, aop_ref32);
> ax_const_l (ax, trace_string_kludge);
> ax_simple (ax, aop_tracenz);
> }
424a456,464
>
> /* But if the register points to a string, assume the value
> will fit on the stack and push it anyway. */
> if (string_trace)
> {
> ax_reg (ax, value->u.reg);
> ax_const_l (ax, trace_string_kludge);
> ax_simple (ax, aop_tracenz);
> }
2491a2532,2535
> trace_string_kludge = 0;
> if (*exp == '/')
> exp = decode_agent_options (exp);
>
Index: ax-gdb.h
===================================================================
RCS file: /cvs/src/src/gdb/ax-gdb.h,v
retrieving revision 1.21
diff -r1.21 ax-gdb.h
114a115
> extern int trace_string_kludge;
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.465
diff -r1.465 remote.c
333a334,336
> /* True if the stub can collect strings using tracenz bytecode. */
> int string_tracing;
>
3714a3718,3727
> static void
> remote_string_tracing_feature (const struct protocol_feature *feature,
> enum packet_support support,
> const char *value)
> {
> struct remote_state *rs = get_remote_state ();
>
> rs->string_tracing = (support == PACKET_ENABLE);
> }
>
3766a3780,3781
> { "tracenz", PACKET_DISABLE,
> remote_string_tracing_feature, -1 },
9742a9758,9765
> static int
> remote_supports_string_tracing (void)
> {
> struct remote_state *rs = get_remote_state ();
>
> return rs->string_tracing;
> }
>
10461a10485
> remote_ops.to_supports_string_tracing = remote_supports_string_tracing;
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.286
diff -r1.286 target.c
674a675
> INHERIT (to_supports_string_tracing, t);
842a844,846
> de_fault (to_supports_string_tracing,
> (int (*) (void))
> return_zero);
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.214
diff -r1.214 target.h
661a662,664
> /* Does this target support the tracenz bytecode for string collection? */
> int (*to_supports_string_tracing) (void);
>
906a910,912
> #define target_supports_string_tracing() \
> (*current_target.to_supports_string_tracing) ()
>
Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.233
diff -r1.233 tracepoint.c
54a55
> #include "cli/cli-utils.h"
576a578,617
> /* Parse any collection options, such as /s for strings. */
>
> char *
> decode_agent_options (char *exp)
> {
> struct value_print_options opts;
>
> if (*exp != '/')
> return exp;
>
> /* Call this to borrow the print elements default for collection
> size. */
> get_user_print_options (&opts);
>
> exp++;
> if (*exp == 's')
> {
> if (target_supports_string_tracing ())
> {
> /* Allow an optional decimal number giving an explicit maximum
> string length, defaulting it to the "print elements" value;
> so "collect/s80 mystr" gets at most 80 bytes of string. */
> trace_string_kludge = opts.print_max;
> exp++;
> if (*exp >= '0' && *exp <= '9')
> trace_string_kludge = atoi (exp);
> while (*exp >= '0' && *exp <= '9')
> exp++;
> }
> else
> error (_("Target does not support \"/s\" option for string tracing."));
> }
> else
> error (_("Undefined collection format \"%c\"."), *exp);
>
> exp = skip_spaces (exp);
>
> return exp;
> }
>
658a700,703
> trace_string_kludge = 0;
> if (*p == '/')
> p = decode_agent_options (p);
>
1315a1361,1364
> trace_string_kludge = 0;
> if (*action_exp == '/')
> action_exp = decode_agent_options (action_exp);
>
2583a2633,2635
> if (*action_exp == '/')
> action_exp = decode_agent_options (action_exp);
>
Index: tracepoint.h
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.h,v
retrieving revision 1.43
diff -r1.43 tracepoint.h
214a215,217
>
> extern char *decode_agent_options (char *exp);
>
Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.463
diff -r1.463 NEWS
116a117,124
> collect[/s] EXPRESSIONS
> The tracepoint collect command now takes an optional modifier "/s"
> that directs it to dereference pointer-to-character types and
> collect the bytes of memory up to a zero byte. The behavior is
> similar to what you see when you use the regular print command on a
> string. An optional integer following the "/s" sets a bound on the
> number of bytes that will be collected.
>
Index: doc/agentexpr.texi
===================================================================
RCS file: /cvs/src/src/gdb/doc/agentexpr.texi,v
retrieving revision 1.15
diff -r1.15 agentexpr.texi
491a492,496
> @item @code{tracenz} (0x2f) @var{addr} @var{size} @result{}
> Record the bytes at @var{addr} in a trace buffer, for later retrieval
> by GDB. Stop at either the first zero byte, or when @var{size} bytes
> have been recorded, whichever occurs first.
>
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.885
diff -r1.885 gdb.texinfo
10669c10669
< @item collect @var{expr1}, @var{expr2}, @dots{}
---
> @item collect@r{[}/@var{mods}@r{]} @var{expr1}, @var{expr2}, @dots{}
10714a10715,10723
> The optional @var{mods} changes the usual handling of the arguments.
> @code{s} requests that pointers to chars be handled as strings, in
> particular collecting the contents of the memory being pointed at, up
> to the first zero. The upper bound is by default the value of the
> @code{print elements} variable; if @code{s} is followed by a decimal
> number, that is the upper bound instead. So for instance
> @samp{collect/s25 mystr} collects as many as 25 characters at
> @samp{mystr}.
>
34709a34719,34723
> @item @samp{tracenz}
> @tab No
> @tab @samp{-}
> @tab No
>
34833a34848,34852
> @item tracenz
> @cindex string tracing, in remote protocol
> The remote stub supports the @samp{tracenz} bytecode for collecting strings.
> See @ref{Bytecode Descriptions} for details about the bytecode.
>
Index: gdbserver/server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.149
diff -r1.149 server.c
1589a1590
> strcat (own_buf, ";tracenz+");
Index: gdbserver/tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/tracepoint.c,v
retrieving revision 1.28
diff -r1.28 tracepoint.c
1211a1212,1214
> static int agent_mem_read_string (struct traceframe *tframe,
> unsigned char *to, CORE_ADDR from,
> ULONGEST len);
4646a4650,4656
> case gdb_agent_op_tracenz:
> agent_mem_read_string (tframe, NULL, (CORE_ADDR) stack[--sp],
> (ULONGEST) top);
> if (--sp >= 0)
> top = stack[sp];
> break;
>
4729a4740,4799
> static int
> agent_mem_read_string (struct traceframe *tframe,
> unsigned char *to, CORE_ADDR from, ULONGEST len)
> {
> unsigned char *buf, *mspace;
> ULONGEST remaining = len;
> unsigned short blocklen, i;
>
> /* To save a bit of space, block lengths are 16-bit, so break large
> requests into multiple blocks. Bordering on overkill for strings,
> but it could happen that someone specifies a large max length. */
> while (remaining > 0)
> {
> size_t sp;
>
> blocklen = (remaining > 65535 ? 65535 : remaining);
> /* We want working space to accumulate nonzero bytes, since
> traceframes must have a predecided size (otherwise it gets
> harder to wrap correctly for the circular case, etc). */
> buf = (unsigned char *) xmalloc (blocklen + 1);
> for (i = 0; i < blocklen; ++i)
> {
> /* Read the string one byte at a time, in case the string is
> at the end of a valid memory area - we don't want a
> correctly-terminated string to engender segvio
> complaints. */
> read_inferior_memory (from + i, buf + i, 1);
>
> if (buf[i] == '\0')
> {
> blocklen = i + 1;
> /* Make sure outer loop stops now too. */
> remaining = blocklen;
> break;
> }
> }
> sp = 1 + sizeof (from) + sizeof (blocklen) + blocklen;
> mspace = add_traceframe_block (tframe, sp);
> if (mspace == NULL)
> {
> xfree (buf);
> return 1;
> }
> /* Identify block as a memory block. */
> *mspace = 'M';
> ++mspace;
> /* Record address and size. */
> memcpy ((void *) mspace, (void *) &from, sizeof (from));
> mspace += sizeof (from);
> memcpy ((void *) mspace, (void *) &blocklen, sizeof (blocklen));
> mspace += sizeof (blocklen);
> /* Copy the string contents. */
> memcpy ((void *) mspace, (void *) buf, blocklen);
> remaining -= blocklen;
> from += blocklen;
> xfree (buf);
> }
> return 0;
> }
>
Index: common/ax.def
===================================================================
RCS file: /cvs/src/src/gdb/common/ax.def,v
retrieving revision 1.4
diff -r1.4 ax.def
89,90c89
< /* We need something here just to make the tables come out ok. */
< DEFOP (invalid, 0, 0, 0, 0, 0x2f)
---
> DEFOP (tracenz, 0, 0, 2, 0, 0x2f)
Index: testsuite/gdb.trace/collection.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.trace/collection.c,v
retrieving revision 1.5
diff -r1.5 collection.c
203a204,218
> int strings_test_func ()
> {
> int i = 0;
> char *locstr, *longloc;
>
> locstr = "abcdef";
> longloc = malloc(500);
> strcpy(longloc, "how now brown cow spam spam spam wonderful wonderful spam");
>
> i += strlen (locstr);
> i += strlen (longloc);
>
> return i; /* Set_Tracepoint_Here */
> }
>
265a281
> i += strings_test_func ();
Index: testsuite/gdb.trace/collection.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.trace/collection.exp,v
retrieving revision 1.27
diff -r1.27 collection.exp
621a622,664
> proc gdb_collect_strings_test { func mystr myrslt mylim msg } {
> global hex
> global cr
> global gdb_prompt
>
> prepare_for_trace_test
>
> # Find the comment-identified line for setting this tracepoint.
> set testline 0
> gdb_test_multiple "list $func, +30" "collect $msg: find tracepoint line" {
> -re "\[\r\n\](\[0-9\]+)\[^\r\n\]+ Set_Tracepoint_Here .*$gdb_prompt" {
> set testline $expect_out(1,string)
> pass "collect $msg: find tracepoint line"
> }
> -re ".*$gdb_prompt " {
> fail "collect $msg: find tracepoint line (skipping strings test)"
> return
> }
> timeout {
> fail "collect $msg: find tracepoint line (skipping strings test)"
> return
> }
> }
>
> gdb_test "trace $testline" \
> "Tracepoint \[0-9\]+ at .*" \
> "collect $msg: set tracepoint"
> gdb_trace_setactions "collect $msg: define actions" \
> "" \
> "collect/s$mylim $mystr" "^$"
>
> # Begin the test.
> run_trace_experiment $msg $func
>
> gdb_test "print $mystr" \
> "\\$\[0-9\]+ = $hex \"$myrslt\".*$cr" \
> "collect $msg: collected local string"
>
> gdb_test "tfind none" \
> "#0 end .*" \
> "collect $msg: cease trace debugging"
> }
>
730a774,780
>
> gdb_collect_strings_test strings_test_func "locstr" "abcdef" "" \
> "local string"
>
> gdb_collect_strings_test strings_test_func "longloc" "how now brown c" 15 \
> "long local string"
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-11-04 22:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-03 15:51 [PATCH] Collecting strings at tracepoints Stan Shebs
2011-10-03 16:39 ` Eli Zaretskii
2011-10-04 19:11 ` Tom Tromey
2011-10-04 19:10 ` Tom Tromey
2011-11-04 22:03 ` Stan Shebs
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox