* [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-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: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-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 ` 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-28 12:26 ` Muhammad Waqas
2013-08-30 16:28 ` Pedro Alves
2013-09-02 16:46 ` Pedro Alves
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-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
* 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
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-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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox