RFA: Patch for corefile support

RFA: Patch for corefile support

[J. Johnston]

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

The attached patch fixes a problem in gdb when a corefile is read in
after a multithreaded application has been debugged.  What happens is that
the thread-db and lin-lwp layers are still around and run into internal
errors.

The solution is simply to unpush the thread-db ops in its mourn_inferior
routine.  If a corefile gets loaded, there is no thread-db to interfere.
If another multi-threaded app gets loaded, the thread_db_new_objfile is
designed to bring back the thread-db layer as needed.

This fix solves another failure in the killed.exp testsuite as well.

Ok to commit?

-- Jeff J.

[-- Attachment #2: thread-db.patch --]
[-- Type: text/plain, Size: 555 bytes --]

Index: thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/thread-db.c,v
retrieving revision 1.28
diff -u -r1.28 thread-db.c
--- thread-db.c	14 Jan 2003 00:49:04 -0000	1.28
+++ thread-db.c	27 Jan 2003 22:58:58 -0000
@@ -1007,6 +1007,11 @@
   proc_handle.pid = 0;
 
   target_beneath->to_mourn_inferior ();
+
+  /* Detach thread_db target ops.  The thread_db_new_objfile routine
+     will reattach them later if needed.  */
+  unpush_target (&thread_db_ops);
+  using_thread_db = 0;
 }
 
 static int

Re: RFA: Patch for corefile support

[Mark Kettenis]

"J. Johnston" <jjohnstn@redhat.com> writes:

> The attached patch fixes a problem in gdb when a corefile is read in
> after a multithreaded application has been debugged.  What happens is that
> the thread-db and lin-lwp layers are still around and run into internal
> errors.
> 
> The solution is simply to unpush the thread-db ops in its mourn_inferior
> routine.  If a corefile gets loaded, there is no thread-db to interfere.
> If another multi-threaded app gets loaded, the thread_db_new_objfile is
> designed to bring back the thread-db layer as needed.
> 
> This fix solves another failure in the killed.exp testsuite as well.
> 
> Ok to commit?

Sorry, no.  AFAICT this will break debugging programs that are
statically linked against libpthread.  As a minimum, this code should
check keep_thread_db before unpushing the target, but even then, I'm
not sure whether this is really OK.

Mark

Re: RFA: Patch for corefile support

[Daniel Jacobowitz]

On Sat, Feb 01, 2003 at 02:22:02PM +0100, Mark Kettenis wrote:
> "J. Johnston" <jjohnstn@redhat.com> writes:
> 
> > The attached patch fixes a problem in gdb when a corefile is read in
> > after a multithreaded application has been debugged.  What happens is that
> > the thread-db and lin-lwp layers are still around and run into internal
> > errors.
> > 
> > The solution is simply to unpush the thread-db ops in its mourn_inferior
> > routine.  If a corefile gets loaded, there is no thread-db to interfere.
> > If another multi-threaded app gets loaded, the thread_db_new_objfile is
> > designed to bring back the thread-db layer as needed.
> > 
> > This fix solves another failure in the killed.exp testsuite as well.
> > 
> > Ok to commit?
> 
> Sorry, no.  AFAICT this will break debugging programs that are
> statically linked against libpthread.  As a minimum, this code should
> check keep_thread_db before unpushing the target, but even then, I'm
> not sure whether this is really OK.

Programs statically linked against libpthread are already broken.  I
have a patch to fix it, but it's so gross that I haven't posted it; I
still can't think of a good way to do it.

Given the way GDB treats targets, we seem to be waffling; someone fixes
core file support and breaks static binaries, or vice versa.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA: Patch for corefile support

[J. Johnston]

Daniel Jacobowitz wrote:
> On Sat, Feb 01, 2003 at 02:22:02PM +0100, Mark Kettenis wrote:
> 
>>"J. Johnston" <jjohnstn@redhat.com> writes:
>>
>>
>>>The attached patch fixes a problem in gdb when a corefile is read in
>>>after a multithreaded application has been debugged.  What happens is that
>>>the thread-db and lin-lwp layers are still around and run into internal
>>>errors.
>>>
>>>The solution is simply to unpush the thread-db ops in its mourn_inferior
>>>routine.  If a corefile gets loaded, there is no thread-db to interfere.
>>>If another multi-threaded app gets loaded, the thread_db_new_objfile is
>>>designed to bring back the thread-db layer as needed.
>>>
>>>This fix solves another failure in the killed.exp testsuite as well.
>>>
>>>Ok to commit?
>>
>>Sorry, no.  AFAICT this will break debugging programs that are
>>statically linked against libpthread.  As a minimum, this code should
>>check keep_thread_db before unpushing the target, but even then, I'm
>>not sure whether this is really OK.
> 
> 
> Programs statically linked against libpthread are already broken.  I
> have a patch to fix it, but it's so gross that I haven't posted it; I
> still can't think of a good way to do it.
> 
> Given the way GDB treats targets, we seem to be waffling; someone fixes
> core file support and breaks static binaries, or vice versa.
> 

So, is there a scenario where my patch would be wrong?  I am seeing what you
discussed.  Statically linked multi-threaded programs don't work with gdb
because we never set up the thread_db_ops layer to begin with
(thread_db_new_objfile never gets called with a non-null objfile with
the target_has_execution flag on).

-- Jeff J.

Re: RFA: Patch for corefile support

[Daniel Jacobowitz]

On Mon, Feb 03, 2003 at 05:58:19PM -0500, J. Johnston wrote:
> 
> 
> Daniel Jacobowitz wrote:
> >On Sat, Feb 01, 2003 at 02:22:02PM +0100, Mark Kettenis wrote:
> >
> >>"J. Johnston" <jjohnstn@redhat.com> writes:
> >>
> >>
> >>>The attached patch fixes a problem in gdb when a corefile is read in
> >>>after a multithreaded application has been debugged.  What happens is 
> >>>that
> >>>the thread-db and lin-lwp layers are still around and run into internal
> >>>errors.
> >>>
> >>>The solution is simply to unpush the thread-db ops in its mourn_inferior
> >>>routine.  If a corefile gets loaded, there is no thread-db to interfere.
> >>>If another multi-threaded app gets loaded, the thread_db_new_objfile is
> >>>designed to bring back the thread-db layer as needed.
> >>>
> >>>This fix solves another failure in the killed.exp testsuite as well.
> >>>
> >>>Ok to commit?
> >>
> >>Sorry, no.  AFAICT this will break debugging programs that are
> >>statically linked against libpthread.  As a minimum, this code should
> >>check keep_thread_db before unpushing the target, but even then, I'm
> >>not sure whether this is really OK.
> >
> >
> >Programs statically linked against libpthread are already broken.  I
> >have a patch to fix it, but it's so gross that I haven't posted it; I
> >still can't think of a good way to do it.
> >
> >Given the way GDB treats targets, we seem to be waffling; someone fixes
> >core file support and breaks static binaries, or vice versa.
> >
> 
> So, is there a scenario where my patch would be wrong?  I am seeing what you
> discussed.  Statically linked multi-threaded programs don't work with gdb
> because we never set up the thread_db_ops layer to begin with
> (thread_db_new_objfile never gets called with a non-null objfile with
> the target_has_execution flag on).

I don't know.  The whole way in which thread_db is initialized right
now is a little bit confusing.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA: Patch for corefile support

[J. Johnston]

Daniel Jacobowitz wrote:
> On Mon, Feb 03, 2003 at 05:58:19PM -0500, J. Johnston wrote:
> 
>>
>>Daniel Jacobowitz wrote:
>>
>>>On Sat, Feb 01, 2003 at 02:22:02PM +0100, Mark Kettenis wrote:
>>>
>>>
>>>>"J. Johnston" <jjohnstn@redhat.com> writes:
>>>>
>>>>
>>>>
>>>>>The attached patch fixes a problem in gdb when a corefile is read in
>>>>>after a multithreaded application has been debugged.  What happens is 
>>>>>that
>>>>>the thread-db and lin-lwp layers are still around and run into internal
>>>>>errors.
>>>>>
>>>>>The solution is simply to unpush the thread-db ops in its mourn_inferior
>>>>>routine.  If a corefile gets loaded, there is no thread-db to interfere.
>>>>>If another multi-threaded app gets loaded, the thread_db_new_objfile is
>>>>>designed to bring back the thread-db layer as needed.
>>>>>
>>>>>This fix solves another failure in the killed.exp testsuite as well.
>>>>>
>>>>>Ok to commit?
>>>>
>>>>Sorry, no.  AFAICT this will break debugging programs that are
>>>>statically linked against libpthread.  As a minimum, this code should
>>>>check keep_thread_db before unpushing the target, but even then, I'm
>>>>not sure whether this is really OK.
>>>
>>>
>>>Programs statically linked against libpthread are already broken.  I
>>>have a patch to fix it, but it's so gross that I haven't posted it; I
>>>still can't think of a good way to do it.
>>>
>>>Given the way GDB treats targets, we seem to be waffling; someone fixes
>>>core file support and breaks static binaries, or vice versa.
>>>
>>
>>So, is there a scenario where my patch would be wrong?  I am seeing what you
>>discussed.  Statically linked multi-threaded programs don't work with gdb
>>because we never set up the thread_db_ops layer to begin with
>>(thread_db_new_objfile never gets called with a non-null objfile with
>>the target_has_execution flag on).
> 
> 
> I don't know.  The whole way in which thread_db is initialized right
> now is a little bit confusing.
> 

Mark, I haven't seen a response from you regarding this.  Do you still oppose
the patch now that it is clear that statically linked threaded programs
never worked in the first place?  If so, could you please elaborate.

-- Jefff J.

Re: RFA: Patch for corefile support

[Michael Snyder]

"J. Johnston" wrote:
> 
> The attached patch fixes a problem in gdb when a corefile is read in
> after a multithreaded application has been debugged.  What happens is that
> the thread-db and lin-lwp layers are still around and run into internal
> errors.
> 
> The solution is simply to unpush the thread-db ops in its mourn_inferior
> routine.  If a corefile gets loaded, there is no thread-db to interfere.
> If another multi-threaded app gets loaded, the thread_db_new_objfile is
> designed to bring back the thread-db layer as needed.
> 
> This fix solves another failure in the killed.exp testsuite as well.
> 
> Ok to commit?
> 
> -- Jeff J.

Hi Jeff, 

After reviewing the discussion, why don't you add a test for
"keep_thread_db" as suggested by Mark, and check this in?
Perhaps with a comment explaining that the debugging of
statically-linked threaded programs is currently broken, but
this will preserve sanity in case it is ever fixed.

Michael


> 
>   -------------------------------------------------------------------------------
> Index: thread-db.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/thread-db.c,v
> retrieving revision 1.28
> diff -u -r1.28 thread-db.c
> --- thread-db.c 14 Jan 2003 00:49:04 -0000      1.28
> +++ thread-db.c 27 Jan 2003 22:58:58 -0000
> @@ -1007,6 +1007,11 @@
>    proc_handle.pid = 0;
> 
>    target_beneath->to_mourn_inferior ();
> +
> +  /* Detach thread_db target ops.  The thread_db_new_objfile routine
> +     will reattach them later if needed.  */
> +  unpush_target (&thread_db_ops);
> +  using_thread_db = 0;
>  }
> 
>  static int

Re: RFA: Patch for corefile support

[J. Johnston]

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


Michael Snyder wrote:
> "J. Johnston" wrote:
> 
>>The attached patch fixes a problem in gdb when a corefile is read in
>>after a multithreaded application has been debugged.  What happens is that
>>the thread-db and lin-lwp layers are still around and run into internal
>>errors.
>>
>>The solution is simply to unpush the thread-db ops in its mourn_inferior
>>routine.  If a corefile gets loaded, there is no thread-db to interfere.
>>If another multi-threaded app gets loaded, the thread_db_new_objfile is
>>designed to bring back the thread-db layer as needed.
>>
>>This fix solves another failure in the killed.exp testsuite as well.
>>
>>Ok to commit?
>>
>>-- Jeff J.
> 
> 
> Hi Jeff, 
> 
> After reviewing the discussion, why don't you add a test for
> "keep_thread_db" as suggested by Mark, and check this in?
> Perhaps with a comment explaining that the debugging of
> statically-linked threaded programs is currently broken, but
> this will preserve sanity in case it is ever fixed.
> 
> Michael
> 
> 

Thanks Michael.  The attached patch has been checked in with the following
ChangeLog.

2003-06-03  Jeff Johnston  <jjohnstn@redhat.com>

         * thread-db.c (thread_db_mourn_inferior): Unpush thread target
         layer if not dealing with a statically-linked threaded program.

-- Jeff J.


[-- Attachment #2: thread-db.patch --]
[-- Type: text/plain, Size: 842 bytes --]

Index: thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/thread-db.c,v
retrieving revision 1.31
diff -u -r1.31 thread-db.c
--- thread-db.c	8 May 2003 22:33:14 -0000	1.31
+++ thread-db.c	3 Jun 2003 20:00:37 -0000
@@ -1011,6 +1011,18 @@
   proc_handle.pid = 0;
 
   target_beneath->to_mourn_inferior ();
+
+  /* Detach thread_db target ops if not dealing with a statically
+     linked threaded program.  This allows a corefile to be debugged
+     after finishing debugging of a threaded program.  At present,
+     debugging a statically-linked threaded program is broken, but
+     the check is added below in the event that it is fixed in the
+     future.  */
+  if (!keep_thread_db)
+    {
+      unpush_target (&thread_db_ops);
+      using_thread_db = 0;
+    }
 }
 
 static int