public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Aditya Kamath1 <Aditya.Kamath1@ibm.com>
To: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>,
	"simark@simark.ca" <simark@simark.ca>,
	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: Re: [PATCH] Enable multi process debugging for AIX
Date: Thu, 10 Nov 2022 10:39:06 +0000	[thread overview]
Message-ID: <CH2PR15MB35441E85211E00CE4305FE0CD6019@CH2PR15MB3544.namprd15.prod.outlook.com> (raw)
In-Reply-To: <e54ccf49ba418f64643f26eb7fcbcd6d4a9796b8.camel@de.ibm.com>


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

Hi Ulrich and Simon,

Please find attached the patch [See: 0001-Enable-multi-process-debugging-for-AIX.patch]

I have taken care of parent child relationship. See find_my_aix_parent () function. You can refer the link https://www.ibm.com/docs/en/aix/7.2?topic=g-getprocs-subroutine for the documentation of the same.

I have also taken care of what if break points hit in other threads while only a child or a parent event is reported in another thread as you mentioned Ulrich or any other event. [You can refer the output I have pasted below]. The while loop will take care.
getprocs Subroutine - IBM<https://www.ibm.com/docs/en/aix/7.2?topic=g-getprocs-subroutine>
Note: The ProcessBuffer parameter of getprocs subroutine contains two struct rusage fields named pi_ru and pi_cru.Each of these fields contains two struct timeval fields named ru_utime and ru_stime.The tv_usec field in both of the struct timeval contain nanoseconds instead of microseconds. These values cone from the struct user fields named U_ru and U_cru.
www.ibm.com


>I understand W_SFWTED is not a bit mask, but simply a value.
>So shouldn't you just test for "status == W_SFWTED"?  Also,
>I'm not sure where the "& 0xff" comes into play.  What are
>the contents of the high bits?  The AIX documentation doesn't
>seem to say ...

So, if there is a fork event then my W_SFWTED = 0x7e = 0x01111110 bits is set in my status flag. The other bits represent other things in the status flag. Therefore, I eliminate other bits with AND operation with 0xff and see only if my first 8 bits are set.. You were right with this "status == W_SFWTED". I corrected it.


Let me know what you think.. Hope this patch works..

Have a nice day ahead.

Thanks and regards,
Aditya.

--------------------------------------------------------------
Consider the program


#include <stdio.h>

#include <unistd.h>

#include <stdlib.h>

#include <pthread.h>

#include <assert.h>


pthread_barrier_t barrier;


#define NUM_THREADS 2


void *

thread_function (void *arg)

{

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

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

     thread when the breakpoint triggers.  */


  pthread_barrier_wait (&barrier);

  pid_t child;


  child = fork ();

  if (child > 0)

    printf ("I am parent \n");

  else{

    printf (" Iam child \n");

    child = fork ();

    if (child > 0)

      printf ("From child I became a parent \n");

    else

      printf ("I am grandchild \n");

  }

  while (1); /* 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);

    }


  while (1)

    sleep (1);


  return 0;

}


Output:-


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

(gdb) b thread_function

Breakpoint 1 at 0x100006f8: file /home/XYZ/gdb_tests/ultimate-multi-thread-fork.c, line 18.

(gdb) set detach-on-fork off

(gdb) r

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

[New Thread 1]

[New Thread 258]

[Switching to Thread 258]


Thread 3 hit Breakpoint 1, thread_function (arg=0x0)

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

18        pthread_barrier_wait (&barrier);

(gdb) c

Continuing.

[New Thread 515]

[Switching to Thread 515]


Thread 4 hit Breakpoint 1, thread_function (arg=warning: (Internal error: pc 0x0 in read in psymtab, but not in symtab.)


0x0)

    at /home/XYZ/gdb_tests/ultimate-multi-thread-fork.c:18

18        pthread_barrier_wait (&barrier);

(gdb) c

Continuing.

[New inferior 2 (process 18612656)]

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

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

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

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

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

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

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

I am parent

[New inferior 3 (process 8454632)]

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

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

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

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

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

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

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

I am parent

^C

Thread 1.1 received signal SIGINT, Interrupt.

[Switching to process 23462236]

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

(gdb)






________________________________
From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Sent: 02 November 2022 14:26
To: simark@simark.ca <simark@simark.ca>; Aditya Kamath1 <Aditya.Kamath1@ibm.com>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: Re: [PATCH] Enable multi process debugging for AIX

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

>>Is there any other way of figuring out what the parent process of
>>the new child is?  Maybe you should just do that when you get the
>>child event.
>
>I fully did not get the above suggestion " Maybe you should just do that when you get the
>child event ". Kindly let me know what you are trying to tell me in this context.

The last iteration of you patch simply has a list of "pending parent"
and "pending child" PIDs.  If you get a fork event for the child, and
there is *any* parent PID in the "pending" list, you automatically
assume that this is the parent of *this* child.

I'm concerned that this may not always be true.  For example, if you
are already debugging multiple processes, and *two* of those fork at
the same time, you'll be getting four ptrace events in GDB - two
events for the two parent processes, and two events for the two new
child processes.  If this happens, you'll need to verify which child
is actually associated with *which* parent.

As you note, on other platforms this relationship is reported as part
of the ptrace event directly, and this is apparently not the case on
AIX.  However, the OS of course still knows what the parent process
of each of the child processes is - so I guess there should be *some*
way to find this out from within GDB.  (E.g. in Linux you could get
at that information by reading some /proc files.)

Bye,
Ulrich


[-- Attachment #2: 0001-Enable-multi-process-debugging-for-AIX.patch --]
[-- Type: application/octet-stream, Size: 9396 bytes --]

From 8b7a0ee753f8bc84f872062c80126e6b04f57384 Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Thu, 10 Nov 2022 03:55:56 -0600
Subject: [PATCH] Enable Multi process debugging for AIX.

---
 gdb/rs6000-aix-nat.c | 247 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 227 insertions(+), 20 deletions(-)

diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index cb141427696..36abf186ba0 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -54,6 +54,10 @@
 #include <sys/ldr.h>
 #include <sys/systemcfg.h>
 
+/* Header files for getting ppid in AIX of a child process.  */
+#include <procinfo.h>
+#include <sys/types.h>
+ 
 /* On AIX4.3+, sys/ldr.h provides different versions of struct ld_info for
    debugging 32-bit and 64-bit processes.  Define a typedef and macros for
    accessing fields in the appropriate structures.  */
@@ -91,10 +95,13 @@ class rs6000_nat_target final : public inf_ptrace_target
 
   ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override;
 
+  /* Fork detection related functions, For adding multi process debugging
+     support.  */
+  void follow_fork (inferior *, ptid_t, target_waitkind, bool, bool) override;
+
 protected:
 
-  void post_startup_inferior (ptid_t ptid) override
-  { /* Nothing.  */ }
+  void post_startup_inferior (ptid_t ptid) override;
 
 private:
   enum target_xfer_status
@@ -107,6 +114,120 @@ class rs6000_nat_target final : public inf_ptrace_target
 
 static rs6000_nat_target the_rs6000_nat_target;
 
+/* The below declaration is to track number of times, parent has
+   reported fork event before its children.  */
+
+static std::list<pid_t> aix_pending_parent;
+
+/* The below declaration is for a child process event that
+   is reported before its corresponding parent process in
+   the event of a fork ().  */
+
+static std::list<pid_t> aix_pending_children;
+
+static void
+aix_remember_child (pid_t pid)
+{
+  aix_pending_children.push_front (pid);
+}
+
+static void
+aix_remember_parent (pid_t pid)
+{
+  aix_pending_parent.push_front (pid);
+}
+
+/* This function returns a parent of a child process.  */
+
+static pid_t
+find_my_aix_parent (pid_t child_pid)
+{
+  struct procsinfo ProcessBuffer1;
+  int ProcessSize1;
+  struct fdsinfo FileBuffer1;
+  int FileSize1;
+  pid_t IndexPointer1 = 0;
+  int Count = 1;
+  while (1)
+  {
+    if (getprocs (&ProcessBuffer1, sizeof (ProcessBuffer1),
+        &FileBuffer1, sizeof (FileBuffer1),  &IndexPointer1,
+        Count) != 1)
+      break; 
+     
+    if (child_pid == ProcessBuffer1.pi_pid)
+      return ProcessBuffer1.pi_ppid;
+  }
+  return 0;
+}
+
+/* In the below function we check if there was any child
+   process pending.  If it exists we return it from the
+   list, otherwise we return a null.  */
+
+static pid_t
+has_my_aix_child_reported (pid_t parent_pid)
+{
+  auto it = aix_pending_children.begin ();
+  pid_t child;
+  pid_t my_parent;
+  while (it != aix_pending_children.end ())
+  {
+    child = *it;
+    my_parent = find_my_aix_parent (child);
+    if (my_parent == parent_pid)
+    {
+      aix_pending_children.erase (it);
+      return child;
+    }
+    it++;
+  }
+  return 0;
+}
+
+/* In the below function we check if there was any parent 
+   process pending.  If it exists we return it from the
+   list, otherwise we return a null.  */
+
+static pid_t
+has_my_aix_parent_reported (pid_t child_pid)
+{
+  auto it = aix_pending_parent.begin ();
+  pid_t parent;
+  pid_t my_parent = find_my_aix_parent (child_pid);
+  while (it != aix_pending_parent.end ())
+  {
+    parent = *it;
+    if (my_parent == parent)
+    {
+      aix_pending_parent.erase (it);
+      return parent;
+    }
+    it++;
+  }
+  return 0;
+}
+
+/* This function checks if there was a fork () event.  */
+
+static bool
+did_aix_inferior_fork (int status)
+{
+  /* If multi-process debug mode is enabled, the status
+     location is set to W_SFWTED.  */
+
+  status = status & 0xff;
+
+  /* Eliminate the last few bits. If the W_SFWTED is set
+     which is equal to 0x7e, it is a fork event otherwise
+     it is not.  */
+
+  if (status != W_SFWTED)
+    return false;
+  else
+    return true;
+}
+
 /* Given REGNO, a gdb register number, return the corresponding
    number suitable for use as a ptrace() parameter.  Return -1 if
    there's no suitable mapping.  Also, set the int pointed to by
@@ -187,6 +308,47 @@ rs6000_ptrace64 (int req, int id, long long addr, int data, void *buf)
   return ret;
 }
 
+void rs6000_nat_target::post_startup_inferior (ptid_t ptid)
+{
+
+  /* In AIX to turn on multi process debugging in ptrace
+     PT_MULTI is the option to be passed,
+     with the process ID which can fork () and
+     the data parameter [fourth parameter] must be 1.  */
+
+  if (!ARCH64 ())
+    rs6000_ptrace32 (PT_MULTI, ptid.pid(), 0, 1, 0);
+  else
+    rs6000_ptrace64 (PT_MULTI, ptid.pid(), 0, 1, 0);
+}
+
+void
+rs6000_nat_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
+                          target_waitkind fork_kind, bool follow_child,
+                                                      bool detach_fork)
+{
+
+  /* Once the fork event is detected the infrun.c code
+     calls the target_follow_fork to take care of
+     follow child and detach the child activity which is
+     done using the function below.  */
+
+  inf_ptrace_target::follow_fork (child_inf, child_ptid, fork_kind,
+                                        follow_child, detach_fork);
+
+  /* If we detach fork and follow child we do not want the child
+     process to geneate events that ptrace can trace.  Hence we
+     detach it.  */
+
+  if (detach_fork && !follow_child)
+  {
+    if (ARCH64 ())
+      rs6000_ptrace64 (PT_DETACH, child_ptid.pid (), 0, 0, 0);
+    else
+      rs6000_ptrace32 (PT_DETACH, child_ptid.pid (), 0, 0, 0);
+  }
+}
+
 /* Fetch register REGNO from the inferior.  */
 
 static void
@@ -501,10 +663,10 @@ ptid_t
 rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 			 target_wait_flags options)
 {
-  pid_t pid;
+  pid_t pid = -1;
   int status, save_errno;
 
-  do
+  while (1) 
     {
       set_sigint_trap ();
 
@@ -514,7 +676,7 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 	  save_errno = errno;
 	}
       while (pid == -1 && errno == EINTR);
-
+      
       clear_sigint_trap ();
 
       if (pid == -1)
@@ -530,22 +692,67 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
       /* Ignore terminated detached child processes.  */
       if (!WIFSTOPPED (status) && find_inferior_pid (this, pid) == nullptr)
 	pid = -1;
+      
+      if (pid != -1)
+      {
+        /* AIX has a couple of strange returns from wait().  */
+
+        /* stop after load" status.  */
+        if (status == 0x57c)
+          ourstatus->set_loaded ();
+        /* 0x7f is signal 0.  */
+        /* 0x17f and 0x137f in hexadecimal are status returned if
+           if we follow parent,
+           a switch is made to a child post parent execution
+           and child continues its execution [user switches to child and
+           presses continue].  */
+        else if (status == 0x7f || status == 0x17f || status == 0x137f)
+          ourstatus->set_spurious ();
+        /* Check for a fork () event.  */
+        else if (did_aix_inferior_fork (status))
+        {
+          /* Checking whether it is a parent or a child event.  */
+
+          /* If the event is a child we check if there was a parent
+             event recorded before.  If yes we got the parent child
+             relationship.  If not we push this child and wait for
+             the next fork () event.  */
+
+          if (find_inferior_pid (this, pid) == nullptr)
+            {
+              pid_t parent_pid = has_my_aix_parent_reported (pid);
+              if (parent_pid > 0)
+              {
+                ourstatus->set_forked (ptid_t (pid));
+                return ptid_t (parent_pid);
+              }
+              aix_remember_child (pid);
+            }
+
+            /* If the event is a parent we check if there was a child
+               event recorded before.  If yes we got the parent child
+               relationship.  If not we push this parent and wait for
+               the next fork () event.  */
+
+          else
+            {
+              pid_t child_pid = has_my_aix_child_reported (pid);
+              if (child_pid > 0)
+              {
+                ourstatus->set_forked (ptid_t (child_pid));
+                return ptid_t (pid);
+              }
+              aix_remember_parent (pid);
+            }   
+          continue;
+        }
+      /* A normal waitstatus.  Let the usual macros deal with it.  */
+      else
+        *ourstatus = host_status_to_waitstatus (status);
+      
+      return ptid_t (pid);
     }
-  while (pid == -1);
-
-  /* AIX has a couple of strange returns from wait().  */
-
-  /* stop after load" status.  */
-  if (status == 0x57c)
-    ourstatus->set_loaded ();
-  /* signal 0.  I have no idea why wait(2) returns with this status word.  */
-  else if (status == 0x7f)
-    ourstatus->set_spurious ();
-  /* A normal waitstatus.  Let the usual macros deal with it.  */
-  else
-    *ourstatus = host_status_to_waitstatus (status);
-
-  return ptid_t (pid);
+  }
 }
 \f
 
-- 
2.31.1


  reply	other threads:[~2022-11-10 10:39 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-19 15:53 Aditya Kamath1
2022-07-19 18:51 ` Simon Marchi
2022-07-22 16:56   ` Aditya Kamath1
2022-08-18 18:59   ` Aditya Kamath1
2022-08-21 17:15     ` Aditya Kamath1
2022-08-22 13:25     ` Ulrich Weigand
2022-08-22 14:19       ` Simon Marchi
2022-08-23  6:52       ` Aditya Kamath1
2022-10-19 10:57       ` Aditya Kamath1
2022-10-19 10:57         ` Aditya Kamath1
2022-10-28 10:59         ` Ulrich Weigand
2022-11-01 13:55           ` Aditya Kamath1
2022-11-02  8:56             ` Ulrich Weigand
2022-11-10 10:39               ` Aditya Kamath1 [this message]
2022-11-14 18:24                 ` Ulrich Weigand
2022-11-15  7:13                   ` Aditya Kamath1
2022-11-15 10:53                     ` Ulrich Weigand
2022-11-15 12:01                       ` Aditya Kamath1
2022-11-15 12:43                         ` Ulrich Weigand
2022-11-15 18:13                           ` Aditya Kamath1

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CH2PR15MB35441E85211E00CE4305FE0CD6019@CH2PR15MB3544.namprd15.prod.outlook.com \
    --to=aditya.kamath1@ibm.com \
    --cc=Ulrich.Weigand@de.ibm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=sangamesh.swamy@in.ibm.com \
    --cc=simark@simark.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).