From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27375 invoked by alias); 29 Jul 2013 09:21:34 -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 27366 invoked by uid 89); 29 Jul 2013 09:21:33 -0000 X-Spam-SWARE-Status: No, score=-4.2 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RDNS_NONE autolearn=no version=3.3.1 Received: from Unknown (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 29 Jul 2013 09:21:33 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1V3jdt-0005LY-54 from Yao_Qi@mentor.com ; Mon, 29 Jul 2013 02:21:25 -0700 Received: from SVR-ORW-FEM-02.mgc.mentorg.com ([147.34.96.206]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Mon, 29 Jul 2013 02:21:24 -0700 Received: from qiyao.dyndns.org (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.2.247.3; Mon, 29 Jul 2013 02:21:24 -0700 Message-ID: <51F633E5.7000302@codesourcery.com> Date: Mon, 29 Jul 2013 09:21:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Muhammad Waqas CC: , , Subject: Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit References: <51F619CE.5040407@codesourcery.com> In-Reply-To: <51F619CE.5040407@codesourcery.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2013-07/txt/msg00695.txt.bz2 On 07/29/2013 03:29 PM, Muhammad Waqas wrote: > 2013-07-24 Muhammad Waqas Wrong e-mail address? Should mention PR number in ChangeLog. > > * breakpoint.c (breakpoint_auto_delete): Remove breakpoint > Remove thread related breakpoints if threads are exited. > "Remove breakpoint" can be removed, IMO. > cvs diff -up gdb/breakpoint.c > > > Index: gdb/breakpoint.c > =================================================================== > RCS file: /cvs/src/src/gdb/breakpoint.c,v > retrieving revision 1.772 > diff -u -p -r1.772 breakpoint.c > --- gdb/breakpoint.c 6 Jul 2013 07:14:54 -0000 1.772 > +++ gdb/breakpoint.c 24 Jul 2013 05:18:24 -0000 > @@ -11910,6 +11910,12 @@ breakpoint_auto_delete (bpstat bs) > { > if (b->disposition == disp_del_at_next_stop) > delete_breakpoint (b); > + else if (b->thread > 0) /* If breakpoint relates to user created > thread Check if it's not alive then delete it*/ This line is too long, and your mailer wrap the patch. > + { > + struct thread_info * tp = find_thread_id (b->thread) ; > + if (tp != NULL && (tp->state == THREAD_EXITED || > !target_thread_alive (tp->ptid))) Likewise. > + delete_breakpoint (b); > + } > } > } > I am wondering why don't we register to thread_exit observer, and remove breakpoints when thread exits. arm-linux-nat.c and ppc-linux-nat.c did something similar to delete thread hw breakpoint. However, I am not sure it is safe to do so considering async/non-stop mode. > gdb/testcase/Changlog > > 2013-07-29 Muhammad Waqas > > * gdb.threads/thread-specific-bp.c: New file It is copied from Jan's example in bugzilla, better to list his name here too. > * gdb.threads/thread-specific-bp.exp: New file. > > > gdb.threads/thread-specific-bp.c Please include these new added files into the patch. So that reviewers can apply your patch to get these new files. We need a copyright header for test files too. > gdb.threads/thread-specific-bp.exp > > #This test verfiy that Breakpoint on a spacific thread is deleted > > standard_testfile > if {[gdb_compile_pthreads \ > "${srcdir}/${subdir}/${srcfile}" \ > "${binfile}" executable {debug} ] != "" } { > return -1 > } > clean_restart ${binfile} > > if [runto main] then { > gdb_breakpoint "start" > gdb_continue_to_breakpoint "start" > 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 > set thre $expect_out(1,string) > gdb_breakpoint [gdb_get_line_number "line # 13"] > gdb_breakpoint "main thread $thre" > gdb_test "info break" ".*breakpoint.*(thread $thre)" > gdb_continue_to_breakpoint "line # 13" > gdb_test "info break" ".*breakpoint.*(thread \[^$thre\])*" > } > This test passes even without your patch to breakpoint_auto_delete above. Please make sure the test fails without your patch, and fails go away when your patch is applied. > > Running target unix > Running ./gdb.threads/thread-specific-bp.exp ... > PASS: gdb.threads/thread-specific-bp.exp: successfully compiled posix > threads test case > PASS: gdb.threads/thread-specific-bp.exp: continue to breakpoint: start > PASS: gdb.threads/thread-specific-bp.exp: thread created > PASS: gdb.threads/thread-specific-bp.exp: info break ^^^^^^^^^^ > PASS: gdb.threads/thread-specific-bp.exp: continue to breakpoint: line # 13 > PASS: gdb.threads/thread-specific-bp.exp: info break ^^^^^^^^^^^ Duplicated test results. Please make them unique. Like, PASS: gdb.threads/thread-specific-bp.exp: info break 1 ... PASS: gdb.threads/thread-specific-bp.exp: info break 2 -- Yao (齐尧)