[PATCH] Fix AIX thread exit events not being reported and UI to show kernel thread ID.

[PATCH] Fix AIX thread exit events not being reported and UI to show kernel thread ID.

[Aditya Kamath1]

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

Respected community members,

Hi,

Thank you for the comments in the [RFC] thread<https://sourceware.org/pipermail/gdb-patches/2024-April/208104.html>.

Please find attached the patch. (See: 0001-Fix-AIX-thread-exit-events-not-being-reported-and-UI.patch)


In AIX, we were not reporting a thread exit event due to which we had a bad UI.

Hello World
Hello World
Hello World
[New Thread 258]
[New Thread 515]
[New Thread 772]
Thread 1 received signal SIGINT, Interrupt.
0xd0611d70 in _p_nsleep () from /usr/lib/libpthreads.a(_shr_xpg5.o)
(gdb) info threads
  Id   Target Id                                          Frame
* 1    Thread 1 (tid 26607979) (tid 26607979, running)    0xd0611d70 in _p_nsleep () from /usr/lib/libpthreads.a(_shr_xpg5.o)
  2    Thread 258 (tid 30998799) (tid 30998799, finished) aix-thread: ptrace (52, 30998799) returned -1 (errno = 3 The process does not exist.)

More on this can be read here<https://sourceware.org/pipermail/gdb-patches/2024-April/208104.html>. [https://sourceware.org/pipermail/gdb-patches/2024-April/208104.html]

So, consider program 1 pasted below this email.

This program creates 6 threads in total.

In AIX once a thread reaches terminated state the information can be obtained using pthdb_pthread_state()

In this patch if the state is terminated then we do not want pbuf to keep the thread in its list so that we can delete the same from GDB’s list i.e. gbuf in sync_threadlists ().

But we faced one problem.


>Linux raises a ptrace() stop when a new thread is created.  When using >libthread_db

>I believe you should be placing a breakpoint in your thread library so that you ?>can

>capture new threads in userspace before they start executing (e.g. in the >internal

>routine in your thread library that eventually calls the thread's start routine).

As John mentioned in the RFC thread we need to set a breakpoint in the thread library or similar to capture the new threads before it begins execution.

This is currently not happening in AIX. Though in the pd_activate () we are successful in pthdb_session_init () before the threads begin execution, in sync_threadlists (), pthdb_pthread () and pthdb_pthread_ptid () return invalid thread ID’s for thread except for the main thread.

This results in break of the for loop and we not capturing the remaining 6 new threads, since pbuf now has only one vector that is the main thread.

So the next time when we come to sync_threadlists () we do have all the threads in pbuf.
But by then these 6 threads completed their execution and are in PST_TERM state i.e. terminated state.

In this stage we did not inform users that the threads are born. At this stage we also have to show the threads exited or terminated.

In order to fix this race I used a variable threads_reported to keep track of how many threads have reported i.e we displayed new thread information. So I want to make use of this to show new threads even though the six threads are in terminated state at this stage.

The condition
if (state == PST_TERM && data->threads_reported > 0)
checks if we are at PST_TERM and if we reported the new threads. If we did not we add the new threads in the pbuf list to allow GDB to display new threads.

For each new thread we increase the threads_reported variable and decrement it for every delete.

By this way we make sure that the next time post the new thread event [Adding the threads to GDB list] display, when we are in sync_threadlists (), the condition
if (state == PST_TERM && data->threads_reported > 0)
holds true and we do not have those terminated threads in the pbuf. This will make sure the terminated threads are removed.

Sample output of GDB of program 1 is pasted below.

Reading symbols from //gdb_tests/continue-pending-status_exit_test...
(gdb) r
Starting program: /gdb_tests/continue-pending-status_exit_test
More threads
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
[New Thread 258 (tid 32178533)]
[New Thread 515 (tid 30343651)]
[New Thread 772 (tid 33554909)]
[New Thread 1029 (tid 24969489)]
[New Thread 1286 (tid 18153945)]
[New Thread 1543 (tid 30736739)]
[Thread 258 (tid 32178533) exited]
[Thread 515 (tid 30343651) exited]
[Thread 772 (tid 33554909) exited]
[Thread 1029 (tid 24969489) exited]
[Thread 1286 (tid 18153945) exited]
[Thread 1543 (tid 30736739) exited]

Thread 1 received signal SIGINT, Interrupt.
0xd0611d70 in _p_nsleep () from /usr/lib/libpthread.a(_shr_xpg5.o)
(gdb) inof threads
Undefined command: "inof".  Try "help".
(gdb) info threads
  Id   Target Id                           Frame
* 1    Thread 1 (tid 31326579) ([running]) 0xd0611d70 in _p_nsleep () from /usr/lib/libpthread.a(_shr_xpg5.o)
(gdb) q
A debugging session is active.

        Inferior 1 [process 9437570] will be killed.

Quit anyway? (y or n) y


All these workarounds are because we have problems in catching the new thread event before the threads start executing. If we have that in place, perhaps we can optimise this patch further.

Kindly let me know your feedback for this patch.

Also, some minor changes in the UI are made in this patch so that thread debugging information is the same like Linux.

Have a nice day ahead.

Thanks and regards,
Aditya.

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <pthread.h>
#include <assert.h>

pthread_barrier_t barrier;

#define NUM_THREADS 3

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);

  printf ("Hello World \n"); /* break here */
}

int
main (void)
{
  int i;

  alarm (300);

  pthread_barrier_init (&barrier, NULL, NUM_THREADS);

  for (i = 0; i < NUM_THREADS; i++)
    {
      pthread_t thread;
      int res;

      res = pthread_create (&thread, NULL,
                            thread_function, NULL);
      assert (res == 0);
    }

  printf ("More threads \n");
  for (i = 0; i < NUM_THREADS; i++)
    {
      pthread_t thread;
      int res;

      res = pthread_create (&thread, NULL,
                thread_function, NULL);
      assert (res == 0);
    }
  while (1)
    sleep (1);

  return 0;
}



[-- Attachment #2: 0001-Fix-AIX-thread-exit-events-not-being-reported-and-UI.patch --]
[-- Type: application/octet-stream, Size: 9238 bytes --]

From f5d0db4320ddab7cc52d56bb8edf626a0fc50ad4 Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Thu, 18 Apr 2024 07:46:13 -0500
Subject: [PATCH] Fix AIX thread exit events not being reported and UI to show
 kernel thread ID.

In AIX when a thread exits we were not showing that a thread exit event happened
and GDB continued to keep the terminated threads.

If we have terminated threads then the UI on info threads command will look like
(gdb) info threads
  Id   Target Id                                          Frame
* 1    Thread 1 (tid 26607979, running)    0xd0611d70 in _p_nsleep () from /usr/lib/libpthreads.a(_shr_xpg5.o)
  2    Thread 258 (tid 30998799, finished) aix-thread: ptrace (52, 30998799) returned -1 (errno = 3 The process does not exist.)

If we see the frame is not getting displayed correctly.

The reason for the same is that in AIX we were not managing thread states. In particular we do not know
when a thread terminates.

The reason being in sync_threadlists () the pbuf and gbuf lists remain the same though certain threads exit.

This patch is a fix to the same.

Also certain UI is changed.

On a new thread born and exit the UI in AIX will be similar to Linux with both user and kernel thread information.

[New Thread 258 (tid 32178533)]
[New Thread 515 (tid 30343651)]
[New Thread 772 (tid 33554909)]
[New Thread 1029 (tid 24969489)]
[New Thread 1286 (tid 18153945)]
[New Thread 1543 (tid 30736739)]
[Thread 258 (tid 32178533) exited]
[Thread 515 (tid 30343651) exited]
[Thread 772 (tid 33554909) exited]
[Thread 1029 (tid 24969489) exited]
[Thread 1286 (tid 18153945) exited]
[Thread 1543 (tid 30736739) exited]

and info threads will look like
(gdb) info threads
  Id   Target Id                           Frame
* 1    Thread 1 (tid 31326579) ([running]) 0xd0611d70 in _p_nsleep () from /usr/lib/libpthread.a(_shr_xpg5.o)
---
 gdb/aix-thread.c | 91 ++++++++++++++++++++++++++++++------------------
 1 file changed, 57 insertions(+), 34 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index c70bd82bc24..5ccccf02879 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -55,6 +55,7 @@
 #include <sys/reg.h>
 #include <sched.h>
 #include <sys/pthdebug.h>
+#include <vector>
 
 #if !HAVE_DECL_GETTHRDS
 extern int getthrds (pid_t, struct thrdsinfo64 *, int, tid_t *, int);
@@ -190,6 +191,9 @@ struct aix_thread_variables
   /* Whether the current architecture is 64-bit.
    Only valid when pd_able is true.  */
   int arch64;
+
+  /* Describes the number of thread born events have been reported.  */
+  int threads_reported = 0;
 };
 
 /* Key to our per-inferior data.  */
@@ -740,11 +744,9 @@ state2str (pthdb_state_t state)
 /* qsort() comparison function for sorting pd_thread structs by pthid.  */
 
 static int
-pcmp (const void *p1v, const void *p2v)
+pcmp (struct pd_thread p1v, struct pd_thread p2v)
 {
-  struct pd_thread *p1 = (struct pd_thread *) p1v;
-  struct pd_thread *p2 = (struct pd_thread *) p2v;
-  return p1->pthid < p2->pthid ? -1 : p1->pthid > p2->pthid;
+  return p1v.pthid < p2v.pthid;
 }
 
 /* ptid comparison function */
@@ -823,7 +825,6 @@ sync_threadlists (pid_t pid)
 {
   int cmd, status;
   int pcount, psize, pi, gcount, gi;
-  struct pd_thread *pbuf;
   struct thread_info **gbuf, **g, *thread;
   pthdb_pthread_t pdtid;
   pthread_t pthid;
@@ -831,12 +832,13 @@ sync_threadlists (pid_t pid)
   process_stratum_target *proc_target = current_inferior ()->process_target ();
   struct aix_thread_variables *data;
   data = get_thread_data_helper_for_pid (pid);
+  std::vector<pd_thread> pbuf;
+  struct pd_thread aix_pd_thread;
 
   /* Accumulate an array of libpthdebug threads sorted by pthread id.  */
 
   pcount = 0;
   psize = 1;
-  pbuf = XNEWVEC (struct pd_thread, psize);
 
   for (cmd = PTHDB_LIST_FIRST;; cmd = PTHDB_LIST_NEXT)
     {
@@ -848,27 +850,36 @@ sync_threadlists (pid_t pid)
       if (status != PTHDB_SUCCESS || pthid == PTHDB_INVALID_PTID)
 	continue;
 
-      if (pcount == psize)
-	{
-	  psize *= 2;
-	  pbuf = (struct pd_thread *) xrealloc (pbuf, 
-						psize * sizeof *pbuf);
-	}
-      pbuf[pcount].pdtid = pdtid;
-      pbuf[pcount].pthid = pthid;
+      aix_pd_thread.pdtid = pdtid;
+      aix_pd_thread.pthid = pthid;
+      pbuf.push_back (aix_pd_thread);
       pcount++;
     }
 
-  for (pi = 0; pi < pcount; pi++)
+  for (auto it = pbuf.begin (); it != pbuf.end ();)
     {
-      status = pthdb_pthread_tid (data->pd_session, pbuf[pi].pdtid, &tid);
+      status = pthdb_pthread_tid (data->pd_session, (*it).pdtid, &tid);
       if (status != PTHDB_SUCCESS)
 	tid = PTHDB_INVALID_TID;
-      pbuf[pi].tid = tid;
-    }
 
-  qsort (pbuf, pcount, sizeof *pbuf, pcmp);
+      (*it).tid = tid;
+      pthdb_state_t state;
+
+      status = pthdb_pthread_state (data->pd_session, (*it).pdtid, &state);
 
+      /* If the thread is terminated and has not reported its existence
+	 or new thread event, then wait for it to report.  We want users
+	 to know threads were added and then delete. So first add them to
+	 pbuf and the next time sync_threadlists is called delete them since
+	 data->threads_reported is guarenteed to be greater than 0.  */
+
+      if (state == PST_TERM && data->threads_reported > 0)
+	pbuf.erase (it);
+      else
+	++it;
+    }
+
+  std::sort (pbuf.begin (), pbuf.end (), pcmp);
   /* Accumulate an array of GDB threads sorted by pid.  */
 
   /* gcount is GDB thread count and pcount is pthreadlib thread count.  */
@@ -881,6 +892,7 @@ sync_threadlists (pid_t pid)
     *g++ = tp;
   qsort (gbuf, gcount, sizeof *gbuf, gcmp);
 
+  pcount = pbuf.size ();
   /* Apply differences between the two arrays to GDB's thread list.  */
 
   for (pi = gi = 0; pi < pcount || gi < gcount;)
@@ -889,6 +901,12 @@ sync_threadlists (pid_t pid)
 	{
 	  delete_thread (gbuf[gi]);
 	  gi++;
+	  data->threads_reported--;
+
+	  /* Since main thread is reported.  */
+
+	  if (data->threads_reported == 0)
+	    data->threads_reported = 1;
 	}
       else if (gi == gcount)
 	{
@@ -901,6 +919,7 @@ sync_threadlists (pid_t pid)
 					 private_thread_info_up (priv));
 
 	  pi++;
+	  data->threads_reported++;
 	}
       else
 	{
@@ -913,7 +932,6 @@ sync_threadlists (pid_t pid)
 	  tid = pbuf[pi].tid;
 
 	  cmp_result = ptid_cmp (pptid, gptid);
-
 	  if (cmp_result == 0)
 	    {
 	      aix_thread_info *priv = get_aix_thread_info (gbuf[gi]);
@@ -943,6 +961,10 @@ sync_threadlists (pid_t pid)
 		{
 		  delete_thread (gbuf[gi]);
 		  gi++;
+		  data->threads_reported--;
+
+		  if (data->threads_reported == 0)
+		    data->threads_reported = 1;
 		}
 	    }
 	  else
@@ -954,11 +976,11 @@ sync_threadlists (pid_t pid)
 	      priv->pdtid = pdtid;
 	      priv->tid = tid;
 	      pi++;
+	      data->threads_reported++;
 	    }
 	}
     }
 
-  xfree (pbuf);
   xfree (gbuf);
 }
 
@@ -2084,10 +2106,17 @@ aix_thread_target::thread_alive (ptid_t ptid)
 std::string
 aix_thread_target::pid_to_str (ptid_t ptid)
 {
-  if (ptid.tid () == 0)
-    return beneath ()->pid_to_str (ptid);
+  thread_info *thread_info = current_inferior ()->find_thread (ptid);
+
+  if (thread_info != NULL && thread_info->priv != NULL)
+    {
+      aix_thread_info *priv = get_aix_thread_info (thread_info);
+
+      return string_printf (_("Thread %s (tid %s)"), pulongest (ptid.tid ()),
+		pulongest (priv->tid));
+    }
 
-  return string_printf (_("Thread %s"), pulongest (ptid.tid ()));
+  return beneath ()->pid_to_str (ptid);
 }
 
 /* Return a printable representation of extra information about
@@ -2098,7 +2127,6 @@ aix_thread_target::extra_thread_info (struct thread_info *thread)
 {
   int status;
   pthdb_pthread_t pdtid;
-  pthdb_tid_t tid;
   pthdb_state_t state;
   pthdb_suspendstate_t suspendstate;
   pthdb_detachstate_t detachstate;
@@ -2115,33 +2143,28 @@ aix_thread_target::extra_thread_info (struct thread_info *thread)
   aix_thread_info *priv = get_aix_thread_info (thread);
 
   pdtid = priv->pdtid;
-  tid = priv->tid;
-
-  if (tid != PTHDB_INVALID_TID)
-    /* i18n: Like "thread-identifier %d, [state] running, suspended" */
-    buf.printf (_("tid %d"), (int)tid);
 
   status = pthdb_pthread_state (data->pd_session, pdtid, &state);
   if (status != PTHDB_SUCCESS)
     state = PST_NOTSUP;
-  buf.printf (", %s", state2str (state));
+  buf.printf ("[%s]", state2str (state));
 
   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"));
+    buf.printf (_("[suspended]"));
 
   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"));
+    buf.printf (_("[detached]"));
 
   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"));
+    buf.printf (_("[cancel pending]"));
 
   buf.write ("", 1);
 
-- 
2.41.0

Re: [PATCH] Fix AIX thread exit events not being reported and UI to show kernel thread ID.

[Tom Tromey]

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

> Please find attached the patch. (See:
> 0001-Fix-AIX-thread-exit-events-not-being-reported-and-UI.patch)

Thanks for the patch.

> So, consider program 1 pasted below this email.

Is there some existing gdb test case that is fixed by this patch?  If
not then I think it would be better if the patch came with a new test.

I can't really comment on the AIX parts of the patch, but I noticed a
few little oddities.

>  static int
> -pcmp (const void *p1v, const void *p2v)
> +pcmp (struct pd_thread p1v, struct pd_thread p2v)
>  {
> -  struct pd_thread *p1 = (struct pd_thread *) p1v;
> -  struct pd_thread *p2 = (struct pd_thread *) p2v;
> -  return p1->pthid < p2->pthid ? -1 : p1->pthid > p2->pthid;
> +  return p1v.pthid < p2v.pthid;
>  }

Comparison functions passed to std::sort should return bool and
(ordinarily) use const reference parameter types.

Personally I'd probably just make this a lambda or maybe an operator<
but it's up to you.

>    pcount = 0;

Declaring this earlier and initializing it here is a bit strange now
that the logic has changed.

>    psize = 1;

This isn't needed any more, I think; and I'm surprised the compiler
didn't complain about it.

>    if (status == PTHDB_SUCCESS && detachstate == PDS_DETACHED)
>      /* i18n: Like "Thread-Id %d, [state] running, detached" */
> -    buf.printf (_(", detached"));
> +    buf.printf (_("[detached]"));
 
>    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"));
> +    buf.printf (_("[cancel pending]"));
 
The comments before these printfs are wrong now.

Tom

Re: [PATCH] Fix AIX thread exit events not being reported and UI to show kernel thread ID.

[John Baldwin]

On 4/18/24 6:26 AM, Aditya Kamath1 wrote:
> Respected community members,
> 
> Hi,
> 
> Thank you for the comments in the [RFC] thread<https://sourceware.org/pipermail/gdb-patches/2024-April/208104.html>.
> 
> Please find attached the patch. (See: 0001-Fix-AIX-thread-exit-events-not-being-reported-and-UI.patch)

Next time can you please send the patch inline as a separate e-mail?  You can use a cover
letter for the contents of this mail.  It is just easier to review in that case.

One thing I think I'm not fully understanding: does a thread never get removed from the
pth list once it has exited?  That is, is it on that list forever?  If so, I think
you could simplify this quite a bit to remove the qsort and the gcount and associated
list, etc.  Instead, I would keep track of exited threads whose exit has been reported
via a new std::unordered_set<> class member in the target to avoid reporting duplicate
events and then do something like:

    for (cmd = PTHDB_LIST_FIRST;; cmd = PTHDB_LIST_NEXT)
      {
        status = pthdb_pthread (data->pd_session, &pdtid, cmd);
        if (status != PTHDB_SUCCESS || pthid == PTHDB_INVALID_PTID)
  	continue;

        status = pthdb_pthread_tid (data->pd_session, pdtid, &tid);
        if (status != PTHDB_SUCCESS)
  	tid = PTHDB_INVALID_TID;

        ptid_t ptid (pid, 0, pthid);
        status = pthdb_pthread_state (data->pd_session, pdtid, &state);
        if (state == PST_TERM)
          {
             /* If this thread's exit was previously reported, ignore. */
             if (exited_threads.count (pdtid) != 0)
                 continue;
          }

        /* If this thread has never been reported to GDB, add it.  */
        if (!in_thread_list (proc_target, ptid))
          {
            aix_thread_info *priv = new aix_thread_info;
            /* init priv */
            add_thread_with_info (proc_target, ptid,
                                  private_thread_info_up (priv));
          }

        if (state == PST_TERM)
          {
            thread_info *thr = proc_target->find_thread (ptid);
            gdb_assert (thr != nullptr);
            delete_thread (thr);
            exited_threads.insert (pdtid);
          }
       }

-- 
John Baldwin

Re: [PATCH] Fix AIX thread exit events not being reported and UI to show kernel thread ID.

[John Baldwin]

On 4/29/24 4:48 AM, Aditya Kamath1 wrote:
> This is an older version of the patch. Sorry for resending this. Kindly ignore the same. I have sent the latest version in another git send-email.

Please use -v <n> (e.g. '-v 2') with a new version of n each time you use git send-email
to send a new version of a patch.  This makes includes the 'n' value in the e-mail
subject making it easier to see which version is the latest version.
  
> Have a nice day ahead.
> 
> Thanks and regards,
> Aditya.
> 
> From: Aditya Vidyadhar Kamath <akamath996@gmail.com>
> Date: Monday, 29 April 2024 at 5:11 PM
> To: tom@tromey.com <tom@tromey.com>
> Cc: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>, gdb-patches@sourceware.org <gdb-patches@sourceware.org>, Aditya Kamath1 <Aditya.Kamath1@ibm.com>, Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
> Subject: [EXTERNAL] [PATCH] Fix AIX thread exit events not being reported and UI to show kernel thread ID.
> From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
> 
> In AIX when a thread exits we were not showing that a thread exit event happened
> and GDB continued to keep the terminated threads.
> 
> If we have terminated threads then the UI on info threads command will look like
> (gdb) info threads
>    Id   Target Id                                          Frame
> * 1    Thread 1 (tid 26607979, running)    0xd0611d70 in _p_nsleep () from /usr/lib/libpthreads.a(_shr_xpg5.o)
>    2    Thread 258 (tid 30998799, finished) aix-thread: ptrace (52, 30998799) returned -1 (errno = 3 The process does not exist.)
> 
> If we see the frame is not getting displayed correctly.
> 
> The reason for the same is that in AIX we were not managing thread states. In particular we do not know
> when a thread terminates.
> 
> The reason being in sync_threadlists () the pbuf and gbuf lists remain the same though certain threads exit.
> 
> This patch is a fix to the same.
> 
> Also certain UI is changed.
> 
> On a new thread born and exit the UI in AIX will be similar to Linux with both user and kernel thread information.
> 
> [New Thread 258 (tid 32178533)]
> [New Thread 515 (tid 30343651)]
> [New Thread 772 (tid 33554909)]
> [New Thread 1029 (tid 24969489)]
> [New Thread 1286 (tid 18153945)]
> [New Thread 1543 (tid 30736739)]
> [Thread 258 (tid 32178533) exited]
> [Thread 515 (tid 30343651) exited]
> [Thread 772 (tid 33554909) exited]
> [Thread 1029 (tid 24969489) exited]
> [Thread 1286 (tid 18153945) exited]
> [Thread 1543 (tid 30736739) exited]
> 
> and info threads will look like
> (gdb) info threads
>    Id   Target Id                           Frame
> * 1    Thread 1 (tid 31326579) ([running]) 0xd0611d70 in _p_nsleep () from /usr/lib/libpthread.a(_shr_xpg5.o)
> ---
>   gdb/aix-thread.c | 91 ++++++++++++++++++++++++++++++------------------
>   1 file changed, 57 insertions(+), 34 deletions(-)
> 
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index c70bd82bc24..5ccccf02879 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -55,6 +55,7 @@
>   #include <sys/reg.h>
>   #include <sched.h>
>   #include <sys/pthdebug.h>
> +#include <vector>
> 
>   #if !HAVE_DECL_GETTHRDS
>   extern int getthrds (pid_t, struct thrdsinfo64 *, int, tid_t *, int);
> @@ -190,6 +191,9 @@ struct aix_thread_variables
>     /* Whether the current architecture is 64-bit.
>      Only valid when pd_able is true.  */
>     int arch64;
> +
> +  /* Describes the number of thread born events have been reported.  */
> +  int threads_reported = 0;
>   };
> 
>   /* Key to our per-inferior data.  */
> @@ -740,11 +744,9 @@ state2str (pthdb_state_t state)
>   /* qsort() comparison function for sorting pd_thread structs by pthid.  */
> 
>   static int
> -pcmp (const void *p1v, const void *p2v)
> +pcmp (struct pd_thread p1v, struct pd_thread p2v)
>   {
> -  struct pd_thread *p1 = (struct pd_thread *) p1v;
> -  struct pd_thread *p2 = (struct pd_thread *) p2v;
> -  return p1->pthid < p2->pthid ? -1 : p1->pthid > p2->pthid;
> +  return p1v.pthid < p2v.pthid;
>   }
> 
>   /* ptid comparison function */
> @@ -823,7 +825,6 @@ sync_threadlists (pid_t pid)
>   {
>     int cmd, status;
>     int pcount, psize, pi, gcount, gi;
> -  struct pd_thread *pbuf;
>     struct thread_info **gbuf, **g, *thread;
>     pthdb_pthread_t pdtid;
>     pthread_t pthid;
> @@ -831,12 +832,13 @@ sync_threadlists (pid_t pid)
>     process_stratum_target *proc_target = current_inferior ()->process_target ();
>     struct aix_thread_variables *data;
>     data = get_thread_data_helper_for_pid (pid);
> +  std::vector<pd_thread> pbuf;
> +  struct pd_thread aix_pd_thread;
> 
>     /* Accumulate an array of libpthdebug threads sorted by pthread id.  */
> 
>     pcount = 0;
>     psize = 1;
> -  pbuf = XNEWVEC (struct pd_thread, psize);
> 
>     for (cmd = PTHDB_LIST_FIRST;; cmd = PTHDB_LIST_NEXT)
>       {
> @@ -848,27 +850,36 @@ sync_threadlists (pid_t pid)
>         if (status != PTHDB_SUCCESS || pthid == PTHDB_INVALID_PTID)
>           continue;
> 
> -      if (pcount == psize)
> -       {
> -         psize *= 2;
> -         pbuf = (struct pd_thread *) xrealloc (pbuf,
> -                                               psize * sizeof *pbuf);
> -       }
> -      pbuf[pcount].pdtid = pdtid;
> -      pbuf[pcount].pthid = pthid;
> +      aix_pd_thread.pdtid = pdtid;
> +      aix_pd_thread.pthid = pthid;
> +      pbuf.push_back (aix_pd_thread);
>         pcount++;
>       }
> 
> -  for (pi = 0; pi < pcount; pi++)
> +  for (auto it = pbuf.begin (); it != pbuf.end ();)
>       {
> -      status = pthdb_pthread_tid (data->pd_session, pbuf[pi].pdtid, &tid);
> +      status = pthdb_pthread_tid (data->pd_session, (*it).pdtid, &tid);
>         if (status != PTHDB_SUCCESS)
>           tid = PTHDB_INVALID_TID;
> -      pbuf[pi].tid = tid;
> -    }
> 
> -  qsort (pbuf, pcount, sizeof *pbuf, pcmp);
> +      (*it).tid = tid;
> +      pthdb_state_t state;
> +
> +      status = pthdb_pthread_state (data->pd_session, (*it).pdtid, &state);
> 
> +      /* If the thread is terminated and has not reported its existence
> +        or new thread event, then wait for it to report.  We want users
> +        to know threads were added and then delete. So first add them to
> +        pbuf and the next time sync_threadlists is called delete them since
> +        data->threads_reported is guarenteed to be greater than 0.  */
> +
> +      if (state == PST_TERM && data->threads_reported > 0)
> +       pbuf.erase (it);
> +      else
> +       ++it;
> +    }
> +
> +  std::sort (pbuf.begin (), pbuf.end (), pcmp);
>     /* Accumulate an array of GDB threads sorted by pid.  */
> 
>     /* gcount is GDB thread count and pcount is pthreadlib thread count.  */
> @@ -881,6 +892,7 @@ sync_threadlists (pid_t pid)
>       *g++ = tp;
>     qsort (gbuf, gcount, sizeof *gbuf, gcmp);
> 
> +  pcount = pbuf.size ();
>     /* Apply differences between the two arrays to GDB's thread list.  */
> 
>     for (pi = gi = 0; pi < pcount || gi < gcount;)
> @@ -889,6 +901,12 @@ sync_threadlists (pid_t pid)
>           {
>             delete_thread (gbuf[gi]);
>             gi++;
> +         data->threads_reported--;
> +
> +         /* Since main thread is reported.  */
> +
> +         if (data->threads_reported == 0)
> +           data->threads_reported = 1;
>           }
>         else if (gi == gcount)
>           {
> @@ -901,6 +919,7 @@ sync_threadlists (pid_t pid)
>                                            private_thread_info_up (priv));
> 
>             pi++;
> +         data->threads_reported++;
>           }
>         else
>           {
> @@ -913,7 +932,6 @@ sync_threadlists (pid_t pid)
>             tid = pbuf[pi].tid;
> 
>             cmp_result = ptid_cmp (pptid, gptid);
> -
>             if (cmp_result == 0)
>               {
>                 aix_thread_info *priv = get_aix_thread_info (gbuf[gi]);
> @@ -943,6 +961,10 @@ sync_threadlists (pid_t pid)
>                   {
>                     delete_thread (gbuf[gi]);
>                     gi++;
> +                 data->threads_reported--;
> +
> +                 if (data->threads_reported == 0)
> +                   data->threads_reported = 1;
>                   }
>               }
>             else
> @@ -954,11 +976,11 @@ sync_threadlists (pid_t pid)
>                 priv->pdtid = pdtid;
>                 priv->tid = tid;
>                 pi++;
> +             data->threads_reported++;
>               }
>           }
>       }
> 
> -  xfree (pbuf);
>     xfree (gbuf);
>   }
> 
> @@ -2084,10 +2106,17 @@ aix_thread_target::thread_alive (ptid_t ptid)
>   std::string
>   aix_thread_target::pid_to_str (ptid_t ptid)
>   {
> -  if (ptid.tid () == 0)
> -    return beneath ()->pid_to_str (ptid);
> +  thread_info *thread_info = current_inferior ()->find_thread (ptid);
> +
> +  if (thread_info != NULL && thread_info->priv != NULL)
> +    {
> +      aix_thread_info *priv = get_aix_thread_info (thread_info);
> +
> +      return string_printf (_("Thread %s (tid %s)"), pulongest (ptid.tid ()),
> +               pulongest (priv->tid));
> +    }
> 
> -  return string_printf (_("Thread %s"), pulongest (ptid.tid ()));
> +  return beneath ()->pid_to_str (ptid);
>   }
> 
>   /* Return a printable representation of extra information about
> @@ -2098,7 +2127,6 @@ aix_thread_target::extra_thread_info (struct thread_info *thread)
>   {
>     int status;
>     pthdb_pthread_t pdtid;
> -  pthdb_tid_t tid;
>     pthdb_state_t state;
>     pthdb_suspendstate_t suspendstate;
>     pthdb_detachstate_t detachstate;
> @@ -2115,33 +2143,28 @@ aix_thread_target::extra_thread_info (struct thread_info *thread)
>     aix_thread_info *priv = get_aix_thread_info (thread);
> 
>     pdtid = priv->pdtid;
> -  tid = priv->tid;
> -
> -  if (tid != PTHDB_INVALID_TID)
> -    /* i18n: Like "thread-identifier %d, [state] running, suspended" */
> -    buf.printf (_("tid %d"), (int)tid);
> 
>     status = pthdb_pthread_state (data->pd_session, pdtid, &state);
>     if (status != PTHDB_SUCCESS)
>       state = PST_NOTSUP;
> -  buf.printf (", %s", state2str (state));
> +  buf.printf ("[%s]", state2str (state));
> 
>     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"));
> +    buf.printf (_("[suspended]"));
> 
>     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"));
> +    buf.printf (_("[detached]"));
> 
>     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"));
> +    buf.printf (_("[cancel pending]"));
> 
>     buf.write ("", 1);
> 
> --
> 2.41.0

-- 
John Baldwin

Re: [PATCH] Fix AIX thread exit events not being reported and UI to show kernel thread ID.

[Aditya Kamath1]

[-- Attachment #1: Type: text/plain, Size: 10420 bytes --]

This is an older version of the patch. Sorry for resending this. Kindly ignore the same. I have sent the latest version in another git send-email.

Have a nice day ahead.

Thanks and regards,
Aditya.

From: Aditya Vidyadhar Kamath <akamath996@gmail.com>
Date: Monday, 29 April 2024 at 5:11 PM
To: tom@tromey.com <tom@tromey.com>
Cc: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>, gdb-patches@sourceware.org <gdb-patches@sourceware.org>, Aditya Kamath1 <Aditya.Kamath1@ibm.com>, Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: [EXTERNAL] [PATCH] Fix AIX thread exit events not being reported and UI to show kernel thread ID.
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>

In AIX when a thread exits we were not showing that a thread exit event happened
and GDB continued to keep the terminated threads.

If we have terminated threads then the UI on info threads command will look like
(gdb) info threads
  Id   Target Id                                          Frame
* 1    Thread 1 (tid 26607979, running)    0xd0611d70 in _p_nsleep () from /usr/lib/libpthreads.a(_shr_xpg5.o)
  2    Thread 258 (tid 30998799, finished) aix-thread: ptrace (52, 30998799) returned -1 (errno = 3 The process does not exist.)

If we see the frame is not getting displayed correctly.

The reason for the same is that in AIX we were not managing thread states. In particular we do not know
when a thread terminates.

The reason being in sync_threadlists () the pbuf and gbuf lists remain the same though certain threads exit.

This patch is a fix to the same.

Also certain UI is changed.

On a new thread born and exit the UI in AIX will be similar to Linux with both user and kernel thread information.

[New Thread 258 (tid 32178533)]
[New Thread 515 (tid 30343651)]
[New Thread 772 (tid 33554909)]
[New Thread 1029 (tid 24969489)]
[New Thread 1286 (tid 18153945)]
[New Thread 1543 (tid 30736739)]
[Thread 258 (tid 32178533) exited]
[Thread 515 (tid 30343651) exited]
[Thread 772 (tid 33554909) exited]
[Thread 1029 (tid 24969489) exited]
[Thread 1286 (tid 18153945) exited]
[Thread 1543 (tid 30736739) exited]

and info threads will look like
(gdb) info threads
  Id   Target Id                           Frame
* 1    Thread 1 (tid 31326579) ([running]) 0xd0611d70 in _p_nsleep () from /usr/lib/libpthread.a(_shr_xpg5.o)
---
 gdb/aix-thread.c | 91 ++++++++++++++++++++++++++++++------------------
 1 file changed, 57 insertions(+), 34 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index c70bd82bc24..5ccccf02879 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -55,6 +55,7 @@
 #include <sys/reg.h>
 #include <sched.h>
 #include <sys/pthdebug.h>
+#include <vector>

 #if !HAVE_DECL_GETTHRDS
 extern int getthrds (pid_t, struct thrdsinfo64 *, int, tid_t *, int);
@@ -190,6 +191,9 @@ struct aix_thread_variables
   /* Whether the current architecture is 64-bit.
    Only valid when pd_able is true.  */
   int arch64;
+
+  /* Describes the number of thread born events have been reported.  */
+  int threads_reported = 0;
 };

 /* Key to our per-inferior data.  */
@@ -740,11 +744,9 @@ state2str (pthdb_state_t state)
 /* qsort() comparison function for sorting pd_thread structs by pthid.  */

 static int
-pcmp (const void *p1v, const void *p2v)
+pcmp (struct pd_thread p1v, struct pd_thread p2v)
 {
-  struct pd_thread *p1 = (struct pd_thread *) p1v;
-  struct pd_thread *p2 = (struct pd_thread *) p2v;
-  return p1->pthid < p2->pthid ? -1 : p1->pthid > p2->pthid;
+  return p1v.pthid < p2v.pthid;
 }

 /* ptid comparison function */
@@ -823,7 +825,6 @@ sync_threadlists (pid_t pid)
 {
   int cmd, status;
   int pcount, psize, pi, gcount, gi;
-  struct pd_thread *pbuf;
   struct thread_info **gbuf, **g, *thread;
   pthdb_pthread_t pdtid;
   pthread_t pthid;
@@ -831,12 +832,13 @@ sync_threadlists (pid_t pid)
   process_stratum_target *proc_target = current_inferior ()->process_target ();
   struct aix_thread_variables *data;
   data = get_thread_data_helper_for_pid (pid);
+  std::vector<pd_thread> pbuf;
+  struct pd_thread aix_pd_thread;

   /* Accumulate an array of libpthdebug threads sorted by pthread id.  */

   pcount = 0;
   psize = 1;
-  pbuf = XNEWVEC (struct pd_thread, psize);

   for (cmd = PTHDB_LIST_FIRST;; cmd = PTHDB_LIST_NEXT)
     {
@@ -848,27 +850,36 @@ sync_threadlists (pid_t pid)
       if (status != PTHDB_SUCCESS || pthid == PTHDB_INVALID_PTID)
         continue;

-      if (pcount == psize)
-       {
-         psize *= 2;
-         pbuf = (struct pd_thread *) xrealloc (pbuf,
-                                               psize * sizeof *pbuf);
-       }
-      pbuf[pcount].pdtid = pdtid;
-      pbuf[pcount].pthid = pthid;
+      aix_pd_thread.pdtid = pdtid;
+      aix_pd_thread.pthid = pthid;
+      pbuf.push_back (aix_pd_thread);
       pcount++;
     }

-  for (pi = 0; pi < pcount; pi++)
+  for (auto it = pbuf.begin (); it != pbuf.end ();)
     {
-      status = pthdb_pthread_tid (data->pd_session, pbuf[pi].pdtid, &tid);
+      status = pthdb_pthread_tid (data->pd_session, (*it).pdtid, &tid);
       if (status != PTHDB_SUCCESS)
         tid = PTHDB_INVALID_TID;
-      pbuf[pi].tid = tid;
-    }

-  qsort (pbuf, pcount, sizeof *pbuf, pcmp);
+      (*it).tid = tid;
+      pthdb_state_t state;
+
+      status = pthdb_pthread_state (data->pd_session, (*it).pdtid, &state);

+      /* If the thread is terminated and has not reported its existence
+        or new thread event, then wait for it to report.  We want users
+        to know threads were added and then delete. So first add them to
+        pbuf and the next time sync_threadlists is called delete them since
+        data->threads_reported is guarenteed to be greater than 0.  */
+
+      if (state == PST_TERM && data->threads_reported > 0)
+       pbuf.erase (it);
+      else
+       ++it;
+    }
+
+  std::sort (pbuf.begin (), pbuf.end (), pcmp);
   /* Accumulate an array of GDB threads sorted by pid.  */

   /* gcount is GDB thread count and pcount is pthreadlib thread count.  */
@@ -881,6 +892,7 @@ sync_threadlists (pid_t pid)
     *g++ = tp;
   qsort (gbuf, gcount, sizeof *gbuf, gcmp);

+  pcount = pbuf.size ();
   /* Apply differences between the two arrays to GDB's thread list.  */

   for (pi = gi = 0; pi < pcount || gi < gcount;)
@@ -889,6 +901,12 @@ sync_threadlists (pid_t pid)
         {
           delete_thread (gbuf[gi]);
           gi++;
+         data->threads_reported--;
+
+         /* Since main thread is reported.  */
+
+         if (data->threads_reported == 0)
+           data->threads_reported = 1;
         }
       else if (gi == gcount)
         {
@@ -901,6 +919,7 @@ sync_threadlists (pid_t pid)
                                          private_thread_info_up (priv));

           pi++;
+         data->threads_reported++;
         }
       else
         {
@@ -913,7 +932,6 @@ sync_threadlists (pid_t pid)
           tid = pbuf[pi].tid;

           cmp_result = ptid_cmp (pptid, gptid);
-
           if (cmp_result == 0)
             {
               aix_thread_info *priv = get_aix_thread_info (gbuf[gi]);
@@ -943,6 +961,10 @@ sync_threadlists (pid_t pid)
                 {
                   delete_thread (gbuf[gi]);
                   gi++;
+                 data->threads_reported--;
+
+                 if (data->threads_reported == 0)
+                   data->threads_reported = 1;
                 }
             }
           else
@@ -954,11 +976,11 @@ sync_threadlists (pid_t pid)
               priv->pdtid = pdtid;
               priv->tid = tid;
               pi++;
+             data->threads_reported++;
             }
         }
     }

-  xfree (pbuf);
   xfree (gbuf);
 }

@@ -2084,10 +2106,17 @@ aix_thread_target::thread_alive (ptid_t ptid)
 std::string
 aix_thread_target::pid_to_str (ptid_t ptid)
 {
-  if (ptid.tid () == 0)
-    return beneath ()->pid_to_str (ptid);
+  thread_info *thread_info = current_inferior ()->find_thread (ptid);
+
+  if (thread_info != NULL && thread_info->priv != NULL)
+    {
+      aix_thread_info *priv = get_aix_thread_info (thread_info);
+
+      return string_printf (_("Thread %s (tid %s)"), pulongest (ptid.tid ()),
+               pulongest (priv->tid));
+    }

-  return string_printf (_("Thread %s"), pulongest (ptid.tid ()));
+  return beneath ()->pid_to_str (ptid);
 }

 /* Return a printable representation of extra information about
@@ -2098,7 +2127,6 @@ aix_thread_target::extra_thread_info (struct thread_info *thread)
 {
   int status;
   pthdb_pthread_t pdtid;
-  pthdb_tid_t tid;
   pthdb_state_t state;
   pthdb_suspendstate_t suspendstate;
   pthdb_detachstate_t detachstate;
@@ -2115,33 +2143,28 @@ aix_thread_target::extra_thread_info (struct thread_info *thread)
   aix_thread_info *priv = get_aix_thread_info (thread);

   pdtid = priv->pdtid;
-  tid = priv->tid;
-
-  if (tid != PTHDB_INVALID_TID)
-    /* i18n: Like "thread-identifier %d, [state] running, suspended" */
-    buf.printf (_("tid %d"), (int)tid);

   status = pthdb_pthread_state (data->pd_session, pdtid, &state);
   if (status != PTHDB_SUCCESS)
     state = PST_NOTSUP;
-  buf.printf (", %s", state2str (state));
+  buf.printf ("[%s]", state2str (state));

   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"));
+    buf.printf (_("[suspended]"));

   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"));
+    buf.printf (_("[detached]"));

   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"));
+    buf.printf (_("[cancel pending]"));

   buf.write ("", 1);

--
2.41.0

[PATCH] Fix AIX thread exit events not being reported and UI to show kernel thread ID.

[Aditya Vidyadhar Kamath]

From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>

In AIX when a thread exits we were not showing that a thread exit event happened
and GDB continued to keep the terminated threads.

If we have terminated threads then the UI on info threads command will look like
(gdb) info threads
  Id   Target Id                                          Frame
* 1    Thread 1 (tid 26607979, running)    0xd0611d70 in _p_nsleep () from /usr/lib/libpthreads.a(_shr_xpg5.o)
  2    Thread 258 (tid 30998799, finished) aix-thread: ptrace (52, 30998799) returned -1 (errno = 3 The process does not exist.)

If we see the frame is not getting displayed correctly.

The reason for the same is that in AIX we were not managing thread states. In particular we do not know
when a thread terminates.

The reason being in sync_threadlists () the pbuf and gbuf lists remain the same though certain threads exit.

This patch is a fix to the same.

Also certain UI is changed.

On a new thread born and exit the UI in AIX will be similar to Linux with both user and kernel thread information.

[New Thread 258 (tid 32178533)]
[New Thread 515 (tid 30343651)]
[New Thread 772 (tid 33554909)]
[New Thread 1029 (tid 24969489)]
[New Thread 1286 (tid 18153945)]
[New Thread 1543 (tid 30736739)]
[Thread 258 (tid 32178533) exited]
[Thread 515 (tid 30343651) exited]
[Thread 772 (tid 33554909) exited]
[Thread 1029 (tid 24969489) exited]
[Thread 1286 (tid 18153945) exited]
[Thread 1543 (tid 30736739) exited]

and info threads will look like
(gdb) info threads
  Id   Target Id                           Frame
* 1    Thread 1 (tid 31326579) ([running]) 0xd0611d70 in _p_nsleep () from /usr/lib/libpthread.a(_shr_xpg5.o)
---
 gdb/aix-thread.c | 217 ++++++++++++-----------------------------------
 1 file changed, 52 insertions(+), 165 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index c70bd82bc24..73839243213 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -55,6 +55,7 @@
 #include <sys/reg.h>
 #include <sched.h>
 #include <sys/pthdebug.h>
+#include <unordered_set>
 
 #if !HAVE_DECL_GETTHRDS
 extern int getthrds (pid_t, struct thrdsinfo64 *, int, tid_t *, int);
@@ -190,6 +191,9 @@ struct aix_thread_variables
   /* Whether the current architecture is 64-bit.
    Only valid when pd_able is true.  */
   int arch64;
+
+  /* Describes the number of thread exit events reported.  */
+  std::unordered_set<pthdb_pthread_t> exited_threads;
 };
 
 /* Key to our per-inferior data.  */
@@ -737,47 +741,6 @@ state2str (pthdb_state_t state)
     }
 }
 
-/* qsort() comparison function for sorting pd_thread structs by pthid.  */
-
-static int
-pcmp (const void *p1v, const void *p2v)
-{
-  struct pd_thread *p1 = (struct pd_thread *) p1v;
-  struct pd_thread *p2 = (struct pd_thread *) p2v;
-  return p1->pthid < p2->pthid ? -1 : p1->pthid > p2->pthid;
-}
-
-/* ptid comparison function */
-
-static int
-ptid_cmp (ptid_t ptid1, ptid_t ptid2)
-{
-  if (ptid1.pid () < ptid2.pid ())
-    return -1;
-  else if (ptid1.pid () > ptid2.pid ())
-    return 1;
-  else if (ptid1.tid () < ptid2.tid ())
-    return -1;
-  else if (ptid1.tid () > ptid2.tid ())
-    return 1;
-  else if (ptid1.lwp () < ptid2.lwp ())
-    return -1;
-  else if (ptid1.lwp () > ptid2.lwp ())
-    return 1;
-  else
-    return 0;
-}
-
-/* qsort() comparison function for sorting thread_info structs by pid.  */
-
-static int
-gcmp (const void *t1v, const void *t2v)
-{
-  struct thread_info *t1 = *(struct thread_info **) t1v;
-  struct thread_info *t2 = *(struct thread_info **) t2v;
-  return ptid_cmp (t1->ptid, t2->ptid);
-}
-
 /* Search through the list of all kernel threads for the thread
    that has stopped on a SIGTRAP signal, and return its TID.
    Return 0 if none found.  */
@@ -822,22 +785,16 @@ static void
 sync_threadlists (pid_t pid)
 {
   int cmd, status;
-  int pcount, psize, pi, gcount, gi;
-  struct pd_thread *pbuf;
-  struct thread_info **gbuf, **g, *thread;
   pthdb_pthread_t pdtid;
   pthread_t pthid;
   pthdb_tid_t tid;
   process_stratum_target *proc_target = current_inferior ()->process_target ();
   struct aix_thread_variables *data;
   data = get_thread_data_helper_for_pid (pid);
+  pthdb_state_t state;
 
   /* Accumulate an array of libpthdebug threads sorted by pthread id.  */
 
-  pcount = 0;
-  psize = 1;
-  pbuf = XNEWVEC (struct pd_thread, psize);
-
   for (cmd = PTHDB_LIST_FIRST;; cmd = PTHDB_LIST_NEXT)
     {
       status = pthdb_pthread (data->pd_session, &pdtid, cmd);
@@ -848,118 +805,44 @@ sync_threadlists (pid_t pid)
       if (status != PTHDB_SUCCESS || pthid == PTHDB_INVALID_PTID)
 	continue;
 
-      if (pcount == psize)
+      ptid_t ptid (pid, 0, pthid);
+      status = pthdb_pthread_state (data->pd_session, pdtid, &state);
+      if (state == PST_TERM)
 	{
-	  psize *= 2;
-	  pbuf = (struct pd_thread *) xrealloc (pbuf, 
-						psize * sizeof *pbuf);
+	  if (data->exited_threads.count (pdtid) != 0)
+	     continue;
 	}
-      pbuf[pcount].pdtid = pdtid;
-      pbuf[pcount].pthid = pthid;
-      pcount++;
-    }
-
-  for (pi = 0; pi < pcount; pi++)
-    {
-      status = pthdb_pthread_tid (data->pd_session, pbuf[pi].pdtid, &tid);
-      if (status != PTHDB_SUCCESS)
-	tid = PTHDB_INVALID_TID;
-      pbuf[pi].tid = tid;
-    }
-
-  qsort (pbuf, pcount, sizeof *pbuf, pcmp);
-
-  /* Accumulate an array of GDB threads sorted by pid.  */
-
-  /* gcount is GDB thread count and pcount is pthreadlib thread count.  */
-
-  gcount = 0;
-  for (thread_info *tp : all_threads (proc_target, ptid_t (pid)))
-    gcount++;
-  g = gbuf = XNEWVEC (struct thread_info *, gcount);
-  for (thread_info *tp : all_threads (proc_target, ptid_t (pid)))
-    *g++ = tp;
-  qsort (gbuf, gcount, sizeof *gbuf, gcmp);
 
-  /* Apply differences between the two arrays to GDB's thread list.  */
-
-  for (pi = gi = 0; pi < pcount || gi < gcount;)
-    {
-      if (pi == pcount)
-	{
-	  delete_thread (gbuf[gi]);
-	  gi++;
-	}
-      else if (gi == gcount)
+      /* If this thread has never been reported to GDB, add it.  */
+      if (!in_thread_list (proc_target, ptid))
 	{
 	  aix_thread_info *priv = new aix_thread_info;
-	  priv->pdtid = pbuf[pi].pdtid;
-	  priv->tid = pbuf[pi].tid;
-
-	  thread = add_thread_with_info (proc_target,
-					 ptid_t (pid, 0, pbuf[pi].pthid),
-					 private_thread_info_up (priv));
-
-	  pi++;
-	}
-      else
-	{
-	  ptid_t pptid, gptid;
-	  int cmp_result;
-
-	  pptid = ptid_t (pid, 0, pbuf[pi].pthid);
-	  gptid = gbuf[gi]->ptid;
-	  pdtid = pbuf[pi].pdtid;
-	  tid = pbuf[pi].tid;
-
-	  cmp_result = ptid_cmp (pptid, gptid);
-
-	  if (cmp_result == 0)
-	    {
-	      aix_thread_info *priv = get_aix_thread_info (gbuf[gi]);
-
-	      priv->pdtid = pdtid;
-	      priv->tid = tid;
-	      pi++;
-	      gi++;
-	    }
-	  else if (cmp_result > 0)
+	  /* init priv */
+	  priv->pdtid = pdtid;
+	  status = pthdb_pthread_tid (data->pd_session, pdtid, &tid);
+	  priv->tid = tid;
+	  /* Check if this is the main thread.  If it is, then change
+	     its ptid and add its private data.  */
+	  if (get_signaled_thread (pid) == tid
+		&& in_thread_list (proc_target, ptid_t (pid)))
 	    {
-	      /* This is to make the main process thread now look
-		 like a thread.  */
-
-	      if (gptid.is_pid ())
-		{
-		  thread_info *tp = proc_target->find_thread (gptid);
-		  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->priv.reset (priv);
-		  gi++;
-		  pi++;
-		}
-	      else
-		{
-		  delete_thread (gbuf[gi]);
-		  gi++;
-		}
+	      thread_info *tp = proc_target->find_thread (ptid_t (pid));
+	      thread_change_ptid (proc_target, ptid_t (pid), ptid);
+	      tp->priv.reset (priv);
 	    }
 	  else
-	    {
-	      thread = add_thread (proc_target, pptid);
+	    add_thread_with_info (proc_target, ptid,
+		private_thread_info_up (priv));
+	}
 
-	      aix_thread_info *priv = new aix_thread_info;
-	      thread->priv.reset (priv);
-	      priv->pdtid = pdtid;
-	      priv->tid = tid;
-	      pi++;
-	    }
+      if (state == PST_TERM)
+	{
+	  thread_info *thr = proc_target->find_thread (ptid);
+	  gdb_assert (thr != nullptr);
+	  delete_thread (thr);
+	  data->exited_threads.insert (pdtid);
 	}
     }
-
-  xfree (pbuf);
-  xfree (gbuf);
 }
 
 /* Iterate_over_threads() callback for locating a thread, using
@@ -2084,10 +1967,17 @@ aix_thread_target::thread_alive (ptid_t ptid)
 std::string
 aix_thread_target::pid_to_str (ptid_t ptid)
 {
-  if (ptid.tid () == 0)
-    return beneath ()->pid_to_str (ptid);
+  thread_info *thread_info = current_inferior ()->find_thread (ptid);
+
+  if (thread_info != NULL && thread_info->priv != NULL)
+    {
+      aix_thread_info *priv = get_aix_thread_info (thread_info);
+
+      return string_printf (_("Thread %s (tid %s)"), pulongest (ptid.tid ()),
+		pulongest (priv->tid));
+    }
 
-  return string_printf (_("Thread %s"), pulongest (ptid.tid ()));
+  return beneath ()->pid_to_str (ptid);
 }
 
 /* Return a printable representation of extra information about
@@ -2098,7 +1988,6 @@ aix_thread_target::extra_thread_info (struct thread_info *thread)
 {
   int status;
   pthdb_pthread_t pdtid;
-  pthdb_tid_t tid;
   pthdb_state_t state;
   pthdb_suspendstate_t suspendstate;
   pthdb_detachstate_t detachstate;
@@ -2115,33 +2004,31 @@ aix_thread_target::extra_thread_info (struct thread_info *thread)
   aix_thread_info *priv = get_aix_thread_info (thread);
 
   pdtid = priv->pdtid;
-  tid = priv->tid;
-
-  if (tid != PTHDB_INVALID_TID)
-    /* i18n: Like "thread-identifier %d, [state] running, suspended" */
-    buf.printf (_("tid %d"), (int)tid);
 
   status = pthdb_pthread_state (data->pd_session, pdtid, &state);
+
+  /* Output should look like Thread %d (tid %d) ([state]).  */
+  /* Example:- Thread 1 (tid 34144587) ([running]).  */
+  /* where state can be running, idle, sleeping, finished,
+     suspended, detached, cancel pending, ready or unknown.  */
+
   if (status != PTHDB_SUCCESS)
     state = PST_NOTSUP;
-  buf.printf (", %s", state2str (state));
+  buf.printf ("[%s]", state2str (state));
 
   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"));
+    buf.printf (_("[suspended]"));
 
   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"));
+    buf.printf (_("[detached]"));
 
   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"));
+    buf.printf (_("[cancel pending]"));
 
   buf.write ("", 1);
 
-- 
2.41.0

[PATCH] Fix AIX thread exit events not being reported and UI to show kernel thread ID.

[Aditya Vidyadhar Kamath]

From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>

In AIX when a thread exits we were not showing that a thread exit event happened
and GDB continued to keep the terminated threads.

If we have terminated threads then the UI on info threads command will look like
(gdb) info threads
  Id   Target Id                                          Frame
* 1    Thread 1 (tid 26607979, running)    0xd0611d70 in _p_nsleep () from /usr/lib/libpthreads.a(_shr_xpg5.o)
  2    Thread 258 (tid 30998799, finished) aix-thread: ptrace (52, 30998799) returned -1 (errno = 3 The process does not exist.)

If we see the frame is not getting displayed correctly.

The reason for the same is that in AIX we were not managing thread states. In particular we do not know
when a thread terminates.

The reason being in sync_threadlists () the pbuf and gbuf lists remain the same though certain threads exit.

This patch is a fix to the same.

Also certain UI is changed.

On a new thread born and exit the UI in AIX will be similar to Linux with both user and kernel thread information.

[New Thread 258 (tid 32178533)]
[New Thread 515 (tid 30343651)]
[New Thread 772 (tid 33554909)]
[New Thread 1029 (tid 24969489)]
[New Thread 1286 (tid 18153945)]
[New Thread 1543 (tid 30736739)]
[Thread 258 (tid 32178533) exited]
[Thread 515 (tid 30343651) exited]
[Thread 772 (tid 33554909) exited]
[Thread 1029 (tid 24969489) exited]
[Thread 1286 (tid 18153945) exited]
[Thread 1543 (tid 30736739) exited]

and info threads will look like
(gdb) info threads
  Id   Target Id                           Frame
* 1    Thread 1 (tid 31326579) ([running]) 0xd0611d70 in _p_nsleep () from /usr/lib/libpthread.a(_shr_xpg5.o)
---
 gdb/aix-thread.c | 91 ++++++++++++++++++++++++++++++------------------
 1 file changed, 57 insertions(+), 34 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index c70bd82bc24..5ccccf02879 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -55,6 +55,7 @@
 #include <sys/reg.h>
 #include <sched.h>
 #include <sys/pthdebug.h>
+#include <vector>
 
 #if !HAVE_DECL_GETTHRDS
 extern int getthrds (pid_t, struct thrdsinfo64 *, int, tid_t *, int);
@@ -190,6 +191,9 @@ struct aix_thread_variables
   /* Whether the current architecture is 64-bit.
    Only valid when pd_able is true.  */
   int arch64;
+
+  /* Describes the number of thread born events have been reported.  */
+  int threads_reported = 0;
 };
 
 /* Key to our per-inferior data.  */
@@ -740,11 +744,9 @@ state2str (pthdb_state_t state)
 /* qsort() comparison function for sorting pd_thread structs by pthid.  */
 
 static int
-pcmp (const void *p1v, const void *p2v)
+pcmp (struct pd_thread p1v, struct pd_thread p2v)
 {
-  struct pd_thread *p1 = (struct pd_thread *) p1v;
-  struct pd_thread *p2 = (struct pd_thread *) p2v;
-  return p1->pthid < p2->pthid ? -1 : p1->pthid > p2->pthid;
+  return p1v.pthid < p2v.pthid;
 }
 
 /* ptid comparison function */
@@ -823,7 +825,6 @@ sync_threadlists (pid_t pid)
 {
   int cmd, status;
   int pcount, psize, pi, gcount, gi;
-  struct pd_thread *pbuf;
   struct thread_info **gbuf, **g, *thread;
   pthdb_pthread_t pdtid;
   pthread_t pthid;
@@ -831,12 +832,13 @@ sync_threadlists (pid_t pid)
   process_stratum_target *proc_target = current_inferior ()->process_target ();
   struct aix_thread_variables *data;
   data = get_thread_data_helper_for_pid (pid);
+  std::vector<pd_thread> pbuf;
+  struct pd_thread aix_pd_thread;
 
   /* Accumulate an array of libpthdebug threads sorted by pthread id.  */
 
   pcount = 0;
   psize = 1;
-  pbuf = XNEWVEC (struct pd_thread, psize);
 
   for (cmd = PTHDB_LIST_FIRST;; cmd = PTHDB_LIST_NEXT)
     {
@@ -848,27 +850,36 @@ sync_threadlists (pid_t pid)
       if (status != PTHDB_SUCCESS || pthid == PTHDB_INVALID_PTID)
 	continue;
 
-      if (pcount == psize)
-	{
-	  psize *= 2;
-	  pbuf = (struct pd_thread *) xrealloc (pbuf, 
-						psize * sizeof *pbuf);
-	}
-      pbuf[pcount].pdtid = pdtid;
-      pbuf[pcount].pthid = pthid;
+      aix_pd_thread.pdtid = pdtid;
+      aix_pd_thread.pthid = pthid;
+      pbuf.push_back (aix_pd_thread);
       pcount++;
     }
 
-  for (pi = 0; pi < pcount; pi++)
+  for (auto it = pbuf.begin (); it != pbuf.end ();)
     {
-      status = pthdb_pthread_tid (data->pd_session, pbuf[pi].pdtid, &tid);
+      status = pthdb_pthread_tid (data->pd_session, (*it).pdtid, &tid);
       if (status != PTHDB_SUCCESS)
 	tid = PTHDB_INVALID_TID;
-      pbuf[pi].tid = tid;
-    }
 
-  qsort (pbuf, pcount, sizeof *pbuf, pcmp);
+      (*it).tid = tid;
+      pthdb_state_t state;
+
+      status = pthdb_pthread_state (data->pd_session, (*it).pdtid, &state);
 
+      /* If the thread is terminated and has not reported its existence
+	 or new thread event, then wait for it to report.  We want users
+	 to know threads were added and then delete. So first add them to
+	 pbuf and the next time sync_threadlists is called delete them since
+	 data->threads_reported is guarenteed to be greater than 0.  */
+
+      if (state == PST_TERM && data->threads_reported > 0)
+	pbuf.erase (it);
+      else
+	++it;
+    }
+
+  std::sort (pbuf.begin (), pbuf.end (), pcmp);
   /* Accumulate an array of GDB threads sorted by pid.  */
 
   /* gcount is GDB thread count and pcount is pthreadlib thread count.  */
@@ -881,6 +892,7 @@ sync_threadlists (pid_t pid)
     *g++ = tp;
   qsort (gbuf, gcount, sizeof *gbuf, gcmp);
 
+  pcount = pbuf.size ();
   /* Apply differences between the two arrays to GDB's thread list.  */
 
   for (pi = gi = 0; pi < pcount || gi < gcount;)
@@ -889,6 +901,12 @@ sync_threadlists (pid_t pid)
 	{
 	  delete_thread (gbuf[gi]);
 	  gi++;
+	  data->threads_reported--;
+
+	  /* Since main thread is reported.  */
+
+	  if (data->threads_reported == 0)
+	    data->threads_reported = 1;
 	}
       else if (gi == gcount)
 	{
@@ -901,6 +919,7 @@ sync_threadlists (pid_t pid)
 					 private_thread_info_up (priv));
 
 	  pi++;
+	  data->threads_reported++;
 	}
       else
 	{
@@ -913,7 +932,6 @@ sync_threadlists (pid_t pid)
 	  tid = pbuf[pi].tid;
 
 	  cmp_result = ptid_cmp (pptid, gptid);
-
 	  if (cmp_result == 0)
 	    {
 	      aix_thread_info *priv = get_aix_thread_info (gbuf[gi]);
@@ -943,6 +961,10 @@ sync_threadlists (pid_t pid)
 		{
 		  delete_thread (gbuf[gi]);
 		  gi++;
+		  data->threads_reported--;
+
+		  if (data->threads_reported == 0)
+		    data->threads_reported = 1;
 		}
 	    }
 	  else
@@ -954,11 +976,11 @@ sync_threadlists (pid_t pid)
 	      priv->pdtid = pdtid;
 	      priv->tid = tid;
 	      pi++;
+	      data->threads_reported++;
 	    }
 	}
     }
 
-  xfree (pbuf);
   xfree (gbuf);
 }
 
@@ -2084,10 +2106,17 @@ aix_thread_target::thread_alive (ptid_t ptid)
 std::string
 aix_thread_target::pid_to_str (ptid_t ptid)
 {
-  if (ptid.tid () == 0)
-    return beneath ()->pid_to_str (ptid);
+  thread_info *thread_info = current_inferior ()->find_thread (ptid);
+
+  if (thread_info != NULL && thread_info->priv != NULL)
+    {
+      aix_thread_info *priv = get_aix_thread_info (thread_info);
+
+      return string_printf (_("Thread %s (tid %s)"), pulongest (ptid.tid ()),
+		pulongest (priv->tid));
+    }
 
-  return string_printf (_("Thread %s"), pulongest (ptid.tid ()));
+  return beneath ()->pid_to_str (ptid);
 }
 
 /* Return a printable representation of extra information about
@@ -2098,7 +2127,6 @@ aix_thread_target::extra_thread_info (struct thread_info *thread)
 {
   int status;
   pthdb_pthread_t pdtid;
-  pthdb_tid_t tid;
   pthdb_state_t state;
   pthdb_suspendstate_t suspendstate;
   pthdb_detachstate_t detachstate;
@@ -2115,33 +2143,28 @@ aix_thread_target::extra_thread_info (struct thread_info *thread)
   aix_thread_info *priv = get_aix_thread_info (thread);
 
   pdtid = priv->pdtid;
-  tid = priv->tid;
-
-  if (tid != PTHDB_INVALID_TID)
-    /* i18n: Like "thread-identifier %d, [state] running, suspended" */
-    buf.printf (_("tid %d"), (int)tid);
 
   status = pthdb_pthread_state (data->pd_session, pdtid, &state);
   if (status != PTHDB_SUCCESS)
     state = PST_NOTSUP;
-  buf.printf (", %s", state2str (state));
+  buf.printf ("[%s]", state2str (state));
 
   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"));
+    buf.printf (_("[suspended]"));
 
   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"));
+    buf.printf (_("[detached]"));
 
   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"));
+    buf.printf (_("[cancel pending]"));
 
   buf.write ("", 1);
 
-- 
2.41.0