[patchv2 1/2] 7.3 regression for corrupted core files threads

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Fri, 20 Apr 2012 17:21:29 +0200, Pedro Alves wrote:
> Anyway, I'd suggest adding a comma, and perhaps a little editing further even:
> 
>       /* Even if libthread_db initializes, if the thread list is
> 	 corrupted, we'd not manage to list any threads.  Better reject this
>          thread_db, and fall back to at least listing LWPs.  */

Used it.


> How about making that a bit more user friendly?  Something like:
> 
>       warning (_("couldn't activate thread debugging using libthread_db: %s"), thread_db_err_str (err));

Used it although in a different context now, it now produces:
	Warning: couldn't activate thread debugging using libthread_db: Cannot find new threads: generic error


> Unfortunately this conflicts with this bit at the end of the function:
> 
>   /* There appears to be a bug in glibc-2.3.6: calls to td_thr_get_info fail
>      with TD_ERR for statically linked executables if td_thr_get_info is
>      called before glibc has initialized itself.  Silently ignore such
>      errors, and let gdb enumerate threads again later.  */
>   thread_db_find_new_threads_silently (inferior_ptid);
> 
> Maybe it'd be good enough to condition your new check on !target_has_execution?

Thanks, I have reimplemented it the right way instead.

I have verified on RHEL-4 with glibc-2.3.4-2.43.x86_64 it really still behaves
correctly for gdb.threads/staticthreads.exp when I have forced the testfile to
link with static /usr/lib64/nptl/*.a from nptl-devel-2.3.4-2.43.x86_64.

No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu.


Thanks,
Jan


gdb/
2012-05-22  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* linux-thread-db.c (inferior_has_bug): New function.
	(thread_db_find_new_threads_silently): Return boolean as checked by
	inferior_has_bug, describe it in the comments.
	(try_thread_db_load_1): Move call to thread_db_find_new_threads_silently
	earlier.  Abort the initialization if it returned non-zero.
	(thread_db_find_new_threads_2): Preinitialize ERR.  Check errors also
	if UNTIL_NO_NEW,

gdb/testsuite/
2012-05-22  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.threads/gcore-thread.exp: Remove variable libthread_db_seen.
	Wrap the test into loop for corefile and core0file.

--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -576,6 +576,37 @@ enable_thread_event (int event, CORE_ADDR *bp)
   return TD_OK;
 }
 
+/* Verify inferior's '\0'-terminated symbol VER_SYMBOL starts with "%d.%d" and
+   return 1 if this version is lower (and not equal) to
+   VER_MAJOR_MIN.VER_MINOR_MIN.  Return 0 in all other cases.  */
+
+static int
+inferior_has_bug (const char *ver_symbol, int ver_major_min, int ver_minor_min)
+{
+  struct minimal_symbol *version_msym;
+  CORE_ADDR version_addr;
+  char *version;
+  int err, got, retval = 0;
+
+  version_msym = lookup_minimal_symbol (ver_symbol, NULL, NULL);
+  if (version_msym == NULL)
+    return 0;
+
+  version_addr = SYMBOL_VALUE_ADDRESS (version_msym);
+  got = target_read_string (version_addr, &version, 32, &err);
+  if (err == 0 && memchr (version, 0, got) == &version[got -1])
+    {
+      int major, minor;
+
+      retval = (sscanf (version, "%d.%d", &major, &minor) == 2
+		&& (major < ver_major_min
+		    || (major == ver_major_min && minor < ver_minor_min)));
+    }
+  xfree (version);
+
+  return retval;
+}
+
 static void
 enable_thread_event_reporting (void)
 {
@@ -643,9 +674,13 @@ enable_thread_event_reporting (void)
     }
 }
 
-/* Same as thread_db_find_new_threads_1, but silently ignore errors.  */
+/* Similar as thread_db_find_new_threads_1, but try to silently ignore errors
+   if appropriate.
 
-static void
+   Return 1 if the caller should abort libthread_db initialization.  Return 0
+   otherwise.  */
+
+static int
 thread_db_find_new_threads_silently (ptid_t ptid)
 {
   volatile struct gdb_exception except;
@@ -655,11 +690,36 @@ thread_db_find_new_threads_silently (ptid_t ptid)
       thread_db_find_new_threads_2 (ptid, 1);
     }
 
-  if (except.reason < 0 && libthread_db_debug)
+  if (except.reason < 0)
     {
-      exception_fprintf (gdb_stderr, except,
-			 "Warning: thread_db_find_new_threads_silently: ");
+      if (libthread_db_debug)
+	exception_fprintf (gdb_stderr, except,
+			   "Warning: thread_db_find_new_threads_silently: ");
+
+      /* There is a bug fixed between nptl 2.6.1 and 2.7 by
+	   commit 7d9d8bd18906fdd17364f372b160d7ab896ce909
+	 where calls to td_thr_get_info fail with TD_ERR for statically linked
+	 executables if td_thr_get_info is called before glibc has initialized
+	 itself.
+	 
+	 If the nptl bug is NOT present in the inferior and still thread_db
+	 reports an error return 1.  It means the inferior has corrupted thread
+	 list and GDB should fall back only to LWPs.
+
+	 If the nptl bug is present in the inferior return 0 to silently ignore
+	 such errors, and let gdb enumerate threads again later.  In such case
+	 GDB cannot properly display LWPs if the inferior thread list is
+	 corrupted.  */
+
+      if (!inferior_has_bug ("nptl_version", 2, 7))
+	{
+	  exception_fprintf (gdb_stderr, except,
+			     _("Warning: couldn't activate thread debugging "
+			       "using libthread_db: "));
+	  return 1;
+	}
     }
+  return 0;
 }
 
 /* Lookup a library in which given symbol resides.
@@ -762,6 +822,14 @@ try_thread_db_load_1 (struct thread_db_info *info)
   info->td_thr_event_enable_p = dlsym (info->handle, "td_thr_event_enable");
   info->td_thr_tls_get_addr_p = dlsym (info->handle, "td_thr_tls_get_addr");
 
+  if (thread_db_find_new_threads_silently (inferior_ptid) != 0)
+    {
+      /* Even if libthread_db initializes, if the thread list is
+         corrupted, we'd not manage to list any threads.  Better reject this
+         thread_db, and fall back to at least listing LWPs.  */
+      return 0;
+    }
+
   printf_unfiltered (_("[Thread debugging using libthread_db enabled]\n"));
 
   if (libthread_db_debug || *libthread_db_search_path)
@@ -785,12 +853,6 @@ try_thread_db_load_1 (struct thread_db_info *info)
   if (target_has_execution)
     enable_thread_event_reporting ();
 
-  /* There appears to be a bug in glibc-2.3.6: calls to td_thr_get_info fail
-     with TD_ERR for statically linked executables if td_thr_get_info is
-     called before glibc has initialized itself.  Silently ignore such
-     errors, and let gdb enumerate threads again later.  */
-  thread_db_find_new_threads_silently (inferior_ptid);
-
   return 1;
 }
 
@@ -1132,6 +1194,12 @@ thread_db_new_objfile (struct objfile *objfile)
      correctly.  */
 
   if (objfile != NULL
+      /* libpthread with separate debug info has its debug info file already
+	 loaded (and notified without successfult thread_db initialization))
+	 the time observer_notify_new_objfile is called for the library itself.
+	 Static executables have their separate debug info loaded already
+	 before the inferior has started.  */
+      && objfile->separate_debug_objfile_backlink == 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
@@ -1612,7 +1680,7 @@ find_new_threads_once (struct thread_db_info *info, int iteration,
 static void
 thread_db_find_new_threads_2 (ptid_t ptid, int until_no_new)
 {
-  td_err_e err;
+  td_err_e err = TD_OK;
   struct thread_db_info *info;
   int pid = ptid_get_pid (ptid);
   int i, loop;
@@ -1628,17 +1696,18 @@ thread_db_find_new_threads_2 (ptid_t ptid, int until_no_new)
 	 The 4 is a heuristic: there is an inherent race here, and I have
 	 seen that 2 iterations in a row are not always sufficient to
 	 "capture" all threads.  */
-      for (i = 0, loop = 0; loop < 4; ++i, ++loop)
-	if (find_new_threads_once (info, i, NULL) != 0)
-	  /* Found some new threads.  Restart the loop from beginning.	*/
-	  loop = -1;
+      for (i = 0, loop = 0; loop < 4 && err == TD_OK; ++i, ++loop)
+	if (find_new_threads_once (info, i, &err) != 0)
+	  {
+	    /* Found some new threads.  Restart the loop from beginning.  */
+	    loop = -1;
+	  }
     }
   else
-    {
-      find_new_threads_once (info, 0, &err);
-      if (err != TD_OK)
-	error (_("Cannot find new threads: %s"), thread_db_err_str (err));
-    }
+    find_new_threads_once (info, 0, &err);
+
+  if (err != TD_OK)
+    error (_("Cannot find new threads: %s"), thread_db_err_str (err));
 }
 
 static void
--- a/gdb/testsuite/gdb.threads/gcore-thread.exp
+++ b/gdb/testsuite/gdb.threads/gcore-thread.exp
@@ -143,11 +143,9 @@ proc load_core { corefile } {
     global gdb_prompt
     global libthread_db_seen
 
-    set libthread_db_seen 0
     gdb_test_multiple "core $corefile" \
 	"re-load generated corefile" {
 	    -re "\\\[Thread debugging using \[^ \r\n\]* enabled\\\]\r\n" {
-		set libthread_db_seen 1
 		exp_continue
 	    }
 	    -re " is not a core dump:.*\r\n$gdb_prompt $" {
@@ -170,39 +168,27 @@ proc load_core { corefile } {
     return 1
 }
 
-if ![load_core $corefile] {
-    return
-}
-
-# FIXME: now what can we test about the thread state?
-# We do not know for certain that there should be at least 
-# three threads, because who knows what kind of many-to-one
-# mapping various OS's may do?  Let's assume that there must
-# be at least two threads:
-
-gdb_test "info threads" ".*${nl}  2 ${horiz}${nl}\\* 1 .*" \
-	"corefile contains at least two threads"
-
-# One thread in the corefile should be in the "thread2" function.
-
-gdb_test "info threads" ".* thread2 .*" \
-	"a corefile thread is executing thread2"
+foreach name { corefile core0file } { with_test_prefix $name {
+    if ![load_core [subst $$name]] {
+	continue
+    }
 
-# The thread2 thread should be marked as the current thread.
+    # FIXME: now what can we test about the thread state?
+    # We do not know for certain that there should be at least 
+    # three threads, because who knows what kind of many-to-one
+    # mapping various OS's may do?  Let's assume that there must
+    # be at least two threads:
 
-gdb_test "info threads" ".*${nl}\\* ${horiz} thread2 .*" \
-	"thread2 is current thread in corefile"
+    gdb_test "info threads" ".*${nl}  2 ${horiz}${nl}\\* 1 .*" \
+	    "corefile contains at least two threads"
 
+    # One thread in the corefile should be in the "thread2" function.
 
-# Test the uninitialized thread list.
+    gdb_test "info threads" ".* thread2 .*" \
+	    "a corefile thread is executing thread2"
 
-if {"$core0file" != "" && [load_core $core0file]} {
-    set test "zeroed-threads cannot be listed"
+    # The thread2 thread should be marked as the current thread.
 
-    if {!$libthread_db_seen} {
-	verbose -log "No libthread_db loaded - -Wl,-z,relro compilation?"
-	xfail $test
-    } else {
-	gdb_test "info threads" "Cannot find new threads: .*" $test
-    }
-}
+    gdb_test "info threads" ".*${nl}\\* ${horiz} thread2 .*" \
+	    "thread2 is current thread in corefile"
+}}