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

Doug Evans
2015-08-28T06:21:49Z

"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)
{