From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19766 invoked by alias); 4 Nov 2011 22:03:56 -0000 Received: (qmail 19757 invoked by uid 22791); 4 Nov 2011 22:03:53 -0000 X-SWARE-Spam-Status: No, hits=-2.6 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,TW_CP,TW_EG X-Spam-Check-By: sourceware.org Received: from elasmtp-banded.atl.sa.earthlink.net (HELO elasmtp-banded.atl.sa.earthlink.net) (209.86.89.70) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 04 Nov 2011 22:03:36 +0000 Received: from [70.170.59.51] (helo=macbook2.local) by elasmtp-banded.atl.sa.earthlink.net with esmtpa (Exim 4.67) (envelope-from ) id 1RMRrK-0004Kx-Lm for gdb-patches@sourceware.org; Fri, 04 Nov 2011 18:03:35 -0400 Message-ID: <4EB4612D.2010700@earthlink.net> Date: Fri, 04 Nov 2011 22:03:00 -0000 From: Stan Shebs User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:7.0.1) Gecko/20110929 Thunderbird/7.0.1 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: [PATCH] Collecting strings at tracepoints References: <4E89D9D9.6050703@mentor.com> In-Reply-To: Content-Type: multipart/mixed; boundary="------------040208010503020301000308" X-ELNK-Trace: ae6f8838ff913eba0cc1426638a40ef67e972de0d01da940f0f0078933b4955767e6cfd8f3bdca93350badd9bab72f9c350badd9bab72f9c350badd9bab72f9c X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-11/txt/msg00124.txt.bz2 This is a multi-part message in MIME format. --------------040208010503020301000308 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2654 On 10/4/11 12:10 PM, Tom Tromey wrote: >>>>>> "Stan" == Stan Shebs 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 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. --------------040208010503020301000308 Content-Type: text/plain; x-mac-type="0"; x-mac-creator="0"; name="tstr-patch-2" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="tstr-patch-2" Content-length: 13148 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" > --------------040208010503020301000308--