Re: [PATCH] 0001-Fix-multi-thread-debug-bug-in-AIX.patch

[Aditya Kamath1 <Aditya.Kamath1@ibm.com>]

[-- Attachment #1.1: Type: text/plain, Size: 18085 bytes --]

Hi Ulrich, Tom and community,

Please find attached the patch. I have written my answers to the previous comments. Kindly let me know if we need more changes. If not kindly push this to the community code. The sample output and programs are pasted below this email.

>I think this would fit better into gdb.threads, given that this is about the
>interaction of multiple inferiors with the threading library on AIX.

This I will do it immediately after this patch is done.

>>     if (user_current_pid != 0)
>>+      inferior_ptid = ptid_t (user_current_pid);

>This seems unrelated to the rest of the changes at first glance.
>Why is this necessary?

So, when we need to be in the right context when we read memory. Before coming into the target wait, we switch_to_no_thread () due to which our inferior_ptid is set to null. Our target_memory needs the correct inferior_ptid.  Also, in case we don't have a ptid_t (pid) and the application is threaded we need the inferior_ptid to be set correctly like shown in the patch. Previously we used switch_to_thread ().. Now if the application is theraded and we only pass ptid_t (user_current_pid) to switch_to_thread () it will crash as main thread looks different or is ptid_t (pid, 0, tid). Hence, we set inferior_ptid to simplify.

>Also, is the "user_current_pid != 0" check even still needed given
>the change to pd_enable() below?

So, this I have removed. You were right.

>By comparison, the Linux version of this in proc-service.c also
>switches the current inferior and address space:
 > scoped_restore_current_inferior restore_inferior;
 > set_current_inferior (ph->thread->inf);
  >scoped_restore_current_program_space restore_current_progspace;
  >set_current_program_space (ph->thread->inf->pspace);
 > scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
  >inferior_ptid = ph->thread->ptid;
> so we should probably do the same for consistency.

So, kindly allow me to disagree with you on this. What is happening is in inferior.c in do_target_wait1 () we call switch_to_inferior_no_thread ().. The function is as follows


void

switch_to_inferior_no_thread (inferior *inf)

{

  set_current_inferior (inf);

  switch_to_no_thread ();

  set_current_program_space (inf->pspace);

}


Here we already set the correct current inferior and program space to the same thing as that if we set in pdc_read_memory like linux. So, it does not make any difference to add the changes like linux does. In the switch_to_no_thread () we set inferior_ptid to null and that is why we only set inferior_ptid in pdc_read_memory and not anything else. So, I suggest we stick to this plan. Secondly, things work if we do not do the same for pdc_write_memory. I have not seen anything not work. So, I don't think it is good to add it there. What say??

>This looks unnecessarily complicated.  Isn't this just
  > *g++ = tp;

This I have changed.

>Is this a change in behavior to current GDB?  I thought if the
>application (whether a single inferior or one of multiple inferiors)
>is threaded in the sense that it uses the libpthread library we
>wanted to show it as threaded, so that the user can e.g. see the
>thread ID in info threads.

So, you are right. I read it somewhere which I am not able to recall that only in multiple threads we need to show as thread. I checked the Linux output. It is what you mentioned. I have removed the gcount ==1 && pcount == 1 condition..

>This logic is still confusing me.  Why is the
 >  gptid.pid () == pptid.pid ()
>check still needed?  I thought we now collected only threads
>of a single process to begin with, so they all ought to have
>the same PID?

>Also, if the point is the gptid.is_pid () check, this can
>really only happen once per inferior, as it is switched
>from non-threaded to threaded mode, right?

So I removed the gptid.pid () == pptid.pid () condition. The reason I had added was the gcount {Thread count per process} was not per process before. I was worried to swap process. Now we do not need it.

As far as the check gptid.is_pid () is concerned, I will suggest we keep it there. If cmp_result is > 0 and we have a main process swap to create a thread. Rest is same in the loop. The reason being handling pi and gi variables becomes complex otherwise. When this swap happens, we need to increment both pi and gi.. Because we have taken care of the main threads in both pthread library and GDB. And this for loop is executed only once. So, the first event is main process being pthreaded. Once the swap happens pi and gi become one and since gcount = pcount = 1 we exit the for loop. Thread addition events comes after this.

>That should just be "if (ptid.tid () == 0)" then.
This is done

>-  pd_deactivate ();
>+  pd_disable ();
>Why is this necessary?  If it is, do we even need two
>separate pd_deactivate and pd_disable routines any more?

So, the process exits then all its threads also exit in the mourn inferior. So, we disable everything. Yes, I removed pd_deactivate ().

>>+      if (s.find ('(') != std::string::npos
>>+        && s.find (member_name) != std::string::npos)
>>+      return object_bfd;

>Ah, I guess you also need to ensure the member_name follows
>immediately after the '(', otherwise there could be confusion
>if the member name happens to be part of the file name as well.

This I have changed as per how you mentioned. Kindly check the patch and let me know :)

Have a nice day ahead.

Thanks and regards,
Aditya.

-------------------------------------------
Code:-


#include <stdio.h>

#include <unistd.h>

#include <stdlib.h>

#include <pthread.h>

#include <assert.h>


pthread_barrier_t barrier;


#define NUM_THREADS 2


void *

thread_function (void *arg)

{

  /* This ensures that the breakpoint is only hit after both threads

     are created, so the test can always switch to the non-event

     thread when the breakpoint triggers.  */


  pthread_barrier_wait (&barrier);

  pid_t child;


  child = fork ();

  if (child > 0)

    printf ("I am parent \n");

  else

  {

    child = fork ();

    if (child > 0)

      printf ("I am child \n");

    else

      printf ("I am grandchild \n");

  }

  while (1); /* break here */

}


int

main (void)

{

  int i;

  pthread_t thread[NUM_THREADS];


  alarm (300);


  pthread_barrier_init (&barrier, NULL, NUM_THREADS);


  for (i = 0; i < NUM_THREADS; i++)

    {

      int res;


      res = pthread_create (&thread[i], NULL,

                            thread_function, NULL);

      assert (res == 0);

    }


  while (1)

  {

    sleep (15);

  }


  return 0;

}

-------------------------------------------------

Output with patch:-


Reading symbols from /home/aditya/gdb_tests/ultimate-multi-thread-fork...

(gdb) set detach-on-fork off

(gdb) r

Starting program: /home/aditya/gdb_tests/ultimate-multi-thread-fork

[New Thread 258]

[New Thread 515]

[New inferior 2 (process 15991124)]

I am parent

[New inferior 3 (process 20840796)]

I am parent

^Cin

Thread 1.1 received signal SIGINT, Interrupt.

[Switching to Thread 1]

0xd0595fb0 in _p_nsleep () from /usr/lib/libpthread.a(shr_xpg5.o)

(gdb) info threads

  Id   Target Id                          Frame

* 1.1  Thread 1 (tid 33947921, running)   0xd0595fb0 in _p_nsleep ()

   from /usr/lib/libpthread.a(shr_xpg5.o)

  1.2  Thread 258 (tid 37421465, running) thread_function (arg=0x0)

    at /home/aditya/gdb_tests/ultimate-multi-thread-fork.c:32

  1.3  Thread 515 (tid 32899441, running) thread_function (arg=0x0)


0x0)

    at /home/aditya/gdb_tests/ultimate-multi-thread-fork.c:32

  2.1  Thread 515 (tid 33751493, running) 0xd0594fc8 in _sigsetmask ()

   from /usr/lib/libpthread.a(shr_xpg5.o)

  3.1  Thread 258 (tid 34931151, running) 0xd0594fc8 in _sigsetmask ()

   from /usr/lib/libpthread.a(shr_xpg5.o)

(gdb)


-----------------------------------------------------------------------
Output without patch:-
Reading symbols from /home/aditya/gdb_tests/ultimate-multi-thread-fork...

(gdb) set detach-on-fork off

(gdb) r

Starting program: /home/aditya/gdb_tests/ultimate-multi-thread-fork

[New Thread 1]

[New Thread 258]

[New Thread 515]

[New inferior 2 (process 11731200)]

I am parent

[New inferior 3 (process 16843200)]

I am parent

^C

Thread 1.1 received signal SIGINT, Interrupt.

0xd0595fb0 in _p_nsleep () from /usr/lib/libpthread.a(shr_xpg5.o)

(gdb) inferior 2

[Switching to inferior 2 [process 11731200] (/home/aditya/gdb_tests/ultimate-multi-thread-fork)]

[Switching to thread 2.1 (process 11731200)]

#0  0xd0594fc8 in ?? ()

(gdb) info threads

  Id   Target Id         Frame

  1.1  process 15270316  0xd0595fb0 in _p_nsleep ()

   from /usr/lib/libpthread.a(shr_xpg5.o)

  1.2  process 15270316  0xd0595fb0 in _p_nsleep ()

   from /usr/lib/libpthread.a(shr_xpg5.o)

  1.3  process 15270316  0xd0595fb0 in _p_nsleep ()

   from /usr/lib/libpthread.a(shr_xpg5.o)

  1.4  process 15270316  0xd0595fb0 in _p_nsleep ()

   from /usr/lib/libpthread.a(shr_xpg5.o)

* 2.1  process 11731200  0xd0594fc8 in ?? ()

  3.1  process 16843200  0xd0594fc8 in ?? ()

(gdb) info sharedlibrary

warning: "/usr/lib/libpthreads.a": member "shr_comm.o" missing.

warning: "/usr/lib/libcrypt.a": member "shr.o" missing.

warning: "/usr/lib/libpthread.a": member "shr_xpg5.o" missing.

warning: "/usr/lib/libc.a": member "shr.o" missing.

warning: Could not load shared library symbols for 4 libraries, e.g. /usr/lib/libpthreads.a(shr_comm.o).

Use the "info sharedlibrary" command to see the complete listing.

Do you need "set solib-search-path" or "set sysroot"?

From        To          Syms Read   Shared Object Library

                        No          /usr/lib/libpthreads.a(shr_comm.o)

                        No          /usr/lib/libcrypt.a(shr.o)

                        No          /usr/lib/libpthread.a(shr_xpg5.o)

                        No          /usr/lib/libc.a(shr.o)
(gdb)


________________________________
From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Sent: 07 February 2023 00:37
To: simark@simark.ca <simark@simark.ca>; Aditya Kamath1 <Aditya.Kamath1@ibm.com>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: Re: [PATCH] 0001-Fix-multi-thread-debug-bug-in-AIX.patch

Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>>I think the question here is simply whether, if you run the
>>test suite both without and with your patch, are any of the
>>FAILs fixed with the patch?   If not, it would be good to
>>create a new test that fails without the patch and succeeds
>>with it, and add that to the test suite.
>
>So, this is something new to me. We will add it as a continuation in the same thread
>after this patch. I will need one information. Which test suite will we add it in?
>gdb.threads or gdb.base? Also, kindly suggest a simple test case that is written that
>I can see and learn. Any simple hello_world program will do. I want to understand how
>that exp file is written and how it compares to tell if a test case is pass or fail.

I think this would fit better into gdb.threads, given that this is about the
interaction of multiple inferiors with the threading library on AIX.

I'd just look at existing test cases in that directory.  For simple tests, we
usually have a .c file and a .exp file with the same name.  The .exp file
starts out with instructions to build the test case, and start it up under
GDB.  Then follow a series of test statements which are verified against
the output of the GDB under test.  As a simple example in a related area,
you can look e.g. at fork-child-threads.{c,exp}.


>Kindly give me feedback for this patch, incase we can do anything better
>or is incorrect.

Some comments:

>@@ -508,14 +550,13 @@ pdc_read_data (pthdb_user_t user_current_pid, void *buf,
>   /* This is needed to eliminate the dependency of current thread
>      which is null so that thread reads the correct target memory.  */
>   {
>-    scoped_restore_current_thread restore_current_thread;
>+    scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
>     /* Before the first inferior is added, we pass inferior_ptid.pid ()
>        from pd_enable () which is 0.  There is no need to switch threads
>        during first initialisation.  In the rest of the callbacks the
>        current thread needs to be correct.  */
>     if (user_current_pid != 0)
>-      switch_to_thread (current_inferior ()->process_target (),
>-                      ptid_t (user_current_pid));
>+      inferior_ptid = ptid_t (user_current_pid);
>     status = target_read_memory (addr, (gdb_byte *) buf, len);
>   }

This seems unrelated to the rest of the changes at first glance.
Why is this necessary?

Also, is the "user_current_pid != 0" check even still needed given
the change to pd_enable() below?

By comparison, the Linux version of this in proc-service.c also
switches the current inferior and address space:

  scoped_restore_current_inferior restore_inferior;
  set_current_inferior (ph->thread->inf);

  scoped_restore_current_program_space restore_current_progspace;
  set_current_program_space (ph->thread->inf->pspace);

  scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
  inferior_ptid = ph->thread->ptid;

so we should probably do the same for consistency.

Also, the same logic will be required in pdc_write_data, where it
is currently missing completely.

>+  for (thread_info *tp : all_threads (proc_target, ptid_t (pid)))
>+  {
>+    **(struct thread_info ***) &g = tp;
>+    (*(struct thread_info ***) &g)++;
>+  }

This looks unnecessarily complicated.  Isn't this just
   *g++ = tp;
?

>+        /* If there is only one thread then we need not make the main
>+           thread look like a thread.  It can stay as a process. This
>+           is useful when we have multiple inferiors, but only one is
>+           threaded.  So we need not make the other inferiors with only
>+           main thread, look like a threaded one.  For example, Thread
>+           1.1, 1.2, 2.1, 3.1 exists then it is useful to skip this for
>+           loop for 2.1 and 3.1 leaving them as main process thread with
>+           a dummy priv set.  */
>+
>+        if (pcount == 1 && gcount == 1)
>+        {
>+          aix_thread_info *priv = new aix_thread_info;
>+          tp = find_thread_ptid (proc_target, gptid);
>+          tp->priv.reset (priv);
>+          break;
>+        }

Is this a change in behavior to current GDB?  I thought if the
application (whether a single inferior or one of multiple inferiors)
is threaded in the sense that it uses the libpthread library we
wanted to show it as threaded, so that the user can e.g. see the
thread ID in info threads.

>+            /* This is to make the main process thread now look
>+               like a thread.  */
>+
>+            if (gptid.is_pid () && gptid.pid () == pptid.pid ())
>+            {
>+              thread_change_ptid (proc_target, gptid, pptid);
>+              aix_thread_info *priv = new aix_thread_info;
>+              priv->pdtid = pbuf[pi].pdtid;
>+              priv->tid = pbuf[pi].tid;
>+              tp = find_thread_ptid (proc_target, pptid);
>+              tp->priv.reset (priv);
>+              pi++;
>+              gi++;
>+            }
>+            else
>+            {
>+              delete_thread (gbuf[gi]);
>+              gi++;
>+            }

This logic is still confusing me.  Why is the
   gptid.pid () == pptid.pid ()
check still needed?  I thought we now collected only threads
of a single process to begin with, so they all ought to have
the same PID?

Also, if the point is the gptid.is_pid () check, this can
really only happen once per inferior, as it is switched
from non-threaded to threaded mode, right?  Maybe it
would simplify the logic to have all that (including
the code under
  if (pcount == 1 && gcount == 1)
above if it is actually needed) in a separate statement
before that loop.

I.e. directly before the loop, have a separate check
whether the current process only has a single thread,
whose ptid_t is still in the pid-only format, and if
so, upgrade it to full TID format using the main thread's
TID.  Only after that, go through the loop to handle
any other threads we may also have.  (At that point,
all GDB threads should already always be in TID format.)

>-  if (!PD_TID (ptid))
>+  if (!(ptid.tid () != 0))

That should just be "if (ptid.tid () == 0)" then.
(Here and in a few other places.)

>@@ -1741,7 +1823,7 @@ aix_thread_target::mourn_inferior ()
> {
>   target_ops *beneath = this->beneath ();
>
>-  pd_deactivate ();
>+  pd_disable ();
>   beneath->mourn_inferior ();
> }

Why is this necessary?  If it is, do we even need two
separate pd_deactivate and pd_disable routines any more?

>@@ -618,6 +618,16 @@ solib_aix_bfd_open (const char *pathname)
>       if (member_name == bfd_get_filename (object_bfd.get ()))
>        break;
>
>+      std::string s = bfd_get_filename (object_bfd.get ());
>+
>+      /* For every inferior after first int bfd system we
>+       will have the pathname instead of the member name
>+       registered. Hence the below condition exists.  */
>+
>+      if (s.find ('(') != std::string::npos
>+        && s.find (member_name) != std::string::npos)
>+      return object_bfd;

Ah, I guess you also need to ensure the member_name follows
immediately after the '(', otherwise there could be confusion
if the member name happens to be part of the file name as well.


Bye,
Ulrich


[-- Attachment #2: 0001-Fix-multi-thread-debug-bug-in-AIX.patch --]
[-- Type: application/octet-stream, Size: 26021 bytes --]

From 1f5e0d208173d36c6f940e4f98f73f9e93934fe9 Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Tue, 7 Feb 2023 05:42:13 -0600
Subject: [PATCH] Fix Multi thread debug bug fix in AIX

In the recent commit 98ed24fb35d89eb20179edf6c12f599c7a9e228e made by Mr. Tom there is a change  in aix-thread.c file that changes

static_cast <aix_thread_info *> in gdb to gdb::checked_static_cast <aix_thread_info *>

AIX folks using the latest version will not be able to debug multi thread programs as a result of it

The error in AIX is as follows:-

internal-error: checked_static_cast: Assertion 'result != nullptr' failed.

The reason is that once the threads are syncronised with sync_threadlists () and threads are added with priv -

We iterate over threads to get the thread who caused the event and return its ptid

However the all_threads_safe () function will always return NULL as thread is yet to be updated in that list

So what happens is we were not setting the top target as threads as

Shared library was not loaded for a new process and

gcount was not counted per process

This patch is a fix for the same.
---
 gdb/aix-thread.c | 327 +++++++++++++++++++++++++++--------------------
 gdb/solib-aix.c  |  14 ++
 2 files changed, 205 insertions(+), 136 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index e556c153576..1d800f8b7be 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -68,10 +68,6 @@ static bool debug_aix_thread;
 #define pthdb_tid_t	tid_t
 #endif
 
-/* Return whether to treat PID as a debuggable thread id.  */
-
-#define PD_TID(ptid)	(pd_active && ptid.tid () != 0)
-
 /* Success and failure values returned by pthdb callbacks.  */
 
 #define PDC_SUCCESS	PTHDB_SUCCESS
@@ -144,24 +140,6 @@ class aix_thread_target final : public target_ops
 
 static aix_thread_target aix_thread_ops;
 
-/* Address of the function that libpthread will call when libpthdebug
-   is ready to be initialized.  */
-
-static CORE_ADDR pd_brk_addr;
-
-/* Whether the current application is debuggable by pthdb.  */
-
-static int pd_able = 0;
-
-/* Whether a threaded application is being debugged.  */
-
-static int pd_active = 0;
-
-/* Whether the current architecture is 64-bit.  
-   Only valid when pd_able is true.  */
-
-static int arch64;
-
 /* Forward declarations for pthdb callbacks.  */
 
 static int pdc_symbol_addrs (pthdb_user_t, pthdb_symbol_t *, int);
@@ -191,9 +169,66 @@ static pthdb_callbacks_t pd_callbacks = {
   NULL
 };
 
-/* Current pthdb session.  */
+/* Aix variable structure.  */
+struct aix_thread_variables 
+{
+  /* Whether the current application is debuggable by pthdb.  */
+  int pd_able;
+
+  /* Whether a threaded application is being debugged.  */
+  int pd_active;
+
+  /* Current pthdb session.  */
+  pthdb_session_t pd_session;
+
+  /* Address of the function that libpthread will call when libpthdebug
+   is ready to be initialized.  */
+  CORE_ADDR pd_brk_addr;
+
+  /* Whether the current architecture is 64-bit.
+   Only valid when pd_able is true.  */
+  int arch64;
+};
+
+/* Key to our per-inferior data.  */
+static const registry<inferior>::key<aix_thread_variables>
+  aix_thread_variables_handle;
 
-static pthdb_session_t pd_session;
+/* Function to Get aix_thread_variables data.  */
+static struct aix_thread_variables*
+get_aix_thread_variables_data (struct inferior *inf)
+{
+  if (inf == NULL)
+    return NULL;
+
+  struct aix_thread_variables* data;
+
+  data = aix_thread_variables_handle.get (inf);
+  if (data == NULL)
+    data = aix_thread_variables_handle.emplace (inf);
+
+  return data;
+}
+
+/* Helper to get data for ptid in a function.  */
+
+static struct aix_thread_variables*
+get_thread_data_helper_for_ptid (ptid_t ptid)
+{
+  inferior *inf = find_inferior_ptid (current_inferior ()->process_target (),
+					ptid);
+  return get_aix_thread_variables_data (inf);
+}
+
+/* Helper to get data for pid in a function.  */
+
+static struct aix_thread_variables*
+get_thread_data_helper_for_pid (pid_t pid)
+{
+  inferior *inf = find_inferior_pid (current_inferior ()->process_target (),
+                                        pid);
+  return get_aix_thread_variables_data (inf);
+}
 
 /* Return a printable representation of pthdebug function return
    STATUS.  */
@@ -318,7 +353,7 @@ pid_to_prc (ptid_t *ptidp)
   ptid_t ptid;
 
   ptid = *ptidp;
-  if (PD_TID (ptid))
+  if (ptid.tid () != 0)
     *ptidp = ptid_t (ptid.pid ());
 }
 
@@ -389,6 +424,9 @@ pdc_read_regs (pthdb_user_t user_current_pid,
   double fprs[ppc_num_fprs];
   struct ptxsprs sprs64;
   struct ptsprs sprs32;
+  struct aix_thread_variables *data;
+  
+  data = get_thread_data_helper_for_pid (user_current_pid);
   
   if (debug_aix_thread)
     gdb_printf (gdb_stdlog, "pdc_read_regs tid=%d flags=%s\n",
@@ -397,7 +435,7 @@ pdc_read_regs (pthdb_user_t user_current_pid,
   /* General-purpose registers.  */
   if (flags & PTHDB_FLAG_GPRS)
     {
-      if (arch64)
+      if (data->arch64)
 	{
 	  if (!ptrace64aix (PTT_READ_GPRS, tid, 
 			    (unsigned long) gprs64, 0, NULL))
@@ -423,7 +461,7 @@ pdc_read_regs (pthdb_user_t user_current_pid,
   /* Special-purpose registers.  */
   if (flags & PTHDB_FLAG_SPRS)
     {
-      if (arch64)
+      if (data->arch64)
 	{
 	  if (!ptrace64aix (PTT_READ_SPRS, tid, 
 			    (unsigned long) &sprs64, 0, NULL))
@@ -456,6 +494,10 @@ pdc_write_regs (pthdb_user_t user_current_pid,
      this is needed, I have implemented what I think it should do,
      however this code is untested.  */
 
+  struct aix_thread_variables *data;
+  
+  data = get_thread_data_helper_for_pid (user_current_pid);
+
   if (debug_aix_thread)
     gdb_printf (gdb_stdlog, "pdc_write_regs tid=%d flags=%s\n",
 		(int) tid, hex_string (flags));
@@ -463,7 +505,7 @@ pdc_write_regs (pthdb_user_t user_current_pid,
   /* General-purpose registers.  */
   if (flags & PTHDB_FLAG_GPRS)
     {
-      if (arch64)
+      if (data->arch64)
 	ptrace64aix (PTT_WRITE_GPRS, tid, 
 		     (unsigned long) context->gpr, 0, NULL);
       else
@@ -479,7 +521,7 @@ pdc_write_regs (pthdb_user_t user_current_pid,
   /* Special-purpose registers.  */
   if (flags & PTHDB_FLAG_SPRS)
     {
-      if (arch64)
+      if (data->arch64)
 	{
 	  ptrace64aix (PTT_WRITE_SPRS, tid, 
 		       (unsigned long) &context->msr, 0, NULL);
@@ -508,14 +550,12 @@ pdc_read_data (pthdb_user_t user_current_pid, void *buf,
   /* This is needed to eliminate the dependency of current thread
      which is null so that thread reads the correct target memory.  */
   {
-    scoped_restore_current_thread restore_current_thread;
+    scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
     /* Before the first inferior is added, we pass inferior_ptid.pid ()
        from pd_enable () which is 0.  There is no need to switch threads
        during first initialisation.  In the rest of the callbacks the
        current thread needs to be correct.  */
-    if (user_current_pid != 0)
-      switch_to_thread (current_inferior ()->process_target (),
-			ptid_t (user_current_pid));
+    inferior_ptid = ptid_t (user_current_pid);
     status = target_read_memory (addr, (gdb_byte *) buf, len);
   }
   ret = status == 0 ? PDC_SUCCESS : PDC_FAILURE;
@@ -639,39 +679,6 @@ pcmp (const void *p1v, const void *p2v)
   return p1->pthid < p2->pthid ? -1 : p1->pthid > p2->pthid;
 }
 
-/* iterate_over_threads() callback for counting GDB threads.
-
-   Do not count the main thread (whose tid is zero).  This matches
-   the list of threads provided by the pthreaddebug library, which
-   does not include that main thread either, and thus allows us
-   to compare the two lists.  */
-
-static int
-giter_count (struct thread_info *thread, void *countp)
-{
-  if (PD_TID (thread->ptid))
-    (*(int *) countp)++;
-  return 0;
-}
-
-/* iterate_over_threads() callback for accumulating GDB thread pids.
-
-   Do not include the main thread (whose tid is zero).  This matches
-   the list of threads provided by the pthreaddebug library, which
-   does not include that main thread either, and thus allows us
-   to compare the two lists.  */
-
-static int
-giter_accum (struct thread_info *thread, void *bufp)
-{
-  if (PD_TID (thread->ptid))
-    {
-      **(struct thread_info ***) bufp = thread;
-      (*(struct thread_info ***) bufp)++;
-    }
-  return 0;
-}
-
 /* ptid comparison function */
 
 static int
@@ -719,7 +726,10 @@ get_signaled_thread (int pid)
 		    sizeof (thrinf), &ktid, 1) != 1)
 	break;
 
-      if (thrinf.ti_cursig == SIGTRAP)
+      /* We also need to keep in mind Trap and interrupt or any
+         signal that needs to be handled in pd_update ().  */
+
+      if (thrinf.ti_cursig)
 	return thrinf.ti_tid;
     }
 
@@ -750,6 +760,11 @@ sync_threadlists (int pid)
   pthdb_pthread_t pdtid;
   pthread_t pthid;
   pthdb_tid_t tid;
+  process_stratum_target *proc_target
+            = current_inferior ()->process_target ();
+  thread_info  *tp;
+  struct aix_thread_variables *data;
+  data = get_thread_data_helper_for_pid (pid);
 
   /* Accumulate an array of libpthdebug threads sorted by pthread id.  */
 
@@ -759,11 +774,11 @@ sync_threadlists (int pid)
 
   for (cmd = PTHDB_LIST_FIRST;; cmd = PTHDB_LIST_NEXT)
     {
-      status = pthdb_pthread (pd_session, &pdtid, cmd);
+      status = pthdb_pthread (data->pd_session, &pdtid, cmd);
       if (status != PTHDB_SUCCESS || pdtid == PTHDB_INVALID_PTHREAD)
 	break;
 
-      status = pthdb_pthread_ptid (pd_session, pdtid, &pthid);
+      status = pthdb_pthread_ptid (data->pd_session, pdtid, &pthid);
       if (status != PTHDB_SUCCESS || pthid == PTHDB_INVALID_PTID)
 	continue;
 
@@ -780,7 +795,7 @@ sync_threadlists (int pid)
 
   for (pi = 0; pi < pcount; pi++)
     {
-      status = pthdb_pthread_tid (pd_session, pbuf[pi].pdtid, &tid);
+      status = pthdb_pthread_tid (data->pd_session, pbuf[pi].pdtid, &tid);
       if (status != PTHDB_SUCCESS)
 	tid = PTHDB_INVALID_TID;
       pbuf[pi].tid = tid;
@@ -790,10 +805,17 @@ sync_threadlists (int pid)
 
   /* Accumulate an array of GDB threads sorted by pid.  */
 
+  /* gcount is GDB thread count and pcount is pthreadlib thread count.  */
+
   gcount = 0;
-  iterate_over_threads (giter_count, &gcount);
+  for (thread_info *tp : all_threads (proc_target, ptid_t (pid)))
+    gcount++;
   g = gbuf = XNEWVEC (struct thread_info *, gcount);
-  iterate_over_threads (giter_accum, &g);
+  for (thread_info *tp : all_threads (proc_target, ptid_t (pid)))
+  {
+    *g = tp;
+    *g++;
+  }
   qsort (gbuf, gcount, sizeof *gbuf, gcmp);
 
   /* Apply differences between the two arrays to GDB's thread list.  */
@@ -810,8 +832,6 @@ sync_threadlists (int pid)
 	  priv->pdtid = pbuf[pi].pdtid;
 	  priv->tid = pbuf[pi].tid;
 
-	  process_stratum_target *proc_target
-	    = current_inferior ()->process_target ();
 	  thread = add_thread_with_info (proc_target,
 					 ptid_t (pid, 0, pbuf[pi].pthid),
 					 priv);
@@ -841,13 +861,28 @@ sync_threadlists (int pid)
 	    }
 	  else if (cmp_result > 0)
 	    {
-	      delete_thread (gbuf[gi]);
-	      gi++;
+	      /* This is to make the main process thread now look
+		 like a thread.  */
+
+	      if (gptid.is_pid ())
+	      {
+		thread_change_ptid (proc_target, gptid, pptid);
+		aix_thread_info *priv = new aix_thread_info;
+		priv->pdtid = pbuf[pi].pdtid;
+		priv->tid = pbuf[pi].tid;
+		tp = find_thread_ptid (proc_target, pptid);
+		tp->priv.reset (priv);
+		pi++;
+		gi++;
+	      }
+	      else
+	      {
+		delete_thread (gbuf[gi]);
+		gi++;
+	      }
 	    }
 	  else
 	    {
-	      process_stratum_target *proc_target
-		= current_inferior ()->process_target ();
 	      thread = add_thread (proc_target, pptid);
 
 	      aix_thread_info *priv = new aix_thread_info;
@@ -887,11 +922,14 @@ pd_update (int pid)
   ptid_t ptid;
   pthdb_tid_t tid;
   struct thread_info *thread = NULL;
+  struct aix_thread_variables *data;
+
+  data = get_thread_data_helper_for_pid (pid);
 
-  if (!pd_active)
+  if (!data->pd_active)
     return ptid_t (pid);
 
-  status = pthdb_session_update (pd_session);
+  status = pthdb_session_update (data->pd_session);
   if (status != PTHDB_SUCCESS)
     return ptid_t (pid);
 
@@ -918,31 +956,20 @@ static ptid_t
 pd_activate (int pid)
 {
   int status;
+  struct aix_thread_variables *data;
+  data = get_thread_data_helper_for_pid (pid);
 		
-  status = pthdb_session_init (pid, arch64 ? PEM_64BIT : PEM_32BIT,
+  status = pthdb_session_init (pid, data->arch64 ? PEM_64BIT : PEM_32BIT,
 			       PTHDB_FLAG_REGS, &pd_callbacks, 
-			       &pd_session);
+			       &data->pd_session);
   if (status != PTHDB_SUCCESS)
     {
       return ptid_t (pid);
     }
-  pd_active = 1;
+  data->pd_active = 1;
   return pd_update (pid);
 }
 
-/* Undo the effects of pd_activate().  */
-
-static void
-pd_deactivate (void)
-{
-  if (!pd_active)
-    return;
-  pthdb_session_destroy (pd_session);
-  
-  pid_to_prc (&inferior_ptid);
-  pd_active = 0;
-}
-
 /* An object file has just been loaded.  Check whether the current
    application is pthreaded, and if so, prepare for thread debugging.  */
 
@@ -952,13 +979,19 @@ pd_enable (void)
   int status;
   char *stub_name;
   struct bound_minimal_symbol ms;
+  struct aix_thread_variables *data;
+
+  if (!inferior_ptid.pid ())
+    return;
+  
+  data = get_thread_data_helper_for_ptid (inferior_ptid);
 
   /* Don't initialize twice.  */
-  if (pd_able)
+  if (data->pd_able)
     return;
 
   /* Check application word size.  */
-  arch64 = register_size (target_gdbarch (), 0) == 8;
+  data->arch64 = register_size (target_gdbarch (), 0) == 8;
 
   /* Check whether the application is pthreaded.  */
   stub_name = NULL;
@@ -972,13 +1005,13 @@ pd_enable (void)
   ms = lookup_minimal_symbol (stub_name, NULL, NULL);
   if (ms.minsym == NULL)
     return;
-  pd_brk_addr = ms.value_address ();
-  if (!create_thread_event_breakpoint (target_gdbarch (), pd_brk_addr))
+  data->pd_brk_addr = ms.value_address ();
+  if (!create_thread_event_breakpoint (target_gdbarch (), data->pd_brk_addr))
     return;
 
   /* Prepare for thread debugging.  */
   current_inferior ()->push_target (&aix_thread_ops);
-  pd_able = 1;
+  data->pd_able = 1; 
 
   /* If we're debugging a core file or an attached inferior, the
      pthread library may already have been initialized, so try to
@@ -991,28 +1024,31 @@ pd_enable (void)
 static void
 pd_disable (void)
 {
-  if (!pd_able)
+  struct aix_thread_variables *data;
+  data = get_thread_data_helper_for_ptid (inferior_ptid);
+
+  if (!data->pd_able)
     return;
-  if (pd_active)
-    pd_deactivate ();
-  pd_able = 0;
+  if (!data->pd_active)
+    return;
+  pthdb_session_destroy (data->pd_session);
+ 
+  pid_to_prc (&inferior_ptid);
+  data->pd_active = 0;
+  data->pd_able = 0;
   current_inferior ()->unpush_target (&aix_thread_ops);
 }
 
 /* new_objfile observer callback.
 
    If OBJFILE is non-null, check whether a threaded application is
-   being debugged, and if so, prepare for thread debugging.
-
-   If OBJFILE is null, stop debugging threads.  */
+   being debugged, and if so, prepare for thread debugging.  */
 
 static void
 new_objfile (struct objfile *objfile)
 {
   if (objfile)
     pd_enable ();
-  else
-    pd_disable ();
 }
 
 /* Attach to process specified by ARGS.  */
@@ -1042,8 +1078,11 @@ aix_thread_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
 {
   struct thread_info *thread;
   pthdb_tid_t tid[2];
+  struct aix_thread_variables *data;
+  
+  data = get_thread_data_helper_for_ptid (ptid);
 
-  if (!PD_TID (ptid))
+  if (ptid.tid () == 0)
     {
       scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
       
@@ -1065,7 +1104,7 @@ aix_thread_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
 	       ptid.lwp ());
       tid[1] = 0;
 
-      if (arch64)
+      if (data->arch64)
 	ptrace64aix (PTT_CONTINUE, tid[0], (long long) 1,
 		     gdb_signal_to_host (sig), (PTRACE_TYPE_ARG5) tid);
       else
@@ -1082,6 +1121,7 @@ ptid_t
 aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
 			 target_wait_flags options)
 {
+  struct aix_thread_variables *data;
   {
     pid_to_prc (&ptid);
 
@@ -1095,8 +1135,10 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
      pid-only ptids.  */
   gdb_assert (ptid.is_pid ());
 
+  data = get_thread_data_helper_for_ptid (ptid);
+
   /* Check whether libpthdebug might be ready to be initialized.  */
-  if (!pd_active && status->kind () == TARGET_WAITKIND_STOPPED
+  if (!data->pd_active && status->kind () == TARGET_WAITKIND_STOPPED
       && status->sig () == GDB_SIGNAL_TRAP)
     {
       process_stratum_target *proc_target
@@ -1105,7 +1147,7 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
       struct gdbarch *gdbarch = regcache->arch ();
 
       if (regcache_read_pc (regcache)
-	  - gdbarch_decr_pc_after_break (gdbarch) == pd_brk_addr)
+	  - gdbarch_decr_pc_after_break (gdbarch) == data->pd_brk_addr)
 	return pd_activate (ptid.pid ());
     }
 
@@ -1229,18 +1271,20 @@ fetch_regs_user_thread (struct regcache *regcache, pthdb_pthread_t pdtid)
   ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep> (gdbarch);
   int status, i;
   pthdb_context_t ctx;
+  struct aix_thread_variables *data;
+  data = get_thread_data_helper_for_ptid (inferior_ptid);
 
   if (debug_aix_thread)
     gdb_printf (gdb_stdlog, 
 		"fetch_regs_user_thread %lx\n", (long) pdtid);
-  status = pthdb_pthread_context (pd_session, pdtid, &ctx);
+  status = pthdb_pthread_context (data->pd_session, pdtid, &ctx);
   if (status != PTHDB_SUCCESS)
     error (_("aix-thread: fetch_registers: pthdb_pthread_context returned %s"),
 	   pd_status2str (status));
 
   /* General-purpose registers.  */
 
-  if (arch64)
+  if (data->arch64)
     supply_gprs64 (regcache, ctx.gpr);
   else
     for (i = 0; i < ppc_num_gprs; i++)
@@ -1253,7 +1297,7 @@ fetch_regs_user_thread (struct regcache *regcache, pthdb_pthread_t pdtid)
 
   /* Special registers.  */
 
-  if (arch64)
+  if (data->arch64)
     supply_sprs64 (regcache, ctx.iar, ctx.msr, ctx.cr, ctx.lr, ctx.ctr,
 			     ctx.xer, ctx.fpscr);
   else
@@ -1288,18 +1332,21 @@ fetch_regs_kernel_thread (struct regcache *regcache, int regno,
   struct ptxsprs sprs64;
   struct ptsprs sprs32;
   int i;
+  struct aix_thread_variables *data;
+  
+  data = get_thread_data_helper_for_ptid (regcache->ptid ());
 
   if (debug_aix_thread)
     gdb_printf (gdb_stdlog,
 		"fetch_regs_kernel_thread tid=%lx regno=%d arch64=%d\n",
-		(long) tid, regno, arch64);
+		(long) tid, regno, data->arch64);
 
   /* General-purpose registers.  */
   if (regno == -1
       || (tdep->ppc_gp0_regnum <= regno
 	  && regno < tdep->ppc_gp0_regnum + ppc_num_gprs))
     {
-      if (arch64)
+      if (data->arch64)
 	{
 	  if (!ptrace64aix (PTT_READ_GPRS, tid, 
 			    (unsigned long) gprs64, 0, NULL))
@@ -1331,7 +1378,7 @@ fetch_regs_kernel_thread (struct regcache *regcache, int regno,
 
   if (regno == -1 || special_register_p (gdbarch, regno))
     {
-      if (arch64)
+      if (data->arch64)
 	{
 	  if (!ptrace64aix (PTT_READ_SPRS, tid, 
 			    (unsigned long) &sprs64, 0, NULL))
@@ -1363,7 +1410,7 @@ aix_thread_target::fetch_registers (struct regcache *regcache, int regno)
   struct thread_info *thread;
   pthdb_tid_t tid;
 
-  if (!PD_TID (regcache->ptid ()))
+  if (regcache->ptid ().tid () == 0)
     beneath ()->fetch_registers (regcache, regno);
   else
     {
@@ -1511,6 +1558,8 @@ store_regs_user_thread (const struct regcache *regcache, pthdb_pthread_t pdtid)
   pthdb_context_t ctx;
   uint32_t int32;
   uint64_t int64;
+  struct aix_thread_variables *data;
+  data = get_thread_data_helper_for_ptid (inferior_ptid);
 
   if (debug_aix_thread)
     gdb_printf (gdb_stdlog, 
@@ -1518,7 +1567,7 @@ store_regs_user_thread (const struct regcache *regcache, pthdb_pthread_t pdtid)
 
   /* Retrieve the thread's current context for its non-register
      values.  */
-  status = pthdb_pthread_context (pd_session, pdtid, &ctx);
+  status = pthdb_pthread_context (data->pd_session, pdtid, &ctx);
   if (status != PTHDB_SUCCESS)
     error (_("aix-thread: store_registers: pthdb_pthread_context returned %s"),
 	   pd_status2str (status));
@@ -1528,7 +1577,7 @@ store_regs_user_thread (const struct regcache *regcache, pthdb_pthread_t pdtid)
   for (i = 0; i < ppc_num_gprs; i++)
     if (REG_VALID == regcache->get_register_status (tdep->ppc_gp0_regnum + i))
       {
-	if (arch64)
+	if (data->arch64)
 	  {
 	    regcache->raw_collect (tdep->ppc_gp0_regnum + i, (void *) &int64);
 	    ctx.gpr[i] = int64;
@@ -1545,7 +1594,7 @@ store_regs_user_thread (const struct regcache *regcache, pthdb_pthread_t pdtid)
     fill_fprs (regcache, ctx.fpr);
 
   /* Special registers (always kept in ctx as 64 bits).  */
-  if (arch64)
+  if (data->arch64)
     {
       fill_sprs64 (regcache, &ctx.iar, &ctx.msr, &ctx.cr, &ctx.lr, &ctx.ctr,
 			     &ctx.xer, &ctx.fpscr);
@@ -1576,7 +1625,7 @@ store_regs_user_thread (const struct regcache *regcache, pthdb_pthread_t pdtid)
 	ctx.fpscr = tmp_fpscr;
     }
 
-  status = pthdb_pthread_setcontext (pd_session, pdtid, &ctx);
+  status = pthdb_pthread_setcontext (data->pd_session, pdtid, &ctx);
   if (status != PTHDB_SUCCESS)
     error (_("aix-thread: store_registers: "
 	     "pthdb_pthread_setcontext returned %s"),
@@ -1602,6 +1651,9 @@ store_regs_kernel_thread (const struct regcache *regcache, int regno,
   double fprs[ppc_num_fprs];
   struct ptxsprs sprs64;
   struct ptsprs  sprs32;
+  struct aix_thread_variables *data;
+  
+  data = get_thread_data_helper_for_ptid (regcache->ptid ());
 
   if (debug_aix_thread)
     gdb_printf (gdb_stdlog, 
@@ -1613,7 +1665,7 @@ store_regs_kernel_thread (const struct regcache *regcache, int regno,
       || (tdep->ppc_gp0_regnum <= regno
 	  && regno < tdep->ppc_gp0_regnum + ppc_num_fprs))
     {
-      if (arch64)
+      if (data->arch64)
 	{
 	  /* Pre-fetch: some regs may not be in the cache.  */
 	  ptrace64aix (PTT_READ_GPRS, tid, (unsigned long) gprs64, 0, NULL);
@@ -1646,7 +1698,7 @@ store_regs_kernel_thread (const struct regcache *regcache, int regno,
 
   if (regno == -1 || special_register_p (gdbarch, regno))
     {
-      if (arch64)
+      if (data->arch64)
 	{
 	  /* Pre-fetch: some registers won't be in the cache.  */
 	  ptrace64aix (PTT_READ_SPRS, tid, 
@@ -1703,7 +1755,7 @@ aix_thread_target::store_registers (struct regcache *regcache, int regno)
   struct thread_info *thread;
   pthdb_tid_t tid;
 
-  if (!PD_TID (regcache->ptid ()))
+  if (regcache->ptid ().tid () == 0)
     beneath ()->store_registers (regcache, regno);
   else
     {
@@ -1741,7 +1793,7 @@ aix_thread_target::mourn_inferior ()
 {
   target_ops *beneath = this->beneath ();
 
-  pd_deactivate ();
+  pd_disable ();
   beneath->mourn_inferior ();
 }
 
@@ -1750,7 +1802,7 @@ aix_thread_target::mourn_inferior ()
 bool
 aix_thread_target::thread_alive (ptid_t ptid)
 {
-  if (!PD_TID (ptid))
+  if (ptid.tid () == 0)
     return beneath ()->thread_alive (ptid);
 
   /* We update the thread list every time the child stops, so all
@@ -1766,7 +1818,7 @@ aix_thread_target::thread_alive (ptid_t ptid)
 std::string
 aix_thread_target::pid_to_str (ptid_t ptid)
 {
-  if (!PD_TID (ptid))
+  if (ptid.tid () == 0)
     return beneath ()->pid_to_str (ptid);
 
   return string_printf (_("Thread %s"), pulongest (ptid.tid ()));
@@ -1786,8 +1838,11 @@ aix_thread_target::extra_thread_info (struct thread_info *thread)
   pthdb_detachstate_t detachstate;
   int cancelpend;
   static char *ret = NULL;
+  struct aix_thread_variables *data;
+
+  data = get_thread_data_helper_for_ptid (thread->ptid);
 
-  if (!PD_TID (thread->ptid))
+  if (thread->ptid.tid () == 0)
     return NULL;
 
   string_file buf;
@@ -1800,24 +1855,24 @@ aix_thread_target::extra_thread_info (struct thread_info *thread)
     /* i18n: Like "thread-identifier %d, [state] running, suspended" */
     buf.printf (_("tid %d"), (int)tid);
 
-  status = pthdb_pthread_state (pd_session, pdtid, &state);
+  status = pthdb_pthread_state (data->pd_session, pdtid, &state);
   if (status != PTHDB_SUCCESS)
     state = PST_NOTSUP;
   buf.printf (", %s", state2str (state));
 
-  status = pthdb_pthread_suspendstate (pd_session, pdtid, 
+  status = pthdb_pthread_suspendstate (data->pd_session, pdtid, 
 				       &suspendstate);
   if (status == PTHDB_SUCCESS && suspendstate == PSS_SUSPENDED)
     /* i18n: Like "Thread-Id %d, [state] running, suspended" */
     buf.printf (_(", suspended"));
 
-  status = pthdb_pthread_detachstate (pd_session, pdtid, 
+  status = pthdb_pthread_detachstate (data->pd_session, pdtid, 
 				      &detachstate);
   if (status == PTHDB_SUCCESS && detachstate == PDS_DETACHED)
     /* i18n: Like "Thread-Id %d, [state] running, detached" */
     buf.printf (_(", detached"));
 
-  pthdb_pthread_cancelpend (pd_session, pdtid, &cancelpend);
+  pthdb_pthread_cancelpend (data->pd_session, pdtid, &cancelpend);
   if (status == PTHDB_SUCCESS && cancelpend)
     /* i18n: Like "Thread-Id %d, [state] running, cancel pending" */
     buf.printf (_(", cancel pending"));
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index f483f54de13..6be81064ebd 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -618,6 +618,20 @@ solib_aix_bfd_open (const char *pathname)
       if (member_name == bfd_get_filename (object_bfd.get ()))
 	break;
 
+      std::string s = bfd_get_filename (object_bfd.get ());
+
+      /* For every inferior after first int bfd system we 
+	 will have the pathname instead of the member name
+	 registered. Hence the below condition exists.  */
+
+      if (s.find ('(') != std::string::npos)
+	{
+	  int pos = s.find ('(');
+	  int len = s.find (')') - s.find ('(');
+	  if (s.substr (pos+1, len-1) == member_name) 
+	    return object_bfd;
+	}
+
       object_bfd = gdb_bfd_openr_next_archived_file (archive_bfd.get (),
 						     object_bfd.get ());
     }
-- 
2.31.1