Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit

[Muhammad Waqas <mwaqas@codesourcery.com>]

[-- 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 --]