[Patch] Mark the thread_event breakpoint as "not inserted" before calling thread_mourn_inferior

[Patch] Mark the thread_event breakpoint as "not inserted" before calling thread_mourn_inferior

[Girish Shilamkar]

[-- Attachment #1: Type: text/plain, Size: 1662 bytes --]

Hi,
	While running gdb-6.4 on Fedora Core 3 for multithread application we
encountered following problem:
-----------------------------------------------------------------------
GNU gdb 6.4
Copyright 2005 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you
are
welcome to change it and/or distribute copies of it under certain
conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for
details.
This GDB was configured as "i686-pc-linux-gnu"...Using host libthread_db
library "/lib/tls/libthread_db.so.1".

(gdb) r
Starting program: /home/girish/gdbl/build-6.4/multithread
[Thread debugging using libthread_db enabled]
[New Thread -151132480 (LWP 10793)]
Thread 0
[New Thread -151135312 (LWP 10796)]
Thread 1
Starting thread 1
[New Thread -161625168 (LWP 10797)]
Starting thread 0
Finishing thread
[Thread -151135312 (LWP 10796) exited]
Finishing thread
[Thread -161625168 (LWP 10797) exited]

Program exited normally.
Couldn't get registers: No such process.
(gdb) q
The program is running.  Exit anyway? (y or n) y
------------------------------------------------------------
	This problem was also seen on powerpc board. 
This problem occurs since gdb tries to remove thread_event breakpoint
after the thread has exited and tries to read the address space since
the breakpoint is still marked as updated.
	In the attached patch we mark the thread_event breakpoint as "not
inserted" before calling thread_mourn_inferior.
	Is this the right way?	
	If yes do we also need to do the same thing when target has been
terminated by a signal?

-Girish

[-- Attachment #2: mark_bpt_clean.patch --]
[-- Type: text/x-patch, Size: 1566 bytes --]

Index: gdb-6.4/gdb/infrun.c
===================================================================
--- gdb-6.4.orig/gdb/infrun.c	2005-11-14 21:05:16.000000000 +0530
+++ gdb-6.4/gdb/infrun.c	2006-01-17 20:57:44.431069776 +0530
@@ -1367,6 +1367,7 @@
 		       value_from_longest (builtin_type_int,
 					   (LONGEST) ecs->ws.value.integer));
       gdb_flush (gdb_stdout);
+      mark_thrd_event_bpt_clean();
       target_mourn_inferior ();
       singlestep_breakpoints_inserted_p = 0;	/*SOFTWARE_SINGLE_STEP_P() */
       stop_print_frame = 0;
Index: gdb-6.4/gdb/breakpoint.c
===================================================================
--- gdb-6.4.orig/gdb/breakpoint.c	2005-05-29 08:43:17.000000000 +0530
+++ gdb-6.4/gdb/breakpoint.c	2006-01-17 21:00:00.899323448 +0530
@@ -4289,6 +4289,17 @@
       delete_breakpoint (b);
 }
 
+void
+mark_thrd_event_bpt_clean (void)
+{
+
+      struct breakpoint *b, *temp;
+
+      ALL_BREAKPOINTS_SAFE (b, temp)
+        if (b->type == bp_thread_event)
+	   b->loc->inserted = 0;
+}
+
 struct captured_parse_breakpoint_args
   {
     char **arg_p;
Index: gdb-6.4/gdb/breakpoint.h
===================================================================
--- gdb-6.4.orig/gdb/breakpoint.h	2005-05-27 02:18:57.000000000 +0530
+++ gdb-6.4/gdb/breakpoint.h	2006-01-17 21:01:07.244237488 +0530
@@ -763,6 +763,8 @@
 
 extern void remove_thread_event_breakpoints (void);
 
+extern void  mark_thrd_event_bpt_clean (void);
+
 extern void disable_breakpoints_in_shlibs (int silent);
 
 extern void re_enable_breakpoints_in_shlibs (void);

Re: [Patch] Mark the thread_event breakpoint as "not inserted" before calling thread_mourn_inferior

[Daniel Jacobowitz]

On Tue, Jan 17, 2006 at 09:08:14PM +0530, Girish Shilamkar wrote:
> This problem occurs since gdb tries to remove thread_event breakpoint
> after the thread has exited and tries to read the address space since
> the breakpoint is still marked as updated.
> 	In the attached patch we mark the thread_event breakpoint as "not
> inserted" before calling thread_mourn_inferior.
> 	Is this the right way?	

Definitely not.  We need more information about this bug: what do you
do to reproduce it, since no one else has encountered it?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [Patch] Mark the thread_event breakpoint as "not inserted" before calling thread_mourn_inferior

[Girish Shilamkar]

[-- Attachment #1: Type: text/plain, Size: 423 bytes --]

> Definitely not.  We need more information about this bug: what do you
> do to reproduce it, since no one else has encountered it?
> 
I have attached .c to reproduce this bug. I am using gdb-6.4 checked out
from cvs, on Fedora Core 3, 32 bit Athlon processor. To reproduce the
problem just run the application under gdb. No breakpoints, backtrace
etc required. When the application exits this error can be seen.

-Girish.

[-- Attachment #2: mult.c --]
[-- Type: text/x-csrc, Size: 577 bytes --]

#include <pthread.h>

#define THR_NUMS 5 
#define MAX_THR 512

void *func(void *num)
{
    int i, j, l, m;

    sleep(1);
    for(i=1;i<10;i++)
      {
	for(j=1;j<10;j++)
	  {
	    l = l+j;	    
	  }
      }
}

int main()
{
    int i, thr_nums;
    pthread_t thr[MAX_THR];

    thr_nums = THR_NUMS;    
    for(i=0;i<thr_nums; i++)
      {
	if ( pthread_create (&(thr[i]), NULL, func, &i ) != 0 )
                            perror("Thread creation failed");
      }

    for (i=0;i<thr_nums;i++)    
      {
	    pthread_join( thr[i], NULL);
      }
    return 0;
}
    
    

Re: [Patch] Mark the thread_event breakpoint as "not inserted" before calling thread_mourn_inferior

[Daniel Jacobowitz]

On Wed, Jan 18, 2006 at 02:44:30PM +0530, Girish Shilamkar wrote:
> > Definitely not.  We need more information about this bug: what do you
> > do to reproduce it, since no one else has encountered it?
> > 
> I have attached .c to reproduce this bug. I am using gdb-6.4 checked out
> from cvs, on Fedora Core 3, 32 bit Athlon processor. To reproduce the
> problem just run the application under gdb. No breakpoints, backtrace
> etc required. When the application exits this error can be seen.

As I suspected something is wrong with your host system, kernel,
environment, or similar.  You may want to try a few others.

drow@caradoc:~% gdb ./mult
GNU gdb 6.4-debian
Copyright 2005 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and
you are
welcome to change it and/or distribute copies of it under certain
conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for
details.
This GDB was configured as "x86_64-linux-gnu"...Using host libthread_db
library "/lib/libthread_db.so.1".

(gdb) r
Starting program: /home/drow/mult 
[Thread debugging using libthread_db enabled]
[New Thread 46912500755888 (LWP 15465)]
[New Thread 1082132832 (LWP 15471)]
[New Thread 1090525536 (LWP 15472)]
[New Thread 1098918240 (LWP 15473)]
[New Thread 1107310944 (LWP 15474)]
[New Thread 1115703648 (LWP 15475)]
[Thread 1082132832 (LWP 15471) exited]
[Thread 1090525536 (LWP 15472) exited]
[Thread 1098918240 (LWP 15473) exited]
[Thread 1107310944 (LWP 15474) exited]
[Thread 1115703648 (LWP 15475) exited]

Program exited normally.
(gdb) 

Verified on both 32-bit and 64-bit hosts.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [Patch] Mark the thread_event breakpoint as "not inserted" before calling thread_mourn_inferior

[Girish Shilamkar]

> As I suspected something is wrong with your host system, kernel,
> environment, or similar.  You may want to try a few others.
	Thanks for the reply. I tried gdb-6.4 on Fedora Core 3 & 4 and always
saw that problem. I further tried gdb-6.3 on the same m/c with same
configuration (only the gdb had changed )on which I had seen this
problem and it worked fine. 
	Then while investigating further about the different behaviour of the
two gdb's i.e. gdb-6.3 and gdb-6.4 I found one difference that gdb-6.3
doesn't have a function called thread_db_xfer_partial.

	The following call trace is same for both the gdbs 

#11 0x08131c24 in target_write_memory (memaddr=4626416, myaddr=0x95a7618
"U",
            len=1) at ../../src/gdb/target.c:1013
#12 0x081305a2 in default_memory_remove_breakpoint (addr=4626416,
            contents_cache=0x95a7618 "U") at ../../src/gdb/mem-
break.c:78
#13 0x08124292 in gdbarch_memory_remove_breakpoint (gdbarch=0x9545f90,
            addr=4626416, contents_cache=0x95a7618 "U")
at ../../src/gdb/gdbarch.c:2960
#14 0x081305e0 in memory_remove_breakpoint (addr=4626416,
            contents_cache=0x95a7618 "U") at ../../src/gdb/mem-
break.c:91
#15 0x080cffdd in remove_breakpoint (b=0x95a7600, is=mark_inserted)
    at ../../src/gdb/breakpoint.c:1452
#16 0x080d82aa in delete_breakpoint (bpt=0x95a7558)
        at ../../src/gdb/breakpoint.c:6772
#17 0x080d4258 in remove_thread_event_breakpoints ()
            at ../../src/gdb/breakpoint.c:4295
#18 0x08096875 in thread_db_mourn_inferior ()
                at ../../src/gdb/linux-thread-db.c:1107
#19 0x0810da19 in handle_inferior_event (ecs=0xfefa4cb0)
                    at ../../src/gdb/infrun.c:1376
                    (More stack frames follow...)

	in target_write_memory, gdb-6.3 calls target_xfer_memory whereas
gdb-6.4 calls xfer_using_stratum this difference is there since
target_stack->to_xfer_partial is set to deafult_xfer_partial for gdb-6.3
whereas it is set to thread_db_xfer_partial for gdb-6.4
	So the further functions which are called for gdb-6.4 are
xfer_using_stratum -> target_xfer_partial -> thread_db_xfer_partial

And in thread_db_xfer_partial it checks whether the thread is alive or
not (target_thread_alive) and while doing so it tries to fetch registers
ptrace(PTRACE_GETREGS,.....) and fails.

	Kindly let me know what's wrong?

Thanks & Regards,
-Girish.
	

Re: [Patch] Mark the thread_event breakpoint as "not inserted" before calling thread_mourn_inferior

[Daniel Jacobowitz]

On Fri, Jan 20, 2006 at 07:47:18PM +0530, Girish Shilamkar wrote:
> > As I suspected something is wrong with your host system, kernel,
> > environment, or similar.  You may want to try a few others.
> 	Thanks for the reply. I tried gdb-6.4 on Fedora Core 3 & 4 and always
> saw that problem. I further tried gdb-6.3 on the same m/c with same
> configuration (only the gdb had changed )on which I had seen this
> problem and it worked fine. 
> 	Then while investigating further about the different behaviour of the
> two gdb's i.e. gdb-6.3 and gdb-6.4 I found one difference that gdb-6.3
> doesn't have a function called thread_db_xfer_partial.
> 
> 	The following call trace is same for both the gdbs 

You left out the interesting part of the trace; why is it fetching
registers?  My GDB doesn't.  Presumably this is something specific to
Red Hat's libthread_db.  Does td_thr_validate_p return TD_OK for a
thread which is actually dead?

> #14 0x081305e0 in memory_remove_breakpoint (addr=4626416,
>             contents_cache=0x95a7618 "U") at ../../src/gdb/mem-
> break.c:91
> #15 0x080cffdd in remove_breakpoint (b=0x95a7600, is=mark_inserted)
>     at ../../src/gdb/breakpoint.c:1452
> #16 0x080d82aa in delete_breakpoint (bpt=0x95a7558)
>         at ../../src/gdb/breakpoint.c:6772
> #17 0x080d4258 in remove_thread_event_breakpoints ()
>             at ../../src/gdb/breakpoint.c:4295
> #18 0x08096875 in thread_db_mourn_inferior ()
>                 at ../../src/gdb/linux-thread-db.c:1107

This suggests where the problem really ought to be fixed.
No point removing breakpoints after the inferior is dead.
generic_mourn_inferior arranges to mark all breakpoints
as uninserted.

Try moving remove_thread_event_breakpoints below the call
to target_beneath->to_mourn_inferior ().

-- 
Daniel Jacobowitz
CodeSourcery

Re: [Patch] Mark the thread_event breakpoint as "not inserted" before calling thread_mourn_inferior

[Girish Shilamkar]

[-- Attachment #1: Type: text/plain, Size: 2096 bytes --]


> You left out the interesting part of the trace; why is it fetching
> registers?  My GDB doesn't.  Presumably this is something specific to
> Red Hat's libthread_db.  Does td_thr_validate_p return TD_OK for a
> thread which is actually dead?
> 
	This is the stack trace td_thr_validate never returns , in fetch_regs
gdb calls perror_with_name (_("Couldn't get registers")); and control
never returns back to fetch_regs. 
----------------------------------------------------------------------
Program exited normally.

Breakpoint 1, fetch_regs (tid=-151132512) at ../../src/gdb/i386-linux-
nat.c:277
277           perror_with_name (_("Couldn't get registers"));
(gdb) bt
#0  fetch_regs (tid=-151132512) at ../../src/gdb/i386-linux-nat.c:277
#1  0x0808fdaf in i386_linux_fetch_inferior_registers (regno=-1)
    at ../../src/gdb/i386-linux-nat.c:513
#2  0x08091848 in thread_db_fetch_registers (regno=-1)
    at ../../src/gdb/linux-thread-db.c:1007
#3  0x0809058c in ps_lgetregs (ph=0xf6fde6a0, lwpid=29054,
gregset=0xfee6383c)
    at ../../src/gdb/proc-service.c:236
#4  0xf6db26b2 in td_ta_map_lwp2thr () from /lib/tls/libthread_db.so.1
#5  0xf6db3872 in td_thr_validate () from /lib/tls/libthread_db.so.1
#6  0x08091c1e in thread_db_thread_alive (ptid=
      {pid = 29054, lwp = 29061, tid = -193094736})
    at ../../src/gdb/linux-thread-db.c:1135
#7  0x080917b1 in thread_db_xfer_partial (ops=0x8280a80,
    object=TARGET_OBJECT_MEMORY, annex=0x0, readbuf=0x0,
    writebuf=0xfee639f0 "U\2174", offset=4626416, len=1)
    at ../../src/gdb/linux-thread-db.c:983
#8  0x0811267a in target_xfer_partial (ops=0x8280a80,
    object=TARGET_OBJECT_MEMORY, annex=0x0, readbuf=0x0,
writebuf=0xfee639f0,
    offset=4626416, len=1) at ../../src/gdb/target.c:864
#9  0x08112894 in xfer_using_stratum (object=TARGET_OBJECT_MEMORY,
annex=0x0,
    offset=4626416, len=Unhandled dwarf expression opcode 0x93


> 
> Try moving remove_thread_event_breakpoints below the call
> to target_beneath->to_mourn_inferior ().
> 
	It works , I have verified it on FC3 & FC4. I have attached the patch.

Thanks,
-Girish.

[-- Attachment #2: gdb-6.4-first_mourn.patch --]
[-- Type: text/x-patch, Size: 626 bytes --]

Index: src/gdb/linux-thread-db.c
===================================================================
--- gdb-cvs.orig/gdb/linux-thread-db.c	2006-01-23 14:20:23.331552656 +0530
+++ gdb-cvs/gdb/linux-thread-db.c	2006-01-23 14:25:29.955938664 +0530
@@ -1104,13 +1104,12 @@
 static void
 thread_db_mourn_inferior (void)
 {
-  remove_thread_event_breakpoints ();
-
   /* Forget about the child's process ID.  We shouldn't need it
      anymore.  */
   proc_handle.pid = 0;
 
   target_beneath->to_mourn_inferior ();
+  remove_thread_event_breakpoints ();
 
   /* Detach thread_db target ops.  */
   unpush_target (&thread_db_ops);

Re: [Patch] Mark the thread_event breakpoint as "not inserted" before calling thread_mourn_inferior

[Daniel Jacobowitz]

On Mon, Jan 23, 2006 at 02:54:51PM +0530, Girish Shilamkar wrote:
> #3  0x0809058c in ps_lgetregs (ph=0xf6fde6a0, lwpid=29054,
> gregset=0xfee6383c)
>     at ../../src/gdb/proc-service.c:236
> #4  0xf6db26b2 in td_ta_map_lwp2thr () from /lib/tls/libthread_db.so.1
> #5  0xf6db3872 in td_thr_validate () from /lib/tls/libthread_db.so.1
> #6  0x08091c1e in thread_db_thread_alive (ptid=
>       {pid = 29054, lwp = 29061, tid = -193094736})
>     at ../../src/gdb/linux-thread-db.c:1135

This is the unexpected bit; not sure why it doesn't happen for me,
but I am not going to investigate further now.

> > Try moving remove_thread_event_breakpoints below the call
> > to target_beneath->to_mourn_inferior ().
> > 
> 	It works , I have verified it on FC3 & FC4. I have attached the patch.

Thanks for testing it.  I've checked in the attached after testing on
x86_64-pc-linux-gnu.

-- 
Daniel Jacobowitz
CodeSourcery

2006-01-24  Daniel Jacobowitz  <dan@codesourcery.com>

	* linux-thread-db.c (thread_db_mourn_inferior): Remove breakpoints
	after mourning the inferior.

Index: linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.13
diff -u -p -r1.13 linux-thread-db.c
--- linux-thread-db.c	17 Jan 2006 14:57:43 -0000	1.13
+++ linux-thread-db.c	24 Jan 2006 21:49:26 -0000
@@ -1104,14 +1104,16 @@ thread_db_post_startup_inferior (ptid_t 
 static void
 thread_db_mourn_inferior (void)
 {
-  remove_thread_event_breakpoints ();
-
   /* Forget about the child's process ID.  We shouldn't need it
      anymore.  */
   proc_handle.pid = 0;
 
   target_beneath->to_mourn_inferior ();
 
+  /* Delete the old thread event breakpoints.  Do this after mourning
+     the inferior, so that we don't try to uninsert them.  */
+  remove_thread_event_breakpoints ();
+
   /* Detach thread_db target ops.  */
   unpush_target (&thread_db_ops);
   using_thread_db = 0;