Re: NPTL libthread_db changes for static debugging

[Roland McGrath <roland at redhat dot com>]

> GDB (nptl_db) has feeling the threading is turned on as `nptl_version' symbol
> already exists (as if `libpthread.so.0' would get loaded) due to `-static'.
> That time still the %fs/%gs (or ia64 %r13) register has value 0.
> Therefore nptl_db tries to read `struct pthread' from address 0 and fails.

Please describe this in terms of the libthread_db interface contract.
I take it the current situation is that before initialization in a
static executable, td_ta_map_lwp2thr returns TD_ERR?  This also makes
td_ta_thr_iter return TD_ERR.  Is that right?

If the gdb changes Daniel described have gone in, then returning
TD_NOTHR would make gdb sufficiently happy.  That is, it sees "zero
threads" but knows that libthread_db exists and that it can enable
event reporting.

Daniel's proposed libc patch generates a fake thread-create event after
initialization.  I don't like that.  AFAICT, it's not really necessary
at all.  gdb can try thread_db operations any time it wants, to get
th_unique if it's available at a later stop.  If nothing else, it will
get the thread-create event for the first pthread_create call (second
thread).  All that's really required is that before initialization is
complete, td_* calls return a coherent error code indicating harmless
uninitializedness (TD_NOTHR) rather than "generic error".  Is that right?

> Wouldn't it be most clean to set the TLS (all?) register(s) by the Linux
> kernel on exec(2) to zero even on the non-x86 platforms?

Indeed, I think it's crazy that they don't clear all registers.  (On
ia64, it clears the "scratch" registers for a setuid program exec'ing,
but not otherwise.)  It means a sufficiently buggy program can have its
failure modes depend not only on known variables like the size of the
arguments and environment on the stack, but on what values happened to
be in what registers at the time of the execve syscall that started the
buggy program.  Those will surely be joys to debug for the poor suckers
with such a bug in their application.  ("It only fails when started
from csh, I guess it must be a csh bug!")

That is a separate issue for the kernel on each platform, which I'd
call a bug worth reporting and fixing there.  But that's outside our
scope here.

Nonetheless, gdb and glibc should work as well as possible with older
kernels regardless, unless it's truly infeasible.

> > Another potential problem is that there's a window between the
> > initialization of TLS and the initialization of the first struct
> > pthread; at that point libthread_db will try to read things like the
> > LWP ID from the struct pthread even though they aren't initialized
> > until later in __pthread_initialize_minimal.  I can probably get
> > that to be a problem even in dynamic executables.  Not sure.

The patch below should also address this problem (I have not tested
it).  It uses a different method to decide that libpthread is not
initialized yet, and returns TD_NOTHR without looking at the possibly
bogus (or zero) register values.

I think this also changes the behavior for dynamically linked programs.
Now you will get TD_NOTHR even after TLS setup, as long as libpthread
is not initialized yet.  In case of a program that only loads
libpthread via dlopen, that could be a long time into the session.
IMHO this is actually an improvement, as previously you would get a
th_unique value for the initial thread even though pthread_self () in
the thread itself would have returned 0.  Is this going to confuse gdb?



Thanks,
Roland


2007-05-13  Roland McGrath  <roland@redhat.com>

	* td_ta_thr_iter.c (iterate_thread_list): Rename FAKE_EMPTY argument
	to FIRST, make it bool.  If NEXT is zero, return TD_NOTHR if FIRST or
	TD_OK otherwise.
	(td_ta_thr_iter): Update caller.

	* td_ta_map_lwp2thr.c (td_ta_map_lwp2thr): Check for __stack_user
	still being uninitialized, and return TD_NOTHR for that case.

Index: nptl_db/td_ta_map_lwp2thr.c
===================================================================
RCS file: /cvs/glibc/libc/nptl_db/td_ta_map_lwp2thr.c,v
retrieving revision 1.5
diff -b -B -p -u -r1.5 td_ta_map_lwp2thr.c
--- nptl_db/td_ta_map_lwp2thr.c	9 Sep 2004 22:50:10 -0000	1.5
+++ nptl_db/td_ta_map_lwp2thr.c	13 May 2007 22:39:54 -0000
@@ -1,5 +1,5 @@
 /* Which thread is running on an LWP?
-   Copyright (C) 2003, 2004 Free Software Foundation, Inc.
+   Copyright (C) 2003, 2004, 2007 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -116,11 +116,23 @@ td_ta_map_lwp2thr (const td_thragent_t *
 	}
     }
 
+  /* We cannot rely on thread registers and such information at all
+     before __pthread_initialize_minimal has gotten far enough.  They
+     sometimes contain garbage that would confuse us, left by the kernel
+     at exec.  So if it looks like initialization is incomplete, bail
+     early with "I see no threads here".  */
+  psaddr_t list;
+  terr = DB_GET_SYMBOL (list, ta, __stack_user);
+  if (terr != TD_OK)
+    return terr;
+  terr = DB_GET_FIELD (list, ta, list, list_t, next, 0);
+  if (terr != TD_OK)
+    return terr;
+  if (list == 0)
+    return TD_NOTHR;
+
   switch (ta->ta_howto)
     {
-    case ta_howto_unknown:
-      return TD_DBERR;
-
     default:
       return TD_DBERR;
 
Index: nptl_db/td_ta_thr_iter.c
===================================================================
RCS file: /cvs/glibc/libc/nptl_db/td_ta_thr_iter.c,v
retrieving revision 1.8
diff -b -B -p -u -r1.8 td_ta_thr_iter.c
--- nptl_db/td_ta_thr_iter.c	4 Apr 2004 00:31:10 -0000	1.8
+++ nptl_db/td_ta_thr_iter.c	13 May 2007 22:39:54 -0000
@@ -1,5 +1,6 @@
 /* Iterate over a process's threads.
-   Copyright (C) 1999,2000,2001,2002,2003,2004 Free Software Foundation, Inc.
+   Copyright (C) 1999,2000,2001,2002,2003,2004,2007
+	Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@redhat.com>, 1999.
 
@@ -24,7 +25,7 @@
 static td_err_e
 iterate_thread_list (td_thragent_t *ta, td_thr_iter_f *callback,
 		     void *cbdata_p, td_thr_state_e state, int ti_pri,
-		     psaddr_t head, int fake_empty)
+		     psaddr_t head, bool first)
 {
   td_err_e err;
   psaddr_t next, ofs;
@@ -39,16 +40,9 @@ iterate_thread_list (td_thragent_t *ta, 
   if (err != TD_OK)
     return err;
 
-  if (next == 0 && fake_empty)
-    {
-      /* __pthread_initialize_minimal has not run.
-	 There is just the main thread to return.  */
-      td_thrhandle_t th;
-      err = td_ta_map_lwp2thr (ta, ps_getpid (ta->ph), &th);
-      if (err == TD_OK)
-	err = callback (&th, cbdata_p) != 0 ? TD_DBERR : TD_OK;
-      return err;
-    }
+  if (next == 0)
+    /* __pthread_initialize_minimal has not run.  */
+    return first ? TD_NOTHR : TD_OK;
 
   /* Cache the offset from struct pthread to its list_t member.  */
   err = DB_GET_FIELD_ADDRESS (ofs, ta, 0, pthread, list, 0);