On 07/29/2013 02:20 PM, Yao Qi wrote: > 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 > Thanks for reviewing the patch. gdb/ChangLog 2013-07-24 Muhammad Waqas PR gdb/11568 * breakpoint.c (breakpoint_auto_delete): add new condition Remove thread related breakpoints if threads are exited. 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 29 Jul 2013 11:22:05 -0000 @@ -11910,6 +11910,15 @@ 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*/ + { + struct thread_info * tp = find_thread_id (b->thread) ; + if (tp != NULL && (tp->state == THREAD_EXITED + || !target_thread_alive (tp->ptid))) + delete_breakpoint (b); + } } } testsuite/Changlog 2013-07-29 Muhammad Waqas Jan Kratochvil * gdb.threads/thread-specific-bp.c: New file. * gdb.threads/thread-specific-bp.exp: New file. find the attached file for testcase and now testcase fails without my patch and passes with my patch Test Run By waqas on Mon Jul 29 15:47:40 2013 Native configuration is x86_64-unknown-linux-gnu === gdb tests === Schedule of variations: unix 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: threaded breakpoint deleted === gdb Summary === # of expected passes 6 /home/waqas/gdb/mygdb/newgdb/src/gdb/testsuite/../../gdb/gdb version 7.6.50.20130724-cvs -nw -nx -data-directory /home/waqas/gdb/mygdb/newgdb/src/gdb/testsuite/../data-directory