[PATCH 0/2] GNU/Linux: Stop using libthread_db/td_ta_thr_iter

[PATCH 0/2] GNU/Linux: Stop using libthread_db/td_ta_thr_iter

[Pedro Alves]

This one made me scratch my head for a while.  My
all-stop-on-top-of-non-stop series was causing odd GDB hangs.  Turned
out to be that we can't use libthread_db/td_ta_thr_iter when threads
are running.  This series stops GDB from using it altogether, when
debugging live programs.

The second patch is the real deal.  The first patch fixes latent bugs
that the second patch's test exposed, though after the second patch,
the buggy function won't be used on modern system anymore...

Pedro Alves (2):
  linux-nat.c: fix a few lin_lwp_attach_lwp issues
  GNU/Linux: Stop using libthread_db/td_ta_thr_iter

 gdb/gdbserver/thread-db.c                          |  18 +-
 gdb/linux-nat.c                                    | 125 +++++++----
 gdb/linux-nat.h                                    |  16 +-
 gdb/linux-thread-db.c                              | 237 ++++++++++-----------
 gdb/nat/linux-procfs.c                             |  13 ++
 gdb/nat/linux-procfs.h                             |   3 +
 .../gdb.threads/multi-create-ns-info-thr.exp       |  50 +++++
 7 files changed, 287 insertions(+), 175 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/multi-create-ns-info-thr.exp

-- 
1.9.3

[PATCH 1/2] linux-nat.c: fix a few lin_lwp_attach_lwp issues

[Pedro Alves]

This function has a few latent bugs that are triggered by a non-stop
mode test that will be added in a subsequent patch.

First, as described in the function's intro comment, the function is
supposed to return 1 if we're already auto attached to the thread, but
haven't processed the PTRACE_EVENT_CLONE event of its parent thread
yet.

Then, we may find that we're trying to attach to a clone child that
hasn't yet stopped for its initial stop, and therefore 'waitpid(...,
WNOHANG)' returns 0.  In that case, we're currently adding the LWP to
the stopped_pids list, which results in linux_handle_extended_wait
skipping the waitpid call on the child, and thus confusing things
later on when the child eventually reports the stop.

Then, the tail end of lin_lwp_attach_lwp always sets the
last_resume_kind of the LWP to resume_stop, which is wrong given that
the user may be doing "info threads" while some threads are running.

And then, the else branch of lin_lwp_attach_lwp always sets the
stopped flag of the LWP.  This branch is reached if the LWP is the
main LWP, which may well be running at this point (to it's wrong to
set its 'stopped' flag).

AFAICS, there's no reason anymore for special-casing the main/leader
LWP here:

- For the "attach" case, linux_nat_attach already adds the main LWP to
the lwp list, and sets its 'stopped' flag.

- For the "run" case, after linux_nat_create_inferior, end up in
linux_nat_wait_1 here:

  /* The first time we get here after starting a new inferior, we may
     not have added it to the LWP list yet - this is the earliest
     moment at which we know its PID.  */
  if (ptid_is_pid (inferior_ptid))
    {
      /* Upgrade the main thread's ptid.  */
      thread_change_ptid (inferior_ptid,
			  ptid_build (ptid_get_pid (inferior_ptid),
				      ptid_get_pid (inferior_ptid), 0));

      lp = add_initial_lwp (inferior_ptid);
      lp->resumed = 1;
    }

... which adds the LWP to the LWP list already, before
lin_lwp_attach_lwp can ever be reached.

gdb/ChangeLog:
2015-02-08  Pedro Alves  <palves@redhat.com>

	* linux-nat.c (lin_lwp_attach_lwp): No longer special case the
	main LWP.  Handle the case of waitpid returning 0 if we're already
	attached to the LWP.  Don't set the LWP's last_resume_kind to
	resume_stop if we already knew about the LWP.
	(linux_nat_filter_event): Add debug logs.
---
 gdb/linux-nat.c | 60 +++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 39 insertions(+), 21 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 169188a..ba62adc 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -994,13 +994,12 @@ lin_lwp_attach_lwp (ptid_t ptid)
   lp = find_lwp_pid (ptid);
   lwpid = ptid_get_lwp (ptid);
 
-  /* We assume that we're already attached to any LWP that has an id
-     equal to the overall process id, and to any LWP that is already
+  /* We assume that we're already attached to any LWP that is already
      in our list of LWPs.  If we're not seeing exit events from threads
      and we've had PID wraparound since we last tried to stop all threads,
      this assumption might be wrong; fortunately, this is very unlikely
      to happen.  */
-  if (lwpid != ptid_get_pid (ptid) && lp == NULL)
+  if (lp == NULL)
     {
       int status, cloned = 0, signalled = 0;
 
@@ -1018,23 +1017,50 @@ lin_lwp_attach_lwp (ptid_t ptid)
 		  /* We've already seen this thread stop, but we
 		     haven't seen the PTRACE_EVENT_CLONE extended
 		     event yet.  */
-		  return 0;
+		  if (debug_linux_nat)
+		    fprintf_unfiltered (gdb_stdlog,
+					"LLAL: attach failed, but already seen "
+					"this thread %s stop\n",
+					target_pid_to_str (ptid));
+		  return 1;
 		}
 	      else
 		{
 		  int new_pid;
 		  int status;
 
-		  /* See if we've got a stop for this new child
-		     pending.  If so, we're already attached.  */
+		  if (debug_linux_nat)
+		    fprintf_unfiltered (gdb_stdlog,
+					"LLAL: attach failed, and haven't seen "
+					"this thread %s stop yet\n",
+					target_pid_to_str (ptid));
+
+		  /* We may or may not be attached to the LWP already.
+		     Try waitpid on it.  If that errors, we're not
+		     attached to the LWP yet.  Otherwise, we're
+		     already attached.  */
 		  gdb_assert (lwpid > 0);
 		  new_pid = my_waitpid (lwpid, &status, WNOHANG);
 		  if (new_pid == -1 && errno == ECHILD)
 		    new_pid = my_waitpid (lwpid, &status, __WCLONE | WNOHANG);
 		  if (new_pid != -1)
 		    {
-		      if (WIFSTOPPED (status))
-			add_to_pid_list (&stopped_pids, lwpid, status);
+		      if (new_pid == 0)
+			{
+			  /* The child hasn't stopped for its initial
+			     SIGSTOP stop yet.  */
+			  if (debug_linux_nat)
+			    fprintf_unfiltered (gdb_stdlog,
+						"LLAL: child hasn't "
+						"stopped yet\n");
+			}
+		      else if (WIFSTOPPED (status))
+			{
+			  if (debug_linux_nat)
+			    fprintf_unfiltered (gdb_stdlog,
+						"LLAL: adding to stopped_pids\n");
+			  add_to_pid_list (&stopped_pids, lwpid, status);
+			}
 		      return 1;
 		    }
 		}
@@ -1061,6 +1087,7 @@ lin_lwp_attach_lwp (ptid_t ptid)
 
       lp = add_lwp (ptid);
       lp->stopped = 1;
+      lp->last_resume_kind = resume_stop;
       lp->cloned = cloned;
       lp->signalled = signalled;
       if (WSTOPSIG (status) != SIGSTOP)
@@ -1079,20 +1106,7 @@ lin_lwp_attach_lwp (ptid_t ptid)
 			      status_to_str (status));
 	}
     }
-  else
-    {
-      /* We assume that the LWP representing the original process is
-         already stopped.  Mark it as stopped in the data structure
-         that the GNU/linux ptrace layer uses to keep track of
-         threads.  Note that this won't have already been done since
-         the main thread will have, we assume, been stopped by an
-         attach from a different layer.  */
-      if (lp == NULL)
-	lp = add_lwp (ptid);
-      lp->stopped = 1;
-    }
 
-  lp->last_resume_kind = resume_stop;
   return 0;
 }
 
@@ -2855,6 +2869,10 @@ linux_nat_filter_event (int lwpid, int status)
 
   if (WIFSTOPPED (status) && !lp)
     {
+      if (debug_linux_nat)
+	fprintf_unfiltered (gdb_stdlog,
+			    "LHEW: saving LWP %ld status %s in stopped_pids list\n",
+			    (long) lwpid, status_to_str (status));
       add_to_pid_list (&stopped_pids, lwpid, status);
       return NULL;
     }
-- 
1.9.3

[PATCH 2/2] GNU/Linux: Stop using libthread_db/td_ta_thr_iter

[Pedro Alves]

TL;DR - GDB can hang if something refreshes the thread list out of the
target while the target is running.  GDB hangs inside td_ta_thr_iter.
The fix is to not use that libthread_db function anymore.

Long version:

Running the testsuite against my all-stop-on-top-of-non-stop series is
still exposing latent non-stop bugs.

I was originally seeing this with the multi-create.exp test, back when
we were still using libthread_db thread event breakpoints.  The
all-stop-on-top-of-non-stop series forces a thread list refresh each
time GDB needs to start stepping over a breakpoint (to pause all
threads).  That test hits the thread event breakpoint often, resulting
in a bunch of step-over operations, thus a bunch of thread list
refreshes while some threads in the target are running.

The commit adds a real non-stop mode test that triggers the issue,
based on multi-create.exp, that does an explicit "info threads" when a
breakpoint is hit.  IOW, it does the same things the as-ns series was
doing when testing multi-create.exp.

The bug is a race, so it unfortunately takes several runs for the test
to trigger it.  In fact, even when setting the test running in a loop,
it sometimes takes several minutes for it to trigger for me.

The race is related to libthread_db's td_ta_thr_iter.  This is
libthread_db's entry point for walking the thread list of the
inferior.

Sometimes, when GDB refreshes the thread list from the target,
libthread_db's td_ta_thr_iter can somehow see glibc's thread list as a
cycle, and get stuck in an infinite loop.

The issue is that when a thread exits, its thread control structure in
glibc is moved from a "used" list to a "cache" list.  These lists are
simply circular linked lists where the "next/prev" pointers are
embedded in the thread control structure itself.  The "next" pointer
of the last element of the list points back to the list's sentinel
"head".  There's only one set of "next/prev" pointers for both lists;
thus a thread can only be in one of the lists at a time, not in both
simultaneously.

So when thread C exits, simplifying, the following happens.  A-C are
threads.  stack_used and stack_cache are the list's heads.

Before:

  stack_used -> A -> B -> C -> (&stack_used)
  stack_cache -> (&stack_cache)

After:

  stack_used -> A -> B -> (&stack_used)
  stack_cache -> C -> (&stack_cache)

td_ta_thr_iter starts by iterating at the list's head's next, and
iterates until it sees a thread whose next pointer points to the
list's head again.  Thus in the before case above, C's next points to
stack_used, indicating end of list.  In the same case, the stack_cache
list is empty.

For each thread being iterated, td_ta_thr_iter reads the whole thread
object out of the inferior.  This includes the thread's "next"
pointer.

In the scenario above, it may happen that td_ta_thr_iter is iterating
thread B and has already read B's thread structure just before thread
C exits and its control structure moves to the cached list.

Now, recall that td_ta_thr_iter is running in the context of GDB, and
there's no locking between GDB and the inferior.  From it's local copy
of B, td_ta_thr_iter believes that the next thread after B is thread
C, so it happilly continues iterating to C, a thread that has already
exited, and is now in the stack cache list.

After iterating C, td_ta_thr_iter finds the stack_cache head, which
because it is not stack_used, td_ta_thr_iter assumes it's just another
thread.  After this, unless the reverse race triggers, GDB gets stuck
in td_ta_thr_iter forever walking the stack_cache list, as no thread
in thatlist has a next pointer that points back to stack_used (the
terminating condition).

Before fully understanding the issue, I tried adding cycle detection
to GDB's td_ta_thr_iter callback.  However, td_ta_thr_iter skips
calling the callback in some cases, which means that it's possible
that the callback isn't called at all, making it impossible for GDB to
break the loop.  I did manage to get GDB stuck in that state more than
once.

Fortunately, we can avoid the issue altogether.  We don't really need
td_ta_thr_iter for live debugging nowadays, given PTRACE_EVENT_CLONE.
We already know how to map and lwp id to a thread id without iterating
(thread_from_lwp), so use that more.

gdb/ChangeLog:
2015-02-08  Pedro Alves  <palves@redhat.com>

	* linux-nat.c (linux_handle_extended_wait): Call
	thread_db_notice_clone whenever a new clone LWP is detected.
	(linux_stop_and_wait_all_lwps, linux_unstop_all_lwps): New
	functions.
	* linux-nat.h (thread_db_attach_lwp): Delete declaration.
	(thread_db_notice_clone, linux_stop_and_wait_all_lwps)
	(linux_unstop_all_lwps): Declare.
	* linux-thread-db.c (struct thread_get_info_inout): Delete.
	(thread_get_info_callback): Delete.
	(thread_from_lwp): Use td_thr_get_info and record_thread.
	(thread_db_attach_lwp): Delete.
	(thread_db_notice_clone): New function.
	(try_thread_db_load_1): If /proc is mounted and shows the
	process'es task list, walk over all LWPs and call thread_from_lwp
	instead of relying on td_ta_thr_iter.
	(attach_thread): Don't call check_thread_signals here.  Split the
	tail part of the function (which adds the thread to the core GDB
	thread list) to ...
	(record_thread): ... this function.  Call check_thread_signals
	here.
	(thread_db_wait): Don't call thread_db_find_new_threads_1.  Always
	call thread_from_lwp.
	(thread_db_update_thread_list): Rename to ...
	(thread_db_update_thread_list_org): ... this.
	(thread_db_update_thread_list): New function.
	(thread_db_find_thread_from_tid): Delete.
	(thread_db_get_ada_task_ptid): Simplify.
	* nat/linux-procfs.c: Include <sys/stat.h>.
	(linux_proc_task_list_dir_exists): New function.
	* nat/linux-procfs.h (linux_proc_task_list_dir_exists): Declare.

gdb/gdbserver/ChangeLog:
2015-02-08  Pedro Alves  <palves@redhat.com>

	* thread-db.c: Include "nat/linux-procfs.h".
	(thread_db_init): Skip listing new threads if the kernel supports
	PTRACE_EVENT_CLONE and /proc/PID/task/ is accessible.

gdb/testsuite/ChangeLog:
2015-02-08  Pedro Alves  <palves@redhat.com>

	* gdb.threads/multi-create-ns-info-thr.exp: New file.
---
 gdb/gdbserver/thread-db.c                          |  18 +-
 gdb/linux-nat.c                                    |  65 +++---
 gdb/linux-nat.h                                    |  16 +-
 gdb/linux-thread-db.c                              | 237 ++++++++++-----------
 gdb/nat/linux-procfs.c                             |  13 ++
 gdb/nat/linux-procfs.h                             |   3 +
 .../gdb.threads/multi-create-ns-info-thr.exp       |  50 +++++
 7 files changed, 248 insertions(+), 154 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/multi-create-ns-info-thr.exp

diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c
index 2185245..a8b6f42 100644
--- a/gdb/gdbserver/thread-db.c
+++ b/gdb/gdbserver/thread-db.c
@@ -29,6 +29,7 @@ static int thread_db_use_events;
 #include "gdb_proc_service.h"
 #include "nat/gdb_thread_db.h"
 #include "gdb_vecs.h"
+#include "nat/linux-procfs.h"
 
 #ifndef USE_LIBTHREAD_DB_DIRECTLY
 #include <dlfcn.h>
@@ -859,7 +860,22 @@ thread_db_init (int use_events)
 	  thread_db_mourn (proc);
 	  return 0;
 	}
-      thread_db_find_new_threads ();
+
+      /* It's best to avoid td_ta_thr_iter if possible.  That walks
+	 data structures in the inferior's address space that may be
+	 corrupted, or, if the target is running, the list may change
+	 while we walk it.  In the latter case, it's possible that a
+	 thread exits just at the exact time that causes GDBserver to
+	 get stuck in an infinite loop.  If the kernel supports clone
+	 events, and /proc/PID/task/ exits, then we already know about
+	 all threads in the process.  When we need info out of
+	 thread_db on a given thread (e.g., for TLS), we'll use
+	 find_one_thread then.  That uses thread_db entry points that
+	 do not walk libpthread's thread list, so should be safe, as
+	 well as more efficient.  */
+      if (use_events
+	  || !linux_proc_task_list_dir_exists (pid_of (proc)))
+	thread_db_find_new_threads ();
       thread_db_look_up_symbols ();
       return 1;
     }
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index ba62adc..9c9daa8 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -264,6 +264,7 @@ async_file_mark (void)
 static int kill_lwp (int lwpid, int signo);
 
 static int stop_callback (struct lwp_info *lp, void *data);
+static int resume_stopped_resumed_lwps (struct lwp_info *lp, void *data);
 
 static void block_child_signals (sigset_t *prev_mask);
 static void restore_child_signals_mask (sigset_t *prev_mask);
@@ -2003,34 +2004,24 @@ linux_handle_extended_wait (struct lwp_info *lp, int status,
 		status = 0;
 	    }
 
-	  if (non_stop)
+	  /* If the thread_db layer is active, let it record the user
+	     level thread id and status, and add the thread to GDB's
+	     list.  */
+	  if (!thread_db_notice_clone (lp->ptid, new_lp->ptid))
 	    {
-	      /* Add the new thread to GDB's lists as soon as possible
-		 so that:
-
-		 1) the frontend doesn't have to wait for a stop to
-		 display them, and,
-
-		 2) we tag it with the correct running state.  */
-
-	      /* If the thread_db layer is active, let it know about
-		 this new thread, and add it to GDB's list.  */
-	      if (!thread_db_attach_lwp (new_lp->ptid))
-		{
-		  /* We're not using thread_db.  Add it to GDB's
-		     list.  */
-		  target_post_attach (ptid_get_lwp (new_lp->ptid));
-		  add_thread (new_lp->ptid);
-		}
+	      /* The process is not using thread_db.  Add the LWP to
+		 GDB's list.  */
+	      target_post_attach (ptid_get_lwp (new_lp->ptid));
+	      add_thread (new_lp->ptid);
+	    }
 
-	      if (!stopping)
-		{
-		  set_running (new_lp->ptid, 1);
-		  set_executing (new_lp->ptid, 1);
-		  /* thread_db_attach_lwp -> lin_lwp_attach_lwp forced
-		     resume_stop.  */
-		  new_lp->last_resume_kind = resume_continue;
-		}
+	  if (!stopping)
+	    {
+	      set_running (new_lp->ptid, 1);
+	      set_executing (new_lp->ptid, 1);
+	      /* thread_db_attach_lwp -> lin_lwp_attach_lwp forced
+		 resume_stop.  */
+	      new_lp->last_resume_kind = resume_continue;
 	    }
 
 	  if (status != 0)
@@ -2285,6 +2276,28 @@ linux_stop_lwp (struct lwp_info *lwp)
   stop_callback (lwp, NULL);
 }
 
+/* See linux-nat.h  */
+
+void
+linux_stop_and_wait_all_lwps (void)
+{
+  /* Stop all LWP's ...  */
+  iterate_over_lwps (minus_one_ptid, stop_callback, NULL);
+
+  /* ... and wait until all of them have reported back that
+     they're no longer running.  */
+  iterate_over_lwps (minus_one_ptid, stop_wait_callback, NULL);
+}
+
+/* See linux-nat.h  */
+
+void
+linux_unstop_all_lwps (void)
+{
+  iterate_over_lwps (minus_one_ptid,
+		     resume_stopped_resumed_lwps, &minus_one_ptid);
+}
+
 /* Return non-zero if LWP PID has a pending SIGINT.  */
 
 static int
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 669450d..8c2ceb5 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -142,7 +142,12 @@ extern struct lwp_info *lwp_list;
 /* Attempt to initialize libthread_db.  */
 void check_for_thread_db (void);
 
-int thread_db_attach_lwp (ptid_t ptid);
+/* Called from the LWP layer to inform the thread_db layer that PARENT
+   spawned CHILD.  Both LWPs are currently stopped.  This function
+   does whatever is required to have the child LWP under the
+   thread_db's control --- e.g., enabling event reporting.  Returns
+   true on success, false if the process isn't using libpthread.  */
+extern int thread_db_notice_clone (ptid_t parent, ptid_t child);
 
 /* Return the set of signals used by the threads library.  */
 extern void lin_thread_get_thread_signals (sigset_t *mask);
@@ -155,6 +160,15 @@ extern int lin_lwp_attach_lwp (ptid_t ptid);
 
 extern void linux_stop_lwp (struct lwp_info *lwp);
 
+/* Stop all LWPs, synchronously.  (Any events that trigger while LWPs
+   are being stopped are left pending.)  */
+extern void linux_stop_and_wait_all_lwps (void);
+
+/* Set resumed LWPs running again, as they were before being stopped
+   with linux_stop_and_wait_all_lwps.  (LWPS with pending events are
+   left stopped.)  */
+extern void linux_unstop_all_lwps (void);
+
 /* Iterator function for lin-lwp's lwp list.  */
 struct lwp_info *iterate_over_lwps (ptid_t filter,
 				    int (*callback) (struct lwp_info *,
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 6b525a0..37fa2ae 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -217,6 +217,13 @@ struct thread_db_info *thread_db_list;
 static void thread_db_find_new_threads_1 (ptid_t ptid);
 static void thread_db_find_new_threads_2 (ptid_t ptid, int until_no_new);
 
+static void check_thread_signals (void);
+
+static void record_thread (struct thread_db_info *info,
+			   struct thread_info *tp,
+			   ptid_t ptid, const td_thrhandle_t *th_p,
+			   const td_thrinfo_t *ti_p);
+
 /* Add the current inferior to the list of processes using libpthread.
    Return a pointer to the newly allocated object that was added to
    THREAD_DB_LIST.  HANDLE is the handle returned by dlopen'ing
@@ -398,12 +405,6 @@ have_threads (ptid_t ptid)
   return iterate_over_threads (have_threads_callback, &pid) != NULL;
 }
 
-struct thread_get_info_inout
-{
-  struct thread_info *thread_info;
-  struct thread_db_info *thread_db_info;
-};
-
 /* A callback function for td_ta_thr_iter, which we use to map all
    threads to LWPs.
 
@@ -414,40 +415,6 @@ struct thread_get_info_inout
    If the thread is a zombie, TD_THR_ZOMBIE is returned.  Otherwise,
    zero is returned to indicate success.  */
 
-static int
-thread_get_info_callback (const td_thrhandle_t *thp, void *argp)
-{
-  td_thrinfo_t ti;
-  td_err_e err;
-  ptid_t thread_ptid;
-  struct thread_get_info_inout *inout;
-  struct thread_db_info *info;
-
-  inout = argp;
-  info = inout->thread_db_info;
-
-  err = info->td_thr_get_info_p (thp, &ti);
-  if (err != TD_OK)
-    error (_("thread_get_info_callback: cannot get thread info: %s"),
-	   thread_db_err_str (err));
-
-  /* Fill the cache.  */
-  thread_ptid = ptid_build (info->pid, ti.ti_lid, 0);
-  inout->thread_info = find_thread_ptid (thread_ptid);
-
-  if (inout->thread_info == NULL)
-    {
-      /* New thread.  Attach to it now (why wait?).  */
-      if (!have_threads (thread_ptid))
- 	thread_db_find_new_threads_1 (thread_ptid);
-      else
-	attach_thread (thread_ptid, thp, &ti);
-      inout->thread_info = find_thread_ptid (thread_ptid);
-      gdb_assert (inout->thread_info != NULL);
-    }
-
-  return 0;
-}
 \f
 /* Fetch the user-level thread id of PTID.  */
 
@@ -455,9 +422,10 @@ static void
 thread_from_lwp (ptid_t ptid)
 {
   td_thrhandle_t th;
+  td_thrinfo_t ti;
   td_err_e err;
   struct thread_db_info *info;
-  struct thread_get_info_inout io = {0};
+  struct thread_info *tp;
 
   /* Just in case td_ta_map_lwp2thr doesn't initialize it completely.  */
   th.th_unique = 0;
@@ -476,56 +444,37 @@ thread_from_lwp (ptid_t ptid)
     error (_("Cannot find user-level thread for LWP %ld: %s"),
 	   ptid_get_lwp (ptid), thread_db_err_str (err));
 
-  /* Long-winded way of fetching the thread info.  */
-  io.thread_db_info = info;
-  io.thread_info = NULL;
-  thread_get_info_callback (&th, &io);
+  err = info->td_thr_get_info_p (&th, &ti);
+  if (err != TD_OK)
+    error (_("thread_get_info_callback: cannot get thread info: %s"),
+	   thread_db_err_str (err));
+
+  /* Fill the cache.  */
+  tp = find_thread_ptid (ptid);
+  record_thread (info, tp, ptid, &th, &ti);
 }
 \f
 
-/* Attach to lwp PTID, doing whatever else is required to have this
-   LWP under the debugger's control --- e.g., enabling event
-   reporting.  Returns true on success.  */
+/* See linux-nat.h.  */
+
 int
-thread_db_attach_lwp (ptid_t ptid)
+thread_db_notice_clone (ptid_t parent, ptid_t child)
 {
   td_thrhandle_t th;
   td_thrinfo_t ti;
   td_err_e err;
   struct thread_db_info *info;
 
-  info = get_thread_db_info (ptid_get_pid (ptid));
+  info = get_thread_db_info (ptid_get_pid (child));
 
   if (info == NULL)
     return 0;
 
-  /* This ptid comes from linux-nat.c, which should always fill in the
-     LWP.  */
-  gdb_assert (ptid_get_lwp (ptid) != 0);
-
-  /* Access an lwp we know is stopped.  */
-  info->proc_handle.ptid = ptid;
-
-  /* If we have only looked at the first thread before libpthread was
-     initialized, we may not know its thread ID yet.  Make sure we do
-     before we add another thread to the list.  */
-  if (!have_threads (ptid))
-    thread_db_find_new_threads_1 (ptid);
-
-  err = info->td_ta_map_lwp2thr_p (info->thread_agent, ptid_get_lwp (ptid),
-				   &th);
-  if (err != TD_OK)
-    /* Cannot find user-level thread.  */
-    return 0;
-
-  err = info->td_thr_get_info_p (&th, &ti);
-  if (err != TD_OK)
-    {
-      warning (_("Cannot get thread info: %s"), thread_db_err_str (err));
-      return 0;
-    }
+  thread_from_lwp (child);
 
-  attach_thread (ptid, &th, &ti);
+  /* If we do not know about the main thread yet, this would be a good
+     time to find it.  */
+  thread_from_lwp (parent);
   return 1;
 }
 
@@ -813,7 +762,30 @@ try_thread_db_load_1 (struct thread_db_info *info)
   info->td_thr_tls_get_addr_p = dlsym (info->handle, "td_thr_tls_get_addr");
   info->td_thr_tlsbase_p = dlsym (info->handle, "td_thr_tlsbase");
 
-  if (thread_db_find_new_threads_silently (inferior_ptid) != 0)
+  /* It's best to avoid td_ta_thr_iter if possible.  That walks data
+     structures in the inferior's address space that may be corrupted,
+     or, if the target is running, may change while we walk them.  If
+     there's execution (and /proc is mounted), then we're already
+     attached to all LWPs.  Use thread_from_lwp, which uses
+     td_ta_map_lwp2thr instead, which does not walk the thread list.
+
+     td_ta_map_lwp2thr uses ps_get_thread_area, but we can't use that
+     currently on core targets, as it uses ptrace directly.  */
+  if (target_has_execution
+      && linux_proc_task_list_dir_exists (ptid_get_pid (inferior_ptid)))
+    {
+      struct lwp_info *lp;
+      int pid = ptid_get_pid (inferior_ptid);
+
+      linux_stop_and_wait_all_lwps ();
+
+      ALL_LWPS (lp)
+	if (ptid_get_pid (lp->ptid) == pid)
+	  thread_from_lwp (lp->ptid);
+
+      linux_unstop_all_lwps ();
+    }
+  else 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
@@ -1294,9 +1266,7 @@ static int
 attach_thread (ptid_t ptid, const td_thrhandle_t *th_p,
 	       const td_thrinfo_t *ti_p)
 {
-  struct private_thread_info *private;
   struct thread_info *tp;
-  td_err_e err;
   struct thread_db_info *info;
 
   /* If we're being called after a TD_CREATE event, we may already
@@ -1329,9 +1299,6 @@ attach_thread (ptid_t ptid, const td_thrhandle_t *th_p,
 	}
     }
 
-  if (target_has_execution)
-    check_thread_signals ();
-
   /* Under GNU/Linux, we have to attach to each and every thread.  */
   if (target_has_execution
       && tp == NULL)
@@ -1355,31 +1322,48 @@ attach_thread (ptid_t ptid, const td_thrhandle_t *th_p,
       /* Otherwise, we sucessfully attached to the thread.  */
     }
 
+  info = get_thread_db_info (ptid_get_pid (ptid));
+  record_thread (info, tp, ptid, th_p, ti_p);
+  return 1;
+}
+
+/* Record a new thread in GDB's thread list.  Creates the thread's
+   private info.  If TP is NULL, creates a new thread.  Otherwise,
+   uses TP.  */
+
+static void
+record_thread (struct thread_db_info *info,
+	       struct thread_info *tp,
+	       ptid_t ptid, const td_thrhandle_t *th_p,
+	       const td_thrinfo_t *ti_p)
+{
+  td_err_e err;
+  struct private_thread_info *private;
+  int new_thread = (tp == NULL);
+
+  /* A thread ID of zero may mean the thread library has not
+     initialized yet.  Leave private == NULL until the thread library
+     has initialized.  */
+  if (ti_p->ti_tid == 0)
+    return;
+
   /* Construct the thread's private data.  */
   private = xmalloc (sizeof (struct private_thread_info));
   memset (private, 0, sizeof (struct private_thread_info));
 
-  /* A thread ID of zero may mean the thread library has not initialized
-     yet.  But we shouldn't even get here if that's the case.  FIXME:
-     if we change GDB to always have at least one thread in the thread
-     list this will have to go somewhere else; maybe private == NULL
-     until the thread_db target claims it.  */
-  gdb_assert (ti_p->ti_tid != 0);
   private->th = *th_p;
   private->tid = ti_p->ti_tid;
   update_thread_state (private, ti_p);
 
   /* Add the thread to GDB's thread list.  */
   if (tp == NULL)
-    add_thread_with_info (ptid, private);
+    tp = add_thread_with_info (ptid, private);
   else
     tp->private = private;
 
-  info = get_thread_db_info (ptid_get_pid (ptid));
-
   /* Enable thread event reporting for this thread, except when
      debugging a core file.  */
-  if (target_has_execution && thread_db_use_events ())
+  if (target_has_execution && thread_db_use_events () && new_thread)
     {
       err = info->td_thr_event_enable_p (th_p, 1);
       if (err != TD_OK)
@@ -1387,7 +1371,8 @@ attach_thread (ptid_t ptid, const td_thrhandle_t *th_p,
 	       target_pid_to_str (ptid), thread_db_err_str (err));
     }
 
-  return 1;
+  if (target_has_execution)
+    check_thread_signals ();
 }
 
 static void
@@ -1573,21 +1558,13 @@ thread_db_wait (struct target_ops *ops,
       return ptid;
     }
 
-  /* If we do not know about the main thread yet, this would be a good time to
-     find it.  */
-  if (ourstatus->kind == TARGET_WAITKIND_STOPPED && !have_threads (ptid))
-    thread_db_find_new_threads_1 (ptid);
-
   if (ourstatus->kind == TARGET_WAITKIND_STOPPED
       && ourstatus->value.sig == GDB_SIGNAL_TRAP)
     /* Check for a thread event.  */
     check_event (ptid);
 
-  if (have_threads (ptid))
-    {
-      /* Fill in the thread's user-level thread id.  */
-      thread_from_lwp (ptid);
-    }
+  /* Fill in the thread's user-level thread id and status.  */
+  thread_from_lwp (ptid);
 
   return ptid;
 }
@@ -1719,6 +1696,9 @@ find_new_threads_once (struct thread_db_info *info, int iteration,
   data.info = info;
   data.new_threads = 0;
 
+  /* See comment in thread_db_update_thread_list.  */
+  gdb_assert (!target_has_execution || thread_db_use_events ());
+
   TRY_CATCH (except, RETURN_MASK_ERROR)
     {
       /* Iterate over all user-space threads to discover new threads.  */
@@ -1797,8 +1777,10 @@ update_thread_core (struct lwp_info *info, void *closure)
   return 0;
 }
 
+/* Update the thread list using td_ta_thr_iter.  */
+
 static void
-thread_db_update_thread_list (struct target_ops *ops)
+thread_db_update_thread_list_td_ta_thr_iter (struct target_ops *ops)
 {
   struct thread_db_info *info;
   struct inferior *inf;
@@ -1822,6 +1804,29 @@ thread_db_update_thread_list (struct target_ops *ops)
 
       thread_db_find_new_threads_1 (thread->ptid);
     }
+}
+
+/* Implement the to_update_thread_list target method for this
+   target.  */
+
+static void
+thread_db_update_thread_list (struct target_ops *ops)
+{
+  /* It's best to avoid td_ta_thr_iter if possible.  That walks data
+     structures in the inferior's address space that may be corrupted,
+     or, if the target is running, the list may change while we walk
+     it.  In the latter case, it's possible that a thread exits just
+     at the exact time that causes GDB to get stuck in an infinite
+     loop.  To avoid pausing all threads whenever the core wants to
+     refresh the thread list, if the kernel supports clone events
+     (meaning we're always already attached to all LWPs), we use
+     thread_from_lwp immediately when we see an LWP stop.  That uses
+     thread_db entry points that do not walk libpthread's thread list,
+     so should be safe, as well as more efficient.  */
+  if (target_has_execution && !thread_db_use_events ())
+    ops->beneath->to_update_thread_list (ops->beneath);
+  else
+    thread_db_update_thread_list_td_ta_thr_iter (ops);
 
   if (target_has_execution)
     iterate_over_lwps (minus_one_ptid /* iterate over all */,
@@ -1954,33 +1959,13 @@ thread_db_get_thread_local_address (struct target_ops *ops,
   return beneath->to_get_thread_local_address (beneath, ptid, lm, offset);
 }
 
-/* Callback routine used to find a thread based on the TID part of
-   its PTID.  */
-
-static int
-thread_db_find_thread_from_tid (struct thread_info *thread, void *data)
-{
-  long *tid = (long *) data;
-
-  if (thread->private->tid == *tid)
-    return 1;
-
-  return 0;
-}
-
 /* Implement the to_get_ada_task_ptid target method for this target.  */
 
 static ptid_t
 thread_db_get_ada_task_ptid (struct target_ops *self, long lwp, long thread)
 {
-  struct thread_info *thread_info;
-
-  thread_db_find_new_threads_1 (inferior_ptid);
-  thread_info = iterate_over_threads (thread_db_find_thread_from_tid, &thread);
-
-  gdb_assert (thread_info != NULL);
-
-  return (thread_info->ptid);
+  /* NPTL uses a 1:1 model, so the LWP id suffices.  */
+  return ptid_build (ptid_get_pid (inferior_ptid), lwp, 0);
 }
 
 static void
diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
index 5dd2b92..1e922b9 100644
--- a/gdb/nat/linux-procfs.c
+++ b/gdb/nat/linux-procfs.c
@@ -20,6 +20,7 @@
 #include "linux-procfs.h"
 #include "filestuff.h"
 #include <dirent.h>
+#include <sys/stat.h>
 
 /* Return the TGID of LWPID from /proc/pid/status.  Returns -1 if not
    found.  */
@@ -251,3 +252,15 @@ linux_proc_attach_tgid_threads (pid_t pid,
 
   closedir (dir);
 }
+
+/* See linux-procfs.h.  */
+
+int
+linux_proc_task_list_dir_exists (pid_t pid)
+{
+  char pathname[128];
+  struct stat buf;
+
+  xsnprintf (pathname, sizeof (pathname), "/proc/%ld/task", (long) pid);
+  return (stat (pathname, &buf) == 0);
+}
diff --git a/gdb/nat/linux-procfs.h b/gdb/nat/linux-procfs.h
index 6f2a404..979ae0d 100644
--- a/gdb/nat/linux-procfs.h
+++ b/gdb/nat/linux-procfs.h
@@ -68,4 +68,7 @@ typedef int (*linux_proc_attach_lwp_func) (ptid_t ptid);
 extern void linux_proc_attach_tgid_threads (pid_t pid,
 					    linux_proc_attach_lwp_func func);
 
+/* Return true if the /proc/PID/task/ directory exists.  */
+extern int linux_proc_task_list_dir_exists (pid_t pid);
+
 #endif /* COMMON_LINUX_PROCFS_H */
diff --git a/gdb/testsuite/gdb.threads/multi-create-ns-info-thr.exp b/gdb/testsuite/gdb.threads/multi-create-ns-info-thr.exp
new file mode 100644
index 0000000..906532e
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/multi-create-ns-info-thr.exp
@@ -0,0 +1,50 @@
+# Copyright (C) 2007-2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test running "info threads" while threads are being created and
+# exiting, in non-stop mode.  Originally based on multi-create.exp.
+
+standard_testfile multi-create.c
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug pthreads}] == -1} {
+    return -1
+}
+
+gdb_test_no_output "set pagination off"
+gdb_test_no_output "set non-stop on"
+
+runto_main
+
+# Create a breakpoint that does "info threads" when hit, which will be
+# just while other threads are being created or exiting.
+set bp_location1 [gdb_get_line_number "set breakpoint 1 here"]
+gdb_breakpoint $srcfile:$bp_location1
+gdb_test "commands\ninfo threads\ncontinue&\nend" ".*" "set breakpoint commands"
+
+set test "continue -a&"
+gdb_test_multiple $test $test {
+    -re "$gdb_prompt " {
+	pass $test
+    }
+}
+
+for {set i 0} {$i < 32} {incr i} {
+    set test "continue to breakpoint $i"
+    gdb_test_multiple "" $test {
+	-re "Breakpoint $decimal,.*$srcfile:$bp_location1" {
+	    pass $test
+	}
+    }
+}
-- 
1.9.3

Re: [PATCH 0/2] GNU/Linux: Stop using libthread_db/td_ta_thr_iter

[Pedro Alves]

On 02/08/2015 07:10 PM, Pedro Alves wrote:
> This one made me scratch my head for a while.  My
> all-stop-on-top-of-non-stop series was causing odd GDB hangs.  Turned
> out to be that we can't use libthread_db/td_ta_thr_iter when threads
> are running.  This series stops GDB from using it altogether, when
> debugging live programs.
>
> The second patch is the real deal.  The first patch fixes latent bugs
> that the second patch's test exposed, though after the second patch,
> the buggy function won't be used on modern system anymore...
>
> Pedro Alves (2):
>   linux-nat.c: fix a few lin_lwp_attach_lwp issues
>   GNU/Linux: Stop using libthread_db/td_ta_thr_iter

I've pushed this in now.

Thanks,
Pedro Alves

Cell multi-arch broken (Re: [PATCH 2/2] GNU/Linux: Stop using libthread_db/td_ta_thr_iter)

[Ulrich Weigand]

Pedro Alves wrote:

> gdb/ChangeLog:
> 2015-02-08  Pedro Alves  <palves@redhat.com>

> @@ -1719,6 +1696,9 @@ find_new_threads_once (struct thread_db_info *info, int iteration,
>    data.info = info;
>    data.new_threads = 0;
>  
> +  /* See comment in thread_db_update_thread_list.  */
> +  gdb_assert (!target_has_execution || thread_db_use_events ());
> +


Now that I've had the occasion to run some tests on Cell/B.E. again,
I've tried to verify combined multi-arch debugging still works, but
noticed that it is currently completely broken; GDB will immediately
run into the assertion you've added above.

In fact, it is so broken that the test suite assumes we're not even
on a Cell/B.E. (since it can't debug the trivial test program), and
silently skips all Cell tests, so I didn't notice in the daily build
reports ...

The reason why we're running into the abort is that the multi-arch
debugging logic attempts to resolve a thread-local variable from
inside the frame unwinders (which is probably not done elsewhere).
This uncovers a code path where the above assertion is wrong:

In thread_db_get_thread_local_address, we have:

  /* If we have not discovered any threads yet, check now.  */
  if (!have_threads (ptid))
    thread_db_find_new_threads_1 (ptid);

Now, note that thread_db_get_thread_local_address is one of the few
remaining routines that always uses the thread DB, even if we do
not use thread events.  However, thread_db_find_new_threads_1 now
assumes it gets only ever called when using thread events, which
ultimately leads to the assert.

As a quick fix, the patch appended below makes it work again;
but this may be a bit overkill since thread_db_update_thread_list
no longer realized that we're only interested in process ptid,
and updates all inferiors.  (Maybe the to_update_thread_list
target callback should get a ptid argument?)

Any suggestions on how best to fix this?

Bye,
Ulrich

P.S. With this fix, the Cell multi-arch tests now run again.
As probably to be expected, this uncovers a series of other
problems where code has bit-rotten in the meantime.  I'll
have other patches to fix this again ...



Index: binutils-gdb/gdb/linux-thread-db.c
===================================================================
--- binutils-gdb.orig/gdb/linux-thread-db.c
+++ binutils-gdb/gdb/linux-thread-db.c
@@ -1853,7 +1853,7 @@ thread_db_get_thread_local_address (stru
 
   /* If we have not discovered any threads yet, check now.  */
   if (!have_threads (ptid))
-    thread_db_find_new_threads_1 (ptid);
+    thread_db_update_thread_list (ops);
 
   /* Find the matching thread.  */
   thread_info = find_thread_ptid (ptid);


-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: Cell multi-arch broken (Re: [PATCH 2/2] GNU/Linux: Stop using libthread_db/td_ta_thr_iter)

[Pedro Alves]

On 08/26/2015 06:39 PM, Ulrich Weigand wrote:

> In fact, it is so broken that the test suite assumes we're not even
> on a Cell/B.E. (since it can't debug the trivial test program), and
> silently skips all Cell tests, so I didn't notice in the daily build
> reports ...

(Sounds like the testsuite could be improved to better detect this.)

> 
> The reason why we're running into the abort is that the multi-arch
> debugging logic attempts to resolve a thread-local variable from
> inside the frame unwinders (which is probably not done elsewhere).
> This uncovers a code path where the above assertion is wrong:

Curious.  Could you point me at this code path?  I can't seem
to find it.  I wonder whether can trigger this assertion
by stopping the inferior before thread_db is initialized (e.g.,
entry point), and then trying to print a tls variable?  (In order
to construct a test case).

> 
> In thread_db_get_thread_local_address, we have:
> 
>   /* If we have not discovered any threads yet, check now.  */
>   if (!have_threads (ptid))
>     thread_db_find_new_threads_1 (ptid);
> 
> Now, note that thread_db_get_thread_local_address is one of the few
> remaining routines that always uses the thread DB, even if we do
> not use thread events.  However, thread_db_find_new_threads_1 now
> assumes it gets only ever called when using thread events, which
> ultimately leads to the assert.
> 
> As a quick fix, the patch appended below makes it work again;
> but this may be a bit overkill since thread_db_update_thread_list
> no longer realized that we're only interested in process ptid,
> and updates all inferiors.  (Maybe the to_update_thread_list
> target callback should get a ptid argument?)
> 
> Any suggestions on how best to fix this?

Try doing it like gdbserver's thread_db_get_tls_address.

...
  lwp = get_thread_lwp (thread);
  if (!lwp->thread_known)
    find_one_thread (thread->entry.id);
...

That is, here what we're really after is the td_thrhandle_t
of the current thread, in order to be able to call
td_thr_tls_get_addr.  There's no need to walk thread_db's
thread list to find that for a single thread.

find_one_thread makes use of td_ta_map_lwp2thr for the
mapping we're after.
On the GDB side, the equivalent is linux-thread-db.c:thread_from_lwp.

Thanks,
Pedro Alves

Re: Cell multi-arch broken (Re: [PATCH 2/2] GNU/Linux: Stop using libthread_db/td_ta_thr_iter)

[Ulrich Weigand]

Pedro Alves wrote:
> On 08/26/2015 06:39 PM, Ulrich Weigand wrote:
> 
> > In fact, it is so broken that the test suite assumes we're not even
> > on a Cell/B.E. (since it can't debug the trivial test program), and
> > silently skips all Cell tests, so I didn't notice in the daily build
> > reports ...
> 
> (Sounds like the testsuite could be improved to better detect this.)

Yes, I think I'll at least set the test case to UNRESOLVED if
something unexpected happens while attempting to detect whether
we have Cell/B.E. hardware.

> > The reason why we're running into the abort is that the multi-arch
> > debugging logic attempts to resolve a thread-local variable from
> > inside the frame unwinders (which is probably not done elsewhere).
> > This uncovers a code path where the above assertion is wrong:
> 
> Curious.  Could you point me at this code path?  I can't seem
> to find it.  I wonder whether can trigger this assertion
> by stopping the inferior before thread_db is initialized (e.g.,
> entry point), and then trying to print a tls variable?  (In order
> to construct a test case).

This is where it triggers for me:

#0  internal_error (file=0x107e0ce0 "/home/uweigand/fsf/binutils-gdb/gdb/linux-thread-db.c", line=1663, fmt=0x107dacd8 "%s: Assertion `%s' failed.")
    at /home/uweigand/fsf/binutils-gdb/gdb/common/errors.c:51
#1  0x00000000100ee6e0 in find_new_threads_once (info=0x10f00de0, iteration=0, errp=0xfffffc6d380) at /home/uweigand/fsf/binutils-gdb/gdb/linux-thread-db.c:1663
#2  0x00000000100ee9ac in thread_db_find_new_threads_2 (ptid=..., until_no_new=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/linux-thread-db.c:1729
#3  0x00000000100eec10 in thread_db_get_thread_local_address (ops=0x10a59fa8, ptid=..., lm=552219129840, offset=4398046642176)
    at /home/uweigand/fsf/binutils-gdb/gdb/linux-thread-db.c:1856
#4  0x00000000102b0cfc in delegate_get_thread_local_address (self=<value optimized out>, arg1=..., arg2=<value optimized out>, arg3=<value optimized out>)
    at /home/uweigand/fsf/binutils-gdb/gdb/target-delegates.c:1931
#5  0x00000000100c5540 in ppc_linux_spe_context (wordsize=0, byte_order=BFD_ENDIAN_BIG, n=0, id=0xfffffc6d660, npc=0xfffffc6d664)
    at /home/uweigand/fsf/binutils-gdb/gdb/ppc-linux-tdep.c:1205
#6  0x00000000100c58f4 in ppu2spu_sniffer (self=<value optimized out>, this_frame=0x10c35fe0, this_prologue_cache=0x10c35ff8)
    at /home/uweigand/fsf/binutils-gdb/gdb/ppc-linux-tdep.c:1341
#7  0x00000000103a276c in frame_unwind_try_unwinder (this_frame=0x10c35fe0, this_cache=0x10c35ff8, unwinder=0x10942b70) at /home/uweigand/fsf/binutils-gdb/gdb/frame-unwind.c:105
#8  0x00000000103a28dc in frame_unwind_find_by_frame (this_frame=0x10c35fe0, this_cache=0x10c35ff8) at /home/uweigand/fsf/binutils-gdb/gdb/frame-unwind.c:160
#9  0x00000000103a0f70 in compute_frame_id (this_frame=0x10c35f10) at /home/uweigand/fsf/binutils-gdb/gdb/frame.c:454
#10 get_prev_frame_if_no_cycle (this_frame=0x10c35f10) at /home/uweigand/fsf/binutils-gdb/gdb/frame.c:1781
#11 0x00000000103a1260 in get_prev_frame_always_1 (this_frame=0x10c35f10) at /home/uweigand/fsf/binutils-gdb/gdb/frame.c:1955
#12 get_prev_frame_always (this_frame=0x10c35f10) at /home/uweigand/fsf/binutils-gdb/gdb/frame.c:1971
#13 0x00000000103a1788 in get_prev_frame (this_frame=0x10c35f10) at /home/uweigand/fsf/binutils-gdb/gdb/frame.c:2213
#14 0x00000000103a19ac in unwind_to_current_frame (ui_out=<value optimized out>, args=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/frame.c:1450
#15 0x000000001027707c in catch_exceptions_with_msg (func_uiout=<value optimized out>, func=@0x10a1b980: 0x103a1990 <unwind_to_current_frame>, func_args=0x10c35f10,
    gdberrmsg=0x0, mask=0) at /home/uweigand/fsf/binutils-gdb/gdb/exceptions.c:187
#16 0x000000001039f2a8 in get_current_frame () at /home/uweigand/fsf/binutils-gdb/gdb/frame.c:1489
#17 0x0000000010264408 in process_event_stop_test (ecs=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/infrun.c:5877
#18 0x0000000010265fac in handle_inferior_event_1 (ecs=0xfffffc6ea08) at /home/uweigand/fsf/binutils-gdb/gdb/infrun.c:5857
#19 handle_inferior_event (ecs=0xfffffc6ea08) at /home/uweigand/fsf/binutils-gdb/gdb/infrun.c:5053
#20 0x0000000010267978 in fetch_inferior_event (client_data=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/infrun.c:3785
#21 0x00000000102864b8 in inferior_event_handler (event_type=INF_REG_EVENT, client_data=0x0) at /home/uweigand/fsf/binutils-gdb/gdb/inf-loop.c:56
#22 0x00000000100f4bb8 in handle_target_event (error=<value optimized out>, client_data=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/linux-nat.c:4706
#23 0x0000000010284374 in handle_file_event (file_ptr=0x10ddc670, ready_mask=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/event-loop.c:708
#24 0x00000000102845dc in gdb_wait_for_event (block=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/event-loop.c:859
#25 0x00000000102849c8 in gdb_do_one_event () at /home/uweigand/fsf/binutils-gdb/gdb/event-loop.c:298
#26 0x0000000010284af4 in start_event_loop () at /home/uweigand/fsf/binutils-gdb/gdb/event-loop.c:347
#27 0x0000000010286408 in cli_command_loop (data=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/event-top.c:186
#28 0x000000001027ae28 in current_interp_command_loop () at /home/uweigand/fsf/binutils-gdb/gdb/interps.c:317
#29 0x000000001027bfc4 in captured_command_loop (data=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/main.c:318
#30 0x0000000010276e14 in catch_errors (func=@0x10a0c950: 0x1027bfa0 <captured_command_loop>, func_args=0x0, errstring=0x108d1078 "", mask=0)
    at /home/uweigand/fsf/binutils-gdb/gdb/exceptions.c:240
#31 0x000000001027cd84 in captured_main (data=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/main.c:1157
#32 0x0000000010276e14 in catch_errors (func=@0x10a0c980: 0x1027c270 <captured_main>, func_args=0xfffffc6f3c0, errstring=0x108d1078 "", mask=0)
    at /home/uweigand/fsf/binutils-gdb/gdb/exceptions.c:240
#33 0x000000001027ba48 in gdb_main (args=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/main.c:1165
#34 0x0000000010093644 in main (argc=<value optimized out>, argv=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/gdb.c:32

I haven't really tried to construct a test case yet ...

> Try doing it like gdbserver's thread_db_get_tls_address.
> 
> ...
>   lwp = get_thread_lwp (thread);
>   if (!lwp->thread_known)
>     find_one_thread (thread->entry.id);
> ...
> 
> That is, here what we're really after is the td_thrhandle_t
> of the current thread, in order to be able to call
> td_thr_tls_get_addr.  There's no need to walk thread_db's
> thread list to find that for a single thread.
> 
> find_one_thread makes use of td_ta_map_lwp2thr for the
> mapping we're after.
> On the GDB side, the equivalent is linux-thread-db.c:thread_from_lwp.

Ah, indeed that works for me.  The attached patch also fixes the
problem for me.

Bye,
Ulrich

Index: binutils-gdb/gdb/linux-thread-db.c
===================================================================
--- binutils-gdb.orig/gdb/linux-thread-db.c
+++ binutils-gdb/gdb/linux-thread-db.c
@@ -1851,13 +1851,16 @@ thread_db_get_thread_local_address (stru
   struct thread_info *thread_info;
   struct target_ops *beneath;
 
-  /* If we have not discovered any threads yet, check now.  */
-  if (!have_threads (ptid))
-    thread_db_find_new_threads_1 (ptid);
-
   /* Find the matching thread.  */
   thread_info = find_thread_ptid (ptid);
 
+  /* We may not have discovered the thread yet.  */
+  if (thread_info != NULL && thread_info->priv == NULL)
+    {
+      thread_from_lwp (ptid);
+      thread_info = find_thread_ptid (ptid);
+    }
+
   if (thread_info != NULL && thread_info->priv != NULL)
     {
       td_err_e err;


-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: Cell multi-arch broken (Re: [PATCH 2/2] GNU/Linux: Stop using libthread_db/td_ta_thr_iter)

[Pedro Alves]

On 08/26/2015 08:01 PM, Ulrich Weigand wrote:
> Pedro Alves wrote:

>> (Sounds like the testsuite could be improved to better detect this.)
> 
> Yes, I think I'll at least set the test case to UNRESOLVED if
> something unexpected happens while attempting to detect whether
> we have Cell/B.E. hardware.

Sounds good to me.

> This is where it triggers for me:
> 

Thanks, I see.

>> find_one_thread makes use of td_ta_map_lwp2thr for the
>> mapping we're after.
>> On the GDB side, the equivalent is linux-thread-db.c:thread_from_lwp.
> 
> Ah, indeed that works for me.  The attached patch also fixes the
> problem for me.
> 

LGTM.

Thanks,
Pedro Alves

Re: Cell multi-arch broken (Re: [PATCH 2/2] GNU/Linux: Stop using libthread_db/td_ta_thr_iter)

[Ulrich Weigand]

Pedro Alves wrote:
> On 08/26/2015 08:01 PM, Ulrich Weigand wrote:
> > Pedro Alves wrote:
> 
> >> (Sounds like the testsuite could be improved to better detect this.)
> > 
> > Yes, I think I'll at least set the test case to UNRESOLVED if
> > something unexpected happens while attempting to detect whether
> > we have Cell/B.E. hardware.
> 
> Sounds good to me.

I've pushed the testcase change (see patch below) ...

> >> find_one_thread makes use of td_ta_map_lwp2thr for the
> >> mapping we're after.
> >> On the GDB side, the equivalent is linux-thread-db.c:thread_from_lwp.
> > 
> > Ah, indeed that works for me.  The attached patch also fixes the
> > problem for me.
> > 
> 
> LGTM.

... and this linux-thread-db fix now.

Thanks,
Ulrich


testsuite/ChangeLog:

	* lib/cell.exp (skip_cell_tests): Report UNRESOLVED on unexpected
	failures to run the test program under GDB.

Index: binutils-gdb/gdb/testsuite/lib/cell.exp
===================================================================
--- binutils-gdb.orig/gdb/testsuite/lib/cell.exp
+++ binutils-gdb/gdb/testsuite/lib/cell.exp
@@ -139,7 +139,7 @@ gdb_caching_proc skip_cell_tests {
             set result 1
         }
         default {
-            verbose -log "\n$me Cell/B.E. hardware not detected (default case)"
+            unresolved "$me: unexpected failure"
             set result 1
         }
     }

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: Cell multi-arch broken (Re: [PATCH 2/2] GNU/Linux: Stop using libthread_db/td_ta_thr_iter)

[Doug Evans]

"Ulrich Weigand" <uweigand@de.ibm.com> writes:
> Ah, indeed that works for me.  The attached patch also fixes the
> problem for me.
>
> Bye,
> Ulrich
>
> Index: binutils-gdb/gdb/linux-thread-db.c
> ===================================================================
> --- binutils-gdb.orig/gdb/linux-thread-db.c
> +++ binutils-gdb/gdb/linux-thread-db.c
> @@ -1851,13 +1851,16 @@ thread_db_get_thread_local_address (stru
>    struct thread_info *thread_info;
>    struct target_ops *beneath;
>  
> -  /* If we have not discovered any threads yet, check now.  */
> -  if (!have_threads (ptid))
> -    thread_db_find_new_threads_1 (ptid);
> -
>    /* Find the matching thread.  */
>    thread_info = find_thread_ptid (ptid);
>  
> +  /* We may not have discovered the thread yet.  */
> +  if (thread_info != NULL && thread_info->priv == NULL)
> +    {
> +      thread_from_lwp (ptid);
> +      thread_info = find_thread_ptid (ptid);
> +    }
> +
>    if (thread_info != NULL && thread_info->priv != NULL)
>      {
>        td_err_e err;

Hi.

Just a thought.

It's kinda clumsy that thread_from_lwp ends with this:

  /* Fill the cache.  */
  tp = find_thread_ptid (ptid);
  record_thread (info, tp, ptid, &th, &ti);

and then we just call find_thread_ptid again after it returns:

> +      thread_from_lwp (ptid);
> +      thread_info = find_thread_ptid (ptid);

One might ask "Why doesn't thread_from_lwp just return thread_info?"

From record_thread things seem to be a be more subtle.
Even if we pass in a non-NULL TP we may still create a new one.

  /* Add the thread to GDB's thread list.  If we already know about a
     thread with this PTID, but it's marked exited, then the kernel
     reused the tid of an old thread.  */
  if (tp == NULL || tp->state == THREAD_EXITED)
    tp = add_thread_with_info (ptid, priv);
  else
    tp->priv = priv;

So it'd be helpful if record_thread also returned tp.
Sound about right?

2015-08-27  Doug Evans  <xdje42@gmail.com>

	* linux-thread-db.c (record_thread): Return the created thread.
	(thread_from_lwp): Likewise.
	(thread_db_get_thread_local_address): Update.

diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 855629b..b5719eb 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -198,10 +198,9 @@ static void thread_db_find_new_threads_2 (ptid_t ptid, int until_no_new);
 
 static void check_thread_signals (void);
 
-static void record_thread (struct thread_db_info *info,
-			   struct thread_info *tp,
-			   ptid_t ptid, const td_thrhandle_t *th_p,
-			   const td_thrinfo_t *ti_p);
+static struct thread_info *record_thread
+  (struct thread_db_info *info, struct thread_info *tp,
+   ptid_t ptid, const td_thrhandle_t *th_p, const td_thrinfo_t *ti_p);
 
 /* Add the current inferior to the list of processes using libpthread.
    Return a pointer to the newly allocated object that was added to
@@ -386,7 +385,7 @@ have_threads (ptid_t ptid)
 \f
 /* Fetch the user-level thread id of PTID.  */
 
-static void
+static struct thread_info *
 thread_from_lwp (ptid_t ptid)
 {
   td_thrhandle_t th;
@@ -419,7 +418,7 @@ thread_from_lwp (ptid_t ptid)
 
   /* Fill the cache.  */
   tp = find_thread_ptid (ptid);
-  record_thread (info, tp, ptid, &th, &ti);
+  return record_thread (info, tp, ptid, &th, &ti);
 }
 \f
 
@@ -1287,10 +1286,10 @@ attach_thread (ptid_t ptid, const td_thrhandle_t *th_p,
 }
 
 /* Record a new thread in GDB's thread list.  Creates the thread's
-   private info.  If TP is NULL, creates a new thread.  Otherwise,
-   uses TP.  */
+   private info.  If TP is NULL or TP is marked as having exited,
+   creates a new thread.  Otherwise, uses TP.  */
 
-static void
+static struct thread_info *
 record_thread (struct thread_db_info *info,
 	       struct thread_info *tp,
 	       ptid_t ptid, const td_thrhandle_t *th_p,
@@ -1304,7 +1303,7 @@ record_thread (struct thread_db_info *info,
      initialized yet.  Leave private == NULL until the thread library
      has initialized.  */
   if (ti_p->ti_tid == 0)
-    return;
+    return tp;
 
   /* Construct the thread's private data.  */
   priv = XCNEW (struct private_thread_info);
@@ -1333,6 +1332,8 @@ record_thread (struct thread_db_info *info,
 
   if (target_has_execution)
     check_thread_signals ();
+
+  return tp;
 }
 
 static void
@@ -1854,10 +1855,7 @@ thread_db_get_thread_local_address (struct target_ops *ops,
 
   /* We may not have discovered the thread yet.  */
   if (thread_info != NULL && thread_info->priv == NULL)
-    {
-      thread_from_lwp (ptid);
-      thread_info = find_thread_ptid (ptid);
-    }
+    thread_info = thread_from_lwp (ptid);
 
   if (thread_info != NULL && thread_info->priv != NULL)
     {

Re: Cell multi-arch broken (Re: [PATCH 2/2] GNU/Linux: Stop using libthread_db/td_ta_thr_iter)

[Ulrich Weigand]

Doug Evans wrote:

> One might ask "Why doesn't thread_from_lwp just return thread_info?"

Yes, I did ask myself that too :-)

> 2015-08-27  Doug Evans  <xdje42@gmail.com>
> 
> 	* linux-thread-db.c (record_thread): Return the created thread.
> 	(thread_from_lwp): Likewise.
> 	(thread_db_get_thread_local_address): Update.

Makes sense to me.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: Cell multi-arch broken (Re: [PATCH 2/2] GNU/Linux: Stop using libthread_db/td_ta_thr_iter)

[Pedro Alves]

On 08/28/2015 07:20 AM, Doug Evans wrote:

> So it'd be helpful if record_thread also returned tp.
> Sound about right?

Sounds fine to me too.

Thanks,
Pedro Alves

Re: Cell multi-arch broken (Re: [PATCH 2/2] GNU/Linux: Stop using libthread_db/td_ta_thr_iter)

[Doug Evans]

Doug Evans <xdje42@gmail.com> writes:
> "Ulrich Weigand" <uweigand@de.ibm.com> writes:
>> Ah, indeed that works for me.  The attached patch also fixes the
>> problem for me.
>>
>> Bye,
>> Ulrich
>>
>> Index: binutils-gdb/gdb/linux-thread-db.c
>> ===================================================================
>> --- binutils-gdb.orig/gdb/linux-thread-db.c
>> +++ binutils-gdb/gdb/linux-thread-db.c
>> @@ -1851,13 +1851,16 @@ thread_db_get_thread_local_address (stru
>>    struct thread_info *thread_info;
>>    struct target_ops *beneath;
>>  
>> -  /* If we have not discovered any threads yet, check now.  */
>> -  if (!have_threads (ptid))
>> -    thread_db_find_new_threads_1 (ptid);
>> -
>>    /* Find the matching thread.  */
>>    thread_info = find_thread_ptid (ptid);
>>  
>> +  /* We may not have discovered the thread yet.  */
>> +  if (thread_info != NULL && thread_info->priv == NULL)
>> +    {
>> +      thread_from_lwp (ptid);
>> +      thread_info = find_thread_ptid (ptid);
>> +    }
>> +
>>    if (thread_info != NULL && thread_info->priv != NULL)
>>      {
>>        td_err_e err;
>
> Hi.
>
> Just a thought.
>
> It's kinda clumsy that thread_from_lwp ends with this:
>
>   /* Fill the cache.  */
>   tp = find_thread_ptid (ptid);
>   record_thread (info, tp, ptid, &th, &ti);
>
> and then we just call find_thread_ptid again after it returns:
>
>> +      thread_from_lwp (ptid);
>> +      thread_info = find_thread_ptid (ptid);
>
> One might ask "Why doesn't thread_from_lwp just return thread_info?"
>
> From record_thread things seem to be a be more subtle.
> Even if we pass in a non-NULL TP we may still create a new one.
>
>   /* Add the thread to GDB's thread list.  If we already know about a
>      thread with this PTID, but it's marked exited, then the kernel
>      reused the tid of an old thread.  */
>   if (tp == NULL || tp->state == THREAD_EXITED)
>     tp = add_thread_with_info (ptid, priv);
>   else
>     tp->priv = priv;
>
> So it'd be helpful if record_thread also returned tp.
> Sound about right?
>
> 2015-08-27  Doug Evans  <xdje42@gmail.com>
>
> 	* linux-thread-db.c (record_thread): Return the created thread.
> 	(thread_from_lwp): Likewise.
> 	(thread_db_get_thread_local_address): Update.

Committed.