[PATCH v2] Speed up next/step while debugging multithreaded programs on AIX.

[PATCH v2] Speed up next/step while debugging multithreaded programs on AIX.

[Aditya Vidyadhar Kamath]

From: Aditya Vidyadhar Kamath <aditya.kamath1@ibm.com>

During go compiler debugging on AIX my collegue and I noticed that
the next command was pretty slow compared to Linux - around 7 seconds
per step when program has atleast 3 or more threads running. A simple
walk through a function took around a minute. In compiler codes this is
slow.

TO measure this properly I wrote a benchmark pasted below. The program spawns
20 worker threads , then stops inside a function with 10 simple
that GDB steps over one by one. The GDB batch script breaks at that
function and issues 10 next commands and then quits. Timing the whole
run gives a fair picture of how much overhead each `next` carries.
Both the .c file and batch file are pasted below.

Before this patch, on AIX 7.3 ppc64 with 20 background threads:

real    1m10.05s
user    0m17.59s
sys     0m20.43s

That is roughly 7 seconds per `next` step.

After this patch:

real    0m20.37s
user    0m5.94s
sys     0m6.53s

About 2 seconds per step.  3.5x faster for the same workload.

Now why was it slow?

Every time the inferior stops -- even for a single-step trap from next
- the AIX thread layer called pd_update(), which did two very expensive
things unconditionally:

1. pthdb_session_update ()

This function walks through all of the pthread library's internal
data structures inside the inferior process, reading them out via
pdc_read_data() callbacks. With 20 threads that is over 300 memory
reads from the inferior on every single stop, even when absolutely
nothing about the threads has changed.

2. sync_threadlists()

This follows the session update with a full rebuild of GDB's thread
list: iterate every thread via pthdb_pthread(), call
pthdb_pthread_tid() for each one, allocate two sorted arrays, diff
them against GDB's internal list, and reconcile the differences.
Again, all of this on every stop, even when the thread population
has not changed at all since the last stop.

On top of that, get_signaled_thread() was calling getthrds() asking for
one kernel thread descriptor at a time, which costs one syscall per
thread in the process.

None of this work is wrong but it is needed when threads actually come
and go.  But during a next or step sequence through a function, the
thread list does not change. Paying for a full rescan on every stop is
something not needed I think.

So what I changed in this patch is:

1. Batch getthrds() calls get_signaled_thread ()

The original code passed 1 as the count argument to getthrds(), so
it did one syscall per kernel thread to find the one that stopped.
Changed to fetch 64 descriptors per call and scan the batch in a
loop. This alone cuts the getthrds() cost from O(N) syscalls to
O(N/64).

2. Track the thread count - aix_thread_variables, pd_activate

Added a last_thread_count field to aix_thread_variables.  It records
how many threads libpthdebug reported the last time a full sync was
done. Initialised to -1 in the constructor so the very
first stop always does a full sync. Also reset to -1 in pd_activate()
so that whenever a new debug session is opened the first stop is
always fully synced regardless of the stop kind.

3. Skip the sync on clean step stops

pd_update() now takes a bool step_stop parameter.  When that is true
and last_thread_count is not -1, it skips pthdb_session_update() and
sync_threadlists() entirely.

When a sync is needed (not a step stop, or first stop after
activation), a new helper count_pthdb_threads() does a lightweight
pass through pthdb_pthread() just to count threads. It only calls
sync_threadlists() if that count differs from last_thread_count.
This avoids the expensive pthdb_pthread_tid() calls and array
allocations on stops where the thread count has not changed.

4. Detect step stops in wait() and pass the flag down

wait() already has the stop kind and signal number before it calls
pd_update(). A TARGET_WAITKIND_STOPPED with GDB_SIGNAL_TRAP is what
the kernel reports for a completed single-step. wait() now computes
step_stop from those two fields and passes it to pd_update().

int current_count = count_pthdb_threads (data);
          if (current_count != data->last_thread_count)

This makes sure we never miss any genuinely coming new threads and are
in sync.

==================
Benchmark program (bench_next.c):

    /* Spawns NUM_THREADS worker threads that spin, then steps through
       10 assignments in do_steps() so GDB can time each next.  */
    #include <stdio.h>
    #include <stdlib.h>
    #include <pthread.h>
    #include <unistd.h>

    #ifndef NUM_THREADS
    #define NUM_THREADS 20
    #endif

    static volatile int keep_running = 1;

    static void *
    worker (void *arg)
    {
      while (keep_running)
        sched_yield ();
      return NULL;
    }

    void
    do_steps (void)
    {
      volatile int a = 1;
      volatile int b = 2;
      volatile int c = a + b;
      volatile int d = c * 2;
      volatile int e = d - a;
      volatile int f = e + c;
      volatile int g = f / 2;
      volatile int h = g + 1;
      volatile int i2 = h * h;
      volatile int j = i2 - b;
      (void)j;
    }

    int
    main (void)
    {
      pthread_t threads[NUM_THREADS];
      int i;

      for (i = 0; i < NUM_THREADS; i++)
        pthread_create (&threads[i], NULL, worker, NULL);

      do_steps ();

      keep_running = 0;
      for (i = 0; i < NUM_THREADS; i++)
        pthread_join (threads[i], NULL);

      return 0;
    }

====================
Build done with
gcc -O0 -g -gdwarf -maix64 -DNUM_THREADS=20 -o bench_next_bin bench_next.c -lpthread
=====================
GDB batch script (bench_next.gdb):

    set pagination off
    set confirm off

    break do_steps
    run

    next
    next
    next
    next
    next
    next
    next
    next
    next
    next

    quit
========================
Then run,
time gdb -batch -x bench_next.gdb ./bench_next_bin
---
 gdb/aix-thread.c | 153 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 130 insertions(+), 23 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index 5ac71d22237..3370736810a 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -173,6 +173,11 @@ static pthdb_callbacks_t pd_callbacks = {
 /* Aix variable structure.  */
 struct aix_thread_variables
 {
+  aix_thread_variables ()
+    : pd_able (0), pd_active (0), pd_session (0), pd_brk_addr (0),
+      arch64 (0), last_thread_count (-1), last_resume_step (0)
+  {}
+
   /* Whether the current application is debuggable by pthdb.  */
   int pd_able;
 
@@ -192,6 +197,19 @@ struct aix_thread_variables
 
   /* Describes the number of thread exit events reported.  */
   std::unordered_set<pthdb_pthread_t> exited_threads;
+
+  /* Last known libpthdebug thread count.  Used to skip sync_threadlists()
+     when the thread population has not changed since the previous stop,
+     which is the common case during stepping.  -1 means unknown/force sync.  */
+  int last_thread_count;
+
+  /* Set to non-zero by resume() when the resume was a software
+     single-step, i.e. single-step breakpoints were inserted for the
+     current thread before the inferior was set running.  Cleared to
+     zero for any other kind of resume.  Used by wait() to distinguish
+     a SIGTRAP from a completed single-step from one caused by a
+     breakpoint, since both look identical at the signal level.  */
+  int last_resume_step;
 };
 
 /* Key to our per-inferior data.  */
@@ -733,6 +751,11 @@ state2str (pthdb_state_t state)
     }
 }
 
+/* Number of thread descriptors to fetch per getthrds() call.  The original
+   code fetched one at a time, which costs one syscall per thread.  Fetching
+   in batches reduces that to one syscall per GETTHRDS_BATCH threads.  */
+#define GETTHRDS_BATCH 64
+
 /* 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.  */
@@ -740,20 +763,18 @@ state2str (pthdb_state_t state)
 static pthdb_tid_t
 get_signaled_thread (int pid)
 {
-  struct thrdsinfo64 thrinf;
+  struct thrdsinfo64 thrinf[GETTHRDS_BATCH];
   tid_t ktid = 0;
+  int count, i;
 
-  while (1)
+  while ((count = getthrds (pid, thrinf, sizeof (thrinf[0]),
+			    &ktid, GETTHRDS_BATCH)) > 0)
     {
-      if (getthrds (pid, &thrinf,
-		    sizeof (thrinf), &ktid, 1) != 1)
-	break;
-
       /* 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;
+      for (i = 0; i < count; i++)
+	if (thrinf[i].ti_cursig)
+	  return thrinf[i].ti_tid;
     }
 
   /* Didn't find any thread stopped on a SIGTRAP signal.  */
@@ -861,13 +882,43 @@ sync_threadlists (pid_t pid)
       }
 }
 
+/* Count the number of live pthreads visible to libpthdebug.
+   Used before sync_threadlists() to check whether the thread population
+   has changed since the last stop.  */
+
+static int
+count_pthdb_threads (struct aix_thread_variables *data)
+{
+  pthdb_pthread_t pdtid;
+  int cmd, n = 0;
+
+  for (cmd = PTHDB_LIST_FIRST;; cmd = PTHDB_LIST_NEXT)
+    {
+      int status = pthdb_pthread (data->pd_session, &pdtid, cmd);
+      if (status != PTHDB_SUCCESS || pdtid == PTHDB_INVALID_PTHREAD)
+	break;
+      n++;
+    }
+  return n;
+}
+
 /* Synchronize libpthdebug's state with the inferior and with GDB,
    generate a composite process/thread <pid> for the current thread,
-   Return the ptid of the event thread if one can be found, else
-   return a pid-only ptid with PID.  */
+   return the ptid of the event thread if one can be found, else
+   return a pid-only ptid with PID.
+
+   STEP_STOP should be set only when the last resume() was a software
+   single-step and the resulting stop is a SIGTRAP.  In that case, if
+   the thread list was already synced at a previous stop, both
+   pthdb_session_update() and sync_threadlists() are skipped.  Those
+   two calls dominate the cost of every stop in a multithreaded program,
+   so skipping them during stepping gives a large speedup.  When
+   STEP_STOP is false - meaning the resume was a free continue, or a
+   temporary breakpoint was used, or the signal is not SIGTRAP - the
+   full sync always runs so that newly created threads are not missed.  */
 
 static ptid_t
-pd_update (pid_t pid)
+pd_update (pid_t pid, bool step_stop = false)
 {
   int status;
   ptid_t ptid;
@@ -880,15 +931,36 @@ pd_update (pid_t pid)
   if (!data->pd_active)
     return ptid_t (pid);
 
-  status = pthdb_session_update (data->pd_session);
-  if (status != PTHDB_SUCCESS)
-    return ptid_t (pid);
-
-  /* Attempt to sync_threadlists () only when debugging object files
-     and not core files since list of threads never change for core
-     files.  */
-  if (target_has_execution ())
-    sync_threadlists (pid);
+  /* Skip the session update and thread list sync when stepping.
+     pthdb_session_update() reads through all pthread data structures in
+     the inferior on every call, and sync_threadlists() follows it with
+     O(N) kernel calls per thread.  During a next/step sequence the thread
+     list does not change, so both calls can be omitted as long as we have
+     done at least one full sync since activation (last_thread_count >= 0).
+     Any other stop kind (breakpoints, real signals) always does a full
+     sync because threads may have been created or destroyed.  */
+  bool need_sync = !step_stop || (data->last_thread_count < 0);
+
+  if (need_sync)
+    {
+      status = pthdb_session_update (data->pd_session);
+      if (status != PTHDB_SUCCESS)
+	return ptid_t (pid);
+
+      /* Attempt to sync_threadlists() only when debugging object files
+	 and not core files since the list of threads never changes for
+	 core files.  Even for live inferiors, skip the sync when the
+	 thread count matches the previous stop.  */
+      if (target_has_execution ())
+	{
+	  int current_count = count_pthdb_threads (data);
+	  if (current_count != data->last_thread_count)
+	    {
+	      sync_threadlists (pid);
+	      data->last_thread_count = current_count;
+	    }
+	}
+    }
 
   /* Define "current thread" as one that just received a trap signal.  */
 
@@ -921,7 +993,12 @@ pd_activate (pid_t pid)
 			       PTHDB_FLAG_REGS, &pd_callbacks,
 			       &data->pd_session);
   if (status == PTHDB_SUCCESS)
-    data->pd_active = 1;
+    {
+      data->pd_active = 1;
+      /* Reset last_thread_count so that the very first stop after
+	 activation always does a full sync_threadlists() pass.  */
+      data->last_thread_count = -1;
+    }
 }
 
 /* AIX implementation of update_thread_list.  */
@@ -1056,6 +1133,21 @@ aix_thread_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
 
   data = get_thread_data_helper_for_ptid (ptid);
 
+  /* Remember whether this resume is a software single-step so that
+     wait() can correctly classify the matching stop.  On AIX the
+     architecture uses software single-stepping (rs6000_software_single_step),
+     so the step argument passed down to target_resume is always 0.
+     The only reliable indicator that GDB is single-stepping is whether
+     single-step breakpoints were inserted for the current thread just
+     before this resume.  A breakpoint SIGTRAP from a user breakpoint
+     and a SIGTRAP from a completed software single-step are otherwise
+     indistinguishable at this layer.  */
+  if (data != nullptr)
+    {
+      struct thread_info *tp = inferior_thread ();
+      data->last_resume_step = thread_has_single_step_breakpoints_set (tp);
+    }
+
   if (ptid.tid () == 0)
     {
       scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
@@ -1125,7 +1217,22 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
 	pd_activate (ptid.pid ());
     }
 
-  return pd_update (ptid.pid ());
+  /* Decide whether to skip the expensive pthdb_session_update() and
+     sync_threadlists() calls in pd_update().  We can only skip them
+     when we know for certain this stop came from a software single-step.
+     On AIX, single-stepping is implemented by inserting breakpoints at
+     the next instruction(s) and resuming with step=0, so the step
+     argument to target_resume is always 0 and cannot be used here.
+     Instead, resume() records whether single-step breakpoints were set
+     at the time of the resume in last_resume_step.  If that flag is set
+     and the stop is a SIGTRAP, this is a completed single-step and the
+     thread list has not changed.  If last_resume_step is 0, GDB either
+     continued freely or planted a different kind of breakpoint, so a
+     new thread could exist and the sync must run.  */
+  bool step_stop = (data->last_resume_step
+		    && status->kind () == TARGET_WAITKIND_STOPPED
+		    && status->sig () == GDB_SIGNAL_TRAP);
+  return pd_update (ptid.pid (), step_stop);
 }
 
 /* Supply AIX altivec registers, both 64 and 32 bit.  */
-- 
2.51.2

Re: [PATCH v2] Speed up next/step while debugging multithreaded programs on AIX.

[Ulrich Weigand]

Aditya Vidyadhar Kamath <akamath996@gmail.com> wrote:

+      /* Attempt to sync_threadlists() only when debugging object
files
+	 and not core files since the list of threads never changes
for
+	 core files.  Even for live inferiors, skip the sync when the
+	 thread count matches the previous stop.  */

This last part seems racy to me.  In principle, it is possible
that one thread terminated and simultaneously one new thread was
created, since the last time we stopped.  In this case, the thread
count would be the same, but the thread list still incorrect.

Given the other test for single-stepping, is this particular
check even still resulting in a noticeable performance
difference?

Bye,
Ulrich

Re: [PATCH v2] Speed up next/step while debugging multithreaded programs on AIX.

[Simon Marchi]

On 2026-09-10 06:16, Aditya Vidyadhar Kamath wrote:
> From: Aditya Vidyadhar Kamath <aditya.kamath1@ibm.com>
> 
> During go compiler debugging on AIX my collegue and I noticed that
> the next command was pretty slow compared to Linux - around 7 seconds
> per step when program has atleast 3 or more threads running. A simple
> walk through a function took around a minute. In compiler codes this is
> slow.
> 
> TO measure this properly I wrote a benchmark pasted below. The program spawns
> 20 worker threads , then stops inside a function with 10 simple
> that GDB steps over one by one. The GDB batch script breaks at that

Seems to be missing a word after "simple".  Statements?

> function and issues 10 next commands and then quits. Timing the whole
> run gives a fair picture of how much overhead each `next` carries.
> Both the .c file and batch file are pasted below.
> 
> Before this patch, on AIX 7.3 ppc64 with 20 background threads:
> 
> real    1m10.05s
> user    0m17.59s
> sys     0m20.43s
> 
> That is roughly 7 seconds per `next` step.
> 
> After this patch:
> 
> real    0m20.37s
> user    0m5.94s
> sys     0m6.53s
> 
> About 2 seconds per step.  3.5x faster for the same workload.
> 
> Now why was it slow?
> 
> Every time the inferior stops -- even for a single-step trap from next
> - the AIX thread layer called pd_update(), which did two very expensive
> things unconditionally:
> 
> 1. pthdb_session_update ()
> 
> This function walks through all of the pthread library's internal
> data structures inside the inferior process, reading them out via
> pdc_read_data() callbacks. With 20 threads that is over 300 memory
> reads from the inferior on every single stop, even when absolutely
> nothing about the threads has changed.
> 
> 2. sync_threadlists()
> 
> This follows the session update with a full rebuild of GDB's thread
> list: iterate every thread via pthdb_pthread(), call
> pthdb_pthread_tid() for each one, allocate two sorted arrays, diff
> them against GDB's internal list, and reconcile the differences.
> Again, all of this on every stop, even when the thread population
> has not changed at all since the last stop.
> 
> On top of that, get_signaled_thread() was calling getthrds() asking for
> one kernel thread descriptor at a time, which costs one syscall per
> thread in the process.
> 
> None of this work is wrong but it is needed when threads actually come
> and go.  But during a next or step sequence through a function, the
> thread list does not change. Paying for a full rescan on every stop is
> something not needed I think.

I mean, it *could* change, you could do "next" over a function that
spawns a thread, a background thread (one other than the one you step)
could spawn a thread, another thread could have exited, etc.

> So what I changed in this patch is:
> 
> 1. Batch getthrds() calls get_signaled_thread ()
> 
> The original code passed 1 as the count argument to getthrds(), so
> it did one syscall per kernel thread to find the one that stopped.
> Changed to fetch 64 descriptors per call and scan the batch in a
> loop. This alone cuts the getthrds() cost from O(N) syscalls to
> O(N/64).

It's unrelated to this patch, but I happen to be looking at aix-thread.c
because of it and I must ask if these things are still relevant, given
the AIX versions and configurations you support nowadays:

1. #if !HAVE_DECL_GETTHRDS
   extern int getthrds (pid_t, struct thrdsinfo64 *, int, tid_t *, int);
   #endif

   The commit message that added this is:

     On newer versions of AIX (6.x and later), this function is actually
     declared in procinfo.h, thus causing a compilation warning when we
     re-declare it ourselves. This patch adds a configure check for that
     function allowing us to declare the function only if the declaration
     isn't already present in one of procinfo system header.

2. /* In AIX 5.1, functions use pthdb_tid_t instead of tid_t.  */
   #ifndef PTHDB_VERSION_3
   #define pthdb_tid_t	tid_t
   #endif

3. #ifdef HAVE_PTRACE64
   # define ptracex(request, pid, addr, data, buf) \
         ptrace64 (request, pid, addr, data, buf)
   #endif

> 2. Track the thread count - aix_thread_variables, pd_activate
> 
> Added a last_thread_count field to aix_thread_variables.  It records
> how many threads libpthdebug reported the last time a full sync was
> done. Initialised to -1 in the constructor so the very
> first stop always does a full sync. Also reset to -1 in pd_activate()
> so that whenever a new debug session is opened the first stop is
> always fully synced regardless of the stop kind.

Ulrich already pointed out that you could have one thread appear and one
disappear, and the count will not change.

> 3. Skip the sync on clean step stops
> 
> pd_update() now takes a bool step_stop parameter.  When that is true
> and last_thread_count is not -1, it skips pthdb_session_update() and
> sync_threadlists() entirely.
> 
> When a sync is needed (not a step stop, or first stop after
> activation), a new helper count_pthdb_threads() does a lightweight
> pass through pthdb_pthread() just to count threads. It only calls
> sync_threadlists() if that count differs from last_thread_count.
> This avoids the expensive pthdb_pthread_tid() calls and array
> allocations on stops where the thread count has not changed.

So yeah, what about doing "next" over a call that spawns a thread, or a
background thread existing while you do a next?

> 4. Detect step stops in wait() and pass the flag down
> 
> wait() already has the stop kind and signal number before it calls
> pd_update(). A TARGET_WAITKIND_STOPPED with GDB_SIGNAL_TRAP is what
> the kernel reports for a completed single-step. wait() now computes
> step_stop from those two fields and passes it to pd_update().

This sounds wrong, as a regular breakpoint is also (typically) reported
as a TARGET_WAITKIND_STOPPED with a SIGTRAP.  In the implementation, you
also check for data->last_resume_step, which is not mentioned here.

> int current_count = count_pthdb_threads (data);
>           if (current_count != data->last_thread_count)
> 
> This makes sure we never miss any genuinely coming new threads and are
> in sync.
> 
> ==================
> Benchmark program (bench_next.c):
> 
>     /* Spawns NUM_THREADS worker threads that spin, then steps through
>        10 assignments in do_steps() so GDB can time each next.  */
>     #include <stdio.h>
>     #include <stdlib.h>
>     #include <pthread.h>
>     #include <unistd.h>
> 
>     #ifndef NUM_THREADS
>     #define NUM_THREADS 20
>     #endif
> 
>     static volatile int keep_running = 1;
> 
>     static void *
>     worker (void *arg)
>     {
>       while (keep_running)
>         sched_yield ();
>       return NULL;
>     }
> 
>     void
>     do_steps (void)
>     {
>       volatile int a = 1;
>       volatile int b = 2;
>       volatile int c = a + b;
>       volatile int d = c * 2;
>       volatile int e = d - a;
>       volatile int f = e + c;
>       volatile int g = f / 2;
>       volatile int h = g + 1;
>       volatile int i2 = h * h;
>       volatile int j = i2 - b;
>       (void)j;
>     }
> 
>     int
>     main (void)
>     {
>       pthread_t threads[NUM_THREADS];
>       int i;
> 
>       for (i = 0; i < NUM_THREADS; i++)
>         pthread_create (&threads[i], NULL, worker, NULL);
> 
>       do_steps ();
> 
>       keep_running = 0;
>       for (i = 0; i < NUM_THREADS; i++)
>         pthread_join (threads[i], NULL);
> 
>       return 0;
>     }
> 
> ====================
> Build done with
> gcc -O0 -g -gdwarf -maix64 -DNUM_THREADS=20 -o bench_next_bin bench_next.c -lpthread
> =====================
> GDB batch script (bench_next.gdb):
> 
>     set pagination off
>     set confirm off
> 
>     break do_steps
>     run
> 
>     next
>     next
>     next
>     next
>     next
>     next
>     next
>     next
>     next
>     next
> 
>     quit
> ========================
> Then run,
> time gdb -batch -x bench_next.gdb ./bench_next_bin
> ---
>  gdb/aix-thread.c | 153 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 130 insertions(+), 23 deletions(-)
> 
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index 5ac71d22237..3370736810a 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -173,6 +173,11 @@ static pthdb_callbacks_t pd_callbacks = {
>  /* Aix variable structure.  */
>  struct aix_thread_variables
>  {
> +  aix_thread_variables ()
> +    : pd_able (0), pd_active (0), pd_session (0), pd_brk_addr (0),
> +      arch64 (0), last_thread_count (-1), last_resume_step (0)
> +  {}
> +
>    /* Whether the current application is debuggable by pthdb.  */
>    int pd_able;
>  
> @@ -192,6 +197,19 @@ struct aix_thread_variables
>  
>    /* Describes the number of thread exit events reported.  */
>    std::unordered_set<pthdb_pthread_t> exited_threads;
> +
> +  /* Last known libpthdebug thread count.  Used to skip sync_threadlists()
> +     when the thread population has not changed since the previous stop,
> +     which is the common case during stepping.  -1 means unknown/force sync.  */
> +  int last_thread_count;
> +
> +  /* Set to non-zero by resume() when the resume was a software
> +     single-step, i.e. single-step breakpoints were inserted for the
> +     current thread before the inferior was set running.  Cleared to
> +     zero for any other kind of resume.  Used by wait() to distinguish
> +     a SIGTRAP from a completed single-step from one caused by a
> +     breakpoint, since both look identical at the signal level.  */
> +  int last_resume_step;
>  };
>  
>  /* Key to our per-inferior data.  */
> @@ -733,6 +751,11 @@ state2str (pthdb_state_t state)
>      }
>  }
>  
> +/* Number of thread descriptors to fetch per getthrds() call.  The original
> +   code fetched one at a time, which costs one syscall per thread.  Fetching
> +   in batches reduces that to one syscall per GETTHRDS_BATCH threads.  */

No need to document what the original code did.  Just keep the first
sentence.

> +#define GETTHRDS_BATCH 64

Prefer:

constexpr int GETTHRDS_BATCH = 64;

> +
>  /* 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.  */

This predates your patch, but is the comment for the get_signaled_thread
function accurate?  It says it looks for a thread that has stopped on a
SIGTRAP signal, but the implementation seems to look for any signal, not
just SIGTRAP.

> @@ -740,20 +763,18 @@ state2str (pthdb_state_t state)
>  static pthdb_tid_t
>  get_signaled_thread (int pid)
>  {
> -  struct thrdsinfo64 thrinf;
> +  struct thrdsinfo64 thrinf[GETTHRDS_BATCH];
>    tid_t ktid = 0;
> +  int count, i;
>  
> -  while (1)
> +  while ((count = getthrds (pid, thrinf, sizeof (thrinf[0]),
> +			    &ktid, GETTHRDS_BATCH)) > 0)

Prefer declaring the variable in the while statement, and separating the
comparison from the assignment.  This should work:

  while (int count = getthrds (pid, thrinf, sizeof (thrinf[0]), &ktid,
			       GETTHRDS_BATCH));
	 count > 0)

>      {
> -      if (getthrds (pid, &thrinf,
> -		    sizeof (thrinf), &ktid, 1) != 1)
> -	break;
> -
>        /* 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;
> +      for (i = 0; i < count; i++)
> +	if (thrinf[i].ti_cursig)
> +	  return thrinf[i].ti_tid;

Declare variable `i` in the for loop.

Simon

Re: [PATCH v2] Speed up next/step while debugging multithreaded programs on AIX.

[Simon Marchi]

On 2026-09-10 09:14, Ulrich Weigand wrote:
> Aditya Vidyadhar Kamath <akamath996@gmail.com> wrote:
> 
> +      /* Attempt to sync_threadlists() only when debugging object
> files
> +	 and not core files since the list of threads never changes
> for
> +	 core files.  Even for live inferiors, skip the sync when the
> +	 thread count matches the previous stop.  */
> 
> This last part seems racy to me.  In principle, it is possible
> that one thread terminated and simultaneously one new thread was
> created, since the last time we stopped.  In this case, the thread
> count would be the same, but the thread list still incorrect.
> 
> Given the other test for single-stepping, is this particular
> check even still resulting in a noticeable performance
> difference?

I would suggest making a patch with just the get_signaled_thread batch
change first, since that one looks fairly safe and low hanging fruit.
Then, measure again and then look for the next optimization.

I would suggest using the "scoped_time_it" class to measure the
execution time of various functions and scopes.  This object measures
the time spent between its construction and destruction.  On
destruction, it prints a line like:

  Time for "DWARF indexing worker": wall 0.195, user 0.130, sys 0.033, user+sys 0.163, 83.6 % CPU

Include that information in your future commit messages (the before and
after).  That will give us an accurate picture of where time is spent,
and how much each optimization helps.

Just sprinkle it like this:

  {
    scoped_time_it time_it ("doing something");

    ...
  }

And then enable it with:

  (gdb) maintenance set per-command time on

Simon

Re: [PATCH v2] Speed up next/step while debugging multithreaded programs on AIX.

[Abhay Kandpal]

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

Hi Aditya,

On 10/09/26 15:46, Aditya Vidyadhar Kamath wrote:
> From: Aditya Vidyadhar Kamath<aditya.kamath1@ibm.com>
>
> During go compiler debugging on AIX my collegue and I noticed that
> the next command was pretty slow compared to Linux - around 7 seconds
> per step when program has atleast 3 or more threads running. A simple
> walk through a function took around a minute. In compiler codes this is
> slow.
>
> TO measure this properly I wrote a benchmark pasted below. The program spawns
> 20 worker threads , then stops inside a function with 10 simple
> that GDB steps over one by one. The GDB batch script breaks at that
> function and issues 10 next commands and then quits. Timing the whole
> run gives a fair picture of how much overhead each `next` carries.
> Both the .c file and batch file are pasted below.
>
> Before this patch, on AIX 7.3 ppc64 with 20 background threads:
>
> real    1m10.05s
> user    0m17.59s
> sys     0m20.43s
>
> That is roughly 7 seconds per `next` step.
>
> After this patch:
>
> real    0m20.37s
> user    0m5.94s
> sys     0m6.53s
>
> About 2 seconds per step.  3.5x faster for the same workload.
>
> Now why was it slow?
>
> Every time the inferior stops -- even for a single-step trap from next
> - the AIX thread layer called pd_update(), which did two very expensive
> things unconditionally:
>
> 1. pthdb_session_update ()
>
> This function walks through all of the pthread library's internal
> data structures inside the inferior process, reading them out via
> pdc_read_data() callbacks. With 20 threads that is over 300 memory
> reads from the inferior on every single stop, even when absolutely
> nothing about the threads has changed.
>
> 2. sync_threadlists()
>
> This follows the session update with a full rebuild of GDB's thread
> list: iterate every thread via pthdb_pthread(), call
> pthdb_pthread_tid() for each one, allocate two sorted arrays, diff
> them against GDB's internal list, and reconcile the differences.
> Again, all of this on every stop, even when the thread population
> has not changed at all since the last stop.
>
> On top of that, get_signaled_thread() was calling getthrds() asking for
> one kernel thread descriptor at a time, which costs one syscall per
> thread in the process.
>
> None of this work is wrong but it is needed when threads actually come
> and go.  But during a next or step sequence through a function, the
> thread list does not change. Paying for a full rescan on every stop is
> something not needed I think.
>
> So what I changed in this patch is:
>
> 1. Batch getthrds() calls get_signaled_thread ()
>
> The original code passed 1 as the count argument to getthrds(), so
> it did one syscall per kernel thread to find the one that stopped.
> Changed to fetch 64 descriptors per call and scan the batch in a
> loop. This alone cuts the getthrds() cost from O(N) syscalls to
> O(N/64).
>
> 2. Track the thread count - aix_thread_variables, pd_activate
>
> Added a last_thread_count field to aix_thread_variables.  It records
> how many threads libpthdebug reported the last time a full sync was
> done. Initialised to -1 in the constructor so the very
> first stop always does a full sync. Also reset to -1 in pd_activate()
> so that whenever a new debug session is opened the first stop is
> always fully synced regardless of the stop kind.
>
> 3. Skip the sync on clean step stops
>
> pd_update() now takes a bool step_stop parameter.  When that is true
> and last_thread_count is not -1, it skips pthdb_session_update() and
> sync_threadlists() entirely.
>
> When a sync is needed (not a step stop, or first stop after
> activation), a new helper count_pthdb_threads() does a lightweight
> pass through pthdb_pthread() just to count threads. It only calls
> sync_threadlists() if that count differs from last_thread_count.
> This avoids the expensive pthdb_pthread_tid() calls and array
> allocations on stops where the thread count has not changed.
>
> 4. Detect step stops in wait() and pass the flag down
>
> wait() already has the stop kind and signal number before it calls
> pd_update(). A TARGET_WAITKIND_STOPPED with GDB_SIGNAL_TRAP is what
> the kernel reports for a completed single-step. wait() now computes
> step_stop from those two fields and passes it to pd_update().
>
> int current_count = count_pthdb_threads (data);
>            if (current_count != data->last_thread_count)
>
> This makes sure we never miss any genuinely coming new threads and are
> in sync.
>
> ==================
> Benchmark program (bench_next.c):
>
>      /* Spawns NUM_THREADS worker threads that spin, then steps through
>         10 assignments in do_steps() so GDB can time each next.  */
>      #include <stdio.h>
>      #include <stdlib.h>
>      #include <pthread.h>
>      #include <unistd.h>
>
>      #ifndef NUM_THREADS
>      #define NUM_THREADS 20
>      #endif
>
>      static volatile int keep_running = 1;
>
>      static void *
>      worker (void *arg)
>      {
>        while (keep_running)
>          sched_yield ();
>        return NULL;
>      }
>
>      void
>      do_steps (void)
>      {
>        volatile int a = 1;
>        volatile int b = 2;
>        volatile int c = a + b;
>        volatile int d = c * 2;
>        volatile int e = d - a;
>        volatile int f = e + c;
>        volatile int g = f / 2;
>        volatile int h = g + 1;
>        volatile int i2 = h * h;
>        volatile int j = i2 - b;
>        (void)j;
>      }
>
>      int
>      main (void)
>      {
>        pthread_t threads[NUM_THREADS];
>        int i;
>
>        for (i = 0; i < NUM_THREADS; i++)
>          pthread_create (&threads[i], NULL, worker, NULL);
>
>        do_steps ();
>
>        keep_running = 0;
>        for (i = 0; i < NUM_THREADS; i++)
>          pthread_join (threads[i], NULL);
>
>        return 0;
>      }
>
> ====================
> Build done with
> gcc -O0 -g -gdwarf -maix64 -DNUM_THREADS=20 -o bench_next_bin bench_next.c -lpthread
> =====================
> GDB batch script (bench_next.gdb):
>
>      set pagination off
>      set confirm off
>
>      break do_steps
>      run
>
>      next
>      next
>      next
>      next
>      next
>      next
>      next
>      next
>      next
>      next
>
>      quit
> ========================
> Then run,
> time gdb -batch -x bench_next.gdb ./bench_next_bin
> ---
>   gdb/aix-thread.c | 153 ++++++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 130 insertions(+), 23 deletions(-)
>
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index 5ac71d22237..3370736810a 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -173,6 +173,11 @@ static pthdb_callbacks_t pd_callbacks = {
>   /* Aix variable structure.  */
>   struct aix_thread_variables
>   {
> +  aix_thread_variables ()
> +    : pd_able (0), pd_active (0), pd_session (0), pd_brk_addr (0),
> +      arch64 (0), last_thread_count (-1), last_resume_step (0)
> +  {}
> +
>     /* Whether the current application is debuggable by pthdb.  */
>     int pd_able;
>   
> @@ -192,6 +197,19 @@ struct aix_thread_variables
>   
>     /* Describes the number of thread exit events reported.  */
>     std::unordered_set<pthdb_pthread_t> exited_threads;
> +
> +  /* Last known libpthdebug thread count.  Used to skip sync_threadlists()
> +     when the thread population has not changed since the previous stop,
> +     which is the common case during stepping.  -1 means unknown/force sync.  */
> +  int last_thread_count;
> +
> +  /* Set to non-zero by resume() when the resume was a software
> +     single-step, i.e. single-step breakpoints were inserted for the
> +     current thread before the inferior was set running.  Cleared to
> +     zero for any other kind of resume.  Used by wait() to distinguish
> +     a SIGTRAP from a completed single-step from one caused by a
> +     breakpoint, since both look identical at the signal level.  */
> +  int last_resume_step;
>   };
>   
>   /* Key to our per-inferior data.  */
> @@ -733,6 +751,11 @@ state2str (pthdb_state_t state)
>       }
>   }
>   
> +/* Number of thread descriptors to fetch per getthrds() call.  The original
> +   code fetched one at a time, which costs one syscall per thread.  Fetching
> +   in batches reduces that to one syscall per GETTHRDS_BATCH threads.  */
> +#define GETTHRDS_BATCH 64
> +
>   /* 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.  */
> @@ -740,20 +763,18 @@ state2str (pthdb_state_t state)
>   static pthdb_tid_t
>   get_signaled_thread (int pid)
>   {
> -  struct thrdsinfo64 thrinf;
> +  struct thrdsinfo64 thrinf[GETTHRDS_BATCH];
>     tid_t ktid = 0;
> +  int count, i;
>   
> -  while (1)
> +  while ((count = getthrds (pid, thrinf, sizeof (thrinf[0]),
> +			    &ktid, GETTHRDS_BATCH)) > 0)
>       {
> -      if (getthrds (pid, &thrinf,
> -		    sizeof (thrinf), &ktid, 1) != 1)
> -	break;
> -
>         /* 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;
> +      for (i = 0; i< count; i++) + if (thrinf[i].ti_cursig) + return thrinf[i].ti_tid; } 
> /* Didn't find any thread stopped on a SIGTRAP signal. */ @@ -861,13 
> +882,43 @@ sync_threadlists (pid_t pid) } } +/* Count the number of 
> live pthreads visible to libpthdebug. + Used before sync_threadlists() 
> to check whether the thread population + has changed since the last 
> stop. */ + +static int +count_pthdb_threads (struct 
> aix_thread_variables *data) +{ + pthdb_pthread_t pdtid; + int cmd, n = 
> 0; + + for (cmd = PTHDB_LIST_FIRST;; cmd = PTHDB_LIST_NEXT) + { + int 
> status = pthdb_pthread (data->pd_session, &pdtid, cmd);
> +      if (status != PTHDB_SUCCESS || pdtid == PTHDB_INVALID_PTHREAD)
> +	break;
> +      n++;
> +    }
> +  return n;
> +}
> +
>   /* Synchronize libpthdebug's state with the inferior and with GDB,
>      generate a composite process/thread <pid> for the current thread,
> -   Return the ptid of the event thread if one can be found, else
> -   return a pid-only ptid with PID.  */
> +   return the ptid of the event thread if one can be found, else
> +   return a pid-only ptid with PID.
> +
> +   STEP_STOP should be set only when the last resume() was a software
> +   single-step and the resulting stop is a SIGTRAP.  In that case, if
> +   the thread list was already synced at a previous stop, both
> +   pthdb_session_update() and sync_threadlists() are skipped.  Those
> +   two calls dominate the cost of every stop in a multithreaded program,
> +   so skipping them during stepping gives a large speedup.  When
> +   STEP_STOP is false - meaning the resume was a free continue, or a
> +   temporary breakpoint was used, or the signal is not SIGTRAP - the
> +   full sync always runs so that newly created threads are not missed.  */
>   
>   static ptid_t
> -pd_update (pid_t pid)
> +pd_update (pid_t pid, bool step_stop = false)
>   {
>     int status;
>     ptid_t ptid;
> @@ -880,15 +931,36 @@ pd_update (pid_t pid)
>     if (!data->pd_active)
>       return ptid_t (pid);
>   
> -  status = pthdb_session_update (data->pd_session);
> -  if (status != PTHDB_SUCCESS)
> -    return ptid_t (pid);
> -
> -  /* Attempt to sync_threadlists () only when debugging object files
> -     and not core files since list of threads never change for core
> -     files.  */
> -  if (target_has_execution ())
> -    sync_threadlists (pid);
> +  /* Skip the session update and thread list sync when stepping.
> +     pthdb_session_update() reads through all pthread data structures in
> +     the inferior on every call, and sync_threadlists() follows it with
> +     O(N) kernel calls per thread.  During a next/step sequence the thread
> +     list does not change, so both calls can be omitted as long as we have
> +     done at least one full sync since activation (last_thread_count >= 0).
> +     Any other stop kind (breakpoints, real signals) always does a full
> +     sync because threads may have been created or destroyed.  */
> +  bool need_sync = !step_stop || (data->last_thread_count < 0);
> +
> +  if (need_sync)
> +    {
> +      status = pthdb_session_update (data->pd_session);
> +      if (status != PTHDB_SUCCESS)
> +	return ptid_t (pid);
> +
> +      /* Attempt to sync_threadlists() only when debugging object files
> +	 and not core files since the list of threads never changes for
> +	 core files.  Even for live inferiors, skip the sync when the
> +	 thread count matches the previous stop.  */
> +      if (target_has_execution ())
> +	{
> +	  int current_count = count_pthdb_threads (data);
> +	  if (current_count != data->last_thread_count)
> +	    {
> +	      sync_threadlists (pid);
> +	      data->last_thread_count = current_count;
> +	    }
> +	}
> +    }
>   
>     /* Define "current thread" as one that just received a trap signal.  */
>   
> @@ -921,7 +993,12 @@ pd_activate (pid_t pid)
>   			       PTHDB_FLAG_REGS, &pd_callbacks,
>   			       &data->pd_session);
>     if (status == PTHDB_SUCCESS)
> -    data->pd_active = 1;
> +    {
> +      data->pd_active = 1;
> +      /* Reset last_thread_count so that the very first stop after
> +	 activation always does a full sync_threadlists() pass.  */
> +      data->last_thread_count = -1;
> +    }
>   }
>   
>   /* AIX implementation of update_thread_list.  */
> @@ -1056,6 +1133,21 @@ aix_thread_target::resume (ptid_t ptid, int step, enum gdb_signal sig)
>   
>     data = get_thread_data_helper_for_ptid (ptid);
>   
> +  /* Remember whether this resume is a software single-step so that
> +     wait() can correctly classify the matching stop.  On AIX the
> +     architecture uses software single-stepping (rs6000_software_single_step),
> +     so the step argument passed down to target_resume is always 0.
> +     The only reliable indicator that GDB is single-stepping is whether
> +     single-step breakpoints were inserted for the current thread just
> +     before this resume.  A breakpoint SIGTRAP from a user breakpoint
> +     and a SIGTRAP from a completed software single-step are otherwise
> +     indistinguishable at this layer.  */
> +  if (data != nullptr)
> +    {
> +      struct thread_info *tp = inferior_thread ();
> +      data->last_resume_step = thread_has_single_step_breakpoints_set (tp);
> +    }
> +

inferior_thread () returns the currently selected thread, which is not
necessarily the thread named by the ptid argument.  Since
thread_has_single_step_breakpoints_set () is a per-thread query, asking
it about the wrong thread would give the wrong answer for
last_resume_step.

The else branch a few lines below already resolves the thread from the
argument:

   thread = current_inferior ()->find_thread (ptid);

Would that be more appropriate here?  I am also unsure what the flag
should be when ptid.tid () == 0 and the resume covers more than one
thread.

Separately, would it be worth adding testsuite coverage for this?  The
benchmark program creates all 20 threads before do_steps (), so the
thread population never changes after the first stop and it cannot
exercise the case the v1 logic got wrong.

Something that creates threads between two breakpoint stops and checks
"info threads" at the second one would cover it, and a "next" over a
call that spawns a thread would cover Simon's point.  I don't have an
AIX machine to run it on, so I can't offer a patch for it myself.

Regards
Abhay

>     if (ptid.tid () == 0)
>       {
>         scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
> @@ -1125,7 +1217,22 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
>   	pd_activate (ptid.pid ());
>       }
>   
> -  return pd_update (ptid.pid ());
> +  /* Decide whether to skip the expensive pthdb_session_update() and
> +     sync_threadlists() calls in pd_update().  We can only skip them
> +     when we know for certain this stop came from a software single-step.
> +     On AIX, single-stepping is implemented by inserting breakpoints at
> +     the next instruction(s) and resuming with step=0, so the step
> +     argument to target_resume is always 0 and cannot be used here.
> +     Instead, resume() records whether single-step breakpoints were set
> +     at the time of the resume in last_resume_step.  If that flag is set
> +     and the stop is a SIGTRAP, this is a completed single-step and the
> +     thread list has not changed.  If last_resume_step is 0, GDB either
> +     continued freely or planted a different kind of breakpoint, so a
> +     new thread could exist and the sync must run.  */
> +  bool step_stop = (data->last_resume_step
> +		    && status->kind () == TARGET_WAITKIND_STOPPED
> +		    && status->sig () == GDB_SIGNAL_TRAP);
> +  return pd_update (ptid.pid (), step_stop);
>   }
>   
>   /* Supply AIX altivec registers, both 64 and 32 bit.  */

[-- Attachment #2: Type: text/html, Size: 17402 bytes --]

Re: [PATCH v2] Speed up next/step while debugging multithreaded programs on AIX.

[Aditya Kamath]

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

Hi Abhay and community members,

Thank you for the feedback so far.

Please see my comments.

>inferior_thread () returns the currently selected thread, >which is not
>necessarily the thread named by the ptid argument.  Since
>thread_has_single_step_breakpoints_set () is a per-thread >query, asking
>it about the wrong thread would give the wrong answer for
>last_resume_step.
>The else branch a few lines below already resolves the >thread from the
>argument:
>  thread = current_inferior ()->find_thread (ptid);
>Would that be more appropriate here?  I am also unsure what >the flag
>should be when ptid.tid () == 0 and the resume covers more >than one
>thread.

>
>Separately, would it be worth adding testsuite coverage for >this?  The
>benchmark program creates all 20 threads before do_steps (), >so the
>thread population never changes after the first stop and it >cannot
>exercise the case the v1 logic got wrong.

>Something that creates threads between two breakpoint stops >and checks
>"info threads" at the second one would cover it, and a >"next" over a
>call that spawns a thread would cover Simon's point.  I >don't have an
>AIX machine to run it on, so I can't offer a patch for it >myself.


Have done the same in v3 version of this patch and also corrected the thread_has_single_step_breakpoints_set () to use ptid. Thanks for the suggestions. Let me know your thoughts and if I missed anything.

Have a nice day.

Thanks and regards,
Aditya.

[-- Attachment #2: Type: text/html, Size: 4197 bytes --]

RE: [PATCH v2] Speed up next/step while debugging multithreaded programs on AIX.

[Aditya Kamath]

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

Hi Simon, Ulrich and community members,

Please see comments below. Thank you very much for the feedback. Special thanks Simon for suggesting something I explored in GDB codebase.


>> This last part seems racy to me.  In principle, it is possible
>> that one thread terminated and simultaneously one new thread was
>> created, since the last time we stopped.  In this case, the thread
>> count would be the same, but the thread list still incorrect.
>>
>> Given the other test for single-stepping, is this particular
>> check even still resulting in a noticeable performance
>> difference?

Thanks Ulrich. In v3 version of the patch I have made sure we use only single step and no counts are used to save time. It is something I did not think of.

>I would suggest making a patch with just the get_signaled_thread batch
>change first, since that one looks fairly safe and low hanging fruit.
>Then, measure again and then look for the next optimization.
>I would suggest using the "scoped_time_it" class to measure the
>execution time of various functions and scopes.  This object measures

Thanks Simon,

In 2nd and 3rd patch of the coming series I have done the same. Sending the patch with this data in the commit message.

Have a nice day ahead.

Thanks and regards,
Aditya.

[-- Attachment #2: Type: text/html, Size: 3399 bytes --]

RE: [PATCH v2] Speed up next/step while debugging multithreaded programs on AIX.

[Aditya Kamath]

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



Hi everyone,

I am sending a 3 patch series to address the concerns, Thank you very much for the review. Here are my thoughts.

>> per step when program has atleast 3 or more threads running. A simple
>> walk through a function took around a minute. In compiler codes this is
>> slow.
>>
>> TO measure this properly I wrote a benchmark pasted below. The program spawns
>> 20 worker threads , then stops inside a function with 10 simple
>> that GDB steps over one by one. The GDB batch script breaks at that

>Seems to be missing a word after "simple".  Statements?

Made the corrections.


>> None of this work is wrong but it is needed when threads actually come
>> and go.  But during a next or step sequence through a function, the
>> thread list does not change. Paying for a full rescan on every stop is
>> something not needed I think.

>I mean, it *could* change, you could do "next" over a function that
>spawns a thread, a background thread (one other than the one you step)
>could spawn a thread, another thread could have exited, etc.

Yes, in the v3 version I have handle this. GDB resumes all threads (to let the callee execute), using ptid.tid() == 0. That path in resume() sets last_resume_step = 0. So when the return-address breakpoint fires, step_stop = false and the full sync runs and any new thread is caught.


>It's unrelated to this patch, but I happen to be looking at aix-thread.c
>because of it and I must ask if these things are still relevant, given
>the AIX versions and configurations you support nowadays:

The first patch in the coming series addresses these. Thanks for pointing out.

>Ulrich already pointed out that you could have one thread appear and one
>disappear, and the count will not change.

Yes, this is something I did not think. Thanks to both of you. The v3 patch implementation is based on single step only.  No thread counts used.

>So yeah, what about doing "next" over a call that spawns a thread, or a
>background thread existing while you do a next?

The fix is safe for both scenarios because step_stop=true is only set when software single-step breakpoints were inserted for the resumed thread which means GDB knows the thread executed exactly one instruction and could not have called pthread_create or pthread_exit. For next over a function call that spawns a thread, GDB resumes all threads (ptid.tid()==0), which explicitly clears last_resume_step=0 at line 1087, so step_stop is never true and sync_threadlists() runs in full, picking up the new thread. For a background thread exiting during next, the single-step skips only apply to the internal one-instruction steps of the stepped thread; as soon as GDB resumes all threads for the next source-line boundary it clears last_resume_step=0, so the following stop calls sync_threadlists() and detects the exit. In both cases sync_threadlists() is always called on any stop where thread population could have changed. The optimization only fires for stops that are pure single-step traps of a single thread. No new thread can be created or destroyed in one instruction, so skipping the session update on those stops is correct. This is what my thought process is.


>> +/* Number of thread descriptors to fetch per getthrds() call.  The original
>> +   code fetched one at a time, which costs one syscall per thread.  Fetching
>> +   in batches reduces that to one syscall per GETTHRDS_BATCH threads.  */

>No need to document what the original code did.  Just keep the first
>sentence.

Sure, corrected the same in second patch of the coming series.


>> +#define GETTHRDS_BATCH 64
>Prefer:
>constexpr int GETTHRDS_BATCH = 64;

Done this in v3 version of the patch.

>> +
>>  /* 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.  */
>This predates your patch, but is the comment for the get_signaled_thread
>function accurate?  It says it looks for a thread that has stopped on a
>SIGTRAP signal, but the implementation seems to look for any signal, not
>just SIGTRAP.

I have corrected this.


>> -  while (1)
>> +  while ((count = getthrds (pid, thrinf, sizeof (thrinf[0]),
>> +                         &ktid, GETTHRDS_BATCH)) > 0)

>>Prefer declaring the variable in the while statement, and separating the
>>comparison from the assignment.  This should work:

 >> while (int count = getthrds (pid, thrinf, sizeof (thrinf[0]), &ktid,
  >>                            GETTHRDS_BATCH));
  >>       count > 0)

>>     {

>> +      for (i = 0; i < count; i++)
>> +     if (thrinf[i].ti_cursig)
>> +       return thrinf[i].ti_tid;

>Declare variable `i` in the for loop.

The above two are addressed in v3 version of this patch. Simon, that works with for loop and not with while.

Have a nice day ahead.

Thanks and regards,
Aditya.



[-- Attachment #2: Type: text/html, Size: 13975 bytes --]