From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25667 invoked by alias); 27 Aug 2013 19:02:47 -0000 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 Received: (qmail 25653 invoked by uid 89); 27 Aug 2013 19:02:47 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 27 Aug 2013 19:02:47 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-6.1 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r7RJ2guc006075 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 27 Aug 2013 15:02:42 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r7RJ2eVe032086; Tue, 27 Aug 2013 15:02:41 -0400 Message-ID: <521CF7D0.5040801@redhat.com> Date: Tue, 27 Aug 2013 19:02:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Muhammad Waqas CC: gdb-patches@sourceware.org Subject: Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit References: <521646F5.1010509@redhat.com> <1377602943-9177-1-git-send-email-mwaqas@codesourcery.com> In-Reply-To: <1377602943-9177-1-git-send-email-mwaqas@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-08/txt/msg00797.txt.bz2 On 08/27/2013 12:29 PM, Muhammad Waqas wrote: > 2013-08-05 Muhammad Waqas > > PR gdb/11568 > * breakpoint.c (remove_threaded_breakpoints): New function. > * breakpoint.c (_initialize_breakpoint): function > remove_threaded_breakpoints registers with thread_exit. Indentation isn't right. See . "Function" should be uppercase. * breakpoint.c (_initialize_breakpoint): Function remove_threaded_breakpoints registers with thread_exit. > 2013-07-24 Muhammad Waqas > Jan Kratochvil > > PR gdb/11568 > * gdb.thread/thread-specific-bp.c: New file. > * gdb.thread/thread-specific-bp.exp: New file. > --- > gdb/breakpoint.c | 19 +++++ > gdb/testsuite/gdb.threads/thread-specific-bp.c | 33 ++++++++ > gdb/testsuite/gdb.threads/thread-specific-bp.exp | 98 ++++++++++++++++++++++ > 3 files changed, 150 insertions(+) > create mode 100644 gdb/testsuite/gdb.threads/thread-specific-bp.c > create mode 100644 gdb/testsuite/gdb.threads/thread-specific-bp.exp > > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index 44bb7a8..588a9b0 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -2928,6 +2928,24 @@ remove_breakpoints (void) > return val; > } > > +/* Used when a thread exits, it will remove breakpoints which > + are related to that thread. */ /* When a thread exits, remove breakpoints that are related to that thread. */ > + > +static void > +remove_threaded_breakpoints (struct thread_info *tp, int silent) > +{ > + struct breakpoint *b, *b_tmp; > + > + ALL_BREAKPOINTS_SAFE (b, b_tmp) > + { > + if (b->thread == tp->num) > + { > + b->disposition = disp_del_at_next_stop; > + b->number = 0; Add a comment: /* Hide it from the user. */ b->number = 0; In an earlier review I remarked that this is deleting a breakpoint silently, while on other similar occasions (local watchpoint, for e.g.) we're verbose about it. What do people think we should do here? > + } > + } > +} > + > + > +if {[gdb_compile_pthreads \ > + "${srcdir}/${subdir}/${srcfile}" \ > + "${binfile}" executable {debug} ] != "" } { > + return -1 > +} > + > +clean_restart ${binfile} > + > +proc check_threaded_breakpoint {mode} { with_test_prefix "$mode" { Please move the with_test_prefix to its own line, and re-indent the rest to follow. (Existing cases that look like that appeared due to a wholesale conversion to with_test_prefix; done that way to avoid touching much else). > + > + global gdb_prompt > + gdb_breakpoint "start" > + gdb_continue_to_breakpoint "start" > + set thre 0 > + > + gdb_test_multiple "info threads" "get thread 1 id" { > + -re "(\[0-9\]+)(\[^\n\r\]*Thread\[^\n\r\]*start.*)($gdb_prompt $)" { > + pass "thread created" > + # get the id of thread Write full sentences please. Uppercase, period. Suggest: # Get the thread's id. > + set thre $expect_out(1,string) > + } > + } > + gdb_breakpoint "main thread $thre" > + gdb_test "info break" ".*breakpoint.*thread $thre" "Breakpoint set" > + gdb_breakpoint [gdb_get_line_number "set break here"] > + > + # Force GDB to update its knowledge on existing threads when this > + # breakpoint is hit. Otherwise, GDB doesn't realize thread $thre > + # has exited and doesn't remove the thread specific breakpoint. > + gdb_test "commands\ninfo threads\nend" "End with.*" "add breakpoint commands" > + gdb_test "thread $thre" "Switching to thread $thre.*" "Thread $thre selected" > + set full_name "continue to breakpoint: set break here" > + > + send_gdb "continue\n" > + gdb_expect 10 { Why not gdb_test_multiple? > + -re "(?:Breakpoint|Temporary breakpoint) .* (at|in) .*\r\n$gdb_prompt $" { Why is this matching temporary breakpoints output? > + pass $full_name > + } > + -re ".*$gdb_prompt $" { > + fail $full_name > + } > + timeout { > + send_gdb "thread 1\n" Hmm, I don't really understand this. Can you explain, please? > + exp_continue > + } > + } > + Thanks, -- Pedro Alves