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

[Doug Evans <dje@google.com>]

[-- 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)