This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Change trace buffer size(v5)
- From: "Abid, Hafiz" <hafiz_abid at mentor dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: <gdb-patches at sourceware dot org>, <stan at codesourcery dot com>, <yao at codesourcery dot com>, <eliz at gnu dot org>
- Date: Fri, 8 Mar 2013 11:29:43 +0000
- Subject: Re: [patch] Change trace buffer size(v5)
Hi Pedro,
Thanks for the review. Here is an updated patch.
It's always good for the submission to be explicit in these
things, and good to consider these issues upfront. I'm interested
in hearing your thoughts on the subject upfront, and the plans
you had, if any.
I gave it some thought, and looked at the code to refresh a bit,
and I think things will still work correctly if the IPA buffer is
either smaller or larger than gdbserver's.
Running the whole set of gdb.trace tests (--directory=gdb.trace)
with a gdbserver and IPA hacked to default to different
buffer sizes once would provide some assurance, though
not that much.
I have run the test cases as you mentioned and did not see any
regression. I have a plan for a future patch to handle IPA buffer
size too.
> I noticed that packet_ok () is used with getpkt thoughout the file.
So I
> removed remote_get_noisy_reply.
OTOH, tracepoint packets call remote_get_noisy_reply, for an
ancient reason, but nowadays to so that we assist the target
with relocation if necessary (qRelocInsn; gdbserver uses it).
This patch uses remote_get_noisy_reply now.
> * remote.c (remote_set_trace_buffer_size): New function.
> (_initialize_remote): Use it.
Missed mentioning the new remote command.
Added.
> +@item QTBuffer:size
> +The remote stub supports the @samp{QTBuffer:size}
(@pxref{QTBuffer:size})
> +packet that allows to change the size of trace buffer.
> +
"of the trace buffer" I think.
Fixed.
If this fails, then $default_size is left uninitialized,
and then further below, we'll get tcl errors for using
an unknown variable. The pattern we usually follow around
such gdb_test_multiple cases, is to preinitialize the variable:
set default_size -1
gdb_test_multiple "tstatus" "tstatus check 1" {
-re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes
free.*" {
set default_size $expect_out(2,string)
}
}
and then something like,
if { $default_size < 0 } {
...
}
Fixed.
> +* New remote packets
> +
> +QTBuffer:size
> + Set the size of trace buffer.
Add "The remote stub reports support for this packet
to gdb's qSupported query."
Fixed.
Thanks,
Abid
2012-03-08 Stan Shebs <stan@codesourcery.com>
Hafiz Abid Qadeer <abidh@codesourcery.com>
gdb/
* NEWS: Mention set and show trace-buffer-size commands.
Mention new packet.
* target.h (struct target_ops): New method
to_set_trace_buffer_size.
(target_set_trace_buffer_size): New macro.
* target.c (update_current_target): Set up new method.
* tracepoint.c (trace_buffer_size): New global.
(start_tracing): Send it to the target.
(set_trace_buffer_size): New function.
(_initialize_tracepoint): Add new setshow for trace-buffer-size.
* remote.c (remote_set_trace_buffer_size): New function.
(_initialize_remote): Use it.
(QTBuffer:size) New remote command.
(PACKET_QTBuffer_size): New enum.
(remote_protocol_features): Add an entry for
PACKET_QTBuffer_size.
gdb/gdbserver/
* tracepoint.c (trace_buffer_size): New global.
(DEFAULT_TRACE_BUFFER_SIZE): New define.
(init_trace_buffer): Change to one-argument function. Allocate
trace buffer memory.
(handle_tracepoint_general_set): Call cmd_bigqtbuffer_size to
handle QTBuffer:size packet.
(cmd_bigqtbuffer_size): New function.
(initialize_tracepoint): Call init_trace_buffer with
DEFAULT_TRACE_BUFFER_SIZE.
* server.c (handle_query): Add QTBuffer:size in the
supported packets.
gdb/doc/
* gdb.texinfo (Starting and Stopping Trace Experiments):
Document
trace-buffer-size set and show commands.
(Tracepoint Packets): Document QTBuffer:size.
(General Query Packets): Document QTBuffer:size.
gdb/testsuite/
* gdb.trace/trace-buffer-size.exp: New file.
* gdb.trace/trace-buffer-size.c: New file.
diff --git a/gdb/NEWS b/gdb/NEWS
index 05fa498..99b8add 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -105,6 +105,10 @@ set debug notification
show debug notification
Control display of debugging info for async remote notification.
+set trace-buffer-size
+show trace-buffer-size
+ Request target to change the size of trace buffer.
+
* Removed commands
** For the Renesas Super-H architecture, the "regs" command has been removed
@@ -160,6 +164,12 @@ show filename-display
feature to be enabled. For more information, see:
http://fedoraproject.org/wiki/Features/MiniDebugInfo
+* New remote packets
+
+QTBuffer:size
+ Set the size of trace buffer. The remote stub reports support for this
+ packet to gdb's qSupported query.
+
*** Changes in GDB 7.5
* GDB now supports x32 ABI. Visit <http://sites.google.com/site/x32abi/>
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5f39d2e..f0caef1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11728,6 +11728,25 @@ for instance if you are looking at frames from a trace file.
@end table
@table @code
+@item set trace-buffer-size @var{n}
+@kindex set trace-buffer-size
+Request that the target use a trace buffer of @var{n} bytes. Not all
+targets will honor the request; they may have a compiled-in size for
+the trace buffer, or some other limitation. Set to a value of
+@code{-1} to let the target use whatever size it likes. This is also
+the default.
+
+@item show trace-buffer-size
+@kindex show trace-buffer-size
+Show the current requested size for the trace buffer. Note that this
+will only match the actual size if the target supports size-setting,
+and was able to handle the requested size. For instance, if the
+target can only change buffer size between runs, this variable will
+not reflect the change until the next run starts. Use @code{tstatus}
+to get a report of the actual buffer size.
+@end table
+
+@table @code
@item set trace-user @var{text}
@kindex set trace-user
@@ -37373,6 +37392,11 @@ These are the currently defined stub features and their properties:
@tab @samp{-}
@tab No
+@item @samp{QTBuffer:size}
+@tab No
+@tab @samp{-}
+@tab No
+
@item @samp{tracenz}
@tab No
@tab @samp{-}
@@ -37527,6 +37551,10 @@ The remote stub supports the @samp{QTEnable} (@pxref{QTEnable}) and
@samp{QTDisable} (@pxref{QTDisable}) packets that allow tracepoints
to be enabled and disabled while a trace experiment is running.
+@item QTBuffer:size
+The remote stub supports the @samp{QTBuffer:size} (@pxref{QTBuffer:size})
+packet that allows to change the size of the trace buffer.
+
@item tracenz
@cindex string tracing, in remote protocol
The remote stub supports the @samp{tracenz} bytecode for collecting strings.
@@ -38450,6 +38478,11 @@ available.
This packet directs the target to use a circular trace buffer if
@var{value} is 1, or a linear buffer if the value is 0.
+@item QTBuffer:size:@var{size}
+This packet directs the target to make the trace buffer be of size
+@var{size} if possible. A value of @code{-1} tells the target to
+use whatever size it prefers.
+
@item QTNotes:@r{[}@var{type}:@var{text}@r{]}@r{[};@var{type}:@var{text}@r{]}@dots{}
@cindex @samp{QTNotes} packet
This packet adds optional textual notes to the trace run. Allowable
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 768eae7..922a5bf 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1637,6 +1637,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
strcat (own_buf, ";qXfer:statictrace:read+");
strcat (own_buf, ";qXfer:traceframe-info:read+");
strcat (own_buf, ";EnableDisableTracepoints+");
+ strcat (own_buf, ";QTBuffer:size+");
strcat (own_buf, ";tracenz+");
}
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 632ee14..8c46483 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -30,6 +30,8 @@
#include "ax.h"
+#define DEFAULT_TRACE_BUFFER_SIZE 5242880 /* 5*1024*1024 */
+
/* This file is built for both GDBserver, and the in-process
agent (IPA), a shared library that includes a tracing agent that is
loaded by the inferior to support fast tracepoints. Fast
@@ -992,6 +994,10 @@ int current_traceframe = -1;
static int circular_trace_buffer;
#endif
+/* Size of the trace buffer. */
+
+static LONGEST trace_buffer_size;
+
/* Pointer to the block of memory that traceframes all go into. */
static unsigned char *trace_buffer_lo;
@@ -1478,9 +1484,13 @@ clear_inferior_trace_buffer (void)
#endif
static void
-init_trace_buffer (unsigned char *buf, int bufsize)
+init_trace_buffer (LONGEST bufsize)
{
- trace_buffer_lo = buf;
+ trace_buffer_size = bufsize;
+
+ /* If we already have a trace buffer, try realloc'ing. */
+ trace_buffer_lo = xrealloc (trace_buffer_lo, bufsize);
+
trace_buffer_hi = trace_buffer_lo + bufsize;
clear_trace_buffer ();
@@ -4020,6 +4030,37 @@ cmd_bigqtbuffer_circular (char *own_buf)
}
static void
+cmd_bigqtbuffer_size (char *own_buf)
+{
+ ULONGEST val;
+ LONGEST sval;
+ char *packet = own_buf;
+
+ /* Can't change the size during a tracing run. */
+ if (tracing)
+ {
+ write_enn (own_buf);
+ return;
+ }
+
+ packet += strlen ("QTBuffer:size:");
+
+ /* -1 is sent as literal "-1". */
+ if (strcmp (packet, "-1") == 0)
+ sval = DEFAULT_TRACE_BUFFER_SIZE;
+ else
+ {
+ unpack_varlen_hex (packet, &val);
+ sval = (LONGEST)val;
+ }
+
+ init_trace_buffer (sval);
+ trace_debug ("Trace buffer is now %s bytes",
+ plongest (trace_buffer_size));
+ write_ok (own_buf);
+}
+
+static void
cmd_qtnotes (char *own_buf)
{
size_t nbytes;
@@ -4143,6 +4184,11 @@ handle_tracepoint_general_set (char *packet)
cmd_bigqtbuffer_circular (packet);
return 1;
}
+ else if (strncmp ("QTBuffer:size:", packet, strlen ("QTBuffer:size:")) == 0)
+ {
+ cmd_bigqtbuffer_size (packet);
+ return 1;
+ }
else if (strncmp ("QTNotes:", packet, strlen ("QTNotes:")) == 0)
{
cmd_qtnotes (packet);
@@ -7228,10 +7274,8 @@ get_timestamp (void)
void
initialize_tracepoint (void)
{
- /* There currently no way to change the buffer size. */
- const int sizeOfBuffer = 5 * 1024 * 1024;
- unsigned char *buf = xmalloc (sizeOfBuffer);
- init_trace_buffer (buf, sizeOfBuffer);
+ /* Start with the default size. */
+ init_trace_buffer (DEFAULT_TRACE_BUFFER_SIZE);
/* Wire trace state variable 1 to be the timestamp. This will be
uploaded to GDB upon connection and become one of its trace state
diff --git a/gdb/remote.c b/gdb/remote.c
index b69c8a8..4978fca 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1280,6 +1280,7 @@ enum {
PACKET_qXfer_fdpic,
PACKET_QDisableRandomization,
PACKET_QAgent,
+ PACKET_QTBuffer_size,
PACKET_MAX
};
@@ -3989,6 +3990,8 @@ static struct protocol_feature remote_protocol_features[] = {
{ "QDisableRandomization", PACKET_DISABLE, remote_supported_packet,
PACKET_QDisableRandomization },
{ "QAgent", PACKET_DISABLE, remote_supported_packet, PACKET_QAgent},
+ { "QTBuffer:size", PACKET_DISABLE,
+ remote_supported_packet, PACKET_QTBuffer_size},
{ "tracenz", PACKET_DISABLE,
remote_string_tracing_feature, -1 },
};
@@ -11038,6 +11041,38 @@ remote_get_min_fast_tracepoint_insn_len (void)
}
}
+static void
+remote_set_trace_buffer_size (LONGEST val)
+{
+ if (remote_protocol_packets[PACKET_QTBuffer_size].support !=
+ PACKET_DISABLE)
+ {
+ struct remote_state *rs = get_remote_state ();
+ char *buf = rs->buf;
+ char *endbuf = rs->buf + get_remote_packet_size ();
+ enum packet_result result;
+
+ gdb_assert (val >= 0 || val == -1);
+ buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:");
+ /* Send -1 as literal "-1" to avoid host size dependency. */
+ if (val < 0)
+ {
+ *buf++ = '-';
+ buf += hexnumstr (buf, (ULONGEST) -val);
+ }
+ else
+ buf += hexnumstr (buf, (ULONGEST) val);
+
+ putpkt (rs->buf);
+ remote_get_noisy_reply (&rs->buf, &rs->buf_size);
+ result = packet_ok (rs->buf,
+ &remote_protocol_packets[PACKET_QTBuffer_size]);
+
+ if (result != PACKET_OK)
+ warning (_("Bogus reply from target: %s"), rs->buf);
+ }
+}
+
static int
remote_set_trace_notes (char *user, char *notes, char *stop_notes)
{
@@ -11215,6 +11250,7 @@ Specify the serial device it is connected to\n\
remote_ops.to_get_min_fast_tracepoint_insn_len = remote_get_min_fast_tracepoint_insn_len;
remote_ops.to_set_disconnected_tracing = remote_set_disconnected_tracing;
remote_ops.to_set_circular_trace_buffer = remote_set_circular_trace_buffer;
+ remote_ops.to_set_trace_buffer_size = remote_set_trace_buffer_size;
remote_ops.to_set_trace_notes = remote_set_trace_notes;
remote_ops.to_core_of_thread = remote_core_of_thread;
remote_ops.to_verify_memory = remote_verify_memory;
@@ -11751,6 +11787,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (&remote_protocol_packets[PACKET_QAgent],
"QAgent", "agent", 0);
+ add_packet_config_cmd (&remote_protocol_packets[PACKET_QTBuffer_size],
+ "QTBuffer:size", "trace-buffer-size", 0);
+
/* Keep the old ``set remote Z-packet ...'' working. Each individual
Z sub-packet has its own set and show commands, but users may
have sets to this variable in their .gdbinit files (or in their
diff --git a/gdb/target.c b/gdb/target.c
index eaf8b31..0b4f39d 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -693,6 +693,7 @@ update_current_target (void)
INHERIT (to_get_min_fast_tracepoint_insn_len, t);
INHERIT (to_set_disconnected_tracing, t);
INHERIT (to_set_circular_trace_buffer, t);
+ INHERIT (to_set_trace_buffer_size, t);
INHERIT (to_set_trace_notes, t);
INHERIT (to_get_tib_address, t);
INHERIT (to_set_permissions, t);
@@ -912,6 +913,9 @@ update_current_target (void)
de_fault (to_set_circular_trace_buffer,
(void (*) (int))
target_ignore);
+ de_fault (to_set_trace_buffer_size,
+ (void (*) (LONGEST))
+ target_ignore);
de_fault (to_set_trace_notes,
(int (*) (char *, char *, char *))
return_zero);
diff --git a/gdb/target.h b/gdb/target.h
index 1971265..c99642d 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -806,6 +806,8 @@ struct target_ops
disconnection - set VAL to 1 to keep tracing, 0 to stop. */
void (*to_set_disconnected_tracing) (int val);
void (*to_set_circular_trace_buffer) (int val);
+ /* Set the size of trace buffer in the target. */
+ void (*to_set_trace_buffer_size) (LONGEST val);
/* Add/change textual notes about the trace run, returning 1 if
successful, 0 otherwise. */
@@ -1700,6 +1702,9 @@ extern char *target_fileio_read_stralloc (const char *filename);
#define target_set_circular_trace_buffer(val) \
(*current_target.to_set_circular_trace_buffer) (val)
+#define target_set_trace_buffer_size(val) \
+ (*current_target.to_set_trace_buffer_size) (val)
+
#define target_set_trace_notes(user,notes,stopnotes) \
(*current_target.to_set_trace_notes) ((user), (notes), (stopnotes))
diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.c b/gdb/testsuite/gdb.trace/trace-buffer-size.c
new file mode 100644
index 0000000..56159c3
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/trace-buffer-size.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2011-2013 Free Software Foundation, Inc.
+
+ 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/>. */
+
+int var = 10;
+
+int
+test_function ()
+{
+ return 0;
+}
+
+int
+main ()
+{
+ test_function ();
+ return 0; /*breakpoint1*/
+}
diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.exp b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
new file mode 100644
index 0000000..832b635
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
@@ -0,0 +1,113 @@
+# Copyright 1998-2013 Free Software Foundation, Inc.
+
+# 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/>.
+
+load_lib "trace-support.exp"
+
+standard_testfile
+
+if [prepare_for_testing ${testfile}.exp $testfile $srcfile \
+ {debug nowarnings}] {
+ untested "failed to prepare for trace tests"
+ return -1
+}
+
+if ![runto_main] {
+ fail "can't run to main to check for trace support"
+ return -1
+}
+
+if ![gdb_target_supports_trace] {
+ unsupported "target does not support trace"
+ return -1;
+}
+
+set BUFFER_SIZE 4
+set default_size -1
+
+# Save default trace buffer size in 'default_size'.
+gdb_test_multiple "tstatus" "tstatus check 1" {
+ -re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes free.*" {
+ set default_size $expect_out(2,string)
+ }
+}
+
+# If we did not get the default size then there is no point in running the
+# tests below.
+if { $default_size < 0 } {
+ fail "Get default buffer size."
+ return -1;
+}
+
+# Change buffer size to 'BUFFER_SIZE'.
+gdb_test_no_output \
+ "set trace-buffer-size $BUFFER_SIZE" \
+ "set trace buffer size 1"
+
+gdb_test "tstatus" \
+ ".*Trace buffer has $decimal bytes of $BUFFER_SIZE bytes free.*" \
+ "tstatus check 2"
+
+gdb_test "show trace-buffer-size $BUFFER_SIZE" \
+ "Requested size of trace buffer is $BUFFER_SIZE.*" \
+ "show trace buffer size"
+
+gdb_test_no_output \
+ "set trace-buffer-size -1" \
+ "set trace buffer size 2"
+
+# Test that tstatus gives us default buffer size now.
+gdb_test "tstatus" \
+ ".*Trace buffer has $decimal bytes of $default_size bytes free.*" \
+ "tstatus check 3"
+
+gdb_test_no_output \
+ "set trace-buffer-size $BUFFER_SIZE" \
+ "set trace buffer size 3"
+
+# We set trace buffer to very small size. Then after running trace,
+# we check if it is full. This will show if setting trace buffer
+# size really worked.
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "breakpoint1"]
+gdb_test "trace test_function" \
+ "Tracepoint \[0-9\]+ at .*" \
+ "set tracepoint at test_function"
+gdb_trace_setactions "Set action for trace point 1" "" \
+ "collect var" "^$"
+gdb_test_no_output "tstart"
+gdb_test "continue" \
+ "Continuing.*Breakpoint $decimal.*" \
+ "run trace experiment 1"
+gdb_test "tstatus" \
+ ".*Trace stopped because the buffer was full.*" \
+ "buffer full check 1"
+
+# Use the default size -- the trace buffer should not end up
+# full this time
+clean_restart ${testfile}
+runto_main
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "breakpoint1"]
+gdb_test "trace test_function" \
+ "Tracepoint \[0-9\]+ at .*" \
+ "set tracepoint at test_function"
+gdb_trace_setactions "Set action for trace point 2" "" \
+ "collect var" "^$"
+gdb_test_no_output "tstart"
+gdb_test "continue" \
+ "Continuing.*Breakpoint $decimal.*" \
+ "run trace experiment 2"
+gdb_test "tstatus" \
+ ".*Trace is running on the target.*" \
+ "buffer full check 2"
+gdb_test_no_output "tstop"
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 2a4b245..0413903 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -170,6 +170,11 @@ static int disconnected_tracing;
static int circular_trace_buffer;
+/* This variable is the requested trace buffer size, or -1 to indicate
+ that we don't care and leave it up to the target to set a size. */
+
+static int trace_buffer_size = -1;
+
/* Textual notes applying to the current and/or future trace runs. */
char *trace_user = NULL;
@@ -1819,6 +1824,7 @@ start_tracing (char *notes)
/* Set some mode flags. */
target_set_disconnected_tracing (disconnected_tracing);
target_set_circular_trace_buffer (circular_trace_buffer);
+ target_set_trace_buffer_size (trace_buffer_size);
if (!notes)
notes = trace_notes;
@@ -3217,6 +3223,13 @@ set_circular_trace_buffer (char *args, int from_tty,
}
static void
+set_trace_buffer_size (char *args, int from_tty,
+ struct cmd_list_element *c)
+{
+ target_set_trace_buffer_size (trace_buffer_size);
+}
+
+static void
set_trace_user (char *args, int from_tty,
struct cmd_list_element *c)
{
@@ -5409,6 +5422,16 @@ up and stopping the trace run."),
&setlist,
&showlist);
+ add_setshow_zuinteger_unlimited_cmd ("trace-buffer-size", no_class,
+ &trace_buffer_size, _("\
+Set requested size of trace buffer."), _("\
+Show requested size of trace buffer."), _("\
+Use this to choose a size for the trace buffer. Some targets\n\
+may have fixed or limited buffer sizes. A value of -1 disables\n\
+any attempt to set the buffer size and lets the target choose."),
+ set_trace_buffer_size, NULL,
+ &setlist, &showlist);
+
add_setshow_string_cmd ("trace-user", class_trace,
&trace_user, _("\
Set the user name to use for current and future trace runs"), _("\