From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24289 invoked by alias); 28 Aug 2013 12:26:36 -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 24280 invoked by uid 89); 28 Aug 2013 12:26:36 -0000 Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 28 Aug 2013 12:26:36 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RDNS_NONE,SPF_HELO_FAIL autolearn=no version=3.3.2 X-HELO: relay1.mentorg.com Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1VEepU-0000bH-DN from Muhammad_Waqas@mentor.com ; Wed, 28 Aug 2013 05:26:32 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Wed, 28 Aug 2013 05:26:31 -0700 Received: from [137.202.157.111] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server (TLS) id 14.2.247.3; Wed, 28 Aug 2013 13:26:29 +0100 Message-ID: <521DEC71.9050001@codesourcery.com> Date: Wed, 28 Aug 2013 12:26:00 -0000 From: Muhammad Waqas User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130804 Thunderbird/17.0.8 MIME-Version: 1.0 To: Pedro Alves CC: 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> <521CF7D0.5040801@redhat.com> In-Reply-To: <521CF7D0.5040801@redhat.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit X-SW-Source: 2013-08/txt/msg00820.txt.bz2 On 08/28/2013 12:02 AM, Pedro Alves wrote: > 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. > > Fixed. >> 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. */ > Fixed. >> + >> +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? > Fixed. >> + } >> + } >> +} >> + > > > >> + >> +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). > Fixed. >> + >> + 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. > Fixed. >> + 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? > Replaced. >> + -re "(?:Breakpoint|Temporary breakpoint) .* (at|in) .*\r\n$gdb_prompt $" { > > Why is this matching temporary breakpoints output? > Removed. >> + 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? > Some time in non-stop when running thread (which is "thre" in this case) exited GDB prompts for commands, instead of selecting thread 1 and continue, so in that case time-out happened and I select thread 1 and continue. >> + exp_continue >> + } >> + } >> + > > Thanks, >