From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17277 invoked by alias); 9 Dec 2011 15:54:05 -0000 Received: (qmail 17268 invoked by uid 22791); 9 Dec 2011 15:54:03 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,TW_XP X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 09 Dec 2011 15:53:49 +0000 Received: from nat-jpt.mentorg.com ([192.94.33.2] helo=PR1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1RZ2lg-00016B-4c from Yao_Qi@mentor.com ; Fri, 09 Dec 2011 07:53:48 -0800 Received: from [127.0.0.1] ([172.16.63.104]) by PR1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Sat, 10 Dec 2011 00:53:45 +0900 Message-ID: <4EE22F07.5070906@codesourcery.com> Date: Fri, 09 Dec 2011 16:03:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0.1) Gecko/20110930 Thunderbird/7.0.1 MIME-Version: 1.0 To: Pedro Alves CC: gdb-patches@sourceware.org, Hui Zhu , Stan Shebs Subject: Re: [PATCH] Fix tracepoint tstart again get gdb_assert References: <4EE1DA93.7080903@codesourcery.com> <201112091433.49620.pedro@codesourcery.com> In-Reply-To: <201112091433.49620.pedro@codesourcery.com> Content-Type: multipart/mixed; boundary="------------070204030006020703040502" 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-12/txt/msg00291.txt.bz2 This is a multi-part message in MIME format. --------------070204030006020703040502 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-length: 1242 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 (齐尧) --------------070204030006020703040502 Content-Type: text/x-patch; name="0001-clear-inserted-flag.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-clear-inserted-flag.patch" Content-length: 717 2011-12-09 Hui Zhu Yao Qi * 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 --------------070204030006020703040502 Content-Type: text/x-patch; name="0002-status-stop.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0002-status-stop.patch" Content-length: 5529 2011-12-09 Yao Qi * 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 . */ + +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 . + +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 --------------070204030006020703040502--