Re: [PATCH] Fix tracepoint tstart again get gdb_assert

Yao Qi
2011-12-09T09:53:49Z

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

On 12/08/2011 11:40 AM, Hui Zhu wrote:
> Hi guys,
>
> I am sorry that I didn't talk clear about the issue of tstatus.
> When we use tstatus, it can auto set the tracepoint back to stop when
> it full or got error. Then user can use tstart without tstop that set
> loc->inserted. So we will still meet that issue if we just set
> loc->inserted in tstop.
>
> For examp:
> This gdb is just set loc->inserted in tstop but not tstatus.
> (gdb) tstart
> (gdb) tstop
> (gdb) tstart
> (gdb) tstop
> (gdb) tstart
> xxx
> xxx
> (gdb) tstatus
> Trace stopped because the buffer was full.
> Buffer contains 0 trace frames (of 49072 created total).
> Trace buffer has 908408 bytes of 10414080 bytes free (91% full).
> Trace will stop if GDB disconnects.
> Not looking at any trace frame.
> (gdb) tstart
> ../../src/gdb/tracepoint.c:1770: internal-error: start_tracing:
> Assertion `!loc->inserted' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n) n
>
> ../../src/gdb/tracepoint.c:1770: internal-error: start_tracing:
> Assertion `!loc->inserted' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Create a core file of GDB? (y or n) n
>

This is the 2nd version of my patch. Compared with 1st version (sent
four hours ago), this version covers more cases. In this patch, a new
observer `trace_stopped' is added. In stop_tracing and
target_get_trace_status, observer `trace_stopped' is notified, and then,
`inserted' flag is cleared. This looks more clear than version 1.

Tested again on x86_64-linux. No regression.

--
Yao (é½å°§)

[-- Attachment #2: 0001-clear-inserted-in-stop_tracing.patch --]
[-- Type: text/x-patch, Size: 3828 bytes --]

2011-12-09 Hui Zhu <teawater@gmail.com>
Yao Qi <yao@codesourcery.com>

* target.c (target_get_trace_status): New function. Call
observer_notify_trace_stopped if trace is stopped.
* target.h (target_get_trace_status): Remove macro.
* tracepoint.c (stop_tracing): Call observer_notify_trace_stopped.
(trace_start_command): Call stop_tracing.
(trace_stopped): New.
(_initialize_tracepoint): Install trace_stopped to observer
`trace_stopped'.

gdb/doc:

2011-12-09 Yao Qi <yao@codesourcery.com>

* observer.texi (trace_stopped): New observer.
---
gdb/doc/observer.texi | 4 ++++
gdb/target.c | 12 ++++++++++++
gdb/target.h | 3 +--
gdb/tracepoint.c | 25 ++++++++++++++++++++++++-
4 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index 689b303..1daf75a 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -188,6 +188,10 @@ A tracepoint has been modified in some way. The argument @var{tpnum}
is the number of the modified tracepoint.
@end deftypefun

+@deftypefun void trace_stopped (void)
+A trace run has been stopped.
+@end deftypefun
+
@deftypefun void architecture_changed (struct gdbarch *@var{newarch})
The current architecture has changed. The argument @var{newarch} is
a pointer to the new architecture.
diff --git a/gdb/target.c b/gdb/target.c
index 5700066..e5bac02 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -44,6 +44,7 @@
#include "exec.h"
#include "inline-frame.h"
#include "tracepoint.h"
+#include "observer.h"

static void target_info (char *, int);

@@ -505,6 +506,17 @@ target_terminal_inferior (void)
(*current_target.to_terminal_inferior) ();
}

+int
+target_get_trace_status(struct trace_status *ts)
+{
+ int ret = (*current_target.to_get_trace_status) (ts);
+
+ if (!ts->running)
+ observer_notify_trace_stopped ();
+
+ return ret;
+}
+
static int
nomemory (CORE_ADDR memaddr, char *myaddr, int len, int write,
struct target_ops *t)
diff --git a/gdb/target.h b/gdb/target.h
index fd488d6..8a6dce9 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1517,8 +1517,7 @@ extern int target_search_memory (CORE_ADDR start_addr,
#define target_trace_set_readonly_regions() \
(*current_target.to_trace_set_readonly_regions) ()

-#define target_get_trace_status(ts) \
- (*current_target.to_get_trace_status) (ts)
+extern int target_get_trace_status(struct trace_status *ts);

#define target_get_tracepoint_status(tp,utp) \
(*current_target.to_get_tracepoint_status) (tp, utp)
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index e00538c..7b4c56a 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1824,6 +1824,8 @@ trace_start_command (char *args, int from_tty)
if (from_tty
&& !query (_("A trace is running already. Start a new run? ")))
error (_("New trace run not started."));
+
+ stop_tracing (NULL);
}

start_tracing (args);
@@ -1857,8 +1859,27 @@ stop_tracing (char *note)
if (!ret && note)
warning ("Target does not support trace notes, note ignored");

- /* Should change in response to reply? */
+ observer_notify_trace_stopped ();
+}
+
+static void
+trace_stopped (void)
+{
+ VEC(breakpoint_p) *tp_vec = NULL;
+ int ix;
+ struct breakpoint *b;
+
current_trace_status ()->running = 0;
+
+ tp_vec = all_tracepoints ();
+ for (ix = 0; VEC_iterate (breakpoint_p, tp_vec, ix, b); ix++)
+ {
+ struct bp_location *loc;
+
+ for (loc = b->loc; loc; loc = loc->next)
+ loc->inserted = 0;
+ }
+ VEC_free (breakpoint_p, tp_vec);
}

/* tstatus command */
@@ -5327,5 +5348,7 @@ Show the notes string to use for future tstop commands"), NULL,

init_tfile_ops ();

+ observer_attach_trace_stopped (trace_stopped);
+
add_target (&tfile_ops);
}
--
1.7.0.4

[-- Attachment #3: 0002-status-stop.patch --]
[-- Type: text/x-patch, Size: 5501 bytes --]

2011-12-09 Yao Qi <yao@codesourcery.com>

* gdb.trace/status-stop.exp: New.
* gdb.trace/status-stop.c: New.
---
gdb/testsuite/gdb.trace/status-stop.c | 48 ++++++++++++
gdb/testsuite/gdb.trace/status-stop.exp | 124 +++++++++++++++++++++++++++++++
2 files changed, 172 insertions(+), 0 deletions(-)
create mode 100644 gdb/testsuite/gdb.trace/status-stop.c
create mode 100644 gdb/testsuite/gdb.trace/status-stop.exp

diff --git a/gdb/testsuite/gdb.trace/status-stop.c b/gdb/testsuite/gdb.trace/status-stop.c
new file mode 100644
index 0000000..9ff69d6
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/status-stop.c
@@ -0,0 +1,48 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2011 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/>. */
+
+static void
+func1 (void)
+{}
+
+int buf[1024];
+
+static void
+func2 (void)
+{}
+
+static void
+end (void)
+{}
+
+int
+main (void)
+{
+ int i;
+
+ func1 ();
+
+ /* We call func2 as many times as possible to make sure that trace is
+ stopped due to trace buffer is full. */
+ for (i = 0; i < 10000; i++)
+ {
+ func2 ();
+ }
+
+ end ();
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/status-stop.exp b/gdb/testsuite/gdb.trace/status-stop.exp
new file mode 100644
index 0000000..6c92b75
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/status-stop.exp
@@ -0,0 +1,124 @@
+# Copyright 2011 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";
+
+set testfile "status-stop"
+set executable $testfile
+set srcfile ${testfile}.c
+set binfile $objdir/$subdir/$testfile
+set expfile $testfile.exp
+
+
+if [prepare_for_testing $expfile $executable $srcfile \
+ {debug nowarnings}] {
+ untested "failed to prepare for trace tests"
+ return -1
+}
+
+# Verify that the sequence of commands "tstart tstop tstart" works well.
+
+proc test_tstart_tstop_tstart { } {
+ global executable
+ global pf_prefix
+ global hex
+
+ set old_pf_prefix $pf_prefix
+ set pf_prefix "$pf_prefix tstart_tstop_tstart:"
+
+ # Start with a fresh gdb.
+ clean_restart ${executable}
+ if ![runto_main] {
+ fail "Can't run to main"
+ set pf_prefix $old_pf_prefix
+ return -1
+ }
+
+ gdb_test "trace func1" "Tracepoint \[0-9\] at $hex: file.*"
+ gdb_test_no_output "tstart"
+
+ gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+ gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to end"
+
+ gdb_test_no_output "tstop"
+
+ gdb_test_no_output "tstart"
+
+ set pf_prefix $old_pf_prefix
+}
+
+# Verify the sequence of commands "tstart tstart" works well.
+
+proc test_tstart_tstart { } {
+ global executable
+ global pf_prefix
+ global hex
+
+ set old_pf_prefix $pf_prefix
+ set pf_prefix "$pf_prefix tstart_tstart:"
+
+ # Start with a fresh gdb.
+ clean_restart ${executable}
+ if ![runto_main] {
+ fail "Can't run to main"
+ set pf_prefix $old_pf_prefix
+ return -1
+ }
+
+ gdb_test "trace func1" "Tracepoint \[0-9\] at $hex: file.*"
+ gdb_test_no_output "tstart"
+
+ gdb_test "tstart" "" "tstart again" "A trace is running already. Start a new run\\? \\(y or n\\) " "y"
+
+ set pf_prefix $old_pf_prefix
+}
+
+# Verify that trace stops clearly when trace buffer is full.
+
+proc test_buffer_full_tstart { } {
+ global executable
+ global pf_prefix
+ global hex
+
+ set old_pf_prefix $pf_prefix
+ set pf_prefix "$pf_prefix buffer_full_tstart:"
+
+ # Start with a fresh gdb.
+ clean_restart ${executable}
+ if ![runto_main] {
+ fail "Can't run to main"
+ set pf_prefix $old_pf_prefix
+ return -1
+ }
+
+ gdb_test "trace func2" "Tracepoint \[0-9\] at $hex: file.*"
+ gdb_trace_setactions "collect buf: define actions" \
+ "" \
+ "collect buf" "^$"
+
+ gdb_test_no_output "tstart"
+ gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+ gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to end"
+
+ gdb_test "tstatus" ".*buffer was full.*"
+ gdb_test_no_output "tstart"
+
+ set old_pf_prefix $pf_prefix
+}
+
+test_tstart_tstop_tstart
+
+test_tstart_tstart
+
+test_buffer_full_tstart
--
1.7.0.4