[hjl@lucon.org: Re: Does gdb 5.2 work with statically linked thread application under Linux?]

[hjl@lucon.org: Re: Does gdb 5.2 work with statically linked thread application under Linux?]

[Daniel Jacobowitz]

Does anyone have a comment on this patch?  It fixes thread_db for
statically linked binaries (which I broke), and I think it's
legitimate.

----- Forwarded message from "H . J . Lu" <hjl@lucon.org> -----

Date: Thu, 7 Mar 2002 15:08:04 -0800
From: "H . J . Lu" <hjl@lucon.org>
Subject: Re: Does gdb 5.2 work with statically linked thread application under Linux?
To: GDB <gdb@sources.redhat.com>

On Thu, Mar 07, 2002 at 05:02:07PM -0500, Daniel Jacobowitz wrote:
> On Thu, Mar 07, 2002 at 01:39:28PM -0800, H . J . Lu wrote:
> > On Thu, Mar 07, 2002 at 02:48:41AM -0500, Daniel Jacobowitz wrote:
> > > On Wed, Mar 06, 2002 at 11:30:50PM -0800, H . J . Lu wrote:
> > > > Does gdb 5.2 work with statically linked thread application under
> > > > Linux? It doesn't work for me at all. It doesn't know any thread.
> > > > I have a patch which works for gdb 5.1. Now it doesn't work for 5.2
> > > > anymore.
> > > 
> > > My fault, I think.
> > > 
> > > The problem is that in a dynamically linked binary
> > > thread_db_new_objfile will be called for every library is loaded.  At
> > > this point current_target is "child", so target_has_execution is true.
> > > But with a static binary, the first time the function is called objfile
> > > is NULL, and the second time current_target is "exec" (which has
> > > target_has_execution set false).
> > > 
> > > Perhaps target_has_execution was not the right check after all, if
> > > "exec" has it set false (which makes fairly little sense to me...) or
> > > perhaps we need to call the hook again later.  Michael, any idea?
> > > 
> > 
> > What is the problem if "|| !target_has_execution" is removed? I removed
> > it. Gdb now works on statically linked thread application as well as
> > core file. Did I miss soemthing?
> 
> Try running 'info threads' on the corefile of a multithreaded
> application.  Recent Linux kernels will allow them to dump core.
> 
> I've also got a patch to put multiple threads into the corefile. 
> There's a sample corefile at:
>   http://crack.them.org/~drow/mtc2.tar.gz
> 
> 
> Without !target_has_execution, we try to use lin-lwp on the coredump. 
> That works very badly.
> 

This patch works for me.


H.J.
----
2002-03-07  H.J. Lu  (hjl@gnu.org)

	* target.h (target_corefile_hook): New prototype.

	* target.c (target_corefile_hook): New.

	* corelow.c (core_open): Call target_corefile_hook if needed.

	* thread-db.c (thread_db_corefile): New.
	(thread_db_new_objfile): Don't check target_has_execution.
	(_initialize_thread_db): Set target_corefile_hook to
	thread_db_corefile.

--- gdb/corelow.c.static	Wed Mar  6 22:30:49 2002
+++ gdb/corelow.c	Thu Mar  7 15:01:41 2002
@@ -303,6 +303,9 @@ core_open (char *filename, int from_tty)
 	     filename, bfd_errmsg (bfd_get_error ()));
     }
 
+  if (target_corefile_hook)
+    target_corefile_hook ();
+
   /* Looks semi-reasonable.  Toss the old core file and work on the new.  */
 
   discard_cleanups (old_chain);	/* Don't free filename any more */
--- gdb/target.c.static	Wed Mar  6 22:31:31 2002
+++ gdb/target.c	Thu Mar  7 14:54:50 2002
@@ -38,6 +38,8 @@
 
 extern int errno;
 
+void (*target_corefile_hook) (void);
+
 static void target_info (char *, int);
 
 static void cleanup_target (struct target_ops *);
--- gdb/target.h.static	Wed Mar  6 22:31:31 2002
+++ gdb/target.h	Thu Mar  7 14:54:38 2002
@@ -985,6 +985,8 @@ extern char *normal_pid_to_str (ptid_t p
 
 extern void (*target_new_objfile_hook) (struct objfile *);
 
+extern void (*target_corefile_hook) (void);
+
 #ifndef target_pid_or_tid_to_str
 #define target_pid_or_tid_to_str(ID) \
      target_pid_to_str (ID)
--- gdb/thread-db.c.static	Wed Mar  6 22:31:31 2002
+++ gdb/thread-db.c	Thu Mar  7 14:59:09 2002
@@ -479,13 +479,25 @@ disable_thread_signals (void)
 }
 
 static void
+thread_db_corefile (void)
+{
+  if (using_thread_db)
+    {
+      /* If the thread_db target is active, deactivate it now.  */
+      gdb_assert (proc_handle.pid == 0);
+      unpush_target (&thread_db_ops);
+      using_thread_db = 0;
+    }
+
+  keep_thread_db = 0;
+}
+
+static void
 thread_db_new_objfile (struct objfile *objfile)
 {
   td_err_e err;
 
-  /* Don't attempt to use thread_db on targets which can not run
-     (core files).  */
-  if (objfile == NULL || !target_has_execution)
+  if (objfile == NULL)
     {
       /* All symbols have been discarded.  If the thread_db target is
          active, deactivate it now.  */
@@ -1040,5 +1052,6 @@ _initialize_thread_db (void)
       /* Add ourselves to objfile event chain.  */
       target_new_objfile_chain = target_new_objfile_hook;
       target_new_objfile_hook = thread_db_new_objfile;
+      target_corefile_hook = thread_db_corefile;
     }
 }


----- End forwarded message -----

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [hjl@lucon.org: Re: Does gdb 5.2 work with statically linked thread application under Linux?]

[H . J . Lu]

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

On Wed, Mar 13, 2002 at 12:50:34PM -0500, Daniel Jacobowitz wrote:
> Does anyone have a comment on this patch?  It fixes thread_db for
> statically linked binaries (which I broke), and I think it's
> legitimate.
> 

I am enclosing an upated patch here. Any targets should be able to
use target_corefile_hook if necessary.


H.J.
---

[-- Attachment #2: gdb-static.patch --]
[-- Type: text/plain, Size: 3110 bytes --]

2002-03-07  H.J. Lu  (hjl@gnu.org)

	* target.h (target_corefile_hook): New prototype.

	* target.c (target_corefile_hook): New.

	* corelow.c (core_open): Call target_corefile_hook if needed.

	* thread-db.c (target_corefile_chain): New.
	(thread_db_corefile): New.
	(thread_db_new_objfile): Don't check target_has_execution.
	(_initialize_thread_db): Save target_corefile_hook in
	target_corefile_chain.
	Set target_corefile_hook to thread_db_corefile.

--- gdb/corelow.c.static	Wed Mar  6 22:30:49 2002
+++ gdb/corelow.c	Thu Mar  7 15:01:41 2002
@@ -303,6 +303,9 @@ core_open (char *filename, int from_tty)
 	     filename, bfd_errmsg (bfd_get_error ()));
     }
 
+  if (target_corefile_hook)
+    target_corefile_hook ();
+
   /* Looks semi-reasonable.  Toss the old core file and work on the new.  */
 
   discard_cleanups (old_chain);	/* Don't free filename any more */
--- gdb/target.c.static	Wed Mar  6 22:31:31 2002
+++ gdb/target.c	Thu Mar  7 14:54:50 2002
@@ -38,6 +38,8 @@
 
 extern int errno;
 
+void (*target_corefile_hook) (void);
+
 static void target_info (char *, int);
 
 static void cleanup_target (struct target_ops *);
--- gdb/target.h.static	Wed Mar  6 22:31:31 2002
+++ gdb/target.h	Thu Mar  7 14:54:38 2002
@@ -985,6 +985,8 @@ extern char *normal_pid_to_str (ptid_t p
 
 extern void (*target_new_objfile_hook) (struct objfile *);
 
+extern void (*target_corefile_hook) (void);
+
 #ifndef target_pid_or_tid_to_str
 #define target_pid_or_tid_to_str(ID) \
      target_pid_to_str (ID)
--- gdb/thread-db.c.static	Wed Mar  6 22:31:31 2002
+++ gdb/thread-db.c	Thu Mar  7 15:20:17 2002
@@ -54,6 +54,9 @@ static struct target_ops *target_beneath
 /* Pointer to the next function on the objfile event chain.  */
 static void (*target_new_objfile_chain) (struct objfile *objfile);
 
+/* Pointer to the next function on the corefile event chain.  */
+static void (*target_corefile_chain) (void);
+
 /* Non-zero if we're using this module's target vector.  */
 static int using_thread_db;
 
@@ -479,13 +482,28 @@ disable_thread_signals (void)
 }
 
 static void
+thread_db_corefile (void)
+{
+  if (using_thread_db)
+    {
+      /* If the thread_db target is active, deactivate it now.  */
+      gdb_assert (proc_handle.pid == 0);
+      unpush_target (&thread_db_ops);
+      using_thread_db = 0;
+    }
+
+  keep_thread_db = 0;
+
+  if (target_corefile_chain)
+    target_corefile_chain ();
+}
+
+static void
 thread_db_new_objfile (struct objfile *objfile)
 {
   td_err_e err;
 
-  /* Don't attempt to use thread_db on targets which can not run
-     (core files).  */
-  if (objfile == NULL || !target_has_execution)
+  if (objfile == NULL)
     {
       /* All symbols have been discarded.  If the thread_db target is
          active, deactivate it now.  */
@@ -1040,5 +1058,7 @@ _initialize_thread_db (void)
       /* Add ourselves to objfile event chain.  */
       target_new_objfile_chain = target_new_objfile_hook;
       target_new_objfile_hook = thread_db_new_objfile;
+      target_corefile_chain = target_corefile_hook;
+      target_corefile_hook = thread_db_corefile;
     }
 }

Re: [hjl@lucon.org: Re: Does gdb 5.2 work with statically linked thread application under Linux?]

[Andrew Cagney]

> --- gdb/corelow.c.static	Wed Mar  6 22:30:49 2002
> +++ gdb/corelow.c	Thu Mar  7 15:01:41 2002
> @@ -303,6 +303,9 @@ core_open (char *filename, int from_tty)
>  	     filename, bfd_errmsg (bfd_get_error ()));
>      }
>  
> +  if (target_corefile_hook)
> +    target_corefile_hook ();
> +
>    /* Looks semi-reasonable.  Toss the old core file and work on the new.  */
>  
>    discard_cleanups (old_chain);	/* Don't free filename any more */
> --- gdb/target.c.static	Wed Mar  6 22:31:31 2002


> --- gdb/thread-db.c.static	Wed Mar  6 22:31:31 2002
> +++ gdb/thread-db.c	Thu Mar  7 14:59:09 2002
> @@ -479,13 +479,25 @@ disable_thread_signals (void)
>  }


The below just feels wrong.  The hook is pulling the thread stratum off 
the stack when, as far as I can tell, there is no compelling reason for 
doing this.

``hey'' something has happened.  At this point, nothing has happened.
>  static void
> +thread_db_corefile (void)
> +{
> +  if (using_thread_db)
> +    {
> +      /* If the thread_db target is active, deactivate it now.  */
> +      gdb_assert (proc_handle.pid == 0);
> +      unpush_target (&thread_db_ops);
> +      using_thread_db = 0;
> +    }
> +
> +  keep_thread_db = 0;
> +}

It is possible to have thread, process and core-file stratum 
simultaneously.  Changing the core-file shouldn't quietly zap the thread 
stratum on top of the process stratum.

I suspect the real problem here is a limitation in the current target 
frame work - it doesn't accomodate having two instances of the thread 
stratum active (one for the process and one for the core file) active at 
the same time :-(

Andrew

Re: [hjl@lucon.org: Re: Does gdb 5.2 work with statically linked thread application under Linux?]

[Daniel Jacobowitz]

On Fri, Mar 15, 2002 at 09:51:06PM -0500, Andrew Cagney wrote:
> >--- gdb/corelow.c.static	Wed Mar  6 22:30:49 2002
> >+++ gdb/corelow.c	Thu Mar  7 15:01:41 2002
> >@@ -303,6 +303,9 @@ core_open (char *filename, int from_tty)
> > 	     filename, bfd_errmsg (bfd_get_error ()));
> >     }
> > 
> >+  if (target_corefile_hook)
> >+    target_corefile_hook ();
> >+
> >   /* Looks semi-reasonable.  Toss the old core file and work on the new.  */
> > 
> >   discard_cleanups (old_chain);	/* Don't free filename any more */
> >--- gdb/target.c.static	Wed Mar  6 22:31:31 2002
> 
> 
> >--- gdb/thread-db.c.static	Wed Mar  6 22:31:31 2002
> >+++ gdb/thread-db.c	Thu Mar  7 14:59:09 2002
> >@@ -479,13 +479,25 @@ disable_thread_signals (void)
> > }
> 
> 
> The below just feels wrong.  The hook is pulling the thread stratum off 
> the stack when, as far as I can tell, there is no compelling reason for 
> doing this.
> 
> ``hey'' something has happened.  At this point, nothing has happened.

What do you mean by "there is no compelling reason"?  Or how should
this be handled?  The issue is that we do not want thread_db to be used
at all for corefiles; we established that last time I touched this
code, I just solved the problem wrong.  Once again, it doesn't work
as-is, and unpushing matches the way the code was trying to behave.

> > static void
> >+thread_db_corefile (void)
> >+{
> >+  if (using_thread_db)
> >+    {
> >+      /* If the thread_db target is active, deactivate it now.  */
> >+      gdb_assert (proc_handle.pid == 0);
> >+      unpush_target (&thread_db_ops);
> >+      using_thread_db = 0;
> >+    }
> >+
> >+  keep_thread_db = 0;
> >+}
> 
> It is possible to have thread, process and core-file stratum 
> simultaneously.  Changing the core-file shouldn't quietly zap the thread 
> stratum on top of the process stratum.
> 
> I suspect the real problem here is a limitation in the current target 
> frame work - it doesn't accomodate having two instances of the thread 
> stratum active (one for the process and one for the core file) active at 
> the same time :-(

The target stack is almost entirely undocumented, unfortunately.  But
from this it seems that changing targets should change the stack
completely; be it a multi-arch thing or a local/remote thing (we've
seen thread_db do the same thing in that case) or a local/core thing.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [hjl@lucon.org: Re: Does gdb 5.2 work with statically linked thread application under Linux?]

[Andrew Cagney]

> The below just feels wrong.  The hook is pulling the thread stratum off 
>> the stack when, as far as I can tell, there is no compelling reason for 
>> doing this.
>> 
>> ``hey'' something has happened.  At this point, nothing has happened.
> 
> 
> What do you mean by "there is no compelling reason"?  Or how should
> this be handled?  The issue is that we do not want thread_db to be used
> at all for corefiles; we established that last time I touched this
> code, I just solved the problem wrong.  Once again, it doesn't work
> as-is, and unpushing matches the way the code was trying to behave.

I'd expect the thread stratum to be unpushed at the same time as the 
stratum below it is also unpushed.  In the code above, nothing is being 
unpushed so I can't see a reason to unpush the thread-db.

A few lines below the call-out is an unpush() call.  Shouldn't that 
unpush any stratum directly dependant on it?

If you don't want thread-db trying to push its self on top of a core 
stratum, why not check for core and ignore the event?

(GNU/Linux doesn't want the thread-db pushing its self on top of a CORE 
stratum but other OS's do (with an N:M thread:lwp mapping for instance).

> It is possible to have thread, process and core-file stratum 
>> simultaneously.  Changing the core-file shouldn't quietly zap the thread 
>> stratum on top of the process stratum.
>> 
>> I suspect the real problem here is a limitation in the current target 
>> frame work - it doesn't accomodate having two instances of the thread 
>> stratum active (one for the process and one for the core file) active at 
>> the same time :-(
> 
> 
> The target stack is almost entirely undocumented, unfortunately.  But
> from this it seems that changing targets should change the stack
> completely; be it a multi-arch thing or a local/remote thing (we've
> seen thread_db do the same thing in that case) or a local/core thing.

Most of GDB is almost entirely undocumented :-)  However, the user guide 
does describe the external interface to this feature:
http://sources.redhat.com/gdb/current/onlinedocs/gdb_16.html#SEC130

If I understand what you're saying correctly, yes.  Unfortunatly it 
isn't implemented that way.

Search for the words ``squashed sandwich'' in the mail archives :-)

Andrew

Re: [hjl@lucon.org: Re: Does gdb 5.2 work with statically linked thread application under Linux?]

[Daniel Jacobowitz]

On Fri, Mar 15, 2002 at 10:56:37PM -0500, Andrew Cagney wrote:
> >The below just feels wrong.  The hook is pulling the thread stratum off 
> >>the stack when, as far as I can tell, there is no compelling reason for 
> >>doing this.
> >>
> >>``hey'' something has happened.  At this point, nothing has happened.
> >
> >
> >What do you mean by "there is no compelling reason"?  Or how should
> >this be handled?  The issue is that we do not want thread_db to be used
> >at all for corefiles; we established that last time I touched this
> >code, I just solved the problem wrong.  Once again, it doesn't work
> >as-is, and unpushing matches the way the code was trying to behave.
> 
> I'd expect the thread stratum to be unpushed at the same time as the 
> stratum below it is also unpushed.  In the code above, nothing is being 
> unpushed so I can't see a reason to unpush the thread-db.
> 
> A few lines below the call-out is an unpush() call.  Shouldn't that 
> unpush any stratum directly dependant on it?

It unpushes only core_ops.  core_ops isn't pushed at this point, we
weren't debugging a corefile before.

> If you don't want thread-db trying to push its self on top of a core 
> stratum, why not check for core and ignore the event?
> 
> (GNU/Linux doesn't want the thread-db pushing its self on top of a CORE 
> stratum but other OS's do (with an N:M thread:lwp mapping for instance).

I can't find the precise message any more, but I believe we'd decided
thread-db and core files was a bad idea without more work on thread-db. 
In any case, Michael Snyder said to me:

>>> Umm... I had to think about this, but no.  You can't debug a corefile
>>> until you kill or detach from the process that you're already
>>> debugging.
>>> When you kill or detach, that ought to take care of the unpush.

Maybe it should, but (probably because of when thread-db gets pushed?)
it definitely does not.  Perhaps that is the real bug?

Should thread_db_detach call unpush_target?  Some targets seem to like
that model, some don't.  The way we load our target in new_objfile_hook
always struck me as somewhat gross.

> Most of GDB is almost entirely undocumented :-)  However, the user guide 
> does describe the external interface to this feature:
> http://sources.redhat.com/gdb/current/onlinedocs/gdb_16.html#SEC130

Yep, it's implementation details that worry me.

> If I understand what you're saying correctly, yes.  Unfortunatly it 
> isn't implemented that way.

It should be pretty easy, I'd think - call unpush at the appropriate
time in detach...

> Search for the words ``squashed sandwich'' in the mail archives :-)

No matches :P

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [hjl@lucon.org: Re: Does gdb 5.2 work with statically linked thread application under Linux?]

[Andrew Cagney]

>> If you don't want thread-db trying to push its self on top of a core 
>> stratum, why not check for core and ignore the event?

What about this?

>> (GNU/Linux doesn't want the thread-db pushing its self on top of a CORE 
>> stratum but other OS's do (with an N:M thread:lwp mapping for instance).
> 
> 
> I can't find the precise message any more, but I believe we'd decided
> thread-db and core files was a bad idea without more work on thread-db. 
> In any case, Michael Snyder said to me:

It isn't thread-db that needs the work, it is the target 
stack/stratum/sandwich :-)

>>>> Umm... I had to think about this, but no.  You can't debug a corefile
>>>> until you kill or detach from the process that you're already
>>>> debugging.
>>>> When you kill or detach, that ought to take care of the unpush.

Two reasons:

Target stratum (thread, process, ...) is limited to only one instance as 
most of each target's state is in static data

The target stratum is one dimentional (for want of a better phrase).  It 
isn't possible to have:

	process:  thread-db -> remote -> ...
	core: thread-db -> corefile -> ...

> Maybe it should, but (probably because of when thread-db gets pushed?)
> it definitely does not.  Perhaps that is the real bug?
> 
> Should thread_db_detach call unpush_target?  Some targets seem to like
> that model, some don't.  The way we load our target in new_objfile_hook
> always struck me as somewhat gross.

I suspect, for the moment, it is the best thing we have. 
new_objfile_hook is the only ``new symbol file'' event available.

Andrew