Re: [PATCH] Fix tracepoint tstart again get gdb_assert

Yao Qi
2011-12-09T15:54:05Z

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

On 12/09/2011 10:33 PM, Pedro Alves wrote:
> I think this is overkill. It's just impossible to be absolutely
> certain a trace run is currently ongoing. If you do "tstatus;tstart",
> the trace can still stop between the two commands. Is there

This is a very good example.

> anything that we benefit from clearing the inserted flag
> on tstatus? I think that if we clear them only, and

No, I don't think we benefit from clearing `inserted' flag on tstatus.
The intention of patch v2 is trying to make gdb's trace status as closed
to remote stub's as possible. Your example above prove that it may make
no sense.

> always, at "tstart" time, we're good. We already clear all
> the inserted flag of all breakpoints and tracepoints on target
> open ("target remote ...", etc.), from within
> breakpoint.c:breakpoint_init_inferior -- that's why you don't
> see this assertion if you disconnect and reconnect, and find
> the target was tracing.
>
> So I think we should just clear the inserted flag of all
> tracepoints in start_tracing, right after the target_trace_init
> call. Possibly even merge the new loop with the existing loop
> that downloads tracepoints.

This sounds right, and makes patch quite simpler :)

--
Yao (é½å°§)

[-- Attachment #2: 0001-clear-inserted-flag.patch --]
[-- Type: text/x-patch, Size: 717 bytes --]

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

* tracepoint.c (start_tracing): Clear `inserted' flag.
---
gdb/tracepoint.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index e00538c..da7298c 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1756,6 +1756,10 @@ start_tracing (char *notes)
struct tracepoint *t = (struct tracepoint *) b;
struct bp_location *loc;

+ /* Clear `inserted' flag. */
+ for (loc = b->loc; loc; loc = loc->next)
+ loc->inserted = 0;
+
if ((b->type == bp_fast_tracepoint
? !may_insert_fast_tracepoints
: !may_insert_tracepoints))
--
1.7.0.4

[-- Attachment #3: 0002-status-stop.patch --]
[-- Type: text/x-patch, Size: 5529 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
\ No newline at end of file
--
1.7.0.4