* [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit
@ 2013-07-29 7:29 Muhammad Waqas
2013-07-29 9:21 ` Yao Qi
0 siblings, 1 reply; 36+ messages in thread
From: Muhammad Waqas @ 2013-07-29 7:29 UTC (permalink / raw)
To: gdb-patches; +Cc: tromey, ali_anwar
In my previous post (
http://sourceware.org/ml/gdb-patches/2013-07/msg00560.html ), Tom ask me
for testcase here is complete patch with testcase .
gdb ChangLog
2013-07-24 Muhammad Waqas <waqas.jamil47@gmail.com>
* breakpoint.c (breakpoint_auto_delete): Remove breakpoint
Remove thread related breakpoints if threads are exited.
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*/
+ {
+ struct thread_info * tp = find_thread_id (b->thread) ;
+ if (tp != NULL && (tp->state == THREAD_EXITED ||
!target_thread_alive (tp->ptid)))
+ delete_breakpoint (b);
+ }
}
}
gdb/testcase/Changlog
2013-07-29 Muhammad Waqas <mwaqas@codesourcery.com>
* gdb.threads/thread-specific-bp.c: New file
* gdb.threads/thread-specific-bp.exp: New file.
gdb.threads/thread-specific-bp.c
#include <stdio.h>
#include <pthread.h>
static void *
start (void *arg)
{
return NULL;
}
int
main (void)
{
pthread_t thread;
pthread_create (&thread, NULL, start, NULL);
pthread_join (thread, NULL);
return 0; /*line # 13*/
}
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\])*"
}
runtest gdb.threads/thread-specific-bp.exp
test Run By waqas on Mon Jul 29 10:57:26 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: info break
=== gdb Summary ===
# of expected passes 6
/home/waqas/gdb/mygdb/gdb-7.6/gdb/testsuite/../../gdb/gdb version 7.6
-nw -nx -data-directory
/home/waqas/gdb/mygdb/gdb-7.6/gdb/testsuite/../data-directory
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-07-29 7:29 [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit Muhammad Waqas @ 2013-07-29 9:21 ` Yao Qi 2013-07-29 11:42 ` Muhammad Waqas 0 siblings, 1 reply; 36+ messages in thread From: Yao Qi @ 2013-07-29 9:21 UTC (permalink / raw) To: Muhammad Waqas; +Cc: gdb-patches, tromey, ali_anwar On 07/29/2013 03:29 PM, Muhammad Waqas wrote: > 2013-07-24 Muhammad Waqas<waqas.jamil47@gmail.com> 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<mwaqas@codesourcery.com> > > * 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 (é½å°§) ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-07-29 9:21 ` Yao Qi @ 2013-07-29 11:42 ` Muhammad Waqas 2013-07-29 14:18 ` Yao Qi 0 siblings, 1 reply; 36+ messages in thread From: Muhammad Waqas @ 2013-07-29 11:42 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches, tromey, ali_anwar [-- Attachment #1: Type: text/plain, Size: 6445 bytes --] 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<waqas.jamil47@gmail.com> > > 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<mwaqas@codesourcery.com> >> >> * 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 <mwaqas@codesourcery.com> 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 <mwaqas@codesourcery.com> Jan Kratochvil <jan.kratochvil@redhat.com> * 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 [-- Attachment #2: patch.tar.gz --] [-- Type: application/x-gzip, Size: 2976 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-07-29 11:42 ` Muhammad Waqas @ 2013-07-29 14:18 ` Yao Qi 2013-07-30 10:34 ` Muhammad Waqas 0 siblings, 1 reply; 36+ messages in thread From: Yao Qi @ 2013-07-29 14:18 UTC (permalink / raw) To: Muhammad Waqas; +Cc: gdb-patches, tromey, ali_anwar On 07/29/2013 07:42 PM, Muhammad Waqas wrote: Please have a look at http://sourceware.org/gdb/wiki/ContributionChecklist , it is very helpful for you to avoid some issues in your patch. > 2013-07-24 Muhammad Waqas<mwaqas@codesourcery.com> ^^^ Two spaces. > > PR gdb/11568 > * breakpoint.c (breakpoint_auto_delete): add new condition > Remove thread related breakpoints if threads are exited. ^^^^^^^ Tab instead of spaces. The changelog entry is like this: PR gdb/11568 * breakpoint.c (breakpoint_auto_delete): 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*/ Please move the comments into the brackets below. Period is missing in your comment, and we need two spaces after period at the end of comment. > + { > + struct thread_info * tp = find_thread_id (b->thread) ; A blank line is needed here. Indentation looks wrong here. > + if (tp != NULL && (tp->state == THREAD_EXITED > + || !target_thread_alive (tp->ptid))) > + delete_breakpoint (b); and here. > + } > } > } > I run thread-specific-bp.exp in async/non-stop mode, and I get a fail. I am afraid your patch only works in all-stop mode. > > testsuite/Changlog > > 2013-07-29 Muhammad Waqas<mwaqas@codesourcery.com> > Jan Kratochvil<jan.kratochvil@redhat.com> ^^^^^^^^^^ Need a tab instead of spaces. Please reference existing entries in ChangeLog to see how to list two authors. > > * gdb.threads/thread-specific-bp.c: New file. > * gdb.threads/thread-specific-bp.exp: New file. Please include these new added files into your patch, so that people can review them. Here is an example (http://sourceware.org/ml/gdb-patches/2013-07/msg00671.html) I include new added file in the patch. I use git, but you can get the similar patch in cvs. > # Copyright (C) 1996-2013 Free Software Foundation, Inc. It should be 2013 only. > gdb_test_multiple "info breakpoint" "get info" { > -re "(\[0-9\]+)(\[^\n\r\]*breakpoint\[^\n\r\]*main.*thread $thre)" { I don't really understand this pattern. Probably, we only want to match "thread $thre" ".*$gdb_prompt $" is missing at the end of the pattern. Please add it. > fail "threaded breakpoint not deleted" > } > -re "(\[0-9\]+)(\[^\n\r\]*breakpoint\[^\n\r\]*main.*\n)" { > pass "threaded breakpoint deleted" fail/pass should be the same. > } > } Something like this: set test "thread-specific breakpoint is deleted" gdb_test_multiple "info breakpoint" $test { -re "thread $thre.*$gdb_prompt $" { fail $test } -re "$gdb_prompt $" { pass $test } } There are some trailing spaces in the test. Please remove them. -- Yao (é½å°§) ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-07-29 14:18 ` Yao Qi @ 2013-07-30 10:34 ` Muhammad Waqas 2013-07-31 2:41 ` Yao Qi 2013-08-01 11:57 ` Pedro Alves 0 siblings, 2 replies; 36+ messages in thread From: Muhammad Waqas @ 2013-07-30 10:34 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches, tromey, ali_anwar On 07/29/2013 07:17 PM, Yao Qi wrote: > On 07/29/2013 07:42 PM, Muhammad Waqas wrote: > > Please have a look at > http://sourceware.org/gdb/wiki/ContributionChecklist , it is very > helpful for you to avoid some issues in your patch. > >> 2013-07-24 Muhammad Waqas<mwaqas@codesourcery.com> > ^^^ Two spaces. > >> >> PR gdb/11568 >> * breakpoint.c (breakpoint_auto_delete): add new condition >> Remove thread related breakpoints if threads are exited. > ^^^^^^^ Tab instead of spaces. > > The changelog entry is like this: > > PR gdb/11568 > * breakpoint.c (breakpoint_auto_delete): 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*/ > > Please move the comments into the brackets below. Period is missing > in your comment, and we need two spaces after period at the end of > comment. > >> + { >> + struct thread_info * tp = find_thread_id (b->thread) ; > > A blank line is needed here. Indentation looks wrong here. > >> + if (tp != NULL && (tp->state == THREAD_EXITED >> + || !target_thread_alive (tp->ptid))) >> + delete_breakpoint (b); > > and here. > >> + } >> } >> } >> > > I run thread-specific-bp.exp in async/non-stop mode, and I get a fail. > I am afraid your patch only works in all-stop mode. > >> >> testsuite/Changlog >> >> 2013-07-29 Muhammad Waqas<mwaqas@codesourcery.com> >> Jan Kratochvil<jan.kratochvil@redhat.com> > ^^^^^^^^^^ > Need a tab instead of spaces. Please reference existing entries in > ChangeLog to see how to list two authors. > >> >> * gdb.threads/thread-specific-bp.c: New file. >> * gdb.threads/thread-specific-bp.exp: New file. > > Please include these new added files into your patch, so that people > can review them. Here is an example > (http://sourceware.org/ml/gdb-patches/2013-07/msg00671.html) I include > new added file in the patch. I use git, but you can get the similar > patch in cvs. > >> # Copyright (C) 1996-2013 Free Software Foundation, Inc. > > It should be 2013 only. > >> gdb_test_multiple "info breakpoint" "get info" { >> -re "(\[0-9\]+)(\[^\n\r\]*breakpoint\[^\n\r\]*main.*thread >> $thre)" { > > I don't really understand this pattern. Probably, we only want to > match "thread $thre" > > ".*$gdb_prompt $" is missing at the end of the pattern. Please add it. > >> fail "threaded breakpoint not deleted" >> } >> -re "(\[0-9\]+)(\[^\n\r\]*breakpoint\[^\n\r\]*main.*\n)" { >> pass "threaded breakpoint deleted" > > fail/pass should be the same. > >> } >> } > > Something like this: > > set test "thread-specific breakpoint is deleted" > gdb_test_multiple "info breakpoint" $test { > -re "thread $thre.*$gdb_prompt $" { > fail $test > } > -re "$gdb_prompt $" { > pass $test > } > } > > There are some trailing spaces in the test. Please remove them. > Index: ChangeLog =================================================================== RCS file: /cvs/src/src/ChangeLog,v retrieving revision 1.1075 diff -u -p -r1.1075 ChangeLog --- ChangeLog 22 Jul 2013 15:17:20 -0000 1.1075 +++ ChangeLog 30 Jul 2013 10:20:09 -0000 @@ -1,3 +1,9 @@ +2013-07-24 Muhammad Waqas <mwaqas@codesourcery.com> + + PR gdb/11568 + * breakpoint.c (breakpoint_auto_delete): Remove thread related + breakpoints if threads are exited. + 2013-07-22 Joel Brobecker <brobecker@adacore.com> * src-release (VER): Use $(TOOL)/common/create-version.sh Index: gdb/breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.773 diff -u -p -r1.773 breakpoint.c --- gdb/breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773 +++ gdb/breakpoint.c 30 Jul 2013 10:20:49 -0000 @@ -11941,6 +11941,17 @@ 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); + + } } } Problem was with my test case, now It works with async/non-stop mode. Index: gdb/testsuite/ChangeLog =================================================================== RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v retrieving revision 1.3748 diff -u -p -r1.3748 ChangeLog --- gdb/testsuite/ChangeLog 26 Jul 2013 18:15:06 -0000 1.3748 +++ gdb/testsuite/ChangeLog 30 Jul 2013 10:21:11 -0000 @@ -1,3 +1,9 @@ +2013-07-24 Muhammad Waqas <mwaqas@codesourccery.com> + Jan Kratochvil <jan.kartochvil@redhat.com> + + *gdb.thread/thread-specific-bp.c: Newfile. + *gdb.thread/thread-specific-bp.exp: Newfile. + 2013-07-26 Keith Seitz <keiths@redhat.com> * gdb.mi/mi-var-child-f.exp: Pass f90 to gdb_compile instead Index: gdb/testsuite/gdb.threads/thread-specific-bp.exp =================================================================== RCS file: gdb/testsuite/gdb.threads/thread-specific-bp.exp diff -N gdb/testsuite/gdb.threads/thread-specific-bp.exp --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ gdb/testsuite/gdb.threads/thread-specific-bp.exp 30 Jul 2013 10:21:24 -0000 @@ -0,0 +1,54 @@ +# Copyright (C) 2013 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 <http://www.gnu.org/licenses/>. + +# This file was written by Muhammad Waqas <mwaqas@codesourcery.com> +# +#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)" "Breakpoint set" + gdb_test "thread $thre" "Switching to thread $thre.*" "Thread $thre selected" + gdb_continue_to_breakpoint "line # 13" + set test "thread-specific breakpoint is deleted" + gdb_test_multiple "info breakpoint" $test { + -re "thread $thre.*$gdb_prompt $" { + fail $test + } + -re "$gdb_prompt $" { + pass $test + } + } +} Index: gdb/testsuite/gdb.threads/thread-specific-bp.c =================================================================== RCS file: gdb/testsuite/gdb.threads/thread-specific-bp.c diff -N gdb/testsuite/gdb.threads/thread-specific-bp.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ gdb/testsuite/gdb.threads/thread-specific-bp.c 30 Jul 2013 10:21:38 -0000 @@ -0,0 +1,34 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 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 <http://www.gnu.org/licenses/>. */ + +#include <stdio.h> +#include <pthread.h> + +static void * +start (void *arg) +{ + return NULL; +} + +int +main (void) +{ + pthread_t thread; + pthread_create (&thread, NULL, start, NULL); + pthread_join (thread, NULL); + return 0; /*line # 13*/ +} ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-07-30 10:34 ` Muhammad Waqas @ 2013-07-31 2:41 ` Yao Qi 2013-08-01 10:51 ` Pedro Alves 2013-08-01 11:57 ` Pedro Alves 1 sibling, 1 reply; 36+ messages in thread From: Yao Qi @ 2013-07-31 2:41 UTC (permalink / raw) To: Muhammad Waqas; +Cc: gdb-patches, tromey, ali_anwar On 07/30/2013 06:33 PM, Muhammad Waqas wrote: > --- gdb/breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773 > +++ gdb/breakpoint.c 30 Jul 2013 10:20:49 -0000 > @@ -11941,6 +11941,17 @@ 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 ^ redundant space after parenthesis. > + || !target_thread_alive (tp->ptid))) > + delete_breakpoint(b); > + > + } > } > } > > Problem was with my test case, now It works with async/non-stop mode. > One more thing below... > + # 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)" "Breakpoint set" > + gdb_test "thread $thre" "Switching to thread $thre.*" "Thread $thre selected" > + gdb_continue_to_breakpoint "line # 13" > + set test "thread-specific breakpoint is deleted" > + gdb_test_multiple "info breakpoint" $test { We continue to line 33, and type command 'info breakpoint' to check the thread-specific breakpoint is disappeared. We take an assumption that when main thread hits breakpoint on line 33, thread 2 has been exited, and GDB has known about it. However, in async/non-stop mode, it is not always true. The exit of thread 2 happens before thread 1 hits breakpoint on line 33. However, the order of 'thread 1 hits breakpoint' and 'GDB knows about thread 2's exit' is not determined, in some runs (in async/non-stop mode), I can get the fail below, continue^M Continuing.^M ^M Breakpoint 3, main () at ../../../../git/gdb/testsuite/gdb.threads/thread-specific-bp.c:33^M 33 return 0; /*line # 13*/^M (gdb) PASS: gdb.threads/thread-specific-bp.exp: continue to breakpoint: line # 13 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ thread 1 hits breakpoint first, [Thread 0xb7fddb40 (LWP 19876) exited]^M ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ GDB is aware of the exist of thread 2 then, so the thread-specific breakpoint is not deleted. info breakpoint^M Num Type Disp Enb Address What^M 1 breakpoint keep y 0x08048487 in main at ../../../../git/gdb/testsuite/gdb.threads/thread-specific-bp.c:31^M breakpoint already hit 1 time^M 2 breakpoint keep y 0x08048477 in start at ../../../../git/gdb/testsuite/gdb.threads/thread-specific-bp.c:24^M breakpoint already hit 1 time^M 3 breakpoint keep y 0x080484bf in main at ../../../../git/gdb/testsuite/gdb.threads/thread-specific-bp.c:33^M breakpoint already hit 1 time^M 4 breakpoint keep y 0x08048487 in main at ../../../../git/gdb/testsuite/gdb.threads/thread-specific-bp.c:31 thread 2^M stop only in thread 2^M (gdb) FAIL: gdb.threads/thread-specific-bp.exp: thread-specific breakpoint is deleted I suggest that we can add a breakpoint command 'info threads' in the breakpoint, which forces GDB to update the list of threads, IMO. I add this below in the test, and didn't see the fail after many runs of test. gdb_breakpoint [gdb_get_line_number "line # 13"] # 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" The patch looks in a good shape now. I don't have any comments. Leave it to people who can approve it. -- Yao (é½å°§) ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-07-31 2:41 ` Yao Qi @ 2013-08-01 10:51 ` Pedro Alves 2013-08-01 10:59 ` Yao Qi 0 siblings, 1 reply; 36+ messages in thread From: Pedro Alves @ 2013-08-01 10:51 UTC (permalink / raw) To: Yao Qi; +Cc: Muhammad Waqas, gdb-patches, tromey, ali_anwar On 07/31/2013 03:40 AM, Yao Qi wrote: > The exit of thread 2 happens before thread 1 hits breakpoint on line 33. > However, the order of 'thread 1 hits breakpoint' and 'GDB knows about > thread 2's exit' is not determined, in some runs (in async/non-stop > mode), I can get the fail below, Hmm. All-stop and non-stop behave different by design. What's the point of making an all-stop test work in non-stop? async vs sync is a different issue though -- that should be transparent. Did you mean just async? -- Pedro Alves ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-01 10:51 ` Pedro Alves @ 2013-08-01 10:59 ` Yao Qi 2013-08-01 11:27 ` Pedro Alves 0 siblings, 1 reply; 36+ messages in thread From: Yao Qi @ 2013-08-01 10:59 UTC (permalink / raw) To: Pedro Alves; +Cc: Muhammad Waqas, gdb-patches, tromey, ali_anwar On 08/01/2013 06:51 PM, Pedro Alves wrote: > Hmm. All-stop and non-stop behave different by design. What's the > point of making an all-stop test work in non-stop? async vs sync Pedro, Is it an all-stop test? I think it should work on both all-stop and non-stop. > is a different issue though -- that should be transparent. Did you > mean just async? I meant async + non-stop. I get one fail from time to time when I run the test with async on and non-stop on. I run the test with async on only, but can't get one fail in ten runs. -- Yao (é½å°§) ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-01 10:59 ` Yao Qi @ 2013-08-01 11:27 ` Pedro Alves 2013-08-01 12:10 ` Yao Qi 0 siblings, 1 reply; 36+ messages in thread From: Pedro Alves @ 2013-08-01 11:27 UTC (permalink / raw) To: Yao Qi; +Cc: Pedro Alves, Muhammad Waqas, gdb-patches, tromey, ali_anwar On 08/01/2013 11:58 AM, Yao Qi wrote: > On 08/01/2013 06:51 PM, Pedro Alves wrote: >> Hmm. All-stop and non-stop behave different by design. What's the >> point of making an all-stop test work in non-stop? async vs sync > > Pedro, > Is it an all-stop test? I think it should work on both all-stop and > non-stop. Then it should explicitly test both modes. Say, move the body to a procedure, and call it twice. Running the gdb.threads/ tests with forced non-stop mode really makes no sense. >> is a different issue though -- that should be transparent. Did you >> mean just async? > > I meant async + non-stop. I get one fail from time to time when I run > the test with async on and non-stop on. I run the test with async on > only, but can't get one fail in ten runs. -- Pedro Alves ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-01 11:27 ` Pedro Alves @ 2013-08-01 12:10 ` Yao Qi 0 siblings, 0 replies; 36+ messages in thread From: Yao Qi @ 2013-08-01 12:10 UTC (permalink / raw) To: Pedro Alves; +Cc: Muhammad Waqas, gdb-patches, tromey, ali_anwar On 08/01/2013 07:27 PM, Pedro Alves wrote: > Then it should explicitly test both modes. Say, move the body > to a procedure, and call it twice. Running the gdb.threads/ > tests with forced non-stop mode really makes no sense. OK, I am fine with it. Then, I withdraw my comment on adding a breakpoint command 'info threads' in the breakpoint on line 33. It is in this mail <http://sourceware.org/ml/gdb-patches/2013-07/msg00805.html> -- Yao (é½å°§) ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-07-30 10:34 ` Muhammad Waqas 2013-07-31 2:41 ` Yao Qi @ 2013-08-01 11:57 ` Pedro Alves 2013-08-01 12:44 ` Muhammad Waqas 1 sibling, 1 reply; 36+ messages in thread From: Pedro Alves @ 2013-08-01 11:57 UTC (permalink / raw) To: Muhammad Waqas; +Cc: Yao Qi, gdb-patches, tromey, ali_anwar On 07/30/2013 11:33 AM, Muhammad Waqas wrote: > On 07/29/2013 07:17 PM, Yao Qi wrote: >> On 07/29/2013 07:42 PM, Muhammad Waqas wrote: >> >> Please have a look at >> http://sourceware.org/gdb/wiki/ContributionChecklist , it is very >> helpful for you to avoid some issues in your patch. >> >>> 2013-07-24 Muhammad Waqas<mwaqas@codesourcery.com> >> ^^^ Two spaces. >> >>> >>> PR gdb/11568 >>> * breakpoint.c (breakpoint_auto_delete): add new condition >>> Remove thread related breakpoints if threads are exited. >> ^^^^^^^ Tab instead of spaces. >> >> The changelog entry is like this: >> >> PR gdb/11568 >> * breakpoint.c (breakpoint_auto_delete): 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*/ >> >> Please move the comments into the brackets below. Period is missing >> in your comment, and we need two spaces after period at the end of >> comment. >> >>> + { >>> + struct thread_info * tp = find_thread_id (b->thread) ; >> >> A blank line is needed here. Indentation looks wrong here. >> >>> + if (tp != NULL && (tp->state == THREAD_EXITED >>> + || !target_thread_alive (tp->ptid))) >>> + delete_breakpoint (b); >> >> and here. >> >>> + } >>> } >>> } >>> >> >> I run thread-specific-bp.exp in async/non-stop mode, and I get a fail. >> I am afraid your patch only works in all-stop mode. >> >>> >>> testsuite/Changlog >>> >>> 2013-07-29 Muhammad Waqas<mwaqas@codesourcery.com> >>> Jan Kratochvil<jan.kratochvil@redhat.com> >> ^^^^^^^^^^ >> Need a tab instead of spaces. Please reference existing entries in >> ChangeLog to see how to list two authors. >> >>> >>> * gdb.threads/thread-specific-bp.c: New file. >>> * gdb.threads/thread-specific-bp.exp: New file. >> >> Please include these new added files into your patch, so that people >> can review them. Here is an example >> (http://sourceware.org/ml/gdb-patches/2013-07/msg00671.html) I include >> new added file in the patch. I use git, but you can get the similar >> patch in cvs. >> >>> # Copyright (C) 1996-2013 Free Software Foundation, Inc. >> >> It should be 2013 only. >> >>> gdb_test_multiple "info breakpoint" "get info" { >>> -re "(\[0-9\]+)(\[^\n\r\]*breakpoint\[^\n\r\]*main.*thread >>> $thre)" { >> >> I don't really understand this pattern. Probably, we only want to >> match "thread $thre" >> >> ".*$gdb_prompt $" is missing at the end of the pattern. Please add it. >> >>> fail "threaded breakpoint not deleted" >>> } >>> -re "(\[0-9\]+)(\[^\n\r\]*breakpoint\[^\n\r\]*main.*\n)" { >>> pass "threaded breakpoint deleted" >> >> fail/pass should be the same. >> >>> } >>> } >> >> Something like this: >> >> set test "thread-specific breakpoint is deleted" >> gdb_test_multiple "info breakpoint" $test { >> -re "thread $thre.*$gdb_prompt $" { >> fail $test >> } >> -re "$gdb_prompt $" { >> pass $test >> } >> } >> >> There are some trailing spaces in the test. Please remove them. >> > Index: ChangeLog > =================================================================== > RCS file: /cvs/src/src/ChangeLog,v > retrieving revision 1.1075 > diff -u -p -r1.1075 ChangeLog > --- ChangeLog 22 Jul 2013 15:17:20 -0000 1.1075 > +++ ChangeLog 30 Jul 2013 10:20:09 -0000 > @@ -1,3 +1,9 @@ > +2013-07-24 Muhammad Waqas <mwaqas@codesourcery.com> > + > + PR gdb/11568 > + * breakpoint.c (breakpoint_auto_delete): Remove thread related > + breakpoints if threads are exited. > + > 2013-07-22 Joel Brobecker <brobecker@adacore.com> > > * src-release (VER): Use $(TOOL)/common/create-version.sh > Index: gdb/breakpoint.c > =================================================================== > RCS file: /cvs/src/src/gdb/breakpoint.c,v > retrieving revision 1.773 > diff -u -p -r1.773 breakpoint.c > --- gdb/breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773 > +++ gdb/breakpoint.c 30 Jul 2013 10:20:49 -0000 > @@ -11941,6 +11941,17 @@ 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. */ "Check" is capitalized in the middle of the sentence. But instead you'd just say: /* Delete thread specific breakpoints whose thread is done. */ > + struct thread_info *tp = find_thread_id (b->thread); > + > + if ( tp != NULL && (tp->state == THREAD_EXITED ^ spurious space. > + || !target_thread_alive (tp->ptid))) > + if ( tp != NULL && (tp->state == THREAD_EXITED > + || !target_thread_alive (tp->ptid))) Do we really need this target_thread_alive call? It means we have extra remote traffic whenever we have a thread specific breakpoint in the list. If the user (or GDB itself) hasn't refreshed the thread list, is there any harm in delaying deleting the breakpoint? But, I think I agree with Yao in that this doesn't look like the right way to do this. In fact, breakpoint_auto_delete is only called when the target reports some stop. If you're trying to delete stale thread specific breakpoints, then you'd want to do that even if the target hasn't reported a stop, right? Say, in non-stop mode, w/ the remote target, if you have two threads, set a thread-specific break on thread 2, and while all threads are running free in busy loops, not reporting any event, keep doing "info threads", and "info break". At some point thread #2 exits. You can see that from "info threads". But "info break" will still show you the stale breakpoint... > + delete_breakpoint(b); Missing space before parens. This deletes the breakpoint silently, which may be surprising. We're not silent when we delete watchpoints on local scopes. > + > + } > } > } > > Problem was with my test case, now It works with async/non-stop mode. What would be interesting is whether it works with both native and remote targets, as with remote targets we only know whether threads are gone when we refresh the thread list, while with native GNU/Linux, presently, the core knows when a thread is gone as soon as it exits. As I said in the other email, if this is supposed to run in non-stop mode too, then let's make the test test both all-stop and non-stop modes explicitly. > +# This file was written by Muhammad Waqas <mwaqas@codesourcery.com> Can I kindly ask you to consider removing this? From the ChangeLog, it's already wrong from the onset. > +#This test verfiy that Breakpoint on a spacific thread is deleted What the typos, missing periods, etc. I'd write: # Verify that a thread-specific breakpoint is deleted when the # corresponding thread is gone. > + > +standard_testfile > + > +if {[gdb_compile_pthreads \ > + "${srcdir}/${subdir}/${srcfile}" \ > + "${binfile}" executable {debug} ] != "" } { > + return -1 > +} > + > +clean_restart ${binfile} > +if [runto main] then { It's more idiomatic to do if [runto main] then { return } <rest of code here> and avoid the indentation. > + > + 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) If the previous test fails, "thre" will hold something unrelated. Initialize "thre" outside the gdb_test_multiple, and extract the id close to the "pass". > + gdb_breakpoint [gdb_get_line_number "line # 13"] > + gdb_breakpoint "main thread $thre" > + gdb_test "info break" ".*breakpoint.*(thread $thre)" "Breakpoint set" Why the parens? > + gdb_test "thread $thre" "Switching to thread $thre.*" "Thread $thre selected" > + gdb_continue_to_breakpoint "line # 13" Why "line # 13" ? Does "13" have any significance? Let's avoid such confusions in the minds of whoever reads the code. It's more usual to write "set break here" or some such in the code instead, and look for that. > Index: gdb/testsuite/gdb.threads/thread-specific-bp.c > =================================================================== > RCS file: gdb/testsuite/gdb.threads/thread-specific-bp.c > diff -N gdb/testsuite/gdb.threads/thread-specific-bp.c > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ gdb/testsuite/gdb.threads/thread-specific-bp.c 30 Jul 2013 10:21:38 -0000 > +#include <stdio.h> This include looks unnecessary. -- Pedro Alves ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-01 11:57 ` Pedro Alves @ 2013-08-01 12:44 ` Muhammad Waqas 2013-08-02 9:45 ` Pedro Alves 0 siblings, 1 reply; 36+ messages in thread From: Muhammad Waqas @ 2013-08-01 12:44 UTC (permalink / raw) To: Pedro Alves; +Cc: Yao Qi, gdb-patches, tromey, ali_anwar On 08/01/2013 04:57 PM, Pedro Alves wrote: > On 07/30/2013 11:33 AM, Muhammad Waqas wrote: >> On 07/29/2013 07:17 PM, Yao Qi wrote: >>> On 07/29/2013 07:42 PM, Muhammad Waqas wrote: >>> >>> Please have a look at >>> http://sourceware.org/gdb/wiki/ContributionChecklist , it is very >>> helpful for you to avoid some issues in your patch. >>> >>>> 2013-07-24 Muhammad Waqas<mwaqas@codesourcery.com> >>> ^^^ Two spaces. >>> >>>> PR gdb/11568 >>>> * breakpoint.c (breakpoint_auto_delete): add new condition >>>> Remove thread related breakpoints if threads are exited. >>> ^^^^^^^ Tab instead of spaces. >>> >>> The changelog entry is like this: >>> >>> PR gdb/11568 >>> * breakpoint.c (breakpoint_auto_delete): 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*/ >>> Please move the comments into the brackets below. Period is missing >>> in your comment, and we need two spaces after period at the end of >>> comment. >>> >>>> + { >>>> + struct thread_info * tp = find_thread_id (b->thread) ; >>> A blank line is needed here. Indentation looks wrong here. >>> >>>> + if (tp != NULL && (tp->state == THREAD_EXITED >>>> + || !target_thread_alive (tp->ptid))) >>>> + delete_breakpoint (b); >>> and here. >>> >>>> + } >>>> } >>>> } >>>> >>> I run thread-specific-bp.exp in async/non-stop mode, and I get a fail. >>> I am afraid your patch only works in all-stop mode. >>> >>>> testsuite/Changlog >>>> >>>> 2013-07-29 Muhammad Waqas<mwaqas@codesourcery.com> >>>> Jan Kratochvil<jan.kratochvil@redhat.com> >>> ^^^^^^^^^^ >>> Need a tab instead of spaces. Please reference existing entries in >>> ChangeLog to see how to list two authors. >>> >>>> * gdb.threads/thread-specific-bp.c: New file. >>>> * gdb.threads/thread-specific-bp.exp: New file. >>> Please include these new added files into your patch, so that people >>> can review them. Here is an example >>> (http://sourceware.org/ml/gdb-patches/2013-07/msg00671.html) I include >>> new added file in the patch. I use git, but you can get the similar >>> patch in cvs. >>> >>>> # Copyright (C) 1996-2013 Free Software Foundation, Inc. >>> It should be 2013 only. >>> >>>> gdb_test_multiple "info breakpoint" "get info" { >>>> -re "(\[0-9\]+)(\[^\n\r\]*breakpoint\[^\n\r\]*main.*thread >>>> $thre)" { >>> I don't really understand this pattern. Probably, we only want to >>> match "thread $thre" >>> >>> ".*$gdb_prompt $" is missing at the end of the pattern. Please add it. >>> >>>> fail "threaded breakpoint not deleted" >>>> } >>>> -re "(\[0-9\]+)(\[^\n\r\]*breakpoint\[^\n\r\]*main.*\n)" { >>>> pass "threaded breakpoint deleted" >>> fail/pass should be the same. >>> >>>> } >>>> } >>> Something like this: >>> >>> set test "thread-specific breakpoint is deleted" >>> gdb_test_multiple "info breakpoint" $test { >>> -re "thread $thre.*$gdb_prompt $" { >>> fail $test >>> } >>> -re "$gdb_prompt $" { >>> pass $test >>> } >>> } >>> >>> There are some trailing spaces in the test. Please remove them. >>> >> Index: ChangeLog >> =================================================================== >> RCS file: /cvs/src/src/ChangeLog,v >> retrieving revision 1.1075 >> diff -u -p -r1.1075 ChangeLog >> --- ChangeLog 22 Jul 2013 15:17:20 -0000 1.1075 >> +++ ChangeLog 30 Jul 2013 10:20:09 -0000 >> @@ -1,3 +1,9 @@ >> +2013-07-24 Muhammad Waqas <mwaqas@codesourcery.com> >> + >> + PR gdb/11568 >> + * breakpoint.c (breakpoint_auto_delete): Remove thread related >> + breakpoints if threads are exited. >> + >> 2013-07-22 Joel Brobecker <brobecker@adacore.com> >> >> * src-release (VER): Use $(TOOL)/common/create-version.sh >> Index: gdb/breakpoint.c >> =================================================================== >> RCS file: /cvs/src/src/gdb/breakpoint.c,v >> retrieving revision 1.773 >> diff -u -p -r1.773 breakpoint.c >> --- gdb/breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773 >> +++ gdb/breakpoint.c 30 Jul 2013 10:20:49 -0000 >> @@ -11941,6 +11941,17 @@ 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. */ > "Check" is capitalized in the middle of the sentence. But instead > you'd just say: > > /* Delete thread specific breakpoints whose thread is done. */ > >> + struct thread_info *tp = find_thread_id (b->thread); >> + >> + if ( tp != NULL && (tp->state == THREAD_EXITED > ^ > spurious space. > >> + || !target_thread_alive (tp->ptid))) >> + if ( tp != NULL && (tp->state == THREAD_EXITED >> + || !target_thread_alive (tp->ptid))) > Do we really need this target_thread_alive call? It > means we have extra remote traffic whenever we have a thread > specific breakpoint in the list. If the user (or GDB itself) > hasn't refreshed the thread list, is there any harm in delaying > deleting the breakpoint? > > But, I think I agree with Yao in that this doesn't look like > the right way to do this. > > In fact, breakpoint_auto_delete is only called when the > target reports some stop. If you're trying to delete > stale thread specific breakpoints, then you'd want to > do that even if the target hasn't reported a stop, right? > > Say, in non-stop mode, w/ the remote target, if you have two > threads, set a thread-specific break on thread 2, and while > all threads are running free in busy loops, not reporting > any event, keep doing "info threads", and "info break". At some > point thread #2 exits. You can see that from "info threads". > But "info break" will still show you the stale breakpoint... If breakpoint will be deleted when thread list is updated through user or GDB and also when info break command is executed by user, Will it work? What will you say about this technique? >> + delete_breakpoint(b); > Missing space before parens. > > This deletes the breakpoint silently, which may be surprising. > We're not silent when we delete watchpoints on local scopes. > >> + >> + } >> } >> } >> >> Problem was with my test case, now It works with async/non-stop mode. > What would be interesting is whether it works with both native > and remote targets, as with remote targets we only know whether > threads are gone when we refresh the thread list, while with > native GNU/Linux, presently, the core knows when a thread is > gone as soon as it exits. As I said in the other email, if > this is supposed to run in non-stop mode too, then let's make > the test test both all-stop and non-stop modes explicitly. > >> +# This file was written by Muhammad Waqas <mwaqas@codesourcery.com> > Can I kindly ask you to consider removing this? From the ChangeLog, > it's already wrong from the onset. > >> +#This test verfiy that Breakpoint on a spacific thread is deleted > What the typos, missing periods, etc. I'd write: > > # Verify that a thread-specific breakpoint is deleted when the > # corresponding thread is gone. > >> + >> +standard_testfile >> + >> +if {[gdb_compile_pthreads \ >> + "${srcdir}/${subdir}/${srcfile}" \ >> + "${binfile}" executable {debug} ] != "" } { >> + return -1 >> +} >> + >> +clean_restart ${binfile} >> +if [runto main] then { > It's more idiomatic to do > > if [runto main] then { > return > } > > <rest of code here> > > and avoid the indentation. > >> + >> + 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) > If the previous test fails, "thre" will hold something unrelated. > Initialize "thre" outside the gdb_test_multiple, and extract the > id close to the "pass". > >> + gdb_breakpoint [gdb_get_line_number "line # 13"] >> + gdb_breakpoint "main thread $thre" >> + gdb_test "info break" ".*breakpoint.*(thread $thre)" "Breakpoint set" > Why the parens? > >> + gdb_test "thread $thre" "Switching to thread $thre.*" "Thread $thre selected" >> + gdb_continue_to_breakpoint "line # 13" > Why "line # 13" ? Does "13" have any significance? Let's > avoid such confusions in the minds of whoever reads the code. > It's more usual to write "set break here" or some such in the > code instead, and look for that. > >> Index: gdb/testsuite/gdb.threads/thread-specific-bp.c >> =================================================================== >> RCS file: gdb/testsuite/gdb.threads/thread-specific-bp.c >> diff -N gdb/testsuite/gdb.threads/thread-specific-bp.c >> --- /dev/null 1 Jan 1970 00:00:00 -0000 >> +++ gdb/testsuite/gdb.threads/thread-specific-bp.c 30 Jul 2013 10:21:38 -0000 >> +#include <stdio.h> > This include looks unnecessary. > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-01 12:44 ` Muhammad Waqas @ 2013-08-02 9:45 ` Pedro Alves 2013-08-05 12:01 ` Muhammad Waqas 0 siblings, 1 reply; 36+ messages in thread From: Pedro Alves @ 2013-08-02 9:45 UTC (permalink / raw) To: Muhammad Waqas; +Cc: Yao Qi, gdb-patches, tromey, ali_anwar On 08/01/2013 01:43 PM, Muhammad Waqas wrote: >>> >> + || !target_thread_alive (tp->ptid))) >>> >> + if ( tp != NULL && (tp->state == THREAD_EXITED >>> >> + || !target_thread_alive (tp->ptid))) >> > Do we really need this target_thread_alive call? It >> > means we have extra remote traffic whenever we have a thread >> > specific breakpoint in the list. If the user (or GDB itself) >> > hasn't refreshed the thread list, is there any harm in delaying >> > deleting the breakpoint? >> > >> > But, I think I agree with Yao in that this doesn't look like >> > the right way to do this. >> > >> > In fact, breakpoint_auto_delete is only called when the >> > target reports some stop. If you're trying to delete >> > stale thread specific breakpoints, then you'd want to >> > do that even if the target hasn't reported a stop, right? >> > >> > Say, in non-stop mode, w/ the remote target, if you have two >> > threads, set a thread-specific break on thread 2, and while >> > all threads are running free in busy loops, not reporting >> > any event, keep doing "info threads", and "info break". At some >> > point thread #2 exits. You can see that from "info threads". >> > But "info break" will still show you the stale breakpoint... > If breakpoint will be deleted when thread list is updated through > user or GDB and also when info break command is executed > by user, Will it work? What will you say about this technique? > I wouldn't even consider special casing "info break". Otherwise, you end up considering whether to handle all sorts of breakpoint manipulation commands, like "enable", "disable", etc., and what to do if the thread is already gone. Tie this to GDB's own lifetime of the inferior's threads. If GDB learns the thread is gone, remove the breakpoint. Otherwise, leave it there. Whether to remove the breakpoint immediately, or mark it as disp_del_at_next_stop (and hide it, say, set its bp->num to 0) is another question. You'll note that clear_thread_inferior_resources does the latter. (That's because most of GDB's native targets can't touch memory of running processes.) -- Pedro Alves ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-02 9:45 ` Pedro Alves @ 2013-08-05 12:01 ` Muhammad Waqas 2013-08-05 13:57 ` Tom Tromey 0 siblings, 1 reply; 36+ messages in thread From: Muhammad Waqas @ 2013-08-05 12:01 UTC (permalink / raw) To: Pedro Alves; +Cc: Yao Qi, gdb-patches, tromey, ali_anwar On 08/02/2013 02:45 PM, Pedro Alves wrote: > On 08/01/2013 01:43 PM, Muhammad Waqas wrote: >>>>>> + || !target_thread_alive (tp->ptid))) >>>>>> + if ( tp != NULL && (tp->state == THREAD_EXITED >>>>>> + || !target_thread_alive (tp->ptid))) >>>> Do we really need this target_thread_alive call? It >>>> means we have extra remote traffic whenever we have a thread >>>> specific breakpoint in the list. If the user (or GDB itself) >>>> hasn't refreshed the thread list, is there any harm in delaying >>>> deleting the breakpoint? >>>> >>>> But, I think I agree with Yao in that this doesn't look like >>>> the right way to do this. >>>> >>>> In fact, breakpoint_auto_delete is only called when the >>>> target reports some stop. If you're trying to delete >>>> stale thread specific breakpoints, then you'd want to >>>> do that even if the target hasn't reported a stop, right? >>>> >>>> Say, in non-stop mode, w/ the remote target, if you have two >>>> threads, set a thread-specific break on thread 2, and while >>>> all threads are running free in busy loops, not reporting >>>> any event, keep doing "info threads", and "info break". At some >>>> point thread #2 exits. You can see that from "info threads". >>>> But "info break" will still show you the stale breakpoint... >> If breakpoint will be deleted when thread list is updated through >> user or GDB and also when info break command is executed >> by user, Will it work? What will you say about this technique? >> > I wouldn't even consider special casing "info break". Otherwise, > you end up considering whether to handle all sorts of breakpoint > manipulation commands, like "enable", "disable", etc., and what > to do if the thread is already gone. > > Tie this to GDB's own lifetime of the inferior's threads. If GDB > learns the thread is gone, remove the breakpoint. Otherwise, > leave it there. Whether to remove the breakpoint immediately, > or mark it as disp_del_at_next_stop (and hide it, say, set its > bp->num to 0) is another question. You'll note that > clear_thread_inferior_resources does the latter. (That's because > most of GDB's native targets can't touch memory of running processes.) > Thanks for review and helping me out. Now I register a function for deletion of breakpoint with thread_exit if breakpoint is on thread so breakpoint will be deleted when thread finished. Changlog 2013-08-05 Muhammad Waqas <mwaqas@codesourcery.com> PR gdb/11568 * breakpoint.c (remove_threaded_breakpoints): New function. * breakpoint.c (install_breakpoint): If breakpoint is on specific thread remove_threaded_breakpoints will be registered with thread_exit. Index: ./../../breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.773 diff -u -p -r1.773 breakpoint.c --- ./../../breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773 +++ ./../../breakpoint.c 5 Aug 2013 11:51:43 -0000 @@ -200,6 +200,8 @@ typedef enum } insertion_state_t; +static void remove_threaded_breakpoints (struct thread_info *tp, int silent); + static int remove_breakpoint (struct bp_location *, insertion_state_t); static int remove_breakpoint_1 (struct bp_location *, insertion_state_t); @@ -2928,6 +2930,21 @@ remove_breakpoints (void) return val; } +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; + } + } +} + /* Remove breakpoints of process PID. */ int @@ -8450,7 +8467,12 @@ install_breakpoint (int internal, struct if (!internal) mention (b); observer_notify_breakpoint_created (b); - + + if (b->thread > 0) + { + observer_attach_thread_exit (remove_threaded_breakpoints); + } + if (update_gll) update_global_location_list (1); } Changlog 2013-07-24 Muhammad Waqas <mwaqas@codesourccery.com> Jan Kratochvil <jan.kartochvil@redhat.com> PR gdb/11568 *gdb.thread/thread-specific-bp.c: Newfile. *gdb.thread/thread-specific-bp.exp: Newfile. testcase now testing All stop and non stop mode explicitly Index: thread-specific-bp.c =================================================================== RCS file: thread-specific-bp.c diff -N thread-specific-bp.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ thread-specific-bp.c 5 Aug 2013 11:51:51 -0000 @@ -0,0 +1,33 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 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 <http://www.gnu.org/licenses/>. */ + +#include <pthread.h> + +static void * +start (void *arg) +{ + return NULL; +} + +int +main (void) +{ + pthread_t thread; + pthread_create (&thread, NULL, start, NULL); + pthread_join (thread, NULL); + return 0; /*set break here*/ +} Index: thread-specific-bp.exp =================================================================== RCS file: thread-specific-bp.exp diff -N thread-specific-bp.exp --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ thread-specific-bp.exp 5 Aug 2013 11:52:03 -0000 @@ -0,0 +1,84 @@ +# Copyright (C) 2013 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 <http://www.gnu.org/licenses/>. +# +# Verify that a thread-specific breakpoint is deleted when the +# corresponding thread is gone. + +standard_testfile + +set mode "All stop" + +if {[gdb_compile_pthreads \ + "${srcdir}/${subdir}/${srcfile}" \ + "${binfile}" executable {debug} ] != "" } { + return -1 +} + +clean_restart ${binfile} + +proc check_threaded_breakpoint {} { + global gdb_prompt mode + 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 "$mode: thread created" + # get the id of thread + set thre $expect_out(1,string) + } + } + 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.*" "$mode: add breakpoint commands" + gdb_breakpoint "main thread $thre" + gdb_test "info break" ".*breakpoint.*thread $thre" "$mode: Breakpoint set" + gdb_test "thread $thre" "Switching to thread $thre.*" "$mode: Thread $thre selected" + gdb_continue_to_breakpoint "$mode: set break here" + set test "$mode: thread-specific breakpoint is deleted" + gdb_test_multiple "info breakpoint" $test { + -re "thread $thre.*$gdb_prompt $" { + fail $test + } + -re "$gdb_prompt $" { + pass $test + } + } +} + +if ![runto_main] { + untested "could not run to main" + return -1 +} + +# Testing in all stop mode. +check_threaded_breakpoint + +clean_restart ${binfile} + +gdb_test "set target-async on" ".*" "Set async mode" +gdb_test "set non-stop on" ".*" "Set non stop mode" + +if ![runto_main] { + untested "could not run to main" + return -1 +} + +# Testing in non-stop+async mode. +set mode "non-stop\\async" +check_threaded_breakpoint ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-05 12:01 ` Muhammad Waqas @ 2013-08-05 13:57 ` Tom Tromey 2013-08-06 6:12 ` Muhammad Waqas 0 siblings, 1 reply; 36+ messages in thread From: Tom Tromey @ 2013-08-05 13:57 UTC (permalink / raw) To: Muhammad Waqas; +Cc: Pedro Alves, Yao Qi, gdb-patches, ali_anwar >>>>> "Muhammad" == Muhammad Waqas <mwaqas@codesourcery.com> writes: Muhammad> insertion_state_t; Muhammad> +static void remove_threaded_breakpoints (struct thread_info *tp, int Muhammad> silent); Muhammad> + Your patch got mangled by your mailer. This makes it hard to check the formatting, so please fix that. Muhammad> +static void Muhammad> +remove_threaded_breakpoints(struct thread_info *tp, int silent) Muhammad> +{ Needs an intro comment. Muhammad> + Muhammad> + if (b->thread > 0) Muhammad> + { Muhammad> + observer_attach_thread_exit (remove_threaded_breakpoints); Muhammad> + } It seems odd to re-register the observer each time. Why not just do it once, at initialization time? Muhammad> 2013-07-24 Muhammad Waqas <mwaqas@codesourccery.com> Muhammad> Jan Kratochvil <jan.kartochvil@redhat.com> Muhammad> PR gdb/11568 Muhammad> *gdb.thread/thread-specific-bp.c: Newfile. Muhammad> *gdb.thread/thread-specific-bp.exp: Newfile. Space after "*" and in "New file". Muhammad> +set mode "All stop" Muhammad> + Muhammad> +if {[gdb_compile_pthreads \ Muhammad> + "${srcdir}/${subdir}/${srcfile}" \ Muhammad> + "${binfile}" executable {debug} ] != "" } { Muhammad> + return -1 Muhammad> +} Muhammad> + Muhammad> +clean_restart ${binfile} Muhammad> + Muhammad> +proc check_threaded_breakpoint {} { Muhammad> + global gdb_prompt mode Make "mode" a parameter. Use with_test_prefix, since otherwise the new .exp will have repeated test names, an gdb anti-pattern. Muhammad> +# Testing in non-stop+async mode. Muhammad> +set mode "non-stop\\async" It's better to simply not use an unusual character. Tom ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-05 13:57 ` Tom Tromey @ 2013-08-06 6:12 ` Muhammad Waqas 2013-08-22 9:42 ` Muhammad Waqas 0 siblings, 1 reply; 36+ messages in thread From: Muhammad Waqas @ 2013-08-06 6:12 UTC (permalink / raw) To: Tom Tromey; +Cc: Pedro Alves, Yao Qi, gdb-patches, ali_anwar On 08/05/2013 06:57 PM, Tom Tromey wrote: >>>>>> "Muhammad" == Muhammad Waqas <mwaqas@codesourcery.com> writes: > Muhammad> insertion_state_t; > Muhammad> +static void remove_threaded_breakpoints (struct thread_info *tp, int > Muhammad> silent); > Muhammad> + > > Your patch got mangled by your mailer. This makes it hard to check the > formatting, so please fix that. > > Muhammad> +static void > Muhammad> +remove_threaded_breakpoints(struct thread_info *tp, int silent) > Muhammad> +{ > > Needs an intro comment. > > Muhammad> + > Muhammad> + if (b->thread > 0) > Muhammad> + { > Muhammad> + observer_attach_thread_exit (remove_threaded_breakpoints); > Muhammad> + } > > It seems odd to re-register the observer each time. > Why not just do it once, at initialization time? > > Muhammad> 2013-07-24 Muhammad Waqas <mwaqas@codesourccery.com> > Muhammad> Jan Kratochvil <jan.kartochvil@redhat.com> > > Muhammad> PR gdb/11568 > Muhammad> *gdb.thread/thread-specific-bp.c: Newfile. > Muhammad> *gdb.thread/thread-specific-bp.exp: Newfile. > > Space after "*" and in "New file". > > Muhammad> +set mode "All stop" > Muhammad> + > Muhammad> +if {[gdb_compile_pthreads \ > Muhammad> + "${srcdir}/${subdir}/${srcfile}" \ > Muhammad> + "${binfile}" executable {debug} ] != "" } { > Muhammad> + return -1 > Muhammad> +} > Muhammad> + > Muhammad> +clean_restart ${binfile} > Muhammad> + > Muhammad> +proc check_threaded_breakpoint {} { > Muhammad> + global gdb_prompt mode > > Make "mode" a parameter. > Use with_test_prefix, since otherwise the new .exp will have repeated > test names, an gdb anti-pattern. > > Muhammad> +# Testing in non-stop+async mode. > Muhammad> +set mode "non-stop\\async" > > It's better to simply not use an unusual character. > > Tom Thanks for reviewing patch. Changlog 2013-08-05 Muhammad Waqas <mwaqas@codesourcery.com> PR gdb/11568 * breakpoint.c (remove_threaded_breakpoints): New function. * breakpoint.c (_initialize_breakpoint): function remove_threaded_breakpoints registerd with thread_exit. Added intro comments for new function. Fix with re-register the observer each time now it register at inilization time only once. Index: ./../../breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.773 diff -u -p -r1.773 breakpoint.c --- ./../../breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773 +++ ./../../breakpoint.c 6 Aug 2013 05:56:35 -0000 @@ -200,6 +200,9 @@ typedef enum } insertion_state_t; +static void remove_threaded_breakpoints (struct thread_info *tp, + int silent); + static int remove_breakpoint (struct bp_location *, insertion_state_t); static int remove_breakpoint_1 (struct bp_location *, insertion_state_t); @@ -2928,6 +2931,24 @@ remove_breakpoints (void) return val; } +/* Used when a thread exits, it will remove breakpoints which + 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; + } + } +} + /* Remove breakpoints of process PID. */ int @@ -16568,4 +16589,5 @@ agent-printf \"printf format string\", a automatic_hardware_breakpoints = 1; observer_attach_about_to_proceed (breakpoint_about_to_proceed); + observer_attach_thread_exit (remove_threaded_breakpoints); } Changlog 2013-07-24 Muhammad Waqas <mwaqas@codesourccery.com> Jan Kratochvil <jan.kartochvil@redhat.com> PR gdb/11568 * gdb.thread/thread-specific-bp.c: New file. * gdb.thread/thread-specific-bp.exp: New file. Fix with testcase Index: thread-specific-bp.c =================================================================== RCS file: thread-specific-bp.c diff -N thread-specific-bp.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ thread-specific-bp.c 6 Aug 2013 05:56:57 -0000 @@ -0,0 +1,33 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 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 <http://www.gnu.org/licenses/>. */ + +#include <pthread.h> + +static void * +start (void *arg) +{ + return NULL; +} + +int +main (void) +{ + pthread_t thread; + pthread_create (&thread, NULL, start, NULL); + pthread_join (thread, NULL); + return 0; /*set break here*/ +} Index: thread-specific-bp.exp =================================================================== RCS file: thread-specific-bp.exp diff -N thread-specific-bp.exp --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ thread-specific-bp.exp 6 Aug 2013 05:57:14 -0000 @@ -0,0 +1,98 @@ +# Copyright (C) 2013 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 <http://www.gnu.org/licenses/>. +# +# Verify that a thread-specific breakpoint is deleted when the +# corresponding thread is gone. + +standard_testfile + +if {[gdb_compile_pthreads \ + "${srcdir}/${subdir}/${srcfile}" \ + "${binfile}" executable {debug} ] != "" } { + return -1 +} + +clean_restart ${binfile} + +proc check_threaded_breakpoint {mode} { with_test_prefix "$mode" { + + 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 + 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 { + -re "(?:Breakpoint|Temporary breakpoint) .* (at|in) .*\r\n$gdb_prompt $" { + pass $full_name + } + -re ".*$gdb_prompt $" { + fail $full_name + } + timeout { + send_gdb "thread 1\n" + exp_continue + } + } + + set test "thread-specific breakpoint is deleted" + gdb_test_multiple "info breakpoint" $test { + -re "thread $thre.*$gdb_prompt $" { + fail $test + } + -re "$gdb_prompt $" { + pass $test + } + }} +} + +if ![runto_main] { + untested "could not run to main" + return -1 +} + +# Testing in all stop mode. +check_threaded_breakpoint "All stop" + +clean_restart ${binfile} + +gdb_test "set target-async on" ".*" "Set async mode" +gdb_test "set non-stop on" ".*" "Set non stop mode" + +if ![runto_main] { + untested "could not run to main" + return -1 +} + +# Testing in non-stop with async mode. +check_threaded_breakpoint "non-stop with async" ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-06 6:12 ` Muhammad Waqas @ 2013-08-22 9:42 ` Muhammad Waqas 2013-08-22 17:14 ` Pedro Alves 0 siblings, 1 reply; 36+ messages in thread From: Muhammad Waqas @ 2013-08-22 9:42 UTC (permalink / raw) To: Tom Tromey; +Cc: Pedro Alves, Yao Qi, gdb-patches, ali_anwar On 08/06/2013 11:12 AM, Muhammad Waqas wrote: > On 08/05/2013 06:57 PM, Tom Tromey wrote: >>>>>>> "Muhammad" == Muhammad Waqas <mwaqas@codesourcery.com> writes: >> Muhammad> insertion_state_t; >> Muhammad> +static void remove_threaded_breakpoints (struct >> thread_info *tp, int >> Muhammad> silent); >> Muhammad> + >> >> Your patch got mangled by your mailer. This makes it hard to check the >> formatting, so please fix that. >> >> Muhammad> +static void >> Muhammad> +remove_threaded_breakpoints(struct thread_info *tp, int >> silent) >> Muhammad> +{ >> >> Needs an intro comment. >> >> Muhammad> + >> Muhammad> + if (b->thread > 0) >> Muhammad> + { >> Muhammad> + observer_attach_thread_exit >> (remove_threaded_breakpoints); >> Muhammad> + } >> >> It seems odd to re-register the observer each time. >> Why not just do it once, at initialization time? >> >> Muhammad> 2013-07-24 Muhammad Waqas <mwaqas@codesourccery.com> >> Muhammad> Jan Kratochvil <jan.kartochvil@redhat.com> >> >> Muhammad> PR gdb/11568 >> Muhammad> *gdb.thread/thread-specific-bp.c: Newfile. >> Muhammad> *gdb.thread/thread-specific-bp.exp: Newfile. >> >> Space after "*" and in "New file". >> >> Muhammad> +set mode "All stop" >> Muhammad> + >> Muhammad> +if {[gdb_compile_pthreads \ >> Muhammad> + "${srcdir}/${subdir}/${srcfile}" \ >> Muhammad> + "${binfile}" executable {debug} ] != "" } { >> Muhammad> + return -1 >> Muhammad> +} >> Muhammad> + >> Muhammad> +clean_restart ${binfile} >> Muhammad> + >> Muhammad> +proc check_threaded_breakpoint {} { >> Muhammad> + global gdb_prompt mode >> >> Make "mode" a parameter. >> Use with_test_prefix, since otherwise the new .exp will have repeated >> test names, an gdb anti-pattern. >> >> Muhammad> +# Testing in non-stop+async mode. >> Muhammad> +set mode "non-stop\\async" >> >> It's better to simply not use an unusual character. >> >> Tom > > Thanks for reviewing patch. > > Changlog > > 2013-08-05 Muhammad Waqas <mwaqas@codesourcery.com> > > PR gdb/11568 > * breakpoint.c (remove_threaded_breakpoints): New function. > * breakpoint.c (_initialize_breakpoint): function > remove_threaded_breakpoints registerd with thread_exit. > > Added intro comments for new function. > Fix with re-register the observer each time now it register > at inilization time only once. > > > Index: ./../../breakpoint.c > =================================================================== > RCS file: /cvs/src/src/gdb/breakpoint.c,v > retrieving revision 1.773 > diff -u -p -r1.773 breakpoint.c > --- ./../../breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773 > +++ ./../../breakpoint.c 6 Aug 2013 05:56:35 -0000 > @@ -200,6 +200,9 @@ typedef enum > } > insertion_state_t; > > +static void remove_threaded_breakpoints (struct thread_info *tp, > + int silent); > + > static int remove_breakpoint (struct bp_location *, insertion_state_t); > static int remove_breakpoint_1 (struct bp_location *, > insertion_state_t); > > @@ -2928,6 +2931,24 @@ remove_breakpoints (void) > return val; > } > > +/* Used when a thread exits, it will remove breakpoints which > + 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; > + } > + } > +} > + > /* Remove breakpoints of process PID. */ > > int > @@ -16568,4 +16589,5 @@ agent-printf \"printf format string\", a > automatic_hardware_breakpoints = 1; > > observer_attach_about_to_proceed (breakpoint_about_to_proceed); > + observer_attach_thread_exit (remove_threaded_breakpoints); > } > > Changlog > > 2013-07-24 Muhammad Waqas <mwaqas@codesourccery.com> > Jan Kratochvil <jan.kartochvil@redhat.com> > > PR gdb/11568 > * gdb.thread/thread-specific-bp.c: New file. > * gdb.thread/thread-specific-bp.exp: New file. > > Fix with testcase > > > Index: thread-specific-bp.c > =================================================================== > RCS file: thread-specific-bp.c > diff -N thread-specific-bp.c > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ thread-specific-bp.c 6 Aug 2013 05:56:57 -0000 > @@ -0,0 +1,33 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2013 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 > <http://www.gnu.org/licenses/>. */ > + > +#include <pthread.h> > + > +static void * > +start (void *arg) > +{ > + return NULL; > +} > + > +int > +main (void) > +{ > + pthread_t thread; > + pthread_create (&thread, NULL, start, NULL); > + pthread_join (thread, NULL); > + return 0; /*set break here*/ > +} > Index: thread-specific-bp.exp > =================================================================== > RCS file: thread-specific-bp.exp > diff -N thread-specific-bp.exp > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ thread-specific-bp.exp 6 Aug 2013 05:57:14 -0000 > @@ -0,0 +1,98 @@ > +# Copyright (C) 2013 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 <http://www.gnu.org/licenses/>. > +# > +# Verify that a thread-specific breakpoint is deleted when the > +# corresponding thread is gone. > + > +standard_testfile > + > +if {[gdb_compile_pthreads \ > + "${srcdir}/${subdir}/${srcfile}" \ > + "${binfile}" executable {debug} ] != "" } { > + return -1 > +} > + > +clean_restart ${binfile} > + > +proc check_threaded_breakpoint {mode} { with_test_prefix "$mode" { > + > + 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 > + 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 { > + -re "(?:Breakpoint|Temporary breakpoint) .* (at|in) > .*\r\n$gdb_prompt $" { > + pass $full_name > + } > + -re ".*$gdb_prompt $" { > + fail $full_name > + } > + timeout { > + send_gdb "thread 1\n" > + exp_continue > + } > + } > + > + set test "thread-specific breakpoint is deleted" > + gdb_test_multiple "info breakpoint" $test { > + -re "thread $thre.*$gdb_prompt $" { > + fail $test > + } > + -re "$gdb_prompt $" { > + pass $test > + } > + }} > +} > + > +if ![runto_main] { > + untested "could not run to main" > + return -1 > +} > + > +# Testing in all stop mode. > +check_threaded_breakpoint "All stop" > + > +clean_restart ${binfile} > + > +gdb_test "set target-async on" ".*" "Set async mode" > +gdb_test "set non-stop on" ".*" "Set non stop mode" > + > +if ![runto_main] { > + untested "could not run to main" > + return -1 > +} > + > +# Testing in non-stop with async mode. > +check_threaded_breakpoint "non-stop with async" > ping ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-22 9:42 ` Muhammad Waqas @ 2013-08-22 17:14 ` Pedro Alves 2013-08-23 5:31 ` Muhammad Waqas 2013-08-27 11:31 ` Muhammad Waqas 0 siblings, 2 replies; 36+ messages in thread From: Pedro Alves @ 2013-08-22 17:14 UTC (permalink / raw) To: Muhammad Waqas; +Cc: Tom Tromey, Yao Qi, gdb-patches, ali_anwar I'm having trouble applying this patch as well. Could you resend it please? -- Pedro Alves ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-22 17:14 ` Pedro Alves @ 2013-08-23 5:31 ` Muhammad Waqas 2013-08-27 11:31 ` Muhammad Waqas 1 sibling, 0 replies; 36+ messages in thread From: Muhammad Waqas @ 2013-08-23 5:31 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, Yao Qi, gdb-patches [-- Attachment #1: Type: text/plain, Size: 6233 bytes --] On 08/22/2013 10:14 PM, Pedro Alves wrote: > I'm having trouble applying this patch as well. > Could you resend it please? > >> Sorry for this. Please find the patch in attachment as well. >> Thanks. gdb/ChangeLog 2013-08-05 Muhammad Waqas <mwaqas@codesourcery.com> PR gdb/11568 * breakpoint.c (remove_threaded_breakpoints): New function. * breakpoint.c (_initialize_breakpoint): function remove_threaded_breakpoints registers with thread_exit. Index: breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.773 diff -u -p -r1.773 breakpoint.c --- breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773 +++ breakpoint.c 23 Aug 2013 05:15:34 -0000 @@ -2928,6 +2928,24 @@ remove_breakpoints (void) return val; } +/* Used when a thread exits, it will remove breakpoints which + 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; + } + } +} + /* Remove breakpoints of process PID. */ int @@ -16568,4 +16586,5 @@ agent-printf \"printf format string\", a automatic_hardware_breakpoints = 1; observer_attach_about_to_proceed (breakpoint_about_to_proceed); + observer_attach_thread_exit (remove_threaded_breakpoints); } 2013-07-24 Muhammad Waqas <mwaqas@codesourccery.com> Jan Kratochvil <jan.kartochvil@redhat.com> PR gdb/11568 * gdb.thread/thread-specific-bp.c: New file. * gdb.thread/thread-specific-bp.exp: New file. Index: thread-specific-bp.c =================================================================== RCS file: thread-specific-bp.c diff -N thread-specific-bp.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ thread-specific-bp.c 23 Aug 2013 04:54:21 -0000 @@ -0,0 +1,33 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 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 <http://www.gnu.org/licenses/>. */ + +#include <pthread.h> + +static void * +start (void *arg) +{ + return NULL; +} + +int +main (void) +{ + pthread_t thread; + pthread_create (&thread, NULL, start, NULL); + pthread_join (thread, NULL); + return 0; /*set break here*/ +} Index: thread-specific-bp.exp =================================================================== RCS file: thread-specific-bp.exp diff -N thread-specific-bp.exp --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ thread-specific-bp.exp 23 Aug 2013 04:54:46 -0000 @@ -0,0 +1,98 @@ +# Copyright (C) 2013 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 <http://www.gnu.org/licenses/>. +# +# Verify that a thread-specific breakpoint is deleted when the +# corresponding thread is gone. + +standard_testfile + +if {[gdb_compile_pthreads \ + "${srcdir}/${subdir}/${srcfile}" \ + "${binfile}" executable {debug} ] != "" } { + return -1 +} + +clean_restart ${binfile} + +proc check_threaded_breakpoint {mode} { with_test_prefix "$mode" { + + 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 + 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 { + -re "(?:Breakpoint|Temporary breakpoint) .* (at|in) .*\r\n$gdb_prompt $" { + pass $full_name + } + -re ".*$gdb_prompt $" { + fail $full_name + } + timeout { + send_gdb "thread 1\n" + exp_continue + } + } + + set test "thread-specific breakpoint is deleted" + gdb_test_multiple "info breakpoint" $test { + -re "thread $thre.*$gdb_prompt $" { + fail $test + } + -re "$gdb_prompt $" { + pass $test + } + }} +} + +if ![runto_main] { + untested "could not run to main" + return -1 +} + +# Testing in all stop mode. +check_threaded_breakpoint "All stop" + +clean_restart ${binfile} + +gdb_test "set target-async on" ".*" "Set async mode" +gdb_test "set non-stop on" ".*" "Set non stop mode" + +if ![runto_main] { + untested "could not run to main" + return -1 +} + +# Testing in non-stop with async mode. +check_threaded_breakpoint "non-stop with async" [-- Attachment #2: breakpoint.c.thread.patch --] [-- Type: text/x-patch, Size: 1039 bytes --] Index: breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.773 diff -u -p -r1.773 breakpoint.c --- breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773 +++ breakpoint.c 23 Aug 2013 05:15:34 -0000 @@ -2928,6 +2928,24 @@ remove_breakpoints (void) return val; } +/* Used when a thread exits, it will remove breakpoints which + 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; + } + } +} + /* Remove breakpoints of process PID. */ int @@ -16568,4 +16586,5 @@ agent-printf \"printf format string\", a automatic_hardware_breakpoints = 1; observer_attach_about_to_proceed (breakpoint_about_to_proceed); + observer_attach_thread_exit (remove_threaded_breakpoints); } [-- Attachment #3: thread-spacific-bp.c.patch --] [-- Type: text/x-patch, Size: 1282 bytes --] Index: thread-specific-bp.c =================================================================== RCS file: thread-specific-bp.c diff -N thread-specific-bp.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ thread-specific-bp.c 23 Aug 2013 04:54:21 -0000 @@ -0,0 +1,33 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 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 <http://www.gnu.org/licenses/>. */ + +#include <pthread.h> + +static void * +start (void *arg) +{ + return NULL; +} + +int +main (void) +{ + pthread_t thread; + pthread_create (&thread, NULL, start, NULL); + pthread_join (thread, NULL); + return 0; /*set break here*/ +} [-- Attachment #4: thread-spacific-bp.exp.patch --] [-- Type: text/x-patch, Size: 3268 bytes --] Index: thread-specific-bp.exp =================================================================== RCS file: thread-specific-bp.exp diff -N thread-specific-bp.exp --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ thread-specific-bp.exp 23 Aug 2013 04:54:46 -0000 @@ -0,0 +1,98 @@ +# Copyright (C) 2013 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 <http://www.gnu.org/licenses/>. +# +# Verify that a thread-specific breakpoint is deleted when the +# corresponding thread is gone. + +standard_testfile + +if {[gdb_compile_pthreads \ + "${srcdir}/${subdir}/${srcfile}" \ + "${binfile}" executable {debug} ] != "" } { + return -1 +} + +clean_restart ${binfile} + +proc check_threaded_breakpoint {mode} { with_test_prefix "$mode" { + + 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 + 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 { + -re "(?:Breakpoint|Temporary breakpoint) .* (at|in) .*\r\n$gdb_prompt $" { + pass $full_name + } + -re ".*$gdb_prompt $" { + fail $full_name + } + timeout { + send_gdb "thread 1\n" + exp_continue + } + } + + set test "thread-specific breakpoint is deleted" + gdb_test_multiple "info breakpoint" $test { + -re "thread $thre.*$gdb_prompt $" { + fail $test + } + -re "$gdb_prompt $" { + pass $test + } + }} +} + +if ![runto_main] { + untested "could not run to main" + return -1 +} + +# Testing in all stop mode. +check_threaded_breakpoint "All stop" + +clean_restart ${binfile} + +gdb_test "set target-async on" ".*" "Set async mode" +gdb_test "set non-stop on" ".*" "Set non stop mode" + +if ![runto_main] { + untested "could not run to main" + return -1 +} + +# Testing in non-stop with async mode. +check_threaded_breakpoint "non-stop with async" ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-22 17:14 ` Pedro Alves 2013-08-23 5:31 ` Muhammad Waqas @ 2013-08-27 11:31 ` Muhammad Waqas 2013-08-27 19:02 ` Pedro Alves 1 sibling, 1 reply; 36+ messages in thread From: Muhammad Waqas @ 2013-08-27 11:31 UTC (permalink / raw) To: palves; +Cc: gdb-patches, Muhammad Waqas 2013-08-05 Muhammad Waqas <mwaqas@codesourcery.com> PR gdb/11568 * breakpoint.c (remove_threaded_breakpoints): New function. * breakpoint.c (_initialize_breakpoint): function remove_threaded_breakpoints registers with thread_exit. 2013-07-24 Muhammad Waqas <mwaqas@codesourccery.com> Jan Kratochvil <jan.kartochvil@redhat.com> 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. */ + +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; + } + } +} + /* Remove breakpoints of process PID. */ int @@ -16587,4 +16605,5 @@ agent-printf \"printf format string\", arg1, arg2, arg3, ..., argn\n\ automatic_hardware_breakpoints = 1; observer_attach_about_to_proceed (breakpoint_about_to_proceed); + observer_attach_thread_exit (remove_threaded_breakpoints); } diff --git a/gdb/testsuite/gdb.threads/thread-specific-bp.c b/gdb/testsuite/gdb.threads/thread-specific-bp.c new file mode 100644 index 0000000..474d667 --- /dev/null +++ b/gdb/testsuite/gdb.threads/thread-specific-bp.c @@ -0,0 +1,33 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 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 <http://www.gnu.org/licenses/>. */ + +#include <pthread.h> + +static void * +start (void *arg) +{ + return NULL; +} + +int +main (void) +{ + pthread_t thread; + pthread_create (&thread, NULL, start, NULL); + pthread_join (thread, NULL); + return 0; /*set break here*/ +} diff --git a/gdb/testsuite/gdb.threads/thread-specific-bp.exp b/gdb/testsuite/gdb.threads/thread-specific-bp.exp new file mode 100644 index 0000000..44df563 --- /dev/null +++ b/gdb/testsuite/gdb.threads/thread-specific-bp.exp @@ -0,0 +1,98 @@ +# Copyright (C) 2013 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 <http://www.gnu.org/licenses/>. +# +# Verify that a thread-specific breakpoint is deleted when the +# corresponding thread is gone. + +standard_testfile + +if {[gdb_compile_pthreads \ + "${srcdir}/${subdir}/${srcfile}" \ + "${binfile}" executable {debug} ] != "" } { + return -1 +} + +clean_restart ${binfile} + +proc check_threaded_breakpoint {mode} { with_test_prefix "$mode" { + + 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 + 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 { + -re "(?:Breakpoint|Temporary breakpoint) .* (at|in) .*\r\n$gdb_prompt $" { + pass $full_name + } + -re ".*$gdb_prompt $" { + fail $full_name + } + timeout { + send_gdb "thread 1\n" + exp_continue + } + } + + set test "thread-specific breakpoint is deleted" + gdb_test_multiple "info breakpoint" $test { + -re "thread $thre.*$gdb_prompt $" { + fail $test + } + -re "$gdb_prompt $" { + pass $test + } + }} +} + +if ![runto_main] { + untested "could not run to main" + return -1 +} + +# Testing in all stop mode. +check_threaded_breakpoint "All stop" + +clean_restart ${binfile} + +gdb_test "set target-async on" ".*" "Set async mode" +gdb_test "set non-stop on" ".*" "Set non stop mode" + +if ![runto_main] { + untested "could not run to main" + return -1 +} + +# Testing in non-stop with async mode. +check_threaded_breakpoint "non-stop with async" -- 1.7.9.5 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-27 11:31 ` Muhammad Waqas @ 2013-08-27 19:02 ` Pedro Alves 2013-08-27 19:06 ` Pedro Alves ` (2 more replies) 0 siblings, 3 replies; 36+ messages in thread From: Pedro Alves @ 2013-08-27 19:02 UTC (permalink / raw) To: Muhammad Waqas; +Cc: gdb-patches On 08/27/2013 12:29 PM, Muhammad Waqas wrote: > 2013-08-05 Muhammad Waqas <mwaqas@codesourcery.com> > > 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 <https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_Formatted_GNU_ChangeLog>. "Function" should be uppercase. * breakpoint.c (_initialize_breakpoint): Function remove_threaded_breakpoints registers with thread_exit. > 2013-07-24 Muhammad Waqas <mwaqas@codesourccery.com> > Jan Kratochvil <jan.kartochvil@redhat.com> > > 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 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-27 19:02 ` Pedro Alves @ 2013-08-27 19:06 ` Pedro Alves 2013-08-28 12:26 ` Muhammad Waqas 2013-08-28 12:26 ` [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit Muhammad Waqas 2 siblings, 0 replies; 36+ messages in thread From: Pedro Alves @ 2013-08-27 19:06 UTC (permalink / raw) To: Muhammad Waqas; +Cc: gdb-patches On 08/27/2013 08:02 PM, Pedro Alves wrote: > On 08/27/2013 12:29 PM, Muhammad Waqas wrote: >> 2013-08-05 Muhammad Waqas <mwaqas@codesourcery.com> >> >> 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 <https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_Formatted_GNU_ChangeLog>. > > "Function" should be uppercase. > I meant to suggest something else, but forgot to go back and edit it in. > * breakpoint.c (_initialize_breakpoint): Function > remove_threaded_breakpoints registers with thread_exit. I have a little trouble parsing that. I suggest: * breakpoint.c (_initialize_breakpoint): Attach remove_threaded_breakpoints as thread_exit observer. -- Pedro Alves ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-27 19:02 ` Pedro Alves 2013-08-27 19:06 ` Pedro Alves @ 2013-08-28 12:26 ` Muhammad Waqas 2013-08-30 16:28 ` Pedro Alves 2013-09-02 16:46 ` Pedro Alves 2013-08-28 12:26 ` [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit Muhammad Waqas 2 siblings, 2 replies; 36+ messages in thread From: Muhammad Waqas @ 2013-08-28 12:26 UTC (permalink / raw) To: palves; +Cc: gdb-patches, Muhammad Waqas 2013-08-05 Muhammad Waqas <mwaqas@codesourcery.com> PR gdb/11568 * breakpoint.c (remove_threaded_breakpoints): New function. (_initialize_breakpoint): Attach remove_threaded_breakpoints as thread_exit observer. 2013-07-24 Muhammad Waqas <mwaqas@codesourccery.com> Jan Kratochvil <jan.kartochvil@redhat.com> PR gdb/11568 * gdb.thread/thread-specific-bp.c: New file. * gdb.thread/thread-specific-bp.exp: New file. --- gdb/breakpoint.c | 26 ++++++ gdb/testsuite/gdb.threads/thread-specific-bp.c | 33 ++++++++ gdb/testsuite/gdb.threads/thread-specific-bp.exp | 98 ++++++++++++++++++++++ 3 files changed, 157 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..724d847 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2928,6 +2928,31 @@ remove_breakpoints (void) return val; } +/* 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) + { + int tmp = b->number; + b->disposition = disp_del_at_next_stop; + /* Hide it from the user. */ + b->number = 0; + + printf_filtered (_("Breakpoint %d deleted\ +because thread %d has been exited\non which this \ +breakpoint is valid.\n"), + tmp,tp->num); + } + } +} + /* Remove breakpoints of process PID. */ int @@ -16587,4 +16612,5 @@ agent-printf \"printf format string\", arg1, arg2, arg3, ..., argn\n\ automatic_hardware_breakpoints = 1; observer_attach_about_to_proceed (breakpoint_about_to_proceed); + observer_attach_thread_exit (remove_threaded_breakpoints); } diff --git a/gdb/testsuite/gdb.threads/thread-specific-bp.c b/gdb/testsuite/gdb.threads/thread-specific-bp.c new file mode 100644 index 0000000..474d667 --- /dev/null +++ b/gdb/testsuite/gdb.threads/thread-specific-bp.c @@ -0,0 +1,33 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 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 <http://www.gnu.org/licenses/>. */ + +#include <pthread.h> + +static void * +start (void *arg) +{ + return NULL; +} + +int +main (void) +{ + pthread_t thread; + pthread_create (&thread, NULL, start, NULL); + pthread_join (thread, NULL); + return 0; /*set break here*/ +} diff --git a/gdb/testsuite/gdb.threads/thread-specific-bp.exp b/gdb/testsuite/gdb.threads/thread-specific-bp.exp new file mode 100644 index 0000000..013bfcd --- /dev/null +++ b/gdb/testsuite/gdb.threads/thread-specific-bp.exp @@ -0,0 +1,98 @@ +# Copyright (C) 2013 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 <http://www.gnu.org/licenses/>. +# +# Verify that a thread-specific breakpoint is deleted when the +# corresponding thread is gone. + +standard_testfile + +if {[gdb_compile_pthreads \ + "${srcdir}/${subdir}/${srcfile}" \ + "${binfile}" executable {debug} ] != "" } { + return -1 +} + +clean_restart ${binfile} + +proc check_threaded_breakpoint {mode} { + with_test_prefix "$mode" { + 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 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" + + gdb_test_multiple "continue" "$full_name" { + -re "(?:Breakpoint) .* (at) .*\r\n$gdb_prompt $" { + pass $full_name + } + -re ".*$gdb_prompt $" { + fail $full_name + } + timeout { + send_gdb "thread 1\n" + exp_continue + } + } + + set test "thread-specific breakpoint is deleted" + gdb_test_multiple "info breakpoint" $test { + -re "thread $thre\n$gdb_prompt $" { + fail $test + } + -re "$gdb_prompt $" { + pass $test + } + } + } +} + +if ![runto_main] { + untested "could not run to main" + return -1 +} + +# Testing in all stop mode. +check_threaded_breakpoint "All stop" + +clean_restart ${binfile} + +gdb_test "set target-async on" ".*" "Set async mode" +gdb_test "set non-stop on" ".*" "Set non stop mode" + +if ![runto_main] { + untested "could not run to main" + return -1 +} + +# Testing in non-stop with async mode. +check_threaded_breakpoint "non-stop with async" -- 1.7.9.5 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-28 12:26 ` Muhammad Waqas @ 2013-08-30 16:28 ` Pedro Alves 2013-09-02 4:06 ` Muhammad Waqas 2013-09-02 16:46 ` Pedro Alves 1 sibling, 1 reply; 36+ messages in thread From: Pedro Alves @ 2013-08-30 16:28 UTC (permalink / raw) To: Muhammad Waqas; +Cc: gdb-patches While reviewing this, I noticed the new test fails for me: FAIL: gdb.threads/thread-specific-bp.exp: All stop: continue to breakpoint: set break here Investigating, I saw the problem is "info threads" messes up the current source for breakpoints: (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: Breakpoint set break 32 Breakpoint 4 at 0x323dc08d90: file pthread_join.c, line 32. ^^^^^^^^^^^^^^^^^^^^^^^ (gdb) commands Does it actually pass for you? I can't see how. I've filed PR15911 for that and sent a fix: https://sourceware.org/ml/gdb-patches/2013-08/msg00935.html With that in place, your test passes, but there are a few things that'll need addressing. Stay tuned. -- Pedro Alves ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-30 16:28 ` Pedro Alves @ 2013-09-02 4:06 ` Muhammad Waqas 2013-09-02 8:39 ` Pedro Alves 0 siblings, 1 reply; 36+ messages in thread From: Muhammad Waqas @ 2013-09-02 4:06 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches Thanks. On 08/30/2013 09:27 PM, Pedro Alves wrote: > While reviewing this, I noticed the new test fails for me: > > FAIL: gdb.threads/thread-specific-bp.exp: All stop: continue to breakpoint: set break here > > Investigating, I saw the problem is "info threads" messes up > the current source for breakpoints: > > (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: Breakpoint set > break 32 > Breakpoint 4 at 0x323dc08d90: file pthread_join.c, line 32. > ^^^^^^^^^^^^^^^^^^^^^^^ > (gdb) commands > > Does it actually pass for you? I can't see how. It passed for me without your patch. PASS: gdb.threads/thread-specific-bp.exp: All stop: continue to breakpoint: set break here Also I didn't got this problem in any run, I tested it many times. (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: Breakpoint set break 32 Breakpoint 4 at 0x4005f7: file ../.././../gdb/gdb/testsuite/./gdb.threads/thread-specific-bp.c, line 32. (gdb) commands ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-09-02 4:06 ` Muhammad Waqas @ 2013-09-02 8:39 ` Pedro Alves 2013-09-02 9:46 ` Muhammad Waqas 0 siblings, 1 reply; 36+ messages in thread From: Pedro Alves @ 2013-09-02 8:39 UTC (permalink / raw) To: Muhammad Waqas; +Cc: gdb-patches On 09/02/2013 05:05 AM, Muhammad Waqas wrote: > Thanks. > On 08/30/2013 09:27 PM, Pedro Alves wrote: > >> Does it actually pass for you? I can't see how. > It passed for me without your patch. > PASS: gdb.threads/thread-specific-bp.exp: All stop: continue to > breakpoint: set break here > > (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: Breakpoint set > break 32 > Breakpoint 4 at 0x4005f7: file > ../.././../gdb/gdb/testsuite/./gdb.threads/thread-specific-bp.c, line 32. > (gdb) commands Hmm. I'm guessing the difference will be you don't have debug info for you glibc. Could you do do make check RUNTESTFLAGS="thread-specific-bp.exp" and send the resulting gdb.log? Thanks, -- Pedro Alves ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-09-02 8:39 ` Pedro Alves @ 2013-09-02 9:46 ` Muhammad Waqas 2013-09-02 10:24 ` Pedro Alves 0 siblings, 1 reply; 36+ messages in thread From: Muhammad Waqas @ 2013-09-02 9:46 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 268 bytes --] On 09/02/2013 01:39 PM, Pedro Alves wrote: > Hmm. I'm guessing the difference will be you don't have debug > info for you glibc. Could you do > > do make check RUNTESTFLAGS="thread-specific-bp.exp" > > and send the resulting gdb.log? find gdb.log in attachment. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: gdb.log --] [-- Type: text/x-log; name="gdb.log", Size: 14052 bytes --] Test Run By waqas on Mon Sep 2 14:42:25 2013 Native configuration is x86_64-unknown-linux-gnu === gdb tests === Schedule of variations: unix Running target unix Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target. Using /usr/share/dejagnu/config/unix.exp as generic interface file for target. Using ../.././../gdb/gdb/testsuite/config/unix.exp as tool-and-target-specific interface file. Running ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.exp ... Executing on host: gcc ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c -g -lpthreads -lm -o /home/waqas/28-gdb/hd/obj/gdb/testsuite/gdb.threads/thread-specific-bp (timeout = 300) spawn gcc ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c -g -lpthreads -lm -o /home/waqas/28-gdb/hd/obj/gdb/testsuite/gdb.threads/thread-specific-bp /usr/bin/ld: cannot find -lpthreads collect2: ld returned 1 exit status compiler exited with status 1 output is: /usr/bin/ld: cannot find -lpthreads collect2: ld returned 1 exit status Executing on host: gcc ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c -g -lpthread -lm -o /home/waqas/28-gdb/hd/obj/gdb/testsuite/gdb.threads/thread-specific-bp (timeout = 300) spawn gcc ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c -g -lpthread -lm -o /home/waqas/28-gdb/hd/obj/gdb/testsuite/gdb.threads/thread-specific-bp PASS: gdb.threads/thread-specific-bp.exp: successfully compiled posix threads test case spawn /home/waqas/28-gdb/hd/obj/gdb/testsuite/../../gdb/gdb -nw -nx -data-directory /home/waqas/28-gdb/hd/obj/gdb/testsuite/../data-directory GNU gdb (GDB) 7.6.50.20130828-cvs Copyright (C) 2013 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-unknown-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word". (gdb) set height 0 (gdb) set width 0 (gdb) dir Reinitialize source path to empty? (y or n) y Source directories searched: $cdir:$cwd (gdb) dir ../.././../gdb/gdb/testsuite/gdb.threads Source directories searched: /home/waqas/28-gdb/hd/obj/gdb/testsuite/../.././../gdb/gdb/testsuite/gdb.threads:$cdir:$cwd (gdb) kill The program is not being run. (gdb) file /home/waqas/28-gdb/hd/obj/gdb/testsuite/gdb.threads/thread-specific-bp Reading symbols from /home/waqas/28-gdb/hd/obj/gdb/testsuite/gdb.threads/thread-specific-bp...done. (gdb) delete breakpoints (gdb) info breakpoints No breakpoints or watchpoints. (gdb) break main Breakpoint 1 at 0x4005cb: file ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c, line 30. (gdb) run Starting program: /home/waqas/28-gdb/hd/obj/gdb/testsuite/gdb.threads/thread-specific-bp [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Breakpoint 1, main () at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:30 30 pthread_create (&thread, NULL, start, NULL); (gdb) break start Breakpoint 2 at 0x4005bc: file ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c, line 23. (gdb) continue Continuing. [New Thread 0x2aaaab4af700 (LWP 5815)] [Switching to Thread 0x2aaaab4af700 (LWP 5815)] Breakpoint 2, start (arg=0x0) at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:23 23 return NULL; (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: continue to breakpoint: start info threads Id Target Id Frame * 2 Thread 0x2aaaab4af700 (LWP 5815) "thread-specific" start (arg=0x0) at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:23 1 Thread 0x2aaaab2ad700 (LWP 5811) "thread-specific" 0x00002aaaaacd9148 in pthread_join () from /lib/x86_64-linux-gnu/libpthread.so.0 (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: thread created break main thread 2 Note: breakpoint 1 (all threads) also set at pc 0x4005cb. Breakpoint 3 at 0x4005cb: file ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c, line 30. (gdb) info break Num Type Disp Enb Address What 1 breakpoint keep y 0x00000000004005cb in main at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:30 breakpoint already hit 1 time 2 breakpoint keep y 0x00000000004005bc in start at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:23 breakpoint already hit 1 time 3 breakpoint keep y 0x00000000004005cb in main at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:30 thread 2 stop only in thread 2 (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: Breakpoint set break 32 Breakpoint 4 at 0x4005f7: file ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c, line 32. (gdb) commands Type commands for breakpoint(s) 4, one per line. End with a line saying just "end". >info threads >end (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: add breakpoint commands thread 2 [Switching to thread 2 (Thread 0x2aaaab4af700 (LWP 5815))] #0 start (arg=0x0) at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:23 23 return NULL; (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: Thread 2 selected continue Continuing. [Thread 0x2aaaab4af700 (LWP 5815) exited] Breakpoint 3 deletedbecause thread 2 has been exited on which this breakpoint is valid. [Switching to Thread 0x2aaaab2ad700 (LWP 5811)] Breakpoint 4, main () at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:32 32 return 0; /*set break here*/ Id Target Id Frame * 1 Thread 0x2aaaab2ad700 (LWP 5811) "thread-specific" main () at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:32 (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: continue to breakpoint: set break here info breakpoint Num Type Disp Enb Address What 1 breakpoint keep y 0x00000000004005cb in main at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:30 breakpoint already hit 1 time 2 breakpoint keep y 0x00000000004005bc in start at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:23 breakpoint already hit 1 time 4 breakpoint keep y 0x00000000004005f7 in main at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:32 breakpoint already hit 1 time info threads (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: thread-specific breakpoint is deleted spawn /home/waqas/28-gdb/hd/obj/gdb/testsuite/../../gdb/gdb -nw -nx -data-directory /home/waqas/28-gdb/hd/obj/gdb/testsuite/../data-directory GNU gdb (GDB) 7.6.50.20130828-cvs Copyright (C) 2013 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-unknown-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word". (gdb) set height 0 (gdb) set width 0 (gdb) dir Reinitialize source path to empty? (y or n) y Source directories searched: $cdir:$cwd (gdb) dir ../.././../gdb/gdb/testsuite/gdb.threads Source directories searched: /home/waqas/28-gdb/hd/obj/gdb/testsuite/../.././../gdb/gdb/testsuite/gdb.threads:$cdir:$cwd (gdb) kill The program is not being run. (gdb) file /home/waqas/28-gdb/hd/obj/gdb/testsuite/gdb.threads/thread-specific-bp Reading symbols from /home/waqas/28-gdb/hd/obj/gdb/testsuite/gdb.threads/thread-specific-bp...done. (gdb) set target-async on (gdb) PASS: gdb.threads/thread-specific-bp.exp: Set async mode set non-stop on (gdb) PASS: gdb.threads/thread-specific-bp.exp: Set non stop mode delete breakpoints (gdb) info breakpoints No breakpoints or watchpoints. (gdb) break main Breakpoint 1 at 0x4005cb: file ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c, line 30. (gdb) run Starting program: /home/waqas/28-gdb/hd/obj/gdb/testsuite/gdb.threads/thread-specific-bp [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Breakpoint 1, main () at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:30 30 pthread_create (&thread, NULL, start, NULL); (gdb) break start Breakpoint 2 at 0x4005bc: file ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c, line 23. (gdb) continue Continuing. [New Thread 0x2aaaab4af700 (LWP 5828)] Breakpoint 2, start (arg=0x0) at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:23 23 return NULL; (gdb) PASS: gdb.threads/thread-specific-bp.exp: non-stop with async: continue to breakpoint: start info threads Id Target Id Frame 2 Thread 0x2aaaab4af700 (LWP 5828) "thread-specific" start (arg=0x0) at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:23 * 1 Thread 0x2aaaab2ad700 (LWP 5824) "thread-specific" (running) (gdb) PASS: gdb.threads/thread-specific-bp.exp: non-stop with async: thread created break main thread 2 Note: breakpoint 1 (all threads) also set at pc 0x4005cb. Breakpoint 3 at 0x4005cb: file ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c, line 30. (gdb) info break Num Type Disp Enb Address What 1 breakpoint keep y 0x00000000004005cb in main at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:30 breakpoint already hit 1 time 2 breakpoint keep y 0x00000000004005bc in start at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:23 breakpoint already hit 1 time 3 breakpoint keep y 0x00000000004005cb in main at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:30 thread 2 stop only in thread 2 (gdb) PASS: gdb.threads/thread-specific-bp.exp: non-stop with async: Breakpoint set break 32 Breakpoint 4 at 0x4005f7: file ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c, line 32. (gdb) commands Type commands for breakpoint(s) 4, one per line. End with a line saying just "end". >info threads >end (gdb) PASS: gdb.threads/thread-specific-bp.exp: non-stop with async: add breakpoint commands thread 2 [Switching to thread 2 (Thread 0x2aaaab4af700 (LWP 5828))] #0 start (arg=0x0) at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:23 23 return NULL; (gdb) PASS: gdb.threads/thread-specific-bp.exp: non-stop with async: Thread 2 selected continue Continuing. [Thread 0x2aaaab4af700 (LWP 5828) exited] Breakpoint 3 deletedbecause thread 2 has been exited on which this breakpoint is valid. Breakpoint 4, main () at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:32 32 return 0; /*set break here*/ Id Target Id Frame * 1 Thread 0x2aaaab2ad700 (LWP 5824) "thread-specific" main () at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:32 (gdb) PASS: gdb.threads/thread-specific-bp.exp: non-stop with async: continue to breakpoint: set break here info breakpoint Num Type Disp Enb Address What 1 breakpoint keep y 0x00000000004005cb in main at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:30 breakpoint already hit 1 time 2 breakpoint keep y 0x00000000004005bc in start at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:23 breakpoint already hit 1 time 4 breakpoint keep y 0x00000000004005f7 in main at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:32 breakpoint already hit 1 time info threads (gdb) PASS: gdb.threads/thread-specific-bp.exp: non-stop with async: thread-specific breakpoint is deleted testcase ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.exp completed in 0 seconds === gdb Summary === # of expected passes 17 Executing on host: /home/waqas/28-gdb/hd/obj/gdb/testsuite/../../gdb/gdb -nw -nx -data-directory /home/waqas/28-gdb/hd/obj/gdb/testsuite/../data-directory --version (timeout = 300) spawn /home/waqas/28-gdb/hd/obj/gdb/testsuite/../../gdb/gdb -nw -nx -data-directory /home/waqas/28-gdb/hd/obj/gdb/testsuite/../data-directory --version GNU gdb (GDB) 7.6.50.20130828-cvs Copyright (C) 2013 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-unknown-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word". /home/waqas/28-gdb/hd/obj/gdb/testsuite/../../gdb/gdb version 7.6.50.20130828-cvs -nw -nx -data-directory /home/waqas/28-gdb/hd/obj/gdb/testsuite/../data-directory runtest completed at Mon Sep 2 14:42:25 2013 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-09-02 9:46 ` Muhammad Waqas @ 2013-09-02 10:24 ` Pedro Alves 2013-09-02 10:32 ` Muhammad Waqas 0 siblings, 1 reply; 36+ messages in thread From: Pedro Alves @ 2013-09-02 10:24 UTC (permalink / raw) To: Muhammad Waqas; +Cc: gdb-patches On 09/02/2013 10:46 AM, Muhammad Waqas wrote: > On 09/02/2013 01:39 PM, Pedro Alves wrote: >> Hmm. I'm guessing the difference will be you don't have debug >> info for you glibc. Could you do >> >> do make check RUNTESTFLAGS="thread-specific-bp.exp" >> >> and send the resulting gdb.log? > > find gdb.log in attachment. Alright, thanks. That's indeed the difference: Yours: > (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: continue to breakpoint: start> info threads > Id Target Id Frame > * 2 Thread 0x2aaaab4af700 (LWP 5815) "thread-specific" start (arg=0x0) at ../.././../gdb/gdb/testsuite/gdb.threads/thread-specific-bp.c:23 > 1 Thread 0x2aaaab2ad700 (LWP 5811) "thread-specific" 0x00002aaaaacd9148 in pthread_join () from /lib/x86_64-linux-gnu/libpthread.so.0 > (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: thread created Mine: > (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: continue to breakpoint: start > info threads > Id Target Id Frame > * 2 Thread 0x7ffff7fca700 (LWP 16383) "thread-specific" start (arg=0x0) at ../../../src/gdb/testsuite/gdb.threads/thread-specific-bp.c:23 > 1 Thread 0x7ffff7fcb740 (LWP 16379) "thread-specific" 0x000000323dc08e60 in pthread_join (threadid=140737353918208, thread_return=0x0) at pthread_join.c:93 > (gdb) PASS: gdb.threads/thread-specific-bp.exp: All stop: thread created I can tell because yours doesn't show the arguments to pthread_join. So in your case the bug doesn't trigger because set_current_sal_from_frame, called from print_stack_frame, does nothing if the frame (in this case pthread_join's) doesn't have a symtab: void set_current_sal_from_frame (struct frame_info *frame, int center) { struct symtab_and_line sal; find_frame_sal (frame, &sal); if (sal.symtab) { if (center) sal.line = max (sal.line - get_lines_to_list () / 2, 1); set_current_source_symtab_and_line (&sal); } } -- Pedro Alves ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-09-02 10:24 ` Pedro Alves @ 2013-09-02 10:32 ` Muhammad Waqas 2013-09-02 10:48 ` Pedro Alves 0 siblings, 1 reply; 36+ messages in thread From: Muhammad Waqas @ 2013-09-02 10:32 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On 09/02/2013 03:24 PM, Pedro Alves wrote: > So in your case the bug doesn't trigger because > set_current_sal_from_frame, called from print_stack_frame, does > nothing if the frame (in this case pthread_join's) doesn't have > a symtab: > > void > set_current_sal_from_frame (struct frame_info *frame, int center) > { > struct symtab_and_line sal; > > find_frame_sal (frame, &sal); > if (sal.symtab) > { > if (center) > sal.line = max (sal.line - get_lines_to_list () / 2, 1); > set_current_source_symtab_and_line (&sal); > } > } Hmm, Thanks Got it. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-09-02 10:32 ` Muhammad Waqas @ 2013-09-02 10:48 ` Pedro Alves 0 siblings, 0 replies; 36+ messages in thread From: Pedro Alves @ 2013-09-02 10:48 UTC (permalink / raw) To: Muhammad Waqas; +Cc: gdb-patches Let me see about working around the bug in the test, so we don't get stuck waiting for my other patch to go in. -- Pedro Alves ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-28 12:26 ` Muhammad Waqas 2013-08-30 16:28 ` Pedro Alves @ 2013-09-02 16:46 ` Pedro Alves 2013-09-02 16:52 ` [PATCH] PR gdb/11568 - delete thread-specific breakpoints on " Pedro Alves 1 sibling, 1 reply; 36+ messages in thread From: Pedro Alves @ 2013-09-02 16:46 UTC (permalink / raw) To: Muhammad Waqas; +Cc: gdb-patches On 08/28/2013 01:26 PM, Muhammad Waqas wrote: >>> >> + 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. This sounds very much like a workaround. In any case, expecting a timeout for regular test flow is just not right. We should instead make sure to select thread 1 _before_ the "continue", so we don't end up with that situation. As discussed before, I got on native GNU/Linux: FAIL: gdb.threads/thread-specific-bp.exp: All stop: continue to breakpoint: set break here I've worked around the problem by setting a breakpoint at a new function "end", rather than relying on "b 32" to do the right thing. > + > +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) > + { > + int tmp = b->number; There's really no need for this temporary. > + b->disposition = disp_del_at_next_stop; > + /* Hide it from the user. */ > + b->number = 0; > + > + printf_filtered (_("Breakpoint %d deleted\ > +because thread %d has been exited\non which this \ > +breakpoint is valid.\n"), Awww, now that's broken... Breakpoint 3 deletedbecause thread 2 has been exited ^^^^^^^^^^^^^^ on which this breakpoint is valid. ^^ In any case, I suggest instead: "Thread-specific breakpoint 3 deleted - thread 2 is gone.\n" "thread-specific" is how the manual refers to these breakpoints - there's a section in the manual called that way. "gone" instead of "exited" because we will do this even if we e.g., detach from the process, or other similar situations when the thread hasn't really exited. It'd be nice if the "Thread-Specific Breakpoints" node in the manual would be updated to mention what we now do when the thread is gone. > + tmp,tp->num); > + } > + } The patch added some spurious trailing whitespace: $ git am /tmp/mbox Applying: Bug 11568 - delete thread-specific breakpoint on the thread exit /home/pedro/gdb/mygit/src/.git/rebase-apply/patch:58: trailing whitespace. /home/pedro/gdb/mygit/src/.git/rebase-apply/patch:60: trailing whitespace. /home/pedro/gdb/mygit/src/.git/rebase-apply/patch:65: trailing whitespace. /home/pedro/gdb/mygit/src/.git/rebase-apply/patch:70: trailing whitespace. /home/pedro/gdb/mygit/src/.git/rebase-apply/patch:157: space before tab in indent. send_gdb "thread 1\n" warning: 5 lines add whitespace errors. > diff --git a/gdb/testsuite/gdb.threads/thread-specific-bp.exp b/gdb/testsuite/gdb.threads/thread-specific-bp.exp > new file mode 100644 > index 0000000..013bfcd > --- /dev/null > +++ b/gdb/testsuite/gdb.threads/thread-specific-bp.exp > @@ -0,0 +1,98 @@ > +# Copyright (C) 2013 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 <http://www.gnu.org/licenses/>. > +# > +# Verify that a thread-specific breakpoint is deleted when the > +# corresponding thread is gone. > + > +standard_testfile > + > +if {[gdb_compile_pthreads \ > + "${srcdir}/${subdir}/${srcfile}" \ > + "${binfile}" executable {debug} ] != "" } { > + return -1 > +} > + > +clean_restart ${binfile} > + > +proc check_threaded_breakpoint {mode} { > + with_test_prefix "$mode" { > + global gdb_prompt It's good to leave an empty line after declarations. > + 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 $)" { There's no need for all these parens, actually. > + pass "thread created" > + # 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"] This is the breakpoint I needed changing to work around PR gdb/15911. > + > + # 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" I don't see much point in doing this in the breakpoint's command itself, rather than simply after hitting the breakpoint. Seems more standard that way. > + gdb_test "thread $thre" "Switching to thread $thre.*" "Thread $thre selected" > + set full_name "continue to breakpoint: set break here" > + > + gdb_test_multiple "continue" "$full_name" { > + -re "(?:Breakpoint) .* (at) .*\r\n$gdb_prompt $" { > + pass $full_name > + } > + -re ".*$gdb_prompt $" { > + fail $full_name > + } > + timeout { > + send_gdb "thread 1\n" > + exp_continue > + } > + } This I've done differently. The test is now switching to thread 1, and issuing "continue -a" in non-stop mode, so to make sure to resume thread 2 as well. > + > + set test "thread-specific breakpoint is deleted" > + gdb_test_multiple "info breakpoint" $test { > + -re "thread $thre\n$gdb_prompt $" { > + fail $test > + } > + -re "$gdb_prompt $" { > + pass $test > + } > + } > + } > +} > + > +if ![runto_main] { > + untested "could not run to main" > + return -1 > +} This can be moved to the test routine... (1) > + > +# Testing in all stop mode. > +check_threaded_breakpoint "All stop" Mixup of uppercase and lowercase gdb.sum output. Let's stick with lowercase. Also note "All stop" vs "non-stop" (hyphen), vs "non stop" below. > + > +clean_restart ${binfile} > + > +gdb_test "set target-async on" ".*" "Set async mode" > +gdb_test "set non-stop on" ".*" "Set non stop mode" > + > +if ![runto_main] { > + untested "could not run to main" > + return -1 > +} (1) ... as both modes do it. Doing that also gets rid of duplicated output in gdb.sum. > + > +# Testing in non-stop with async mode. > +check_threaded_breakpoint "non-stop with async" non-stop is always async. Here's the diff I got on top of yours. I'll send out the final diff in a bit. --- gdb/breakpoint.c | 17 +-- gdb/testsuite/gdb.threads/thread-specific-bp.c | 16 ++- gdb/testsuite/gdb.threads/thread-specific-bp.exp | 123 ++++++++++++++-------- 3 files changed, 95 insertions(+), 61 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 724d847..734dfd6 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2940,15 +2940,14 @@ remove_threaded_breakpoints (struct thread_info *tp, int silent) { if (b->thread == tp->num) { - int tmp = b->number; - b->disposition = disp_del_at_next_stop; - /* Hide it from the user. */ - b->number = 0; - - printf_filtered (_("Breakpoint %d deleted\ -because thread %d has been exited\non which this \ -breakpoint is valid.\n"), - tmp,tp->num); + b->disposition = disp_del_at_next_stop; + + printf_filtered (_("\ +Thread-specific breakpoint %d deleted - thread %d is gone.\n"), + b->number, tp->num); + + /* Hide it from the user. */ + b->number = 0; } } } diff --git a/gdb/testsuite/gdb.threads/thread-specific-bp.c b/gdb/testsuite/gdb.threads/thread-specific-bp.c index 474d667..5ca02c7 100644 --- a/gdb/testsuite/gdb.threads/thread-specific-bp.c +++ b/gdb/testsuite/gdb.threads/thread-specific-bp.c @@ -1,17 +1,17 @@ /* This testcase is part of GDB, the GNU debugger. - + Copyright 2013 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 <http://www.gnu.org/licenses/>. */ @@ -23,11 +23,17 @@ start (void *arg) return NULL; } +static void +end (void) +{ +} + int main (void) { pthread_t thread; pthread_create (&thread, NULL, start, NULL); pthread_join (thread, NULL); - return 0; /*set break here*/ + end (); + return 0; } diff --git a/gdb/testsuite/gdb.threads/thread-specific-bp.exp b/gdb/testsuite/gdb.threads/thread-specific-bp.exp index 013bfcd..9f7d142 100644 --- a/gdb/testsuite/gdb.threads/thread-specific-bp.exp +++ b/gdb/testsuite/gdb.threads/thread-specific-bp.exp @@ -26,47 +26,87 @@ if {[gdb_compile_pthreads \ clean_restart ${binfile} -proc check_threaded_breakpoint {mode} { +# Extract and return the thread ID of the thread stopped at function +# FUNC. + +proc get_thread_id {func} { + global gdb_prompt + + set thre -1 + set test "get $func thread id" + gdb_test_multiple "info threads" $test { + -re "(\[0-9\]+)\[^\n\r\]*Thread\[^\n\r\]*$func.*$gdb_prompt $" { + # Get the thread's id. + set thre $expect_out(1,string) + pass $test + } + } + + return $thre +} + +proc check_thread_specific_breakpoint {mode} { with_test_prefix "$mode" { global gdb_prompt + + if ![runto_main] { + untested "could not run to main" + return -1 + } + + set main_thre [get_thread_id "main"] + if { $main_thre < 0 } { + return -1 + } + 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 thread's id. - set thre $expect_out(1,string) - } + set start_thre [get_thread_id "start"] + if { $start_thre < 0 } { + return -1 } - 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" - - gdb_test_multiple "continue" "$full_name" { - -re "(?:Breakpoint) .* (at) .*\r\n$gdb_prompt $" { - pass $full_name - } - -re ".*$gdb_prompt $" { - fail $full_name + + # Set a thread-specific breakpoint at "main". This can't ever + # be hit, but that's OK. + gdb_breakpoint "main thread $start_thre" + gdb_test "info break" ".*breakpoint.*thread $start_thre" "breakpoint set" + + # Set breakpoint at a place only reacheable after the "start" + # thread exits. + gdb_breakpoint "end" + + # Switch back to the main thread, and resume all threads. The + # "start" thread exits, and the main thread reaches "end". + gdb_test "thread $main_thre" \ + "Switching to thread $main_thre.*" \ + "thread $main_thre selected" + + if { $mode == "non-stop" } { + set cmd "continue -a" + } else { + set cmd "continue" + } + gdb_test "$cmd" \ + "Breakpoint .* end .* at .*" \ + "continue to end" + + # Force GDB to update the thread list. Otherwise, depending + # on target, GDB may not realize that the start thread has + # exited and thus not remove the thread specific breakpoint. + set test "thread start is gone" + gdb_test_multiple "info threads" $test { + -re "\[0-9\]+.*start.*$gdb_prompt $" { + fail $test } - timeout { - send_gdb "thread 1\n" - exp_continue + -re "$gdb_prompt $" { + pass $test } } - set test "thread-specific breakpoint is deleted" + set test "thread-specific breakpoint was deleted" gdb_test_multiple "info breakpoint" $test { - -re "thread $thre\n$gdb_prompt $" { + -re "thread $start_thre\n$gdb_prompt $" { fail $test } -re "$gdb_prompt $" { @@ -76,23 +116,12 @@ proc check_threaded_breakpoint {mode} { } } -if ![runto_main] { - untested "could not run to main" - return -1 -} - -# Testing in all stop mode. -check_threaded_breakpoint "All stop" +# Test all-stop mode. +check_thread_specific_breakpoint "all-stop" clean_restart ${binfile} -gdb_test "set target-async on" ".*" "Set async mode" -gdb_test "set non-stop on" ".*" "Set non stop mode" - -if ![runto_main] { - untested "could not run to main" - return -1 -} - -# Testing in non-stop with async mode. -check_threaded_breakpoint "non-stop with async" +# Test non-stop mode. +gdb_test_no_output "set target-async on" "set async mode" +gdb_test_no_output "set non-stop on" "set non-stop mode" +check_thread_specific_breakpoint "non-stop" ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] PR gdb/11568 - delete thread-specific breakpoints on thread exit 2013-09-02 16:46 ` Pedro Alves @ 2013-09-02 16:52 ` Pedro Alves 2013-09-09 16:07 ` Tom Tromey 0 siblings, 1 reply; 36+ messages in thread From: Pedro Alves @ 2013-09-02 16:52 UTC (permalink / raw) To: gdb-patches; +Cc: Muhammad Waqas On 09/02/2013 05:46 PM, Pedro Alves wrote: > Here's the diff I got on top of yours. I'll send out the > final diff in a bit. Here it is. Let me know what you all think. --------- Subject: PR gdb/11568 - delete thread-specific breakpoints on thread exit Bug 11568 - delete thread-specific breakpoints on thread exit PR gdb/11568 is about thread-specific breakpoints being left behind when the corresponding thread exits. Currently: (gdb) b start thread 2 Breakpoint 3 at 0x400614: file thread-specific-bp.c, line 23. (gdb) b end Breakpoint 4 at 0x40061f: file thread-specific-bp.c, line 29. (gdb) c Continuing. [Thread 0x7ffff7fcb700 (LWP 14925) exited] [Switching to Thread 0x7ffff7fcc740 (LWP 14921)] Breakpoint 4, end () at thread-specific-bp.c:29 29 } (gdb) info threads Id Target Id Frame * 1 Thread 0x7ffff7fcc740 (LWP 14921) "thread-specific" end () at thread-specific-bp.c:29 (gdb) info breakpoints Num Type Disp Enb Address What 2 breakpoint keep y 0x0000000000400614 in start at thread-specific-bp.c:23 breakpoint already hit 1 time 3 breakpoint keep y 0x0000000000400614 in start at thread-specific-bp.c:23 thread 2 stop only in thread 2 4 breakpoint keep y 0x000000000040061f in end at thread-specific-bp.c:29 breakpoint already hit 1 time Note that the thread-specific breakpoint 3 stayed around, even though thread 2 is gone. There's no way that breakpoint can trigger again (*), so the PR argues that the breakpoint should just be removed, like local watchpoints. I'm ambivalent on this -- it could be reasonable to disable the breakpoint (kind of like breakpoint in shared library code when the DSO is unloaded), so the user could still use it as visual template for creating other breakpoints (copy/paste command lists, etc.), or we could have a way to change to which thread a breakpoint applies. But, several people pushed this direction, and I don't plan on arguing... (*) - actually, there is ... thread numbers are reset on "run", so the user could do "break foo thread 2", "run", and expect the breakpoint to hit again on the second thread. But given gdb's thread numbering can't really be stable, that'd only work sufficiently well for thread 1, so we'd better call it unsupported. So with the patch, whenever a thread is deleted from GDB's list, GDB goes through the thread-specific breakpoints and deletes corresponding breakpoints. Since this is user-visible, GDB prints out: Thread-specific breakpoint 3 deleted - thread 2 is gone. And of course, we end up with: (gdb) info breakpoints Num Type Disp Enb Address What 2 breakpoint keep y 0x0000000000400614 in start at thread-specific-bp.c:23 breakpoint already hit 1 time 4 breakpoint keep y 0x000000000040061f in end at thread-specific-bp.c:29 breakpoint already hit 1 time 2013-09-02 Muhammad Waqas <mwaqas@codesourcery.com> Pedro Alves <palves@redhat.com> PR gdb/11568 * breakpoint.c (remove_threaded_breakpoints): New function. (_initialize_breakpoint): Attach remove_threaded_breakpoints as thread_exit observer. 2013-09-02 Muhammad Waqas <mwaqas@codesourccery.com> Jan Kratochvil <jan.kartochvil@redhat.com> Pedro Alves <palves@redhat.com> PR gdb/11568 * gdb.thread/thread-specific-bp.c: New file. * gdb.thread/thread-specific-bp.exp: New file. --- gdb/breakpoint.c | 25 ++++ gdb/testsuite/gdb.threads/thread-specific-bp.c | 39 +++++++ gdb/testsuite/gdb.threads/thread-specific-bp.exp | 127 ++++++++++++++++++++++ 3 files changed, 191 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..734dfd6 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2928,6 +2928,30 @@ remove_breakpoints (void) return val; } +/* 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; + + printf_filtered (_("\ +Thread-specific breakpoint %d deleted - thread %d is gone.\n"), + b->number, tp->num); + + /* Hide it from the user. */ + b->number = 0; + } + } +} + /* Remove breakpoints of process PID. */ int @@ -16587,4 +16611,5 @@ agent-printf \"printf format string\", arg1, arg2, arg3, ..., argn\n\ automatic_hardware_breakpoints = 1; observer_attach_about_to_proceed (breakpoint_about_to_proceed); + observer_attach_thread_exit (remove_threaded_breakpoints); } diff --git a/gdb/testsuite/gdb.threads/thread-specific-bp.c b/gdb/testsuite/gdb.threads/thread-specific-bp.c new file mode 100644 index 0000000..5ca02c7 --- /dev/null +++ b/gdb/testsuite/gdb.threads/thread-specific-bp.c @@ -0,0 +1,39 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 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 <http://www.gnu.org/licenses/>. */ + +#include <pthread.h> + +static void * +start (void *arg) +{ + return NULL; +} + +static void +end (void) +{ +} + +int +main (void) +{ + pthread_t thread; + pthread_create (&thread, NULL, start, NULL); + pthread_join (thread, NULL); + end (); + return 0; +} diff --git a/gdb/testsuite/gdb.threads/thread-specific-bp.exp b/gdb/testsuite/gdb.threads/thread-specific-bp.exp new file mode 100644 index 0000000..9f7d142 --- /dev/null +++ b/gdb/testsuite/gdb.threads/thread-specific-bp.exp @@ -0,0 +1,127 @@ +# Copyright (C) 2013 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 <http://www.gnu.org/licenses/>. +# +# Verify that a thread-specific breakpoint is deleted when the +# corresponding thread is gone. + +standard_testfile + +if {[gdb_compile_pthreads \ + "${srcdir}/${subdir}/${srcfile}" \ + "${binfile}" executable {debug} ] != "" } { + return -1 +} + +clean_restart ${binfile} + +# Extract and return the thread ID of the thread stopped at function +# FUNC. + +proc get_thread_id {func} { + global gdb_prompt + + set thre -1 + set test "get $func thread id" + gdb_test_multiple "info threads" $test { + -re "(\[0-9\]+)\[^\n\r\]*Thread\[^\n\r\]*$func.*$gdb_prompt $" { + # Get the thread's id. + set thre $expect_out(1,string) + pass $test + } + } + + return $thre +} + +proc check_thread_specific_breakpoint {mode} { + with_test_prefix "$mode" { + global gdb_prompt + + if ![runto_main] { + untested "could not run to main" + return -1 + } + + set main_thre [get_thread_id "main"] + if { $main_thre < 0 } { + return -1 + } + + gdb_breakpoint "start" + gdb_continue_to_breakpoint "start" + + set start_thre [get_thread_id "start"] + if { $start_thre < 0 } { + return -1 + } + + # Set a thread-specific breakpoint at "main". This can't ever + # be hit, but that's OK. + gdb_breakpoint "main thread $start_thre" + gdb_test "info break" ".*breakpoint.*thread $start_thre" "breakpoint set" + + # Set breakpoint at a place only reacheable after the "start" + # thread exits. + gdb_breakpoint "end" + + # Switch back to the main thread, and resume all threads. The + # "start" thread exits, and the main thread reaches "end". + gdb_test "thread $main_thre" \ + "Switching to thread $main_thre.*" \ + "thread $main_thre selected" + + if { $mode == "non-stop" } { + set cmd "continue -a" + } else { + set cmd "continue" + } + gdb_test "$cmd" \ + "Breakpoint .* end .* at .*" \ + "continue to end" + + # Force GDB to update the thread list. Otherwise, depending + # on target, GDB may not realize that the start thread has + # exited and thus not remove the thread specific breakpoint. + set test "thread start is gone" + gdb_test_multiple "info threads" $test { + -re "\[0-9\]+.*start.*$gdb_prompt $" { + fail $test + } + -re "$gdb_prompt $" { + pass $test + } + } + + set test "thread-specific breakpoint was deleted" + gdb_test_multiple "info breakpoint" $test { + -re "thread $start_thre\n$gdb_prompt $" { + fail $test + } + -re "$gdb_prompt $" { + pass $test + } + } + } +} + +# Test all-stop mode. +check_thread_specific_breakpoint "all-stop" + +clean_restart ${binfile} + +# Test non-stop mode. +gdb_test_no_output "set target-async on" "set async mode" +gdb_test_no_output "set non-stop on" "set non-stop mode" +check_thread_specific_breakpoint "non-stop" ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] PR gdb/11568 - delete thread-specific breakpoints on thread exit 2013-09-02 16:52 ` [PATCH] PR gdb/11568 - delete thread-specific breakpoints on " Pedro Alves @ 2013-09-09 16:07 ` Tom Tromey 2013-09-17 19:36 ` Pedro Alves 0 siblings, 1 reply; 36+ messages in thread From: Tom Tromey @ 2013-09-09 16:07 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches, Muhammad Waqas >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: Pedro> Here it is. Let me know what you all think. It looks good to me. Pedro> There's no way that breakpoint can trigger again (*), so the PR argues Pedro> that the breakpoint should just be removed, like local watchpoints. Pedro> I'm ambivalent on this -- it could be reasonable to disable the Pedro> breakpoint (kind of like breakpoint in shared library code when the Pedro> DSO is unloaded), so the user could still use it as visual template Pedro> for creating other breakpoints (copy/paste command lists, etc.), or we Pedro> could have a way to change to which thread a breakpoint applies. But, Pedro> several people pushed this direction, and I don't plan on arguing... I've sometimes wished for a way to modify a breakpoint in place. There may be another PR about this. But it's fine to put in this patch now and remove it again later if anyone actually implements breakpoint modification. Tom ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] PR gdb/11568 - delete thread-specific breakpoints on thread exit 2013-09-09 16:07 ` Tom Tromey @ 2013-09-17 19:36 ` Pedro Alves 2013-09-19 14:48 ` [COMMIT PATCH] Fix regressions caused by thread-specific breakpoint deletion. (was: Re: [PATCH] PR gdb/11568 - delete thread-specific breakpoints on thread exit) Pedro Alves 0 siblings, 1 reply; 36+ messages in thread From: Pedro Alves @ 2013-09-17 19:36 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, Muhammad Waqas On 09/09/2013 05:07 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: > > Pedro> Here it is. Let me know what you all think. > > It looks good to me. Thanks. I've checked it in now. > Pedro> There's no way that breakpoint can trigger again (*), so the PR argues > Pedro> that the breakpoint should just be removed, like local watchpoints. > Pedro> I'm ambivalent on this -- it could be reasonable to disable the > Pedro> breakpoint (kind of like breakpoint in shared library code when the > Pedro> DSO is unloaded), so the user could still use it as visual template > Pedro> for creating other breakpoints (copy/paste command lists, etc.), or we > Pedro> could have a way to change to which thread a breakpoint applies. But, > Pedro> several people pushed this direction, and I don't plan on arguing... > > I've sometimes wished for a way to modify a breakpoint in place. > There may be another PR about this. > But it's fine to put in this patch now and remove it again later if > anyone actually implements breakpoint modification. Yeah. -- Pedro Alves ^ permalink raw reply [flat|nested] 36+ messages in thread
* [COMMIT PATCH] Fix regressions caused by thread-specific breakpoint deletion. (was: Re: [PATCH] PR gdb/11568 - delete thread-specific breakpoints on thread exit) 2013-09-17 19:36 ` Pedro Alves @ 2013-09-19 14:48 ` Pedro Alves 0 siblings, 0 replies; 36+ messages in thread From: Pedro Alves @ 2013-09-19 14:48 UTC (permalink / raw) To: gdb-patches; +Cc: Muhammad Waqas The recent change to make GDB auto-delete thread-specific breakpoints when the corresponding thread is deleted (https://sourceware.org/ml/gdb-patches/2013-09/msg00038.html) caused gdb.base/nextoverexit.exp to regress. Breakpoint 1, main () at .../gdb/testsuite/gdb.base/nextoverexit.c:21 21 exit (0); (gdb) next [Inferior 1 (process 25208) exited normally] Thread-specific breakpoint -5 deleted - thread 1 is gone. Thread-specific breakpoint -6 deleted - thread 1 is gone. Thread-specific breakpoint -7 deleted - thread 1 is gone. Thread-specific breakpoint 0 deleted - thread 1 is gone. (gdb) FAIL: gdb.base/nextoverexit.exp: next over exit (the program exited) We shouldn't be seeing this for internal or momentary breakpoints. In fact, we shouldn't even be trying to delete them, as whatever created them will take care or it, and therefore it's dangerous to delete them behind the creator's back. I thought it'd still be good to tag thread-specific internal/momentary breakpoints such that we'll no longer try to keep them insert in the target, as they'll cause stops and thread hops in other threads, so I tried disabling them instead. That caused a problem when following a child fork, and detaching from the parent, as we try to reset the step-resume etc. breakpoints to the new child's thread (breakpoint_re_set_thread), after the parent thread is already gone (and the breakpoints are marked disabled). I fixed that by re-enabling internal/momentary breakpoints there, but, that didn't feel super safe either (maybe we'd need a new flag in struct breakpoint instead, to tag the thread-specific breakpoint as "not to be inserted"). It felt like I was heading down a design rat hole, and, other things will usually delete internal/momentary breakpoints soon enough, so I left that little optimization for some other day. So, internal/momentary breakpoints are no longer deleted/disabled at all, and we end up with a one-liner fix. Tested on x86_64 Fedora 17. gdb/ 2013-09-19 Pedro Alves <palves@redhat.com> * breakpoint.c (remove_threaded_breakpoints): Skip non-user breakpoints. --- gdb/breakpoint.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 734dfd6..c132e24 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2938,7 +2938,7 @@ remove_threaded_breakpoints (struct thread_info *tp, int silent) ALL_BREAKPOINTS_SAFE (b, b_tmp) { - if (b->thread == tp->num) + if (b->thread == tp->num && user_breakpoint_p (b)) { b->disposition = disp_del_at_next_stop; -- 1.7.11.7 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit 2013-08-27 19:02 ` Pedro Alves 2013-08-27 19:06 ` Pedro Alves 2013-08-28 12:26 ` Muhammad Waqas @ 2013-08-28 12:26 ` Muhammad Waqas 2 siblings, 0 replies; 36+ messages in thread From: Muhammad Waqas @ 2013-08-28 12:26 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches 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 <mwaqas@codesourcery.com> >> >> 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 <https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_Formatted_GNU_ChangeLog>. > > "Function" should be uppercase. > > * breakpoint.c (_initialize_breakpoint): Function > remove_threaded_breakpoints registers with thread_exit. > > Fixed. >> 2013-07-24 Muhammad Waqas <mwaqas@codesourccery.com> >> Jan Kratochvil <jan.kartochvil@redhat.com> >> >> 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, > ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2013-09-19 14:48 UTC | newest] Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-07-29 7:29 [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit Muhammad Waqas 2013-07-29 9:21 ` Yao Qi 2013-07-29 11:42 ` Muhammad Waqas 2013-07-29 14:18 ` Yao Qi 2013-07-30 10:34 ` Muhammad Waqas 2013-07-31 2:41 ` Yao Qi 2013-08-01 10:51 ` Pedro Alves 2013-08-01 10:59 ` Yao Qi 2013-08-01 11:27 ` Pedro Alves 2013-08-01 12:10 ` Yao Qi 2013-08-01 11:57 ` Pedro Alves 2013-08-01 12:44 ` Muhammad Waqas 2013-08-02 9:45 ` Pedro Alves 2013-08-05 12:01 ` Muhammad Waqas 2013-08-05 13:57 ` Tom Tromey 2013-08-06 6:12 ` Muhammad Waqas 2013-08-22 9:42 ` Muhammad Waqas 2013-08-22 17:14 ` Pedro Alves 2013-08-23 5:31 ` Muhammad Waqas 2013-08-27 11:31 ` Muhammad Waqas 2013-08-27 19:02 ` Pedro Alves 2013-08-27 19:06 ` Pedro Alves 2013-08-28 12:26 ` Muhammad Waqas 2013-08-30 16:28 ` Pedro Alves 2013-09-02 4:06 ` Muhammad Waqas 2013-09-02 8:39 ` Pedro Alves 2013-09-02 9:46 ` Muhammad Waqas 2013-09-02 10:24 ` Pedro Alves 2013-09-02 10:32 ` Muhammad Waqas 2013-09-02 10:48 ` Pedro Alves 2013-09-02 16:46 ` Pedro Alves 2013-09-02 16:52 ` [PATCH] PR gdb/11568 - delete thread-specific breakpoints on " Pedro Alves 2013-09-09 16:07 ` Tom Tromey 2013-09-17 19:36 ` Pedro Alves 2013-09-19 14:48 ` [COMMIT PATCH] Fix regressions caused by thread-specific breakpoint deletion. (was: Re: [PATCH] PR gdb/11568 - delete thread-specific breakpoints on thread exit) Pedro Alves 2013-08-28 12:26 ` [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit Muhammad Waqas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox