Re: [rfa] Delete keep_thread_db

Re: [rfa] Delete keep_thread_db

[M.M. Kettenis]

Michael Snyder <msnyder@redhat.com> wrote:

> Andrew Cagney wrote:
> > It would appear that, at one stage, this variable had something to do
> > with corefiles, static programs and threads.  That's no longer the case
> > so this deletes the stray global.
> >
> > Tested on FC3 with no regressions,
> >
> > Ok?
> > Andrew
>
> I would have to defer to Mark Kettenis, since he added the
> variable, and from the ChangeLogs, seems to be the only one
> who's touched it.

The code defenitely was there to deal with static binaries.  Andrew, how did you come to the conclusion that it is no longer useful?  AFAIK there is no static threads test in the testsuite.

Anyway, I don't care about this code anymore.  I've given up on debugging-with threads on Linux.  thread-db.c should renamed into linux-thread.c or somesuch.  It, and lin-lwp.c have become a mess of workarounds around workarounds around workarounds that probably only work if you run a specific combination of glibc and patched kernel.

So I'm not going to explicitly OK or this patch.  Just rely on your own judgement here Andrew.

Mark

Re: [rfa] Delete keep_thread_db

[Andrew Cagney]

> Michael Snyder <msnyder@redhat.com> wrote: 
> 
> 
>>> Andrew Cagney wrote:
>>
>>>> > It would appear that, at one stage, this variable had something to do 
>>>> > with corefiles, static programs and threads.  That's no longer the case 
>>>> > so this deletes the stray global.
>>>> > 
>>>> > Tested on FC3 with no regressions,
>>>> > 
>>>> > Ok?
>>>> > Andrew
>>
>>> 
>>> I would have to defer to Mark Kettenis, since he added the
>>> variable, and from the ChangeLogs, seems to be the only one
>>> who's touched it.
> 
> 
> The code defenitely was there to deal with static binaries.  Andrew, how did you come to the conclusion that it is no longer useful?  AFAIK there is no static threads test in the testsuite.

I recently added a test case for this feature - staticthreads.exp.
It fails.

> Anyway, I don't care about this code anymore.  I've given up on debugging-with threads on Linux.  thread-db.c should renamed into linux-thread.c or somesuch.  It, and lin-lwp.c have become a mess of workarounds around workarounds around workarounds that probably only work if you run a specific combination of glibc and patched kernel.
> 
> So I'm not going to explicitly OK or this patch.  Just rely on your own judgement here Andrew.

Andrew

Re: [rfa] Delete keep_thread_db

[Andrew Cagney]

> So I'm not going to explicitly OK or this patch.  Just rely on your own judgement here Andrew. 

It's in, lets see what else falls out when I shake the tree :-)

Andrew

Re: [rfa] Delete keep_thread_db

[Daniel Jacobowitz]

On Wed, Sep 08, 2004 at 07:00:00AM +0000, M.M. Kettenis wrote:
> Michael Snyder <msnyder@redhat.com> wrote:
> 
> > Andrew Cagney wrote:
> > > It would appear that, at one stage, this variable had something to do
> > > with corefiles, static programs and threads.  That's no longer the case
> > > so this deletes the stray global.
> > >
> > > Tested on FC3 with no regressions,
> > >
> > > Ok?
> > > Andrew
> >
> > I would have to defer to Mark Kettenis, since he added the
> > variable, and from the ChangeLogs, seems to be the only one
> > who's touched it.
> 
> The code defenitely was there to deal with static binaries.  Andrew,
> how did you come to the conclusion that it is no longer useful? 
> AFAIK there is no static threads test in the testsuite.

FYI, I believe this is what happened: keep_thread_db was added to deal
with static binaries.  It was necessary.  Core file debugging did not
work.

Core file debugging was then fixed, which broke static binaries.

keep_thread_db handles the fact that we only used the load-symfile
event to initialize thread_db, not any inferior_created event also.
If you use an observer after inferior creation for this, it won't be
necessary.  I don't see much point in removing it earlier than fixing
the bug; it's not "stray" yet, just broken.  But do whichever you want.

> Anyway, I don't care about this code anymore.  I've given up on
> debugging-with threads on Linux.  thread-db.c should renamed into
> linux-thread.c or somesuch.  It, and lin-lwp.c have become a mess of
> workarounds around workarounds around workarounds that probably only
> work if you run a specific combination of glibc and patched kernel.

I do care about this code.  It needs some major surgery, but it is
still useful (and, while fragile, not quite as fragile as all that). If
no one else is interested in maintaining it then I would like to.

-- 
Daniel Jacobowitz

Re: [rfa] Delete keep_thread_db

[Michael Snyder]

Daniel Jacobowitz wrote:
> On Wed, Sep 08, 2004 at 07:00:00AM +0000, M.M. Kettenis wrote:
> 

>>Anyway, I don't care about this code anymore.  I've given up on
>>debugging-with threads on Linux.  thread-db.c should renamed into
>>linux-thread.c or somesuch.  It, and lin-lwp.c have become a mess of
>>workarounds around workarounds around workarounds that probably only
>>work if you run a specific combination of glibc and patched kernel.
> 
> 
> I do care about this code.  It needs some major surgery, but it is
> still useful (and, while fragile, not quite as fragile as all that). If
> no one else is interested in maintaining it then I would like to.
> 

I'm still interested.  Let's share it?

[rfa] Delete keep_thread_db

[Andrew Cagney]

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

It would appear that, at one stage, this variable had something to do 
with corefiles, static programs and threads.  That's no longer the case 
so this deletes the stray global.

Tested on FC3 with no regressions,

Ok?
Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 2866 bytes --]

Index: ChangeLog
2004-09-05  Andrew Cagney  <cagney@gnu.org>

	* thread-db.c (keep_thread_db): Delete.
	(thread_db_new_objfile, thread_db_create_inferior)
	(thread_db_mourn_inferior): Simplify assuming !keep_thread_db.

Index: thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/thread-db.c,v
retrieving revision 1.43
diff -p -u -r1.43 thread-db.c
--- thread-db.c	22 Jul 2004 01:31:49 -0000	1.43
+++ thread-db.c	5 Sep 2004 15:57:45 -0000
@@ -63,10 +63,6 @@ static void (*target_new_objfile_chain) 
 /* Non-zero if we're using this module's target vector.  */
 static int using_thread_db;
 
-/* Non-zero if we have to keep this module's target vector active
-   across re-runs.  */
-static int keep_thread_db;
-
 /* Non-zero if we have determined the signals used by the threads
    library.  */
 static int thread_signals;
@@ -673,8 +669,6 @@ thread_db_new_objfile (struct objfile *o
 	  using_thread_db = 0;
 	}
 
-      keep_thread_db = 0;
-
       goto quit;
     }
 
@@ -703,18 +697,6 @@ thread_db_new_objfile (struct objfile *o
       push_target (&thread_db_ops);
       using_thread_db = 1;
 
-      /* If the thread library was detected in the main symbol file
-         itself, we assume that the program was statically linked
-         against the thread library and well have to keep this
-         module's target vector activated until forever...  Well, at
-         least until all symbols have been discarded anyway (see
-         above).  */
-      if (objfile == symfile_objfile)
-	{
-	  gdb_assert (proc_handle.pid == 0);
-	  keep_thread_db = 1;
-	}
-
       /* We can only poke around if there actually is a child process.
          If there is no child process alive, postpone the steps below
          until one has been created.  */
@@ -1106,12 +1088,8 @@ static void
 thread_db_create_inferior (char *exec_file, char *allargs, char **env,
 			   int from_tty)
 {
-  if (!keep_thread_db)
-    {
-      unpush_target (&thread_db_ops);
-      using_thread_db = 0;
-    }
-
+  unpush_target (&thread_db_ops);
+  using_thread_db = 0;
   target_beneath->to_create_inferior (exec_file, allargs, env, from_tty);
 }
 
@@ -1141,17 +1119,9 @@ thread_db_mourn_inferior (void)
 
   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;
-    }
+  /* Detach thread_db target ops.  */
+  unpush_target (&thread_db_ops);
+  using_thread_db = 0;
 }
 
 static int

Re: [rfa] Delete keep_thread_db

[Michael Snyder]

Andrew Cagney wrote:
> It would appear that, at one stage, this variable had something to do 
> with corefiles, static programs and threads.  That's no longer the case 
> so this deletes the stray global.
> 
> Tested on FC3 with no regressions,
> 
> Ok?
> Andrew

I would have to defer to Mark Kettenis, since he added the
variable, and from the ChangeLogs, seems to be the only one
who's touched it.



> ------------------------------------------------------------------------
> 
> Index: ChangeLog
> 2004-09-05  Andrew Cagney  <cagney@gnu.org>
> 
> 	* thread-db.c (keep_thread_db): Delete.
> 	(thread_db_new_objfile, thread_db_create_inferior)
> 	(thread_db_mourn_inferior): Simplify assuming !keep_thread_db.
> 
> Index: thread-db.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/thread-db.c,v
> retrieving revision 1.43
> diff -p -u -r1.43 thread-db.c
> --- thread-db.c	22 Jul 2004 01:31:49 -0000	1.43
> +++ thread-db.c	5 Sep 2004 15:57:45 -0000
> @@ -63,10 +63,6 @@ static void (*target_new_objfile_chain) 
>  /* Non-zero if we're using this module's target vector.  */
>  static int using_thread_db;
>  
> -/* Non-zero if we have to keep this module's target vector active
> -   across re-runs.  */
> -static int keep_thread_db;
> -
>  /* Non-zero if we have determined the signals used by the threads
>     library.  */
>  static int thread_signals;
> @@ -673,8 +669,6 @@ thread_db_new_objfile (struct objfile *o
>  	  using_thread_db = 0;
>  	}
>  
> -      keep_thread_db = 0;
> -
>        goto quit;
>      }
>  
> @@ -703,18 +697,6 @@ thread_db_new_objfile (struct objfile *o
>        push_target (&thread_db_ops);
>        using_thread_db = 1;
>  
> -      /* If the thread library was detected in the main symbol file
> -         itself, we assume that the program was statically linked
> -         against the thread library and well have to keep this
> -         module's target vector activated until forever...  Well, at
> -         least until all symbols have been discarded anyway (see
> -         above).  */
> -      if (objfile == symfile_objfile)
> -	{
> -	  gdb_assert (proc_handle.pid == 0);
> -	  keep_thread_db = 1;
> -	}
> -
>        /* We can only poke around if there actually is a child process.
>           If there is no child process alive, postpone the steps below
>           until one has been created.  */
> @@ -1106,12 +1088,8 @@ static void
>  thread_db_create_inferior (char *exec_file, char *allargs, char **env,
>  			   int from_tty)
>  {
> -  if (!keep_thread_db)
> -    {
> -      unpush_target (&thread_db_ops);
> -      using_thread_db = 0;
> -    }
> -
> +  unpush_target (&thread_db_ops);
> +  using_thread_db = 0;
>    target_beneath->to_create_inferior (exec_file, allargs, env, from_tty);
>  }
>  
> @@ -1141,17 +1119,9 @@ thread_db_mourn_inferior (void)
>  
>    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;
> -    }
> +  /* Detach thread_db target ops.  */
> +  unpush_target (&thread_db_ops);
> +  using_thread_db = 0;
>  }
>  
>  static int