[RFA] Only try to load libthread_db when we load libpthread.

[RFA] Only try to load libthread_db when we load libpthread.

[Doug Evans]

Hi.

System libraries are typically last in the list of libraries to load,
and in a program with lots of shared libraries this results
in lots of pointless attempts to load libthread_db.

No regressions in amd64-linux,
but I can imagine it misses some cases.

2011-10-05  Doug Evans  <dje@google.com>

	* linux-thread-db.c (thread_db_new_objfile): Only try to load
	libthread_db when we load libpthread.

Index: linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.91
diff -u -p -r1.91 linux-thread-db.c
--- linux-thread-db.c	13 Sep 2011 19:27:01 -0000	1.91
+++ linux-thread-db.c	5 Oct 2011 18:16:17 -0000
@@ -1083,7 +1083,12 @@ thread_db_new_objfile (struct objfile *o
   /* This observer must always be called with inferior_ptid set
      correctly.  */
 
-  if (objfile != NULL)
+  if (objfile != NULL
+      /* Only check for thread_db if we loaded libpthread.
+	 Libpthread can be near the end of the list of shared libraries to
+	 load, and in an app of several thousand shared libraries, this can
+	 otherwise be painful.  */
+      && libpthread_name_p (objfile->name))
     check_for_thread_db ();
 }
 

Re: [RFA] Only try to load libthread_db when we load libpthread.

[Pedro Alves]

On Wednesday 05 October 2011 19:27:05, Doug Evans wrote:
> 2011-10-05  Doug Evans  <dje@google.com>
> 
>         * linux-thread-db.c (thread_db_new_objfile): Only try to load
>         libthread_db when we load libpthread.

Makes sense to me.

> No regressions in amd64-linux,
> but I can imagine it misses some cases.

Yeah.  I think we'll no longer activate thread_db when debugging core
files of static executables (e.g., a core of gdb.threads/staticthreads).
It works with live debugging since we call check_for_thread_db
from linux_child_post_attach/linux_child_post_startup_inferior.
Maybe moving that to an inferior_created observer in
linux-thread-db.c would work.

-- 
Pedro Alves

Re: [RFA] Only try to load libthread_db when we load libpthread.

[Pedro Alves]

On Thursday 06 October 2011 12:22:24, Pedro Alves wrote:
> On Wednesday 05 October 2011 19:27:05, Doug Evans wrote:
> > 2011-10-05  Doug Evans  <dje@google.com>
> > 
> >         * linux-thread-db.c (thread_db_new_objfile): Only try to load
> >         libthread_db when we load libpthread.
> 
> Makes sense to me.
> 
> > No regressions in amd64-linux,
> > but I can imagine it misses some cases.
> 
> Yeah.  I think we'll no longer activate thread_db when debugging core
> files of static executables (e.g., a core of gdb.threads/staticthreads).
> It works with live debugging since we call check_for_thread_db
> from linux_child_post_attach/linux_child_post_startup_inferior.
> Maybe moving that to an inferior_created observer in
> linux-thread-db.c would work.

And all the talk about executables made me realize something else.  :-)

For static threaded executables, we'll want to check for thread
db when the symbols of the main executable are (re)loaded too.
I don't recall off hand if there's a flag in the objfile to
know that it's from the main executable though.

-- 
Pedro Alves

Re: [RFA] Only try to load libthread_db when we load libpthread.

[Doug Evans]

On Thu, Oct 6, 2011 at 12:56 PM, Pedro Alves <pedro@codesourcery.com> wrote:
>
> On Thursday 06 October 2011 12:22:24, Pedro Alves wrote:
> > On Wednesday 05 October 2011 19:27:05, Doug Evans wrote:
> > > 2011-10-05  Doug Evans  <dje@google.com>
> > >
> > >         * linux-thread-db.c (thread_db_new_objfile): Only try to load
> > >         libthread_db when we load libpthread.
> >
> > Makes sense to me.
> >
> > > No regressions in amd64-linux,
> > > but I can imagine it misses some cases.
> >
> > Yeah.  I think we'll no longer activate thread_db when debugging core
> > files of static executables (e.g., a core of gdb.threads/staticthreads).
> > It works with live debugging since we call check_for_thread_db
> > from linux_child_post_attach/linux_child_post_startup_inferior.
> > Maybe moving that to an inferior_created observer in
> > linux-thread-db.c would work.
>
> And all the talk about executables made me realize something else.  :-)
>
> For static threaded executables, we'll want to check for thread
> db when the symbols of the main executable are (re)loaded too.
> I don't recall off hand if there's a flag in the objfile to
> know that it's from the main executable though.

In what scenario?
[what would the user type?]

Re: [RFA] Only try to load libthread_db when we load libpthread.

[Pedro Alves]

On Thursday 06 October 2011 21:08:08, Doug Evans wrote:
> On Thu, Oct 6, 2011 at 12:56 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> >
> > On Thursday 06 October 2011 12:22:24, Pedro Alves wrote:
> > > On Wednesday 05 October 2011 19:27:05, Doug Evans wrote:
> > > > 2011-10-05  Doug Evans  <dje@google.com>
> > > >
> > > >         * linux-thread-db.c (thread_db_new_objfile): Only try to load
> > > >         libthread_db when we load libpthread.
> > >
> > > Makes sense to me.
> > >
> > > > No regressions in amd64-linux,
> > > > but I can imagine it misses some cases.
> > >
> > > Yeah.  I think we'll no longer activate thread_db when debugging core
> > > files of static executables (e.g., a core of gdb.threads/staticthreads).
> > > It works with live debugging since we call check_for_thread_db
> > > from linux_child_post_attach/linux_child_post_startup_inferior.
> > > Maybe moving that to an inferior_created observer in
> > > linux-thread-db.c would work.
> >
> > And all the talk about executables made me realize something else.  :-)
> >
> > For static threaded executables, we'll want to check for thread
> > db when the symbols of the main executable are (re)loaded too.
> > I don't recall off hand if there's a flag in the objfile to
> > know that it's from the main executable though.
> 
> In what scenario?
> [what would the user type?]

(gdb) file wrong_executable
(gdb) attach PID or core-file core.1234
whooops!
(gdb) file right_executable

-- 
Pedro Alves

Re: [RFA] Only try to load libthread_db when we load libpthread.

[Doug Evans]

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

On Thu, Oct 6, 2011 at 1:25 PM, Pedro Alves <pedro@codesourcery.com> wrote:
>
> On Thursday 06 October 2011 21:08:08, Doug Evans wrote:
> > On Thu, Oct 6, 2011 at 12:56 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> > >
> > > On Thursday 06 October 2011 12:22:24, Pedro Alves wrote:
> > > > On Wednesday 05 October 2011 19:27:05, Doug Evans wrote:
> > > > > 2011-10-05  Doug Evans  <dje@google.com>
> > > > >
> > > > >         * linux-thread-db.c (thread_db_new_objfile): Only try to load
> > > > >         libthread_db when we load libpthread.
> > > >
> > > > Makes sense to me.
> > > >
> > > > > No regressions in amd64-linux,
> > > > > but I can imagine it misses some cases.
> > > >
> > > > Yeah.  I think we'll no longer activate thread_db when debugging core
> > > > files of static executables (e.g., a core of gdb.threads/staticthreads).
> > > > It works with live debugging since we call check_for_thread_db
> > > > from linux_child_post_attach/linux_child_post_startup_inferior.
> > > > Maybe moving that to an inferior_created observer in
> > > > linux-thread-db.c would work.
> > >
> > > And all the talk about executables made me realize something else.  :-)
> > >
> > > For static threaded executables, we'll want to check for thread
> > > db when the symbols of the main executable are (re)loaded too.
> > > I don't recall off hand if there's a flag in the objfile to
> > > know that it's from the main executable though.
> >
> > In what scenario?
> > [what would the user type?]
>
> (gdb) file wrong_executable
> (gdb) attach PID or core-file core.1234
> whooops!
> (gdb) file right_executable

What would the user expect to be able to do next?
I ask because I played with it, and things don't necessarily work,
e.g. if wrong_executable didn't have libpthread.
Perhaps more smarts could be added to file to make this work, or maybe
a new command could be added to (effectively) reattach so that the
initialization that attach does was redone (in case one is
uncomfortable with having file do that.  Though I think commands
shouldn't try to do too much.  Otherwise one could say "Why doesn't
attach automagically redo the "file" command since if the first "file"
wasn't done it would anyway?"  Maybe attach should at least warn the
user though).

Anyways, I think checking for the main symbol file is reasonable (even
if more work is needed), so how about this?

2011-10-06  Doug Evans  <dje@google.com>

        * linux-thread-db.c (thread_db_new_objfile): Only try to load
        libthread_db when we load libpthread or the main symbol file.
        * objfiles.h (OBJF_MAINLINE): Define.
        * symfile.c (symbol_file_add_with_addrs_or_offsets): Pass it to
        allocate_objfile when appropriate.

[-- Attachment #2: gdb-111006-libthread-db-2.patch.txt --]
[-- Type: text/plain, Size: 3073 bytes --]

2011-10-06  Doug Evans  <dje@google.com>

	* linux-thread-db.c (thread_db_new_objfile): Only try to load
	libthread_db when we load libpthread or the main symbol file.
	* objfiles.h (OBJF_MAINLINE): Define.
	* symfile.c (symbol_file_add_with_addrs_or_offsets): Pass it to
	allocate_objfile when appropriate.

Index: linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.91
diff -u -p -r1.91 linux-thread-db.c
--- linux-thread-db.c	13 Sep 2011 19:27:01 -0000	1.91
+++ linux-thread-db.c	6 Oct 2011 21:39:32 -0000
@@ -1083,7 +1083,16 @@ thread_db_new_objfile (struct objfile *o
   /* This observer must always be called with inferior_ptid set
      correctly.  */
 
-  if (objfile != NULL)
+  if (objfile != NULL
+      /* Only check for thread_db if we loaded libpthread,
+	 or if this is the main symbol file.
+	 We need to check OBJF_MAINLINE in case this is a statically
+	 linked executable.
+	 For dynamically linked executables, libpthread can be near the end
+	 of the list of shared libraries to load, and in an app of several
+	 thousand shared libraries, this can otherwise be painful.  */
+      && ((objfile->flags & OBJF_MAINLINE) != 0
+	  || libpthread_name_p (objfile->name)))
     check_for_thread_db ();
 }
 
Index: objfiles.h
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.h,v
retrieving revision 1.85
diff -u -p -r1.85 objfiles.h
--- objfiles.h	14 Jun 2011 16:49:41 -0000	1.85
+++ objfiles.h	6 Oct 2011 21:39:32 -0000
@@ -196,7 +196,8 @@ struct objfile
 
     CORE_ADDR addr_low;
 
-    /* Some flag bits for this objfile.  */
+    /* Some flag bits for this objfile.
+       The values are defined by OBJF_*.  */
 
     unsigned short flags;
 
@@ -434,6 +435,11 @@ struct objfile
 
 #define OBJF_PSYMTABS_READ (1 << 4)
 
+/* Set if this is the main symbol file
+   (as opposed to symbol file for dynamically loaded code).  */
+
+#define OBJF_MAINLINE (1 << 5)
+
 /* The object file that contains the runtime common minimal symbols
    for SunOS4.  Note that this objfile has no associated BFD.  */
 
Index: symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.316
diff -u -p -r1.316 symfile.c
--- symfile.c	29 Sep 2011 02:04:25 -0000	1.316
+++ symfile.c	6 Oct 2011 21:39:34 -0000
@@ -1084,6 +1084,7 @@ symbol_file_add_with_addrs_or_offsets (b
   const int should_print = ((from_tty || info_verbose)
 			    && (readnow_symbol_files
 				|| (add_flags & SYMFILE_NO_READ) == 0));
+  const int mainline = add_flags & SYMFILE_MAINLINE;
 
   if (readnow_symbol_files)
     {
@@ -1102,7 +1103,7 @@ symbol_file_add_with_addrs_or_offsets (b
       && !query (_("Load new symbol table from \"%s\"? "), name))
     error (_("Not confirmed."));
 
-  objfile = allocate_objfile (abfd, flags);
+  objfile = allocate_objfile (abfd, flags | (mainline ? OBJF_MAINLINE : 0));
   discard_cleanups (my_cleanups);
 
   if (parent)

Re: [RFA] Only try to load libthread_db when we load libpthread.

[Doug Evans]

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

On Thu, Oct 6, 2011 at 2:55 PM, Doug Evans <dje@google.com> wrote:
>
> On Thu, Oct 6, 2011 at 1:25 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> >
> > On Thursday 06 October 2011 21:08:08, Doug Evans wrote:
> > > On Thu, Oct 6, 2011 at 12:56 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> > > >
> > > > On Thursday 06 October 2011 12:22:24, Pedro Alves wrote:
> > > > > On Wednesday 05 October 2011 19:27:05, Doug Evans wrote:
> > > > > > 2011-10-05  Doug Evans  <dje@google.com>
> > > > > >
> > > > > >         * linux-thread-db.c (thread_db_new_objfile): Only try to load
> > > > > >         libthread_db when we load libpthread.
> > > > >
> > > > > Makes sense to me.
> > > > >
> > > > > > No regressions in amd64-linux,
> > > > > > but I can imagine it misses some cases.
> > > > >
> > > > > Yeah.  I think we'll no longer activate thread_db when debugging core
> > > > > files of static executables (e.g., a core of gdb.threads/staticthreads).
> > > > > It works with live debugging since we call check_for_thread_db
> > > > > from linux_child_post_attach/linux_child_post_startup_inferior.
> > > > > Maybe moving that to an inferior_created observer in
> > > > > linux-thread-db.c would work.
> > > >
> > > > And all the talk about executables made me realize something else.  :-)
> > > >
> > > > For static threaded executables, we'll want to check for thread
> > > > db when the symbols of the main executable are (re)loaded too.
> > > > I don't recall off hand if there's a flag in the objfile to
> > > > know that it's from the main executable though.
> > >
> > > In what scenario?
> > > [what would the user type?]
> >
> > (gdb) file wrong_executable
> > (gdb) attach PID or core-file core.1234
> > whooops!
> > (gdb) file right_executable
>
> What would the user expect to be able to do next?
> I ask because I played with it, and things don't necessarily work,
> e.g. if wrong_executable didn't have libpthread.
> Perhaps more smarts could be added to file to make this work, or maybe
> a new command could be added to (effectively) reattach so that the
> initialization that attach does was redone (in case one is
> uncomfortable with having file do that.  Though I think commands
> shouldn't try to do too much.  Otherwise one could say "Why doesn't
> attach automagically redo the "file" command since if the first "file"
> wasn't done it would anyway?"  Maybe attach should at least warn the
> user though).
>
> Anyways, I think checking for the main symbol file is reasonable (even
> if more work is needed), so how about this?
>
> 2011-10-06  Doug Evans  <dje@google.com>
>
>        * linux-thread-db.c (thread_db_new_objfile): Only try to load
>        libthread_db when we load libpthread or the main symbol file.
>        * objfiles.h (OBJF_MAINLINE): Define.
>        * symfile.c (symbol_file_add_with_addrs_or_offsets): Pass it to
>        allocate_objfile when appropriate.

Blech, I forgot one bit of cleanup.
Revised patch attached.

[-- Attachment #2: gdb-111006-libthread-db-3.patch.txt --]
[-- Type: text/plain, Size: 3364 bytes --]

2011-10-06  Doug Evans  <dje@google.com>

	* linux-thread-db.c (thread_db_new_objfile): Only try to load
	libthread_db when we load libpthread or the main symbol file.
	* objfiles.h (OBJF_MAINLINE): Define.
	* symfile.c (symbol_file_add_with_addrs_or_offsets): Pass it to
	allocate_objfile when appropriate.

Index: linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.91
diff -u -p -r1.91 linux-thread-db.c
--- linux-thread-db.c	13 Sep 2011 19:27:01 -0000	1.91
+++ linux-thread-db.c	6 Oct 2011 22:03:15 -0000
@@ -1083,7 +1083,16 @@ thread_db_new_objfile (struct objfile *o
   /* This observer must always be called with inferior_ptid set
      correctly.  */
 
-  if (objfile != NULL)
+  if (objfile != NULL
+      /* Only check for thread_db if we loaded libpthread,
+	 or if this is the main symbol file.
+	 We need to check OBJF_MAINLINE in case this is a statically
+	 linked executable.
+	 For dynamically linked executables, libpthread can be near the end
+	 of the list of shared libraries to load, and in an app of several
+	 thousand shared libraries, this can otherwise be painful.  */
+      && ((objfile->flags & OBJF_MAINLINE) != 0
+	  || libpthread_name_p (objfile->name)))
     check_for_thread_db ();
 }
 
Index: objfiles.h
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.h,v
retrieving revision 1.85
diff -u -p -r1.85 objfiles.h
--- objfiles.h	14 Jun 2011 16:49:41 -0000	1.85
+++ objfiles.h	6 Oct 2011 22:03:15 -0000
@@ -196,7 +196,8 @@ struct objfile
 
     CORE_ADDR addr_low;
 
-    /* Some flag bits for this objfile.  */
+    /* Some flag bits for this objfile.
+       The values are defined by OBJF_*.  */
 
     unsigned short flags;
 
@@ -434,6 +435,11 @@ struct objfile
 
 #define OBJF_PSYMTABS_READ (1 << 4)
 
+/* Set if this is the main symbol file
+   (as opposed to symbol file for dynamically loaded code).  */
+
+#define OBJF_MAINLINE (1 << 5)
+
 /* The object file that contains the runtime common minimal symbols
    for SunOS4.  Note that this objfile has no associated BFD.  */
 
Index: symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.316
diff -u -p -r1.316 symfile.c
--- symfile.c	29 Sep 2011 02:04:25 -0000	1.316
+++ symfile.c	6 Oct 2011 22:03:15 -0000
@@ -1081,6 +1081,7 @@ symbol_file_add_with_addrs_or_offsets (b
   struct cleanup *my_cleanups;
   const char *name = bfd_get_filename (abfd);
   const int from_tty = add_flags & SYMFILE_VERBOSE;
+  const int mainline = add_flags & SYMFILE_MAINLINE;
   const int should_print = ((from_tty || info_verbose)
 			    && (readnow_symbol_files
 				|| (add_flags & SYMFILE_NO_READ) == 0));
@@ -1097,12 +1098,12 @@ symbol_file_add_with_addrs_or_offsets (b
      interactively wiping out any existing symbols.  */
 
   if ((have_full_symbols () || have_partial_symbols ())
-      && (add_flags & SYMFILE_MAINLINE)
+      && mainline
       && from_tty
       && !query (_("Load new symbol table from \"%s\"? "), name))
     error (_("Not confirmed."));
 
-  objfile = allocate_objfile (abfd, flags);
+  objfile = allocate_objfile (abfd, flags | (mainline ? OBJF_MAINLINE : 0));
   discard_cleanups (my_cleanups);
 
   if (parent)

Re: [RFA] Only try to load libthread_db when we load libpthread.

[Pedro Alves]

On Thursday 06 October 2011 23:05:45, Doug Evans wrote:

> > What would the user expect to be able to do next?
> > I ask because I played with it, and things don't necessarily work,
> > e.g. if wrong_executable didn't have libpthread.
> > Perhaps more smarts could be added to file to make this work, or maybe
> > a new command could be added to (effectively) reattach so that the
> > initialization that attach does was redone (in case one is
> > uncomfortable with having file do that.  Though I think commands
> > shouldn't try to do too much.  Otherwise one could say "Why doesn't
> > attach automagically redo the "file" command since if the first "file"
> > wasn't done it would anyway?"  Maybe attach should at least warn the
> > user though).

Well, at least things should work the same as before, which IMO
is good thing as it makes this change an optimization only.

> Blech, I forgot one bit of cleanup.
> Revised patch attached.

Still breaks activating thread_db when debugging cores of
static executables.

$ ./gdb -nx -q ./testsuite/gdb.threads/staticthreads 
Reading symbols from /home/pedro/gdb/baseline/build/gdb/testsuite/gdb.threads/staticthreads...done.
(gdb) start
Temporary breakpoint 1 at 0x400548: file ../../../src/gdb/testsuite/gdb.threads/staticthreads.c, line 48.
Starting program: /home/pedro/gdb/baseline/build/gdb/testsuite/gdb.threads/staticthreads 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Temporary breakpoint 1, main (argc=1, argv=0x7fffffffe038) at ../../../src/gdb/testsuite/gdb.threads/staticthreads.c:48
48      {
(gdb) gcore core.test
Saved corefile core.test

Before:

$ ./gdb -nx -q ./testsuite/gdb.threads/staticthreads -c ./core.test 
Reading symbols from /home/pedro/gdb/baseline/build/gdb/testsuite/gdb.threads/staticthreads...done.
[New LWP 32418]
[Thread debugging using libthread_db enabled]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Core was generated by `/home/pedro/gdb/baseline/build/gdb/testsuite/gdb.threads/staticthreads'.
Program terminated with signal 5, Trace/breakpoint trap.
#0  main (argc=1, argv=0x7fffffffe038) at ../../../src/gdb/testsuite/gdb.threads/staticthreads.c:48
48      {
(gdb) 

After:

$ ./gdb -nx -q ./testsuite/gdb.threads/staticthreads -c ./core.test 
Reading symbols from /home/pedro/gdb/baseline/build/gdb/testsuite/gdb.threads/staticthreads...done.
[New LWP 32418]
Core was generated by `/home/pedro/gdb/baseline/build/gdb/testsuite/gdb.threads/staticthreads'.
Program terminated with signal 5, Trace/breakpoint trap.
#0  main (argc=1, argv=0x7fffffffe038) at ../../../src/gdb/testsuite/gdb.threads/staticthreads.c:48
48      {
(gdb) 

Did you try this suggestion?:

> I think we'll no longer activate thread_db when debugging core
> files of static executables (e.g., a core of gdb.threads/staticthreads).
> It works with live debugging since we call check_for_thread_db
> from linux_child_post_attach/linux_child_post_startup_inferior.
> Maybe moving that to an inferior_created observer in
> linux-thread-db.c would work.

-- 
Pedro Alves

Re: [RFA] Only try to load libthread_db when we load libpthread.

[Doug Evans]

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

On Fri, Oct 7, 2011 at 4:09 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> Well, at least things should work the same as before, which IMO
> is good thing as it makes this change an optimization only.

That's the intent.

>> Blech, I forgot one bit of cleanup.
>> Revised patch attached.
>
> Still breaks activating thread_db when debugging cores of
> static executables.

Used wrong sandbox for the before case, sigh.

[...]

> Did you try this suggestion?:
>
>> I think we'll no longer activate thread_db when debugging core
>> files of static executables (e.g., a core of gdb.threads/staticthreads).
>> It works with live debugging since we call check_for_thread_db
>> from linux_child_post_attach/linux_child_post_startup_inferior.
>> Maybe moving that to an inferior_created observer in
>> linux-thread-db.c would work.

It turns out things work today accidentally.
At least I doubt it's by design. :-)
My previous patch didn't work because it skipped calling
check_for_thread_db when the new_objfile observer was called for
vsyscall (which incidentally is called by the inferior_created
observer for vsyscall, heh).
[Which explains why it didn't surprise me when my before-test failed,
that's a pretty unexpected way to load libthread_db.]

How about this.

2011-10-09  Doug Evans  <dje@google.com>

        * linux-thread-db.c (thread_db_new_objfile): Only try to load
        libthread_db when we load libpthread.
        (thread_db_inferior_created): New function.
        (_initialize_thread_db): Attach inferior_created observer.

[-- Attachment #2: gdb-111009-libthread-db-4.patch.txt --]
[-- Type: text/plain, Size: 2129 bytes --]

2011-10-09  Doug Evans  <dje@google.com>

	* linux-thread-db.c (thread_db_new_objfile): Only try to load
	libthread_db when we load libpthread.
	(thread_db_inferior_created): New function.
	(_initialize_thread_db): Attach inferior_created observer.

Index: linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.91
diff -u -p -r1.91 linux-thread-db.c
--- linux-thread-db.c	13 Sep 2011 19:27:01 -0000	1.91
+++ linux-thread-db.c	10 Oct 2011 03:52:27 -0000
@@ -1077,16 +1077,32 @@ check_for_thread_db (void)
     return;
 }
 
+/* This function is called via the new_objfile observer.  */
+
 static void
 thread_db_new_objfile (struct objfile *objfile)
 {
   /* This observer must always be called with inferior_ptid set
      correctly.  */
 
-  if (objfile != NULL)
+  if (objfile != NULL
+      /* Only check for thread_db if we loaded libpthread.
+	 For dynamically linked executables, libpthread can be near the end
+	 of the list of shared libraries to load, and in an app of several
+	 thousand shared libraries, this can otherwise be painful.  */
+      && libpthread_name_p (objfile->name))
     check_for_thread_db ();
 }
 
+/* This function is called via the inferior_created observer.
+   This handles the case of debugging statically linked executables.  */
+
+static void
+thread_db_inferior_created (struct target_ops *target, int from_tty)
+{
+  check_for_thread_db ();
+}
+
 /* Attach to a new thread.  This function is called when we receive a
    TD_CREATE event or when we iterate over all threads and find one
    that wasn't already in our list.  Returns true on success.  */
@@ -1845,4 +1861,9 @@ When non-zero, libthread-db debugging is
 
   /* Add ourselves to objfile event chain.  */
   observer_attach_new_objfile (thread_db_new_objfile);
+
+  /* Add ourselves to inferior_created event chain.
+     This is needed to handle debugging statically linked programs where
+     the new_objfile observer won't get called for libpthread.  */
+  observer_attach_inferior_created (thread_db_inferior_created);
 }

Re: [RFA] Only try to load libthread_db when we load libpthread.

[Pedro Alves]

On Monday 10 October 2011 06:01:00, Doug Evans wrote:
> On Fri, Oct 7, 2011 at 4:09 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> > Well, at least things should work the same as before, which IMO
> > is good thing as it makes this change an optimization only.
> 
> That's the intent.
> 
> > Still breaks activating thread_db when debugging cores of
> > static executables.

> It turns out things work today accidentally.
> At least I doubt it's by design. :-)

Yeah.  :-)

> My previous patch didn't work because it skipped calling
> check_for_thread_db when the new_objfile observer was called for
> vsyscall (which incidentally is called by the inferior_created
> observer for vsyscall, heh).
> [Which explains why it didn't surprise me when my before-test failed,
> that's a pretty unexpected way to load libthread_db.]
> 
> How about this.

This version goes back to breaking the "file right_executable" core
or attach cases (with static binaries) that the previous patch
fixed.  :-)  You need _both_ the observer, and the OBJF_MAINLINE thing.
The observer handles the case of loading the static executable into gdb before
loading the core or attaching to a process (gdb exec -c core), and
the OBJF_MAINLINE thing handles the opposite scenario (gdb -c core; file exec).

> +/* This function is called via the inferior_created observer.
> +   This handles the case of debugging statically linked executables.  */

As mentioned before, !cores are handled by:

static void
linux_child_post_attach (int pid)
{
  linux_enable_event_reporting (pid_to_ptid (pid));
  check_for_thread_db ();
  linux_enable_tracesysgood (pid_to_ptid (pid));
}

static void
linux_child_post_startup_inferior (ptid_t ptid)
{
  linux_enable_event_reporting (ptid);
  check_for_thread_db ();
  linux_enable_tracesysgood (ptid);
}

So this handles the case of debugging _cores_ of statically
linked executables, as those don't use linux-nat.c.  With this
new observer, we could even remove those two check_for_thread_db
calls above.

> +
> +static void
> +thread_db_inferior_created (struct target_ops *target, int from_tty)

This is okay with me with the OBJF_MAINLINE changes merged back in,
and the comment mentioned above adjusted.

Thanks.

-- 
Pedro Alves

Re: [RFA] Only try to load libthread_db when we load libpthread.

[Doug Evans]

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

On Mon, Oct 10, 2011 at 11:22 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> This version goes back to breaking the "file right_executable" core
> or attach cases (with static binaries) that the previous patch
> fixed.  :-)
> [...]
> (gdb -c core; file exec).

That scenario I hadn't thought of.
Thanks, at least the code will be there for a documented reason.
Note that the non-static case of that sequence was and still is broken.

I checked this in.

2011-10-10  Doug Evans  <dje@google.com>

        * linux-thread-db.c (thread_db_new_objfile): Only try to load
        libthread_db when we load libpthread or the main symbol file.
        (thread_db_inferior_created): New function.
        (_initialize_thread_db): Attach inferior_created observer.
        * linux-nat.c (linux_child_post_attach): Remove call to
        check_for_thread_db.
        (linux_child_post_startup_inferior): Ditto.
        * objfiles.h (OBJF_MAINLINE): Define.
        * symfile.c (symbol_file_add_with_addrs_or_offsets): Pass it to
        allocate_objfile when appropriate.

[-- Attachment #2: gdb-111010-libthread-db-5.patch.txt --]
[-- Type: text/plain, Size: 5416 bytes --]

2011-10-10  Doug Evans  <dje@google.com>

	* linux-thread-db.c (thread_db_new_objfile): Only try to load
	libthread_db when we load libpthread or the main symbol file.
	(thread_db_inferior_created): New function.
	(_initialize_thread_db): Attach inferior_created observer.
	* linux-nat.c (linux_child_post_attach): Remove call to
	check_for_thread_db.
	(linux_child_post_startup_inferior): Ditto.
	* objfiles.h (OBJF_MAINLINE): Define.
	* symfile.c (symbol_file_add_with_addrs_or_offsets): Pass it to
	allocate_objfile when appropriate.

Index: linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.c,v
retrieving revision 1.216
diff -u -p -r1.216 linux-nat.c
--- linux-nat.c	7 Oct 2011 12:06:46 -0000	1.216
+++ linux-nat.c	11 Oct 2011 02:44:46 -0000
@@ -571,7 +571,6 @@ static void
 linux_child_post_attach (int pid)
 {
   linux_enable_event_reporting (pid_to_ptid (pid));
-  check_for_thread_db ();
   linux_enable_tracesysgood (pid_to_ptid (pid));
 }
 
@@ -579,7 +578,6 @@ static void
 linux_child_post_startup_inferior (ptid_t ptid)
 {
   linux_enable_event_reporting (ptid);
-  check_for_thread_db ();
   linux_enable_tracesysgood (ptid);
 }
 
Index: linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.91
diff -u -p -r1.91 linux-thread-db.c
--- linux-thread-db.c	13 Sep 2011 19:27:01 -0000	1.91
+++ linux-thread-db.c	11 Oct 2011 02:44:46 -0000
@@ -1077,16 +1077,37 @@ check_for_thread_db (void)
     return;
 }
 
+/* This function is called via the new_objfile observer.  */
+
 static void
 thread_db_new_objfile (struct objfile *objfile)
 {
   /* This observer must always be called with inferior_ptid set
      correctly.  */
 
-  if (objfile != NULL)
+  if (objfile != NULL
+      /* Only check for thread_db if we loaded libpthread,
+	 or if this is the main symbol file.
+	 We need to check OBJF_MAINLINE to handle the case of debugging
+	 a statically linked executable AND the symbol file is specified AFTER
+	 the core file is loaded (e.g., gdb -c core ; file foo).
+	 For dynamically linked executables, libpthread can be near the end
+	 of the list of shared libraries to load, and in an app of several
+	 thousand shared libraries, this can otherwise be painful.  */
+      && ((objfile->flags & OBJF_MAINLINE) != 0
+	  || libpthread_name_p (objfile->name)))
     check_for_thread_db ();
 }
 
+/* This function is called via the inferior_created observer.
+   This handles the case of debugging statically linked executables.  */
+
+static void
+thread_db_inferior_created (struct target_ops *target, int from_tty)
+{
+  check_for_thread_db ();
+}
+
 /* Attach to a new thread.  This function is called when we receive a
    TD_CREATE event or when we iterate over all threads and find one
    that wasn't already in our list.  Returns true on success.  */
@@ -1845,4 +1866,9 @@ When non-zero, libthread-db debugging is
 
   /* Add ourselves to objfile event chain.  */
   observer_attach_new_objfile (thread_db_new_objfile);
+
+  /* Add ourselves to inferior_created event chain.
+     This is needed to handle debugging statically linked programs where
+     the new_objfile observer won't get called for libpthread.  */
+  observer_attach_inferior_created (thread_db_inferior_created);
 }
Index: objfiles.h
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.h,v
retrieving revision 1.85
diff -u -p -r1.85 objfiles.h
--- objfiles.h	14 Jun 2011 16:49:41 -0000	1.85
+++ objfiles.h	11 Oct 2011 02:44:46 -0000
@@ -196,7 +196,8 @@ struct objfile
 
     CORE_ADDR addr_low;
 
-    /* Some flag bits for this objfile.  */
+    /* Some flag bits for this objfile.
+       The values are defined by OBJF_*.  */
 
     unsigned short flags;
 
@@ -434,6 +435,11 @@ struct objfile
 
 #define OBJF_PSYMTABS_READ (1 << 4)
 
+/* Set if this is the main symbol file
+   (as opposed to symbol file for dynamically loaded code).  */
+
+#define OBJF_MAINLINE (1 << 5)
+
 /* The object file that contains the runtime common minimal symbols
    for SunOS4.  Note that this objfile has no associated BFD.  */
 
Index: symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.316
diff -u -p -r1.316 symfile.c
--- symfile.c	29 Sep 2011 02:04:25 -0000	1.316
+++ symfile.c	11 Oct 2011 02:44:46 -0000
@@ -1081,6 +1081,7 @@ symbol_file_add_with_addrs_or_offsets (b
   struct cleanup *my_cleanups;
   const char *name = bfd_get_filename (abfd);
   const int from_tty = add_flags & SYMFILE_VERBOSE;
+  const int mainline = add_flags & SYMFILE_MAINLINE;
   const int should_print = ((from_tty || info_verbose)
 			    && (readnow_symbol_files
 				|| (add_flags & SYMFILE_NO_READ) == 0));
@@ -1097,12 +1098,12 @@ symbol_file_add_with_addrs_or_offsets (b
      interactively wiping out any existing symbols.  */
 
   if ((have_full_symbols () || have_partial_symbols ())
-      && (add_flags & SYMFILE_MAINLINE)
+      && mainline
       && from_tty
       && !query (_("Load new symbol table from \"%s\"? "), name))
     error (_("Not confirmed."));
 
-  objfile = allocate_objfile (abfd, flags);
+  objfile = allocate_objfile (abfd, flags | (mainline ? OBJF_MAINLINE : 0));
   discard_cleanups (my_cleanups);
 
   if (parent)