[PATCH] Use current_inferior ()->pid for AIX

[PATCH] Use current_inferior ()->pid for AIX

[Aditya Vidyadhar Kamath]

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

Hi all,



Attaching the patch for fetching the inferior process ID using current_inferior() function in AIX.



AIX is still using inferior_ptid.pid() to get the inferior pid instead of the new object current_inferior().With this it is not possible to debug any sample program as the it fails with assertion check for pid!=0.

In the gdb to access the process id of the debugged inferior process, a function current_inferior is used which returns the struct type variable inferior. As current_inferior() holds the inferior pid one must use this object to hold the inferior pid. The attached patch is to the current_inferior() object to get the inferior pid and continue debugging.



This can be demonstrated using the below sample program.



#include <stdio.h>

#include <stdlib.h>

int main()

{

   int i = 1;

   return 0;

}



Sample output without patch:

(gdb) b main

Breakpoint 1 at 0x1000070c: file test.cc, line 3.

(gdb) r

Starting program: /home/gdb_tests/test

inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.

A problem internal to GDB has been detected,

further debugging may prove unreliable.

----- Backtrace -----

0x100e874a3 ???

0x100e8766b ???

0x10003724b ???

0x100037697 ???

0x1000363f3 ???

0x1000593a3 ???

0x1000594ff ???

0x10053aa5b ???

0x1002f6e37 ???

0x1002f2e7f ???

0x100b26ca3 ???

0x1003029bf ???

0x10077e373 ???

0x10077b107 ???

0x100001dff ???

0x100002007 ???

0x10000421b ???

0x1000042ef ???

0x100000a9f ???

0x100000583 ???

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

inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.



Output with patch:

(gdb) b main

Breakpoint 1 at 0x1000070c: file test.cc, line 3.

(gdb) r

Starting program: /home/gdb_tests/test

Breakpoint 1, main () at test.cc:3

3 int i = 1;



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



Summary of the gdb.base testsuites.



Without Patch
------------------------

# of expected passes

8096

# of unexpected failures

2160

# of unexpected successes

1

# of expected failures

4

# of known failures

5

# of unresolved testcases

113

# of untested testcases

83

# of unsupported tests

40

# of paths in test names

2

# of duplicate test names

13

With Patch
-------------------

# of expected passes

13831

# of unexpected failures

7397

# of unexpected successes

1

# of expected failures

11

# of known failures

6

# of unresolved testcases

78

# of untested testcases

88

# of unsupported tests

63

# of paths in test names

1

# of duplicate test names

2

(See attached file: ChangeLog)(See attached file: current_inferior.patch).



Thanks and regards,

Aditya.


[-- Attachment #2: ChangeLog --]
[-- Type: application/octet-stream, Size: 386 bytes --]

2022-03-15  Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com>

	*aix-thread.c (get_signaled_thread): Use current_inferior->pid() instead of
	inferior_ptid.pid() in AIX.
	(sync_threadlists): Likewise.
	(pd_update) : Likewise.
	(pd_activate): Likewise.
	(pd_deactivate): Likewise.
	*rs6000-aix-nat.c (xfer_partial): Likewise.
	(wait): Likewise.
	(xfer_shared_libraries): Likewise.

[-- Attachment #3: current_inferior.patch --]
[-- Type: application/octet-stream, Size: 2806 bytes --]

--- ./gdb/aix-thread.c_orig	2022-03-15 02:26:39 +0000
+++ ./gdb/aix-thread.c	2022-03-03 07:21:16 +0000
@@ -708,7 +708,7 @@
 
   while (1)
     {
-      if (getthrds (inferior_ptid.pid (), &thrinf,
+      if (getthrds ( current_inferior ()->pid, &thrinf,
 		    sizeof (thrinf), &ktid, 1) != 1)
 	break;
 
@@ -791,7 +791,7 @@
 
   /* Apply differences between the two arrays to GDB's thread list.  */
 
-  infpid = inferior_ptid.pid ();
+  infpid = current_inferior ()->pid;
   for (pi = gi = 0; pi < pcount || gi < gcount;)
     {
       if (pi == pcount)
@@ -883,11 +883,11 @@
   struct thread_info *thread = NULL;
 
   if (!pd_active)
-    return inferior_ptid;
+    return ptid_t (current_inferior ()->pid);
 
   status = pthdb_session_update (pd_session);
   if (status != PTHDB_SUCCESS)
-    return inferior_ptid;
+   return ptid_t (current_inferior ()->pid);
 
   sync_threadlists ();
 
@@ -897,7 +897,7 @@
   if (tid != 0)
     thread = iterate_over_threads (iter_tid, &tid);
   if (!thread)
-    ptid = inferior_ptid;
+    ptid = ptid_t (current_inferior ()->pid); 
   else
     {
       ptid = thread->ptid;
@@ -921,7 +921,7 @@
 			       &pd_session);
   if (status != PTHDB_SUCCESS)
     {
-      return inferior_ptid;
+      return ptid_t (current_inferior ()->pid);
     }
   pd_active = 1;
   return pd_update (set_infpid);
@@ -932,11 +932,12 @@
 static void
 pd_deactivate (void)
 {
+  ptid_t ptdrtn = ptid_t (current_inferior ()->pid);
   if (!pd_active)
     return;
   pthdb_session_destroy (pd_session);
   
-  pid_to_prc (&inferior_ptid);
+  pid_to_prc (&ptdrtn); 
   pd_active = 0;
 }
 
--- gdb/rs6000-aix-nat.c_orig	2022-03-15 02:27:22 +0000
+++ gdb/rs6000-aix-nat.c	2022-03-11 05:37:57 +0000
@@ -397,7 +397,7 @@
 				 ULONGEST offset, ULONGEST len,
 				 ULONGEST *xfered_len)
 {
-  pid_t pid = inferior_ptid.pid ();
+  pid_t pid =  current_inferior ()->pid;
   int arch64 = ARCH64 ();
 
   switch (object)
@@ -525,15 +525,14 @@
 
 	  /* Claim it exited with unknown signal.  */
 	  ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN);
-	  return inferior_ptid;
+	  return ptid_t (current_inferior ()->pid);
 	}
 
       /* Ignore terminated detached child processes.  */
-      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
+      if (!WIFSTOPPED (status) && pid != pid_t(current_inferior ()->pid))
 	pid = -1;
     }
   while (pid == -1);
   /* AIX has a couple of strange returns from wait().  */
 
   /* stop after load" status.  */
@@ -658,7 +657,7 @@
   if (writebuf)
     return TARGET_XFER_E_IO;
 
-  gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (inferior_ptid);
+   gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (ptid_t (current_inferior ()->pid)); 
   result = rs6000_aix_ld_info_to_xml (target_gdbarch (), ldi_buf.data (),
 				      readbuf, offset, len, 1);
 

Re: [PATCH] Use current_inferior ()->pid for AIX

[Simon Marchi]

On 2022-03-29 02:58, Aditya Vidyadhar Kamath via Gdb-patches wrote:
> Hi all,
> 
> 
> 
> Attaching the patch for fetching the inferior process ID using current_inferior() function in AIX.

Hi,

I am unable to apply the patch using git-am.  Can you send it using "git send-email" (ideally), or as
a fallback, generate it using "git format-patch" and attach it?

Thanks,

Simon

RE: [PATCH] Use current_inferior ()->pid for AIX

[Aditya Vidyadhar Kamath]

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

Hi,

Please find attached the patch.

Thanks,
Aditya.
________________________________
From: Simon Marchi <simon.marchi@polymtl.ca>
Sent: Tuesday, March 29, 2022 6:31 PM
To: Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com>; Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: [EXTERNAL] Re: [PATCH] Use current_inferior ()->pid for AIX



On 2022-03-29 02:58, Aditya Vidyadhar Kamath via Gdb-patches wrote:
> Hi all,
>
>
>
> Attaching the patch for fetching the inferior process ID using current_inferior() function in AIX.

Hi,

I am unable to apply the patch using git-am.  Can you send it using "git send-email" (ideally), or as
a fallback, generate it using "git format-patch" and attach it?

Thanks,

Simon

[-- Attachment #2: 0001-Use-current_inferior-pid-for-AIX.patch --]
[-- Type: application/octet-stream, Size: 3522 bytes --]

From eb10e0ebc422d01f5ce96786f0bcf78a4f57d7ef Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath
 <adityakamath@li-ee942a4f-4ab2-47e9-8494-fb53eafbb9e1.ibm.com>
Date: Wed, 30 Mar 2022 11:19:14 +0530
Subject: [PATCH] Use current_inferior ()->pid for AIX.

---
 gdb/aix-thread.c     | 15 ++++++++-------
 gdb/rs6000-aix-nat.c |  8 ++++----
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index 85be4c15f1c..9331de3beb2 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -708,7 +708,7 @@ get_signaled_thread (void)
 
   while (1)
     {
-      if (getthrds (inferior_ptid.pid (), &thrinf,
+      if (getthrds (current_inferior ()->pid, &thrinf,
 		    sizeof (thrinf), &ktid, 1) != 1)
 	break;
 
@@ -791,7 +791,7 @@ sync_threadlists (void)
 
   /* Apply differences between the two arrays to GDB's thread list.  */
 
-  infpid = inferior_ptid.pid ();
+  infpid = current_inferior ()->pid;
   for (pi = gi = 0; pi < pcount || gi < gcount;)
     {
       if (pi == pcount)
@@ -883,11 +883,11 @@ pd_update (int set_infpid)
   struct thread_info *thread = NULL;
 
   if (!pd_active)
-    return inferior_ptid;
+    return ptid_t (current_inferior ()->pid);
 
   status = pthdb_session_update (pd_session);
   if (status != PTHDB_SUCCESS)
-    return inferior_ptid;
+    return ptid_t (current_inferior ()->pid);
 
   sync_threadlists ();
 
@@ -897,7 +897,7 @@ pd_update (int set_infpid)
   if (tid != 0)
     thread = iterate_over_threads (iter_tid, &tid);
   if (!thread)
-    ptid = inferior_ptid;
+    ptid = ptid_t (current_inferior ()->pid);
   else
     {
       ptid = thread->ptid;
@@ -921,7 +921,7 @@ pd_activate (int set_infpid)
 			       &pd_session);
   if (status != PTHDB_SUCCESS)
     {
-      return inferior_ptid;
+      return ptid_t (current_inferior ()->pid);
     }
   pd_active = 1;
   return pd_update (set_infpid);
@@ -932,11 +932,12 @@ pd_activate (int set_infpid)
 static void
 pd_deactivate (void)
 {
+  ptid_t ptdrtn = ptid_t (current_inferior ()->pid)	
   if (!pd_active)
     return;
   pthdb_session_destroy (pd_session);
   
-  pid_to_prc (&inferior_ptid);
+  pid_to_prc (&ptdrtn);
   pd_active = 0;
 }
 
diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index 8563aea313a..0b8e4d2c687 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -397,7 +397,7 @@ rs6000_nat_target::xfer_partial (enum target_object object,
 				 ULONGEST offset, ULONGEST len,
 				 ULONGEST *xfered_len)
 {
-  pid_t pid = inferior_ptid.pid ();
+  pid_t pid = current_inferior ()->pid;
   int arch64 = ARCH64 ();
 
   switch (object)
@@ -525,11 +525,11 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 
 	  /* Claim it exited with unknown signal.  */
 	  ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN);
-	  return inferior_ptid;
+	  return ptid_t (current_inferior ()->pid);
 	}
 
       /* Ignore terminated detached child processes.  */
-      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
+      if (!WIFSTOPPED (status) && pid != pid_t(current_inferior ()->pid))
 	pid = -1;
     }
   while (pid == -1);
@@ -658,7 +658,7 @@ rs6000_nat_target::xfer_shared_libraries
   if (writebuf)
     return TARGET_XFER_E_IO;
 
-  gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (inferior_ptid);
+  gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (ptid_t(current_inferior ()->pid));
   result = rs6000_aix_ld_info_to_xml (target_gdbarch (), ldi_buf.data (),
 				      readbuf, offset, len, 1);
 
-- 
2.27.0

RE: [PATCH] Use current_inferior ()->pid for AIX

[Aditya Vidyadhar Kamath]

Any updates in regards to this?

Thanks.
________________________________
From: Gdb-patches <gdb-patches-bounces+aditya.vidyadhar.kamath=ibm.com@sourceware.org> on behalf of Aditya Vidyadhar Kamath via Gdb-patches <gdb-patches@sourceware.org>
Sent: Wednesday, March 30, 2022 6:34 PM
To: Simon Marchi <simon.marchi@polymtl.ca>; Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: [EXTERNAL] RE: [PATCH] Use current_inferior ()->pid for AIX

Hi,

Please find attached the patch.

Thanks,
Aditya.
________________________________
From: Simon Marchi <simon.marchi@polymtl.ca>
Sent: Tuesday, March 29, 2022 6:31 PM
To: Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com>; Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: [EXTERNAL] Re: [PATCH] Use current_inferior ()->pid for AIX



On 2022-03-29 02:58, Aditya Vidyadhar Kamath via Gdb-patches wrote:
> Hi all,
>
>
>
> Attaching the patch for fetching the inferior process ID using current_inferior() function in AIX.

Hi,

I am unable to apply the patch using git-am.  Can you send it using "git send-email" (ideally), or as
a fallback, generate it using "git format-patch" and attach it?

Thanks,

Simon

Re: [PATCH] Use current_inferior ()->pid for AIX

[Simon Marchi]

Hi Aditya,

I don't think that using current_inferior throughout is a good solution
to your problems.  It will be case by case, unfortunately there's no
easy way out of this but to dig in GDB and understand its internals a
bit.

> From eb10e0ebc422d01f5ce96786f0bcf78a4f57d7ef Mon Sep 17 00:00:00 2001
> From: Aditya Vidyadhar Kamath
>  <adityakamath@li-ee942a4f-4ab2-47e9-8494-fb53eafbb9e1.ibm.com>
> Date: Wed, 30 Mar 2022 11:19:14 +0530
> Subject: [PATCH] Use current_inferior ()->pid for AIX.
> 
> ---
>  gdb/aix-thread.c     | 15 ++++++++-------
>  gdb/rs6000-aix-nat.c |  8 ++++----
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index 85be4c15f1c..9331de3beb2 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -708,7 +708,7 @@ get_signaled_thread (void)
>  
>    while (1)
>      {
> -      if (getthrds (inferior_ptid.pid (), &thrinf,
> +      if (getthrds (current_inferior ()->pid, &thrinf,
>  		    sizeof (thrinf), &ktid, 1) != 1)

This function doesn't seem to depend on the current inferior/thread, so
it could easily be changed to receive the pid as a parameter.

>  	break;
>  
> @@ -791,7 +791,7 @@ sync_threadlists (void)
>  
>    /* Apply differences between the two arrays to GDB's thread list.  */
>  
> -  infpid = inferior_ptid.pid ();
> +  infpid = current_inferior ()->pid;
>    for (pi = gi = 0; pi < pcount || gi < gcount;)
>      {
>        if (pi == pcount)
> @@ -883,11 +883,11 @@ pd_update (int set_infpid)
>    struct thread_info *thread = NULL;
>  
>    if (!pd_active)
> -    return inferior_ptid;
> +    return ptid_t (current_inferior ()->pid);
>  
>    status = pthdb_session_update (pd_session);
>    if (status != PTHDB_SUCCESS)
> -    return inferior_ptid;
> +    return ptid_t (current_inferior ()->pid);

Here, the pd_update function seems to want to return the event ptid
unmodified.  So, perhaps that ptid could be passed by the caller and
returned here.  Or, pd_update could be changed to return an
optional<ptid_t> and the caller deals with returning the right ptid.

>  
>    sync_threadlists ();
>  
> @@ -897,7 +897,7 @@ pd_update (int set_infpid)
>    if (tid != 0)
>      thread = iterate_over_threads (iter_tid, &tid);
>    if (!thread)
> -    ptid = inferior_ptid;
> +    ptid = ptid_t (current_inferior ()->pid);
>    else
>      {
>        ptid = thread->ptid;
> @@ -921,7 +921,7 @@ pd_activate (int set_infpid)
>  			       &pd_session);
>    if (status != PTHDB_SUCCESS)
>      {
> -      return inferior_ptid;
> +      return ptid_t (current_inferior ()->pid);

This is the same idea as in pd_update.

>      }
>    pd_active = 1;
>    return pd_update (set_infpid);
> @@ -932,11 +932,12 @@ pd_activate (int set_infpid)
>  static void
>  pd_deactivate (void)
>  {
> +  ptid_t ptdrtn = ptid_t (current_inferior ()->pid)	
>    if (!pd_active)
>      return;
>    pthdb_session_destroy (pd_session);
>    
> -  pid_to_prc (&inferior_ptid);
> +  pid_to_prc (&ptdrtn);

Since pid_to_prc only writes to ptdrtn, it essentially a no-op.  I would
try just removing that call.  It was used to modify inferior_ptid after
the thread layer was disabled, which does not seem useful to me.

> diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
> index 8563aea313a..0b8e4d2c687 100644
> --- a/gdb/rs6000-aix-nat.c
> +++ b/gdb/rs6000-aix-nat.c
> @@ -397,7 +397,7 @@ rs6000_nat_target::xfer_partial (enum target_object object,
>  				 ULONGEST offset, ULONGEST len,
>  				 ULONGEST *xfered_len)
>  {
> -  pid_t pid = inferior_ptid.pid ();
> +  pid_t pid = current_inferior ()->pid;
>    int arch64 = ARCH64 ();
>  
>    switch (object)

Here, inferior_ptid should be valid, this change should not be
necessary.

> @@ -525,11 +525,11 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
>  
>  	  /* Claim it exited with unknown signal.  */
>  	  ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN);
> -	  return inferior_ptid;
> +	  return ptid_t (current_inferior ()->pid);
>  	}
>  
>        /* Ignore terminated detached child processes.  */
> -      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
> +      if (!WIFSTOPPED (status) && pid != pid_t(current_inferior ()->pid))
>  	pid = -1;
>      }
>    while (pid == -1);

The target_ops::wait method implemenattions should ne rely on the
current inferior either.  I don't think that this target supports
debugging multiple processes at the same time, but imagine that it does,
eventually.  When rs6000_nat_target::wait gets called, the current
inferior is one of the inferiors managed by that target, at random.  So
what the wait method needs to do is fetch an event from the OS, and
returning, regardless of which inferior it applies to (except if a ptid
argument is given).  So, you should never need to use either
inferior_ptid nor current_inferior in wait.

> @@ -658,7 +658,7 @@ rs6000_nat_target::xfer_shared_libraries
>    if (writebuf)
>      return TARGET_XFER_E_IO;
>  
> -  gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (inferior_ptid);
> +  gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (ptid_t(current_inferior ()->pid));
>    result = rs6000_aix_ld_info_to_xml (target_gdbarch (), ldi_buf.data (),
>  				      readbuf, offset, len, 1);

This is used by xfer_partial, where inferior_ptid should be valid, so
this change shouldn't be necessary.

Simon

RE: [PATCH] Use current_inferior ()->pid for AIX

[Aditya Vidyadhar Kamath]

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

Hi,

A case by case analysis as suggested by you the last time has been done.

While the child process [the program to be debugged] is running inferior_ptid.pid() will not contain the child process ID. One has to use pid_t(current_inferior ()->pid) inorder to get the child process in AIX. GDB may allow you to execute many programmes at the same time. Numerous threads of execution can be initiated from multiple executables in each of multiple processes.

GDB uses an inferior object to represent the status of each programme run. An inferior usually correlates to a process, although it is more broad and may be used to targets without processes as well. Inferiors can be produced before a process runs and kept after it has completed. Inferiors have their own unique IDs, which are not the same as process ids. Each inferior may have several threads going through it. As a result, inferior ptid.pid may not need to contain the current inferior thread id. To obtain the inferior process ID, we must utilise current inferior()->pid in particular areas.

When the child process's pd_active variable is not set i.e. the process is not active one has to use the pid_t(current_inferior ()->pid) instead of inferior_ptid.pid() in AIX.

This can be demonstrated by the following code:


#include <stdio.h>

#include <stdlib.h>

int main()

{

   int i = 1;

   return 0;

}

Sample output without patch changes only in aix-thread.c at line 883 and not in rs6000-aix-nat.c:


inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.

A problem internal to GDB has been detected,

further debugging may prove unreliable.

----- Backtrace -----

0x100e87543 ???

0x100e8770b ???

0x10003724b ???

0x100037697 ???

0x1000363f3 ???

0x1000593a3 ???

0x1000594ff ???

0x10053aa9b ???

0x1002f6e7b ???

0x1002f2ec3 ???

0x100b26d43 ???

0x100302a03 ???

0x10077e413 ???

0x10077b1a7 ???

0x100001dff ???

0x100002007 ???

0x10000421b ???

0x1000042ef ???

0x100000a9f ???

0x100000583 ???

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

inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.


Sample output with patch changes in aix-thread.c and without patch changes in rs6000-aix-nat.c:


Child process unexpectedly missing: There are no child processes..

Program terminated with signal ?, Unknown signal.

The program no longer exists.

Output with patch:

(gdb) b main

Breakpoint 1 at 0x1000070c: file test.cc, line 3.

(gdb) r

Starting program: /home/gdb_tests/test

Breakpoint 1, main () at test.cc:3

3 int i = 1;


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

When there are multiple threads running in a debugging process, inferior_ptid will not have the exact current inferior thread. Instead we need to use ptid_t (current_inferior ()->pid) to get it in AIX.


This can be shown by the following 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)

{

pthread_barrier_wait (&barrier);

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;

}



Sample output without patch changes in aix-thread.c at line 921 and 883 and not in rs6000-aix-nat.c:


inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.

A problem internal to GDB has been detected,

further debugging may prove unreliable.

----- Backtrace -----

0x100e87543 ???

0x100e8770b ???

0x10003724b ???

0x100037697 ???

0x1000363f3 ???

0x1000593a3 ???

0x1000594ff ???

0x10053aa9b ???

0x1002f6e7b ???

0x1002f2ec3 ???

0x100b26d43 ???

0x100302a03 ???

0x10077e413 ???

0x10077b1a7 ???

0x100001dff ???

0x100002007 ???

0x10000421b ???

0x1000042ef ???

0x100000a9f ???

0x100000583 ???

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

inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.


Output with patch:

(gdb) b main

Breakpoint 1 at 0x1000070c: file test.cc, line 3.

(gdb) r

Starting program: /home/gdb_tests/test

Breakpoint 1, main () at test.cc:3

3 int i = 1;



Summary of the gdb.base testsuites.



Without Patch
------------------------

# of expected passes

8096

# of unexpected failures

2160

# of unexpected successes

1

# of expected failures

4

# of known failures

5

# of unresolved testcases

113

# of untested testcases

83

# of unsupported tests

40

# of paths in test names

2

# of duplicate test names

13

With Patch
------------------------


# of expected passes

13822

# of unexpected failures

                    7399

# of unexpected successes

1

# of expected failures

11

# of known failures

6

# of unresolved testcases

78

# of untested testcases

88

# of unsupported tests

63

# of paths in test names

1

# of duplicate test names

2


(See attached file: 0001-Use-current_inferior-pid-for-AIX)


Thanks and regards,

Aditya.




________________________________
From: Simon Marchi <simon.marchi@polymtl.ca>
Sent: Tuesday, April 5, 2022 6:17 PM
To: Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com>; Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: [EXTERNAL] Re: [PATCH] Use current_inferior ()->pid for AIX

Hi Aditya,

I don't think that using current_inferior throughout is a good solution
to your problems.  It will be case by case, unfortunately there's no
easy way out of this but to dig in GDB and understand its internals a
bit.

> From eb10e0ebc422d01f5ce96786f0bcf78a4f57d7ef Mon Sep 17 00:00:00 2001
> From: Aditya Vidyadhar Kamath
>  <adityakamath@li-ee942a4f-4ab2-47e9-8494-fb53eafbb9e1.ibm.com>
> Date: Wed, 30 Mar 2022 11:19:14 +0530
> Subject: [PATCH] Use current_inferior ()->pid for AIX.
>
> ---
>  gdb/aix-thread.c     | 15 ++++++++-------
>  gdb/rs6000-aix-nat.c |  8 ++++----
>  2 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index 85be4c15f1c..9331de3beb2 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -708,7 +708,7 @@ get_signaled_thread (void)
>
>    while (1)
>      {
> -      if (getthrds (inferior_ptid.pid (), &thrinf,
> +      if (getthrds (current_inferior ()->pid, &thrinf,
>                    sizeof (thrinf), &ktid, 1) != 1)

This function doesn't seem to depend on the current inferior/thread, so
it could easily be changed to receive the pid as a parameter.

>        break;
>
> @@ -791,7 +791,7 @@ sync_threadlists (void)
>
>    /* Apply differences between the two arrays to GDB's thread list.  */
>
> -  infpid = inferior_ptid.pid ();
> +  infpid = current_inferior ()->pid;
>    for (pi = gi = 0; pi < pcount || gi < gcount;)
>      {
>        if (pi == pcount)
> @@ -883,11 +883,11 @@ pd_update (int set_infpid)
>    struct thread_info *thread = NULL;
>
>    if (!pd_active)
> -    return inferior_ptid;
> +    return ptid_t (current_inferior ()->pid);
>
>    status = pthdb_session_update (pd_session);
>    if (status != PTHDB_SUCCESS)
> -    return inferior_ptid;
> +    return ptid_t (current_inferior ()->pid);

Here, the pd_update function seems to want to return the event ptid
unmodified.  So, perhaps that ptid could be passed by the caller and
returned here.  Or, pd_update could be changed to return an
optional<ptid_t> and the caller deals with returning the right ptid.

>
>    sync_threadlists ();
>
> @@ -897,7 +897,7 @@ pd_update (int set_infpid)
>    if (tid != 0)
>      thread = iterate_over_threads (iter_tid, &tid);
>    if (!thread)
> -    ptid = inferior_ptid;
> +    ptid = ptid_t (current_inferior ()->pid);
>    else
>      {
>        ptid = thread->ptid;
> @@ -921,7 +921,7 @@ pd_activate (int set_infpid)
>                               &pd_session);
>    if (status != PTHDB_SUCCESS)
>      {
> -      return inferior_ptid;
> +      return ptid_t (current_inferior ()->pid);

This is the same idea as in pd_update.

>      }
>    pd_active = 1;
>    return pd_update (set_infpid);
> @@ -932,11 +932,12 @@ pd_activate (int set_infpid)
>  static void
>  pd_deactivate (void)
>  {
> +  ptid_t ptdrtn = ptid_t (current_inferior ()->pid)
>    if (!pd_active)
>      return;
>    pthdb_session_destroy (pd_session);
>
> -  pid_to_prc (&inferior_ptid);
> +  pid_to_prc (&ptdrtn);

Since pid_to_prc only writes to ptdrtn, it essentially a no-op.  I would
try just removing that call.  It was used to modify inferior_ptid after
the thread layer was disabled, which does not seem useful to me.

> diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
> index 8563aea313a..0b8e4d2c687 100644
> --- a/gdb/rs6000-aix-nat.c
> +++ b/gdb/rs6000-aix-nat.c
> @@ -397,7 +397,7 @@ rs6000_nat_target::xfer_partial (enum target_object object,
>                                 ULONGEST offset, ULONGEST len,
>                                 ULONGEST *xfered_len)
>  {
> -  pid_t pid = inferior_ptid.pid ();
> +  pid_t pid = current_inferior ()->pid;
>    int arch64 = ARCH64 ();
>
>    switch (object)

Here, inferior_ptid should be valid, this change should not be
necessary.

> @@ -525,11 +525,11 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
>
>          /* Claim it exited with unknown signal.  */
>          ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN);
> -       return inferior_ptid;
> +       return ptid_t (current_inferior ()->pid);
>        }
>
>        /* Ignore terminated detached child processes.  */
> -      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
> +      if (!WIFSTOPPED (status) && pid != pid_t(current_inferior ()->pid))
>        pid = -1;
>      }
>    while (pid == -1);

The target_ops::wait method implemenattions should ne rely on the
current inferior either.  I don't think that this target supports
debugging multiple processes at the same time, but imagine that it does,
eventually.  When rs6000_nat_target::wait gets called, the current
inferior is one of the inferiors managed by that target, at random.  So
what the wait method needs to do is fetch an event from the OS, and
returning, regardless of which inferior it applies to (except if a ptid
argument is given).  So, you should never need to use either
inferior_ptid nor current_inferior in wait.

> @@ -658,7 +658,7 @@ rs6000_nat_target::xfer_shared_libraries
>    if (writebuf)
>      return TARGET_XFER_E_IO;
>
> -  gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (inferior_ptid);
> +  gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (ptid_t(current_inferior ()->pid));
>    result = rs6000_aix_ld_info_to_xml (target_gdbarch (), ldi_buf.data (),
>                                      readbuf, offset, len, 1);

This is used by xfer_partial, where inferior_ptid should be valid, so
this change shouldn't be necessary.

Simon

[-- Attachment #2: 0001-Use-current_inferior-pid-for-AIX.patch --]
[-- Type: application/octet-stream, Size: 1476 bytes --]

From 8ffe8f82e27cff8643644a5f3ac8c9e7f3e3a460 Mon Sep 17 00:00:00 2001
From: "aditya@ibm" <aditya.vidyadhar.kamath@ibm.com>
Date: Tue, 12 Apr 2022 07:08:46 -0500
Subject: [PATCH] Use current_inferior ()->pid for AIX

---
 gdb/aix-thread.c     | 4 ++--
 gdb/rs6000-aix-nat.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index 6a4b469788a..47d612c960b 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -883,7 +883,7 @@ pd_update (int set_infpid)
   struct thread_info *thread = NULL;
 
   if (!pd_active)
-    return inferior_ptid;
+    return ptid_t (current_inferior ()->pid);
 
   status = pthdb_session_update (pd_session);
   if (status != PTHDB_SUCCESS)
@@ -921,7 +921,7 @@ pd_activate (int set_infpid)
 			       &pd_session);
   if (status != PTHDB_SUCCESS)
     {
-      return inferior_ptid;
+      return ptid_t (current_inferior ()->pid);
     }
   pd_active = 1;
   return pd_update (set_infpid);
diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index 5cf1214c65e..d4d181b155e 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -529,7 +529,7 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 	}
 
       /* Ignore terminated detached child processes.  */
-      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
+      if (!WIFSTOPPED (status) && pid != pid_t(current_inferior ()->pid))
 	pid = -1;
     }
   while (pid == -1);
-- 
2.31.1

RE: [PATCH] Use current_inferior ()->pid for AIX

[Aditya Vidyadhar Kamath]

Any update on this?

Kindly let me know.

Thanks and regards,
Aditya.
________________________________
From: Gdb-patches <gdb-patches-bounces+aditya.vidyadhar.kamath=ibm.com@sourceware.org> on behalf of Aditya Vidyadhar Kamath via Gdb-patches <gdb-patches@sourceware.org>
Sent: Tuesday, April 12, 2022 7:02 PM
To: Simon Marchi <simon.marchi@polymtl.ca>; Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: [EXTERNAL] RE: [PATCH] Use current_inferior ()->pid for AIX

Hi,

A case by case analysis as suggested by you the last time has been done.

While the child process [the program to be debugged] is running inferior_ptid.pid() will not contain the child process ID. One has to use pid_t(current_inferior ()->pid) inorder to get the child process in AIX. GDB may allow you to execute many programmes at the same time. Numerous threads of execution can be initiated from multiple executables in each of multiple processes.

GDB uses an inferior object to represent the status of each programme run. An inferior usually correlates to a process, although it is more broad and may be used to targets without processes as well. Inferiors can be produced before a process runs and kept after it has completed. Inferiors have their own unique IDs, which are not the same as process ids. Each inferior may have several threads going through it. As a result, inferior ptid.pid may not need to contain the current inferior thread id. To obtain the inferior process ID, we must utilise current inferior()->pid in particular areas.

When the child process's pd_active variable is not set i.e. the process is not active one has to use the pid_t(current_inferior ()->pid) instead of inferior_ptid.pid() in AIX.

This can be demonstrated by the following code:


#include <stdio.h>

#include <stdlib.h>

int main()

{

   int i = 1;

   return 0;

}

Sample output without patch changes only in aix-thread.c at line 883 and not in rs6000-aix-nat.c:


inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.

A problem internal to GDB has been detected,

further debugging may prove unreliable.

----- Backtrace -----

0x100e87543 ???

0x100e8770b ???

0x10003724b ???

0x100037697 ???

0x1000363f3 ???

0x1000593a3 ???

0x1000594ff ???

0x10053aa9b ???

0x1002f6e7b ???

0x1002f2ec3 ???

0x100b26d43 ???

0x100302a03 ???

0x10077e413 ???

0x10077b1a7 ???

0x100001dff ???

0x100002007 ???

0x10000421b ???

0x1000042ef ???

0x100000a9f ???

0x100000583 ???

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

inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.


Sample output with patch changes in aix-thread.c and without patch changes in rs6000-aix-nat.c:


Child process unexpectedly missing: There are no child processes..

Program terminated with signal ?, Unknown signal.

The program no longer exists.

Output with patch:

(gdb) b main

Breakpoint 1 at 0x1000070c: file test.cc, line 3.

(gdb) r

Starting program: /home/gdb_tests/test

Breakpoint 1, main () at test.cc:3

3 int i = 1;


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

When there are multiple threads running in a debugging process, inferior_ptid will not have the exact current inferior thread. Instead we need to use ptid_t (current_inferior ()->pid) to get it in AIX.


This can be shown by the following 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)

{

pthread_barrier_wait (&barrier);

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;

}



Sample output without patch changes in aix-thread.c at line 921 and 883 and not in rs6000-aix-nat.c:


inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.

A problem internal to GDB has been detected,

further debugging may prove unreliable.

----- Backtrace -----

0x100e87543 ???

0x100e8770b ???

0x10003724b ???

0x100037697 ???

0x1000363f3 ???

0x1000593a3 ???

0x1000594ff ???

0x10053aa9b ???

0x1002f6e7b ???

0x1002f2ec3 ???

0x100b26d43 ???

0x100302a03 ???

0x10077e413 ???

0x10077b1a7 ???

0x100001dff ???

0x100002007 ???

0x10000421b ???

0x1000042ef ???

0x100000a9f ???

0x100000583 ???

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

inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.


Output with patch:

(gdb) b main

Breakpoint 1 at 0x1000070c: file test.cc, line 3.

(gdb) r

Starting program: /home/gdb_tests/test

Breakpoint 1, main () at test.cc:3

3 int i = 1;



Summary of the gdb.base testsuites.



Without Patch
------------------------

# of expected passes

8096

# of unexpected failures

2160

# of unexpected successes

1

# of expected failures

4

# of known failures

5

# of unresolved testcases

113

# of untested testcases

83

# of unsupported tests

40

# of paths in test names

2

# of duplicate test names

13

With Patch
------------------------


# of expected passes

13822

# of unexpected failures

                    7399

# of unexpected successes

1

# of expected failures

11

# of known failures

6

# of unresolved testcases

78

# of untested testcases

88

# of unsupported tests

63

# of paths in test names

1

# of duplicate test names

2


(See attached file: 0001-Use-current_inferior-pid-for-AIX)


Thanks and regards,

Aditya.




________________________________
From: Simon Marchi <simon.marchi@polymtl.ca>
Sent: Tuesday, April 5, 2022 6:17 PM
To: Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com>; Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: [EXTERNAL] Re: [PATCH] Use current_inferior ()->pid for AIX

Hi Aditya,

I don't think that using current_inferior throughout is a good solution
to your problems.  It will be case by case, unfortunately there's no
easy way out of this but to dig in GDB and understand its internals a
bit.

> From eb10e0ebc422d01f5ce96786f0bcf78a4f57d7ef Mon Sep 17 00:00:00 2001
> From: Aditya Vidyadhar Kamath
>  <adityakamath@li-ee942a4f-4ab2-47e9-8494-fb53eafbb9e1.ibm.com>
> Date: Wed, 30 Mar 2022 11:19:14 +0530
> Subject: [PATCH] Use current_inferior ()->pid for AIX.
>
> ---
>  gdb/aix-thread.c     | 15 ++++++++-------
>  gdb/rs6000-aix-nat.c |  8 ++++----
>  2 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index 85be4c15f1c..9331de3beb2 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -708,7 +708,7 @@ get_signaled_thread (void)
>
>    while (1)
>      {
> -      if (getthrds (inferior_ptid.pid (), &thrinf,
> +      if (getthrds (current_inferior ()->pid, &thrinf,
>                    sizeof (thrinf), &ktid, 1) != 1)

This function doesn't seem to depend on the current inferior/thread, so
it could easily be changed to receive the pid as a parameter.

>        break;
>
> @@ -791,7 +791,7 @@ sync_threadlists (void)
>
>    /* Apply differences between the two arrays to GDB's thread list.  */
>
> -  infpid = inferior_ptid.pid ();
> +  infpid = current_inferior ()->pid;
>    for (pi = gi = 0; pi < pcount || gi < gcount;)
>      {
>        if (pi == pcount)
> @@ -883,11 +883,11 @@ pd_update (int set_infpid)
>    struct thread_info *thread = NULL;
>
>    if (!pd_active)
> -    return inferior_ptid;
> +    return ptid_t (current_inferior ()->pid);
>
>    status = pthdb_session_update (pd_session);
>    if (status != PTHDB_SUCCESS)
> -    return inferior_ptid;
> +    return ptid_t (current_inferior ()->pid);

Here, the pd_update function seems to want to return the event ptid
unmodified.  So, perhaps that ptid could be passed by the caller and
returned here.  Or, pd_update could be changed to return an
optional<ptid_t> and the caller deals with returning the right ptid.

>
>    sync_threadlists ();
>
> @@ -897,7 +897,7 @@ pd_update (int set_infpid)
>    if (tid != 0)
>      thread = iterate_over_threads (iter_tid, &tid);
>    if (!thread)
> -    ptid = inferior_ptid;
> +    ptid = ptid_t (current_inferior ()->pid);
>    else
>      {
>        ptid = thread->ptid;
> @@ -921,7 +921,7 @@ pd_activate (int set_infpid)
>                               &pd_session);
>    if (status != PTHDB_SUCCESS)
>      {
> -      return inferior_ptid;
> +      return ptid_t (current_inferior ()->pid);

This is the same idea as in pd_update.

>      }
>    pd_active = 1;
>    return pd_update (set_infpid);
> @@ -932,11 +932,12 @@ pd_activate (int set_infpid)
>  static void
>  pd_deactivate (void)
>  {
> +  ptid_t ptdrtn = ptid_t (current_inferior ()->pid)
>    if (!pd_active)
>      return;
>    pthdb_session_destroy (pd_session);
>
> -  pid_to_prc (&inferior_ptid);
> +  pid_to_prc (&ptdrtn);

Since pid_to_prc only writes to ptdrtn, it essentially a no-op.  I would
try just removing that call.  It was used to modify inferior_ptid after
the thread layer was disabled, which does not seem useful to me.

> diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
> index 8563aea313a..0b8e4d2c687 100644
> --- a/gdb/rs6000-aix-nat.c
> +++ b/gdb/rs6000-aix-nat.c
> @@ -397,7 +397,7 @@ rs6000_nat_target::xfer_partial (enum target_object object,
>                                 ULONGEST offset, ULONGEST len,
>                                 ULONGEST *xfered_len)
>  {
> -  pid_t pid = inferior_ptid.pid ();
> +  pid_t pid = current_inferior ()->pid;
>    int arch64 = ARCH64 ();
>
>    switch (object)

Here, inferior_ptid should be valid, this change should not be
necessary.

> @@ -525,11 +525,11 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
>
>          /* Claim it exited with unknown signal.  */
>          ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN);
> -       return inferior_ptid;
> +       return ptid_t (current_inferior ()->pid);
>        }
>
>        /* Ignore terminated detached child processes.  */
> -      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
> +      if (!WIFSTOPPED (status) && pid != pid_t(current_inferior ()->pid))
>        pid = -1;
>      }
>    while (pid == -1);

The target_ops::wait method implemenattions should ne rely on the
current inferior either.  I don't think that this target supports
debugging multiple processes at the same time, but imagine that it does,
eventually.  When rs6000_nat_target::wait gets called, the current
inferior is one of the inferiors managed by that target, at random.  So
what the wait method needs to do is fetch an event from the OS, and
returning, regardless of which inferior it applies to (except if a ptid
argument is given).  So, you should never need to use either
inferior_ptid nor current_inferior in wait.

> @@ -658,7 +658,7 @@ rs6000_nat_target::xfer_shared_libraries
>    if (writebuf)
>      return TARGET_XFER_E_IO;
>
> -  gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (inferior_ptid);
> +  gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (ptid_t(current_inferior ()->pid));
>    result = rs6000_aix_ld_info_to_xml (target_gdbarch (), ldi_buf.data (),
>                                      readbuf, offset, len, 1);

This is used by xfer_partial, where inferior_ptid should be valid, so
this change shouldn't be necessary.

Simon

RE: [PATCH] Use current_inferior ()->pid for AIX

[Aditya Vidyadhar Kamath]

Hi Simon,

Any update on this?

Kindly let me know.

Thanks and regards,
Aditya.
________________________________
From: Gdb-patches <gdb-patches-bounces+aditya.vidyadhar.kamath=ibm.com@sourceware.org> on behalf of Aditya Vidyadhar Kamath via Gdb-patches <gdb-patches@sourceware.org>
Sent: Tuesday, April 12, 2022 7:02 PM
To: Simon Marchi <simon.marchi@polymtl.ca>; Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: [EXTERNAL] RE: [PATCH] Use current_inferior ()->pid for AIX

Hi,

A case by case analysis as suggested by you the last time has been done.

While the child process [the program to be debugged] is running inferior_ptid.pid() will not contain the child process ID. One has to use pid_t(current_inferior ()->pid) inorder to get the child process in AIX. GDB may allow you to execute many programmes at the same time. Numerous threads of execution can be initiated from multiple executables in each of multiple processes.

GDB uses an inferior object to represent the status of each programme run. An inferior usually correlates to a process, although it is more broad and may be used to targets without processes as well. Inferiors can be produced before a process runs and kept after it has completed. Inferiors have their own unique IDs, which are not the same as process ids. Each inferior may have several threads going through it. As a result, inferior ptid.pid may not need to contain the current inferior thread id. To obtain the inferior process ID, we must utilise current inferior()->pid in particular areas.

When the child process's pd_active variable is not set i.e. the process is not active one has to use the pid_t(current_inferior ()->pid) instead of inferior_ptid.pid() in AIX.

This can be demonstrated by the following code:


#include <stdio.h>

#include <stdlib.h>

int main()

{

   int i = 1;

   return 0;

}

Sample output without patch changes only in aix-thread.c at line 883 and not in rs6000-aix-nat.c:


inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.

A problem internal to GDB has been detected,

further debugging may prove unreliable.

----- Backtrace -----

0x100e87543 ???

0x100e8770b ???

0x10003724b ???

0x100037697 ???

0x1000363f3 ???

0x1000593a3 ???

0x1000594ff ???

0x10053aa9b ???

0x1002f6e7b ???

0x1002f2ec3 ???

0x100b26d43 ???

0x100302a03 ???

0x10077e413 ???

0x10077b1a7 ???

0x100001dff ???

0x100002007 ???

0x10000421b ???

0x1000042ef ???

0x100000a9f ???

0x100000583 ???

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

inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.


Sample output with patch changes in aix-thread.c and without patch changes in rs6000-aix-nat.c:


Child process unexpectedly missing: There are no child processes..

Program terminated with signal ?, Unknown signal.

The program no longer exists.

Output with patch:

(gdb) b main

Breakpoint 1 at 0x1000070c: file test.cc, line 3.

(gdb) r

Starting program: /home/gdb_tests/test

Breakpoint 1, main () at test.cc:3

3 int i = 1;


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

When there are multiple threads running in a debugging process, inferior_ptid will not have the exact current inferior thread. Instead we need to use ptid_t (current_inferior ()->pid) to get it in AIX.


This can be shown by the following 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)

{

pthread_barrier_wait (&barrier);

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;

}



Sample output without patch changes in aix-thread.c at line 921 and 883 and not in rs6000-aix-nat.c:


inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.

A problem internal to GDB has been detected,

further debugging may prove unreliable.

----- Backtrace -----

0x100e87543 ???

0x100e8770b ???

0x10003724b ???

0x100037697 ???

0x1000363f3 ???

0x1000593a3 ???

0x1000594ff ???

0x10053aa9b ???

0x1002f6e7b ???

0x1002f2ec3 ???

0x100b26d43 ???

0x100302a03 ???

0x10077e413 ???

0x10077b1a7 ???

0x100001dff ???

0x100002007 ???

0x10000421b ???

0x1000042ef ???

0x100000a9f ???

0x100000583 ???

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

inferior.c:303: internal-error: find_inferior_pid: Assertion `pid != 0' failed.


Output with patch:

(gdb) b main

Breakpoint 1 at 0x1000070c: file test.cc, line 3.

(gdb) r

Starting program: /home/gdb_tests/test

Breakpoint 1, main () at test.cc:3

3 int i = 1;



Summary of the gdb.base testsuites.



Without Patch
------------------------

# of expected passes

8096

# of unexpected failures

2160

# of unexpected successes

1

# of expected failures

4

# of known failures

5

# of unresolved testcases

113

# of untested testcases

83

# of unsupported tests

40

# of paths in test names

2

# of duplicate test names

13

With Patch
------------------------


# of expected passes

13822

# of unexpected failures

                    7399

# of unexpected successes

1

# of expected failures

11

# of known failures

6

# of unresolved testcases

78

# of untested testcases

88

# of unsupported tests

63

# of paths in test names

1

# of duplicate test names

2


(See attached file: 0001-Use-current_inferior-pid-for-AIX)


Thanks and regards,

Aditya.




________________________________
From: Simon Marchi <simon.marchi@polymtl.ca>
Sent: Tuesday, April 5, 2022 6:17 PM
To: Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com>; Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: [EXTERNAL] Re: [PATCH] Use current_inferior ()->pid for AIX

Hi Aditya,

I don't think that using current_inferior throughout is a good solution
to your problems.  It will be case by case, unfortunately there's no
easy way out of this but to dig in GDB and understand its internals a
bit.

> From eb10e0ebc422d01f5ce96786f0bcf78a4f57d7ef Mon Sep 17 00:00:00 2001
> From: Aditya Vidyadhar Kamath
>  <adityakamath@li-ee942a4f-4ab2-47e9-8494-fb53eafbb9e1.ibm.com>
> Date: Wed, 30 Mar 2022 11:19:14 +0530
> Subject: [PATCH] Use current_inferior ()->pid for AIX.
>
> ---
>  gdb/aix-thread.c     | 15 ++++++++-------
>  gdb/rs6000-aix-nat.c |  8 ++++----
>  2 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index 85be4c15f1c..9331de3beb2 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -708,7 +708,7 @@ get_signaled_thread (void)
>
>    while (1)
>      {
> -      if (getthrds (inferior_ptid.pid (), &thrinf,
> +      if (getthrds (current_inferior ()->pid, &thrinf,
>                    sizeof (thrinf), &ktid, 1) != 1)

This function doesn't seem to depend on the current inferior/thread, so
it could easily be changed to receive the pid as a parameter.

>        break;
>
> @@ -791,7 +791,7 @@ sync_threadlists (void)
>
>    /* Apply differences between the two arrays to GDB's thread list.  */
>
> -  infpid = inferior_ptid.pid ();
> +  infpid = current_inferior ()->pid;
>    for (pi = gi = 0; pi < pcount || gi < gcount;)
>      {
>        if (pi == pcount)
> @@ -883,11 +883,11 @@ pd_update (int set_infpid)
>    struct thread_info *thread = NULL;
>
>    if (!pd_active)
> -    return inferior_ptid;
> +    return ptid_t (current_inferior ()->pid);
>
>    status = pthdb_session_update (pd_session);
>    if (status != PTHDB_SUCCESS)
> -    return inferior_ptid;
> +    return ptid_t (current_inferior ()->pid);

Here, the pd_update function seems to want to return the event ptid
unmodified.  So, perhaps that ptid could be passed by the caller and
returned here.  Or, pd_update could be changed to return an
optional<ptid_t> and the caller deals with returning the right ptid.

>
>    sync_threadlists ();
>
> @@ -897,7 +897,7 @@ pd_update (int set_infpid)
>    if (tid != 0)
>      thread = iterate_over_threads (iter_tid, &tid);
>    if (!thread)
> -    ptid = inferior_ptid;
> +    ptid = ptid_t (current_inferior ()->pid);
>    else
>      {
>        ptid = thread->ptid;
> @@ -921,7 +921,7 @@ pd_activate (int set_infpid)
>                               &pd_session);
>    if (status != PTHDB_SUCCESS)
>      {
> -      return inferior_ptid;
> +      return ptid_t (current_inferior ()->pid);

This is the same idea as in pd_update.

>      }
>    pd_active = 1;
>    return pd_update (set_infpid);
> @@ -932,11 +932,12 @@ pd_activate (int set_infpid)
>  static void
>  pd_deactivate (void)
>  {
> +  ptid_t ptdrtn = ptid_t (current_inferior ()->pid)
>    if (!pd_active)
>      return;
>    pthdb_session_destroy (pd_session);
>
> -  pid_to_prc (&inferior_ptid);
> +  pid_to_prc (&ptdrtn);

Since pid_to_prc only writes to ptdrtn, it essentially a no-op.  I would
try just removing that call.  It was used to modify inferior_ptid after
the thread layer was disabled, which does not seem useful to me.

> diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
> index 8563aea313a..0b8e4d2c687 100644
> --- a/gdb/rs6000-aix-nat.c
> +++ b/gdb/rs6000-aix-nat.c
> @@ -397,7 +397,7 @@ rs6000_nat_target::xfer_partial (enum target_object object,
>                                 ULONGEST offset, ULONGEST len,
>                                 ULONGEST *xfered_len)
>  {
> -  pid_t pid = inferior_ptid.pid ();
> +  pid_t pid = current_inferior ()->pid;
>    int arch64 = ARCH64 ();
>
>    switch (object)

Here, inferior_ptid should be valid, this change should not be
necessary.

> @@ -525,11 +525,11 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
>
>          /* Claim it exited with unknown signal.  */
>          ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN);
> -       return inferior_ptid;
> +       return ptid_t (current_inferior ()->pid);
>        }
>
>        /* Ignore terminated detached child processes.  */
> -      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
> +      if (!WIFSTOPPED (status) && pid != pid_t(current_inferior ()->pid))
>        pid = -1;
>      }
>    while (pid == -1);

The target_ops::wait method implemenattions should ne rely on the
current inferior either.  I don't think that this target supports
debugging multiple processes at the same time, but imagine that it does,
eventually.  When rs6000_nat_target::wait gets called, the current
inferior is one of the inferiors managed by that target, at random.  So
what the wait method needs to do is fetch an event from the OS, and
returning, regardless of which inferior it applies to (except if a ptid
argument is given).  So, you should never need to use either
inferior_ptid nor current_inferior in wait.

> @@ -658,7 +658,7 @@ rs6000_nat_target::xfer_shared_libraries
>    if (writebuf)
>      return TARGET_XFER_E_IO;
>
> -  gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (inferior_ptid);
> +  gdb::byte_vector ldi_buf = rs6000_ptrace_ldinfo (ptid_t(current_inferior ()->pid));
>    result = rs6000_aix_ld_info_to_xml (target_gdbarch (), ldi_buf.data (),
>                                      readbuf, offset, len, 1);

This is used by xfer_partial, where inferior_ptid should be valid, so
this change shouldn't be necessary.

Simon

Re: [PATCH] Use current_inferior ()->pid for AIX

[Simon Marchi]

> diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
> index 5cf1214c65e..d4d181b155e 100644
> --- a/gdb/rs6000-aix-nat.c
> +++ b/gdb/rs6000-aix-nat.c
> @@ -529,7 +529,7 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
>  	}
>
>        /* Ignore terminated detached child processes.  */
> -      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
> +      if (!WIFSTOPPED (status) && pid != pid_t(current_inferior ()->pid))

As stated in my previous message, you should not assume which is the
current inferior, when entering the wait method.  The current code was
written a while ago, probably before GDB gained support for
multi-inferior.  So if we are to change this code, we should bring it up
to current standards, as much as possible.

Imagine that your target is currently debugging two inferiors executing
simultaneously (I don't know if the AIX target supports that, but let's
pretend that it does).  The event you get from waitpid could be for
either inferior.  So you can't simply ignore an event if it is for the
non-current inferior.

When you have a multi-threaded (pthread) inferior on AIX and call
waitpid on it, what does it return when some thread get a signal?  Does
it return the pid of the process, or a specific tid?  My reading of the
code in aix-thread.c and of the snippet above makes me think that the
kernel returns the process' pid.  And then get_signaled_thread in
aix-thread.c takes care of finding the specific thread that got a
signal, somehow.

So my intuition is that here it would make sense to call
find_inferior_pid with the value returned by waitpid, to see if this is
for an inferior we know or for a "terminated detached child process", as
the comment above says.  If we have no inferior with that pid, we can
probably ignore the event.  If we have an inferior with that pid, then
we return its pid and status.

Note that inferior_ptid variable at this point will still be null_ptid
(and that's ok).  Back in aix_thread_target::wait (if using the
aix-thread target), the simplest thing to do is probably to call
find_inferior_pid with the pid returned by `beneath ()->wait` and make
it the current inferior with switch_to_inferior_no_thread.  Then, the
rest of the code (pd_update and pd_activate) can correctly assume that
they have the right current inferior, so can use
`current_inferior()->pid`.

I don't know if that actually makes sense.  I would actually try it on
an AIX machine on the GCC compile farm, but I can't get GDB to build on
it, and I don't really have time to look into it:

  CXX      thread-pool.o
In file included from ../../src/binutils-gdb/gdbsupport/thread-pool.cc:21:0:
../../src/binutils-gdb/gdbsupport/../gdbsupport/thread-pool.h: In member function 'std::future<void> gdb::thread_pool::post_task(std::function<void()>&&)':
../../src/binutils-gdb/gdbsupport/../gdbsupport/thread-pool.h:68:3: error: return type 'class std::future<void>' is incomplete

Simon

Re: [PATCH] Use current_inferior ()->pid for AIX

[Simon Marchi]

Hi,

Re-adding gdb-patches, since it's information useful to everybody.

On 2022-05-29 23:41, Aditya Vidyadhar Kamath wrote:
> Hi Simon,
> 
> Thank you so much for the feedback. Yes it makes sense. As I was trying to fix this we need one more information that will help us. 
> 
> Once an inferior finishes its execution where is the inferior_ptid's pid variable set to 0. Or who is the one [file name or function name] that updates inferior_ptid class variable pid to 0. 

This is done in the mourn_inferior target method.  Many targets call the
generic_mourn_inferior function, which calls switch_to_no_thread, which
sets current_thread_ to nullptr and inferior_ptid to null_ptid.

Simon

RE: [PATCH] Use current_inferior ()->pid for AIX

[Aditya Vidyadhar Kamath]

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

Hi all,

While testing programs in AIX I noticed that GDB crashes when an
inferior exits, with this error:

  inferior.c:293: internal- error: inferior* find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0' failed.

When a process exits inferior_ptid.pid is set to 0. Unfortunately, the rs6000-aix-nat target is still relying on the value of
inferior_ptid in the case where an inferior exits - we return the
value of inferior_ptid as the pid of the process that exited.

The waitpid() system call suspends execution of the calling process until a child specified by pid argument has changed state. Once the inferior [assuming there is only one inferior] dies, waitpid() has no child to return.

Due to this an ERRCHLD error is returned thereby returning an inferior_ptid.pid with 0 leading to this assertion failure.

This patch is a fix to the same where we use the pid returned by the beneath wait using waitpid and adjust our inferior_ptid so that the rest of the code will take in the right values of both inferior_ptid and current_inferior.

The following are the test results after running gdb.base test suite with the patch.
# of expected passes 26244
# of unexpected failures 4230
# of unexpected successes 1
# of expected failures 17
# of known failures 26
# of unresolved testcases 110
# of untested testcases 79
# of unsupported tests 62
# of paths in test names 1
# of duplicate test names 4

The following are test results after running gdb.base test suite without the patch.
# of expected passes 12935
# of unexpected failures 1988
# of unexpected successes 1
# of expected failures 3
# of known failures 6
# of unresolved testcases 159
# of untested testcases 77
# of unsupported tests 39
# of paths in test names 2
# of duplicate test names 13

Please find attached the patch. [See 0001-Fix-gdb_assert-pid-0-assertion-failure-in-AIX.patch].

Have a nice day ahead,

Thanks and regards,
Aditya.


________________________________
From: Simon Marchi <simon.marchi@polymtl.ca>
Sent: Monday, May 30, 2022 6:15 PM
To: Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>; Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
Subject: [EXTERNAL] Re: [PATCH] Use current_inferior ()->pid for AIX

Hi,

Re-adding gdb-patches, since it's information useful to everybody.

On 2022-05-29 23:41, Aditya Vidyadhar Kamath wrote:
> Hi Simon,
>
> Thank you so much for the feedback. Yes it makes sense. As I was trying to fix this we need one more information that will help us.
>
> Once an inferior finishes its execution where is the inferior_ptid's pid variable set to 0. Or who is the one [file name or function name] that updates inferior_ptid class variable pid to 0.

This is done in the mourn_inferior target method.  Many targets call the
generic_mourn_inferior function, which calls switch_to_no_thread, which
sets current_thread_ to nullptr and inferior_ptid to null_ptid.

Simon

[-- Attachment #2: 0001-Fix-gdb_assert-pid-0-assertion-failure-in-AIX.patch --]
[-- Type: application/octet-stream, Size: 1703 bytes --]

From f8259070349b075f65a4b9fe0a888543a6864f6e Mon Sep 17 00:00:00 2001
From: "aditya@ibm" <aditya.vidyadhar.kamath@ibm.com>
Date: Fri, 10 Jun 2022 08:50:46 -0500
Subject: [PATCH] Fix gdb_assert (pid != 0); assertion failure in AIX

---
 gdb/aix-thread.c     | 5 +++++
 gdb/rs6000-aix-nat.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index ecd8200b692..3037d73c5c4 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -1079,6 +1079,7 @@ ptid_t
 aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
 			 target_wait_flags options)
 {
+  struct inferior *inf;
   {
     scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
 
@@ -1091,6 +1092,10 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
   if (ptid.pid () == -1)
     return ptid_t (-1);
 
+  inf = find_inferior_pid(current_inferior ()->process_target (),ptid.pid());
+  if(inf != NULL){
+   inferior_ptid = ptid;
+  }
   /* Check whether libpthdebug might be ready to be initialized.  */
   if (!pd_active && status->kind () == TARGET_WAITKIND_STOPPED
       && status->sig () == GDB_SIGNAL_TRAP)
diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index 8563aea313a..8a0c074be8f 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -529,7 +529,7 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 	}
 
       /* Ignore terminated detached child processes.  */
-      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
+      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid () && inferior_ptid.pid () != 0)
 	pid = -1;
     }
   while (pid == -1);
-- 
2.31.1

RE: [PATCH] Use current_inferior ()->pid for AIX

[Aditya Vidyadhar Kamath]

Hi all,

Kindly give us a feedback for this update.

Thanks and regards,
Aditya
________________________________
From: Gdb-patches <gdb-patches-bounces+aditya.vidyadhar.kamath=ibm.com@sourceware.org> on behalf of Aditya Vidyadhar Kamath via Gdb-patches <gdb-patches@sourceware.org>
Sent: Friday, June 10, 2022 8:17 PM
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>; Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
Subject: [EXTERNAL] RE: [PATCH] Use current_inferior ()->pid for AIX

Hi all,

While testing programs in AIX I noticed that GDB crashes when an
inferior exits, with this error:

  inferior.c:293: internal- error: inferior* find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0' failed.

When a process exits inferior_ptid.pid is set to 0. Unfortunately, the rs6000-aix-nat target is still relying on the value of
inferior_ptid in the case where an inferior exits - we return the
value of inferior_ptid as the pid of the process that exited.

The waitpid() system call suspends execution of the calling process until a child specified by pid argument has changed state. Once the inferior [assuming there is only one inferior] dies, waitpid() has no child to return.

Due to this an ERRCHLD error is returned thereby returning an inferior_ptid.pid with 0 leading to this assertion failure.

This patch is a fix to the same where we use the pid returned by the beneath wait using waitpid and adjust our inferior_ptid so that the rest of the code will take in the right values of both inferior_ptid and current_inferior.

The following are the test results after running gdb.base test suite with the patch.
# of expected passes 26244
# of unexpected failures 4230
# of unexpected successes 1
# of expected failures 17
# of known failures 26
# of unresolved testcases 110
# of untested testcases 79
# of unsupported tests 62
# of paths in test names 1
# of duplicate test names 4

The following are test results after running gdb.base test suite without the patch.
# of expected passes 12935
# of unexpected failures 1988
# of unexpected successes 1
# of expected failures 3
# of known failures 6
# of unresolved testcases 159
# of untested testcases 77
# of unsupported tests 39
# of paths in test names 2
# of duplicate test names 13

Please find attached the patch. [See 0001-Fix-gdb_assert-pid-0-assertion-failure-in-AIX.patch].

Have a nice day ahead,

Thanks and regards,
Aditya.


________________________________
From: Simon Marchi <simon.marchi@polymtl.ca>
Sent: Monday, May 30, 2022 6:15 PM
To: Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>; Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
Subject: [EXTERNAL] Re: [PATCH] Use current_inferior ()->pid for AIX

Hi,

Re-adding gdb-patches, since it's information useful to everybody.

On 2022-05-29 23:41, Aditya Vidyadhar Kamath wrote:
> Hi Simon,
>
> Thank you so much for the feedback. Yes it makes sense. As I was trying to fix this we need one more information that will help us.
>
> Once an inferior finishes its execution where is the inferior_ptid's pid variable set to 0. Or who is the one [file name or function name] that updates inferior_ptid class variable pid to 0.

This is done in the mourn_inferior target method.  Many targets call the
generic_mourn_inferior function, which calls switch_to_no_thread, which
sets current_thread_ to nullptr and inferior_ptid to null_ptid.

Simon

RE: [PATCH] Use current_inferior ()->pid for AIX

[Aditya Vidyadhar Kamath]

Hi all,

Kindly give us a feedback for this patch update.

Thanks and regards,
Aditya

________________________________
From: Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com>
Sent: Wednesday, June 15, 2022 9:33 AM
To: Simon Marchi <simon.marchi@polymtl.ca>; Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>; Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
Subject: Re: [EXTERNAL] RE: [PATCH] Use current_inferior ()->pid for AIX

Hi all,

Kindly give us a feedback for this update.

Thanks and regards,
Aditya
________________________________
From: Gdb-patches <gdb-patches-bounces+aditya.vidyadhar.kamath=ibm.com@sourceware.org> on behalf of Aditya Vidyadhar Kamath via Gdb-patches <gdb-patches@sourceware.org>
Sent: Friday, June 10, 2022 8:17 PM
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>; Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
Subject: [EXTERNAL] RE: [PATCH] Use current_inferior ()->pid for AIX

Hi all,

While testing programs in AIX I noticed that GDB crashes when an
inferior exits, with this error:

  inferior.c:293: internal- error: inferior* find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0' failed.

When a process exits inferior_ptid.pid is set to 0. Unfortunately, the rs6000-aix-nat target is still relying on the value of
inferior_ptid in the case where an inferior exits - we return the
value of inferior_ptid as the pid of the process that exited.

The waitpid() system call suspends execution of the calling process until a child specified by pid argument has changed state. Once the inferior [assuming there is only one inferior] dies, waitpid() has no child to return.

Due to this an ERRCHLD error is returned thereby returning an inferior_ptid.pid with 0 leading to this assertion failure.

This patch is a fix to the same where we use the pid returned by the beneath wait using waitpid and adjust our inferior_ptid so that the rest of the code will take in the right values of both inferior_ptid and current_inferior.

The following are the test results after running gdb.base test suite with the patch.
# of expected passes 26244
# of unexpected failures 4230
# of unexpected successes 1
# of expected failures 17
# of known failures 26
# of unresolved testcases 110
# of untested testcases 79
# of unsupported tests 62
# of paths in test names 1
# of duplicate test names 4

The following are test results after running gdb.base test suite without the patch.
# of expected passes 12935
# of unexpected failures 1988
# of unexpected successes 1
# of expected failures 3
# of known failures 6
# of unresolved testcases 159
# of untested testcases 77
# of unsupported tests 39
# of paths in test names 2
# of duplicate test names 13

Please find attached the patch. [See 0001-Fix-gdb_assert-pid-0-assertion-failure-in-AIX.patch].

Have a nice day ahead,

Thanks and regards,
Aditya.


________________________________
From: Simon Marchi <simon.marchi@polymtl.ca>
Sent: Monday, May 30, 2022 6:15 PM
To: Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>; Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
Subject: [EXTERNAL] Re: [PATCH] Use current_inferior ()->pid for AIX

Hi,

Re-adding gdb-patches, since it's information useful to everybody.

On 2022-05-29 23:41, Aditya Vidyadhar Kamath wrote:
> Hi Simon,
>
> Thank you so much for the feedback. Yes it makes sense. As I was trying to fix this we need one more information that will help us.
>
> Once an inferior finishes its execution where is the inferior_ptid's pid variable set to 0. Or who is the one [file name or function name] that updates inferior_ptid class variable pid to 0.

This is done in the mourn_inferior target method.  Many targets call the
generic_mourn_inferior function, which calls switch_to_no_thread, which
sets current_thread_ to nullptr and inferior_ptid to null_ptid.

Simon

Fw: RE: [PATCH] Use current_inferior ()->pid for AIX

[Aditya Vidyadhar Kamath]

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

Hi,

We have worked our way out through the pid!=0 assertion failure.

Currently we also plan to come out soon with the patches for fork support as well in AIX.

It will be great if we could get a review to the patch [Forwarded in this email] whenever you find time.

Have a nice day ahead.

Thanks and regards,
Aditya

________________________________
From: Gdb-patches <gdb-patches-bounces+aditya.vidyadhar.kamath=ibm.com@sourceware.org> on behalf of Aditya Vidyadhar Kamath via Gdb-patches <gdb-patches@sourceware.org>
Sent: Friday, June 10, 2022 8:17 PM
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>; Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
Subject: [EXTERNAL] RE: [PATCH] Use current_inferior ()->pid for AIX

Hi all,

While testing programs in AIX I noticed that GDB crashes when an
inferior exits, with this error:

  inferior.c:293: internal- error: inferior* find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0' failed.

When a process exits inferior_ptid.pid is set to 0. Unfortunately, the rs6000-aix-nat target is still relying on the value of
inferior_ptid in the case where an inferior exits - we return the
value of inferior_ptid as the pid of the process that exited.

The waitpid() system call suspends execution of the calling process until a child specified by pid argument has changed state. Once the inferior [assuming there is only one inferior] dies, waitpid() has no child to return.

Due to this an ERRCHLD error is returned thereby returning an inferior_ptid.pid with 0 leading to this assertion failure.

This patch is a fix to the same where we use the pid returned by the beneath wait using waitpid and adjust our inferior_ptid so that the rest of the code will take in the right values of both inferior_ptid and current_inferior.

The following are the test results after running gdb.base test suite with the patch.
# of expected passes 26244
# of unexpected failures 4230
# of unexpected successes 1
# of expected failures 17
# of known failures 26
# of unresolved testcases 110
# of untested testcases 79
# of unsupported tests 62
# of paths in test names 1
# of duplicate test names 4

The following are test results after running gdb.base test suite without the patch.
# of expected passes 12935
# of unexpected failures 1988
# of unexpected successes 1
# of expected failures 3
# of known failures 6
# of unresolved testcases 159
# of untested testcases 77
# of unsupported tests 39
# of paths in test names 2
# of duplicate test names 13

Please find attached the patch. [See 0001-Fix-gdb_assert-pid-0-assertion-failure-in-AIX.patch].

Have a nice day ahead,

Thanks and regards,
Aditya.


________________________________
From: Simon Marchi <simon.marchi@polymtl.ca>
Sent: Monday, May 30, 2022 6:15 PM
To: Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>; Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
Subject: [EXTERNAL] Re: [PATCH] Use current_inferior ()->pid for AIX

Hi,

Re-adding gdb-patches, since it's information useful to everybody.

On 2022-05-29 23:41, Aditya Vidyadhar Kamath wrote:
> Hi Simon,
>
> Thank you so much for the feedback. Yes it makes sense. As I was trying to fix this we need one more information that will help us.
>
> Once an inferior finishes its execution where is the inferior_ptid's pid variable set to 0. Or who is the one [file name or function name] that updates inferior_ptid class variable pid to 0.

This is done in the mourn_inferior target method.  Many targets call the
generic_mourn_inferior function, which calls switch_to_no_thread, which
sets current_thread_ to nullptr and inferior_ptid to null_ptid.

Simon

[-- Attachment #2: 0001-Fix-gdb_assert-pid-0-assertion-failure-in-AIX.patch --]
[-- Type: application/octet-stream, Size: 1703 bytes --]

From f8259070349b075f65a4b9fe0a888543a6864f6e Mon Sep 17 00:00:00 2001
From: "aditya@ibm" <aditya.vidyadhar.kamath@ibm.com>
Date: Fri, 10 Jun 2022 08:50:46 -0500
Subject: [PATCH] Fix gdb_assert (pid != 0); assertion failure in AIX

---
 gdb/aix-thread.c     | 5 +++++
 gdb/rs6000-aix-nat.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index ecd8200b692..3037d73c5c4 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -1079,6 +1079,7 @@ ptid_t
 aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
 			 target_wait_flags options)
 {
+  struct inferior *inf;
   {
     scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
 
@@ -1091,6 +1092,10 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
   if (ptid.pid () == -1)
     return ptid_t (-1);
 
+  inf = find_inferior_pid(current_inferior ()->process_target (),ptid.pid());
+  if(inf != NULL){
+   inferior_ptid = ptid;
+  }
   /* Check whether libpthdebug might be ready to be initialized.  */
   if (!pd_active && status->kind () == TARGET_WAITKIND_STOPPED
       && status->sig () == GDB_SIGNAL_TRAP)
diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index 8563aea313a..8a0c074be8f 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -529,7 +529,7 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 	}
 
       /* Ignore terminated detached child processes.  */
-      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
+      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid () && inferior_ptid.pid () != 0)
 	pid = -1;
     }
   while (pid == -1);
-- 
2.31.1

Re: Fw: RE: [PATCH] Use current_inferior ()->pid for AIX

[Simon Marchi]

On 6/27/22 08:55, Aditya Vidyadhar Kamath via Gdb-patches wrote:
> Hi,
> 
> We have worked our way out through the pid!=0 assertion failure.
> 
> Currently we also plan to come out soon with the patches for fork support as well in AIX.
> 
> It will be great if we could get a review to the patch [Forwarded in this email] whenever you find time.
> 
> Have a nice day ahead.
> 
> Thanks and regards,
> Aditya

Hi,

I am unable to put time on GDB right now.  I can take a look at this in a few weeks when
I go back to working on GDB.  In the mean time, if someone else wants to review this
(especially someone familiar with AIX), please go ahead.

Simon

Re: Fw: RE: [PATCH] Use current_inferior ()->pid for AIX

[Simon Marchi]

On 6/27/22 08:55, Aditya Vidyadhar Kamath via Gdb-patches wrote:

> Hi,

>

> We have worked our way out through the pid!=0 assertion failure.

>

> Currently we also plan to come out soon with the patches for fork support as well in AIX.

>

> It will be great if we could get a review to the patch [Forwarded in this email] whenever you find time.

>

> Have a nice day ahead.

>

> Thanks and regards,

> Aditya



Hi Aditya,



I looked at your patch, and unfortunately I don't understand how it

improves things.  In my past messages, I tried to explain that the root

of the problem is that the wait methods code is relying on the entry

value of inferior_ptid, when it shouldn't.  I don't see how adding one

more reference to inferior_ptid in rs6000_nat_target::wait helps.



Given your goal is to support forks (and thus multi-process, I

suppose?), try to write the code in the wait method in such a way that

it doesn't rely on the inferior_ptid value or current inferior value on

entry.  The typical pattern for the wait methods is that they fetch some

event (using waitpid in your case) and then figure out which inferior

from the inferior list this event applies to.



Simon

RE: Fw: RE: [PATCH] Fix assert pid != 0 assertion failure in AIX

[Aditya Vidyadhar Kamath]

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


Morning Simon.

The reason we were adding one more inferior_ptid!= 0 , condition is the previous condition in the and logic i.e. pid != inferior_ptd.pid() will satisfy as -1 is not equal to 0. [inferior_ptid is set to null before coming into wait]. So, in the next iteration since the process has exited waitpid (), will lead to ERRCHILD error though the current iteration fetched the right pid using waitpid ().

However, we get your point that inferior_ptid should not be used initially [for any condition check till we fetch the pid using waitpid ()] as it is being reset.

Please find attached our modified patch where we do a check of the inferior being in the list. I hope this solution matches to what you suggested.

[See 0001-Fix-gdb_assert-pid-0-assertion-failure-in-AIX.patch file attached to this email]

Have a great day.

Thanks and regards,
Aditya.


[-- Attachment #2: 0001-Fix-gdb_assert-pid-0-assertion-failure-in-AIX.patch --]
[-- Type: application/octet-stream, Size: 1482 bytes --]

From a1c5ab5338a5d46eab675a85c28a9b00256d395a Mon Sep 17 00:00:00 2001
From: "aditya@ibm" <aditya.vidyadhar.kamath@ibm.com>
Date: Tue, 5 Jul 2022 23:05:18 -0500
Subject: [PATCH] Fix gdb_assert (pid != 0); assertion failure in AIX

---
 gdb/aix-thread.c     | 2 ++
 gdb/rs6000-aix-nat.c | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index ecd8200b692..e5c287a3fad 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -1091,6 +1091,8 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
   if (ptid.pid () == -1)
     return ptid_t (-1);
 
+  inferior_ptid = ptid;
+
   /* Check whether libpthdebug might be ready to be initialized.  */
   if (!pd_active && status->kind () == TARGET_WAITKIND_STOPPED
       && status->sig () == GDB_SIGNAL_TRAP)
diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index 8563aea313a..24071a3742f 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -525,11 +525,11 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 
 	  /* Claim it exited with unknown signal.  */
 	  ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN);
-	  return inferior_ptid;
+	  return ptid_t(pid);
 	}
 
       /* Ignore terminated detached child processes.  */
-      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
+      if (!WIFSTOPPED (status) && find_inferior_pid(this,pid) == NULL)
 	pid = -1;
     }
   while (pid == -1);
-- 
2.31.1

Re: Fw: RE: [PATCH] Fix assert pid != 0 assertion failure in AIX

[Simon Marchi]

On 7/6/22 00:25, Aditya Vidyadhar Kamath wrote:
>
> Morning Simon.
>
> The reason we were adding one more inferior_ptid!= 0 , condition is the previous condition in the and logic i.e. pid != inferior_ptd.pid() will satisfy as -1 is not equal to 0. [inferior_ptid is set to null before coming into wait]. So, in the next iteration since the process has exited waitpid (), will lead to ERRCHILD error though the current iteration fetched the right pid using waitpid ().
>
> However, we get your point that inferior_ptid should not be used initially [for any condition check till we fetch the pid using waitpid ()] as it is being reset.
>
> Please find attached our modified patch where we do a check of the inferior being in the list. I hope this solution matches to what you suggested.
>
> [See 0001-Fix-gdb_assert-pid-0-assertion-failure-in-AIX.patch file attached to this email]
>
> Have a great day.
>
> Thanks and regards,
> Aditya.
>


> From a1c5ab5338a5d46eab675a85c28a9b00256d395a Mon Sep 17 00:00:00 2001
> From: "aditya@ibm" <aditya.vidyadhar.kamath@ibm.com>
> Date: Tue, 5 Jul 2022 23:05:18 -0500
> Subject: [PATCH] Fix gdb_assert (pid != 0); assertion failure in AIX
>
> ---
>  gdb/aix-thread.c     | 2 ++
>  gdb/rs6000-aix-nat.c | 4 ++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index ecd8200b692..e5c287a3fad 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -1091,6 +1091,8 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
>    if (ptid.pid () == -1)
>      return ptid_t (-1);
>
> +  inferior_ptid = ptid;

I get why you are doing this, because the other functions (pd_update and
friends) then use it.  However, it would be nicer to just change them
all to not use inferior_ptid, but take whatever information is needed
through parameters.  See below.

> +
>    /* Check whether libpthdebug might be ready to be initialized.  */
>    if (!pd_active && status->kind () == TARGET_WAITKIND_STOPPED
>        && status->sig () == GDB_SIGNAL_TRAP)
> diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
> index 8563aea313a..24071a3742f 100644
> --- a/gdb/rs6000-aix-nat.c
> +++ b/gdb/rs6000-aix-nat.c
> @@ -525,11 +525,11 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
>
>  	  /* Claim it exited with unknown signal.  */
>  	  ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN);
> -	  return inferior_ptid;
> +	  return ptid_t(pid);

This is not right, as we return a "signalled" event with a
minus_one_ptid (pid is -1 here).  This is unexpected to the core of GDB,
because a "signalled" event means that some inferior has received a
fatal signal.  So the returned ptid should say which inferior received
the signal.

The code in rs6000_nat_target::wait appears to have been copied from
inf_ptrace_target::wait (the base class of rs6000_nat_target).  In
inf_ptrace_target::wait, that snippet has been changed to return an
"ignore" status in that case, so I suppose we should to that here.  The
change in inf_ptrace_target::wait was done here:

  https://gitlab.com/gnutools/binutils-gdb/-/commit/85e8c48c73a5c39a6980f9b2bd16ec96062fc4c3

See my patch below.

>  	}
>
>        /* Ignore terminated detached child processes.  */
> -      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
> +      if (!WIFSTOPPED (status) && find_inferior_pid(this,pid) == NULL)

I think this is correct.  Just make sure to add spaces where appropriate.
And we prefer nullptr over NULL for new code.  See my patch below.

I managed to build GDB on gcc119, on the GCC compile farm and wrote the
patch below that at least fix things enough to be able to debug a simple
program.  I tried a multi-threaded program, and while gdb did not crash,
I was not able to see the multiple threads, so there's more work to do.
But at least, this should be a good starting point.  Please let me know
what you think.

Simon


From e9e35416a45a8454dc87cabf9462e6cf4040d088 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Wed, 6 Jul 2022 13:39:22 -0400
Subject: [PATCH] gdb: fix {rs6000_nat_target,aix_thread_target}::wait to not
 use inferior_ptid

Trying to run a simple program (empty main) on AIX, I get:

    (gdb) run
    Starting program: /scratch/simark/build/gdb/a.out
    Child process unexpectedly missing: There are no child processes..
    ../../src/binutils-gdb/gdb/inferior.c:304: internal-error: find_inferior_pid: Assertion `pid != 0' failed.
    A problem internal to GDB has been detected,
    further debugging may prove unreliable.
    ----- Backtrace -----
    0x10ef12a8 gdb_internal_backtrace_1()
            ../../src/binutils-gdb/gdb/bt-utils.c:122
    0x10ef1470 gdb_internal_backtrace()
            ../../src/binutils-gdb/gdb/bt-utils.c:168
    0x1004d368 internal_vproblem(internal_problem*, char const*, int, char const*, char*)
            ../../src/binutils-gdb/gdb/utils.c:396
    0x1004d8a8 internal_verror(char const*, int, char const*, char*)
            ../../src/binutils-gdb/gdb/utils.c:476
    0x1004c424 internal_error(char const*, int, char const*, ...)
            ../../src/binutils-gdb/gdbsupport/errors.cc:55
    0x102ab344 find_inferior_pid(process_stratum_target*, int)
            ../../src/binutils-gdb/gdb/inferior.c:304
    0x102ab4a4 find_inferior_ptid(process_stratum_target*, ptid_t)
            ../../src/binutils-gdb/gdb/inferior.c:318
    0x1061bae8 find_thread_ptid(process_stratum_target*, ptid_t)
            ../../src/binutils-gdb/gdb/thread.c:519
    0x10319e98 handle_inferior_event(execution_control_state*)
            ../../src/binutils-gdb/gdb/infrun.c:5532
    0x10315544 fetch_inferior_event()
            ../../src/binutils-gdb/gdb/infrun.c:4221
    0x10952e34 inferior_event_handler(inferior_event_type)
            ../../src/binutils-gdb/gdb/inf-loop.c:41
    0x1032640c infrun_async_inferior_event_handler(void*)
            ../../src/binutils-gdb/gdb/infrun.c:9548
    0x10673188 check_async_event_handlers()
            ../../src/binutils-gdb/gdb/async-event.c:335
    0x1066fce4 gdb_do_one_event()
            ../../src/binutils-gdb/gdbsupport/event-loop.cc:214
    0x10001a94 start_event_loop()
            ../../src/binutils-gdb/gdb/main.c:411
    0x10001ca0 captured_command_loop()
            ../../src/binutils-gdb/gdb/main.c:471
    0x10003d74 captured_main(void*)
            ../../src/binutils-gdb/gdb/main.c:1329
    0x10003e48 gdb_main(captured_main_args*)
            ../../src/binutils-gdb/gdb/main.c:1344
    0x10000744 main
            ../../src/binutils-gdb/gdb/gdb.c:32
    ---------------------
    ../../src/binutils-gdb/gdb/inferior.c:304: internal-error: find_inferior_pid: Assertion `pid != 0' failed.
    A problem internal to GDB has been detected,
    further debugging may prove unreliable.
    Quit this debugging session? (y or n)

This is due to some bit-rot in the AIX port, still relying on the entry
value of inferior_ptid in the wait methods.

Problem #1 is in rs6000_nat_target::wait, here:

      /* Ignore terminated detached child processes.  */
      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
	pid = -1;

At this point, waitpid has returned an "exited" status for some pid, so
pid is non-zero.  Since inferior_ptid is set to null_ptid on entry, the
pid returned by wait is not equal to `inferior_ptid.pid ()`, so we reset
pid to -1 and go to waiting again.  Since there are not more children to
wait for, waitpid then returns -1 so we get here:

      if (pid == -1)
	{
	  gdb_printf (gdb_stderr,
		      _("Child process unexpectedly missing: %s.\n"),
		      safe_strerror (save_errno));

	  /* Claim it exited with unknown signal.  */
	  ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN);
	  return inferior_ptid;
	}

We therefore return a "signalled" status with a null_ptid (again,
inferior_ptid is null_ptid).  This confuses infrun, because if the
target returns a "signalled" status, it should be coupled with a ptid
for an inferior that exists.

So, the first step is to fix the snippets above to not use
inferior_ptid.  In the first snippet, use find_inferior_pid to see if
we know the event process.  If there is no inferior with that pid, we
assume it's a detached child process to we ignore the event.  That
should be enough to fix the problem, because it should make it so we
won't go into the second snippet.  But still, fix the second snippet to
return an "ignore" status.  This is copied from inf_ptrace_target::wait,
which is where rs6000_nat_target::wait appears to be copied from in the
first place.

These changes, are not sufficient, as the aix_thread_target, which sits
on top of rs6000_nat_target, also relies on inferior_ptid.
aix_thread_target::wait, by calling pd_update, assumes that
rs6000_nat_target has set inferior_ptid to the appropriate value (the
ptid of the event thread), but that's not the case.  pd_update
returns inferior_ptid - null_ptid - and therefore
aix_thread_target::wait returns null_ptid too, and we still hit the
assert shown above.

Fix this by changing pd_activate, pd_update, sync_threadlists and
get_signaled_thread to all avoid using inferior_ptid.  Instead, they
accept as a parameter the pid of the process we are working on.

With this patch, I am able to run the program to completion:

    (gdb) r
    Starting program: /scratch/simark/build/gdb/a.out
    [Inferior 1 (process 11010794) exited normally]

As well as break on main:

    (gdb) b main
    Breakpoint 1 at 0x1000036c
    (gdb) r
    Starting program: /scratch/simark/build/gdb/a.out

    Breakpoint 1, 0x1000036c in main ()
    (gdb) c
    Continuing.
    [Inferior 1 (process 26083688) exited normally]

Change-Id: I7c2613bbefe487d75fa1a0c0994423471d961ee9
---
 gdb/aix-thread.c     | 59 +++++++++++++++++++++-----------------------
 gdb/rs6000-aix-nat.c |  7 +++---
 2 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index ecd8200b6928..d47f5132592a 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -701,14 +701,14 @@ gcmp (const void *t1v, const void *t2v)
    Return 0 if none found.  */

 static pthdb_tid_t
-get_signaled_thread (void)
+get_signaled_thread (int pid)
 {
   struct thrdsinfo64 thrinf;
   tid_t ktid = 0;

   while (1)
     {
-      if (getthrds (inferior_ptid.pid (), &thrinf,
+      if (getthrds (pid, &thrinf,
 		    sizeof (thrinf), &ktid, 1) != 1)
 	break;

@@ -734,9 +734,9 @@ get_signaled_thread (void)
        have difficulty with certain call patterns */

 static void
-sync_threadlists (void)
+sync_threadlists (int pid)
 {
-  int cmd, status, infpid;
+  int cmd, status;
   int pcount, psize, pi, gcount, gi;
   struct pd_thread *pbuf;
   struct thread_info **gbuf, **g, *thread;
@@ -790,8 +790,6 @@ sync_threadlists (void)
   qsort (gbuf, gcount, sizeof *gbuf, gcmp);

   /* Apply differences between the two arrays to GDB's thread list.  */
-
-  infpid = inferior_ptid.pid ();
   for (pi = gi = 0; pi < pcount || gi < gcount;)
     {
       if (pi == pcount)
@@ -808,7 +806,7 @@ sync_threadlists (void)
 	  process_stratum_target *proc_target
 	    = current_inferior ()->process_target ();
 	  thread = add_thread_with_info (proc_target,
-					 ptid_t (infpid, 0, pbuf[pi].pthid),
+					 ptid_t (pid, 0, pbuf[pi].pthid),
 					 priv);

 	  pi++;
@@ -818,7 +816,7 @@ sync_threadlists (void)
 	  ptid_t pptid, gptid;
 	  int cmp_result;

-	  pptid = ptid_t (infpid, 0, pbuf[pi].pthid);
+	  pptid = ptid_t (pid, 0, pbuf[pi].pthid);
 	  gptid = gbuf[gi]->ptid;
 	  pdtid = pbuf[pi].pdtid;
 	  tid = pbuf[pi].tid;
@@ -872,10 +870,11 @@ iter_tid (struct thread_info *thread, void *tidp)

 /* Synchronize libpthdebug's state with the inferior and with GDB,
    generate a composite process/thread <pid> for the current thread,
-   set inferior_ptid to <pid> if SET_INFPID, and return <pid>.  */
+   Return the ptid of the event thread if one can be found, else
+   return a pid-only ptid with PID.  */

 static ptid_t
-pd_update (int set_infpid)
+pd_update (int pid)
 {
   int status;
   ptid_t ptid;
@@ -883,36 +882,33 @@ pd_update (int set_infpid)
   struct thread_info *thread = NULL;

   if (!pd_active)
-    return inferior_ptid;
+    return ptid_t (pid);

   status = pthdb_session_update (pd_session);
   if (status != PTHDB_SUCCESS)
-    return inferior_ptid;
+    return ptid_t (pid);

-  sync_threadlists ();
+  sync_threadlists (pid);

   /* Define "current thread" as one that just received a trap signal.  */

-  tid = get_signaled_thread ();
+  tid = get_signaled_thread (pid);
   if (tid != 0)
     thread = iterate_over_threads (iter_tid, &tid);
   if (!thread)
-    ptid = inferior_ptid;
+    ptid = ptid_t (pid);
   else
-    {
-      ptid = thread->ptid;
-      if (set_infpid)
-	switch_to_thread (thread);
-    }
+    ptid = thread->ptid;
+
   return ptid;
 }

 /* Try to start debugging threads in the current process.
-   If successful and SET_INFPID, set inferior_ptid to reflect the
-   current thread.  */
+   If successful and there exists and we can find an event thread, return a ptid
+   for that thread.  Otherwise, return a ptid-only ptid using PID.  */

 static ptid_t
-pd_activate (int set_infpid)
+pd_activate (int pid)
 {
   int status;
 		
@@ -921,10 +917,10 @@ pd_activate (int set_infpid)
 			       &pd_session);
   if (status != PTHDB_SUCCESS)
     {
-      return inferior_ptid;
+      return ptid_t (pid);
     }
   pd_active = 1;
-  return pd_update (set_infpid);
+  return pd_update (pid);
 }

 /* Undo the effects of pd_activate().  */
@@ -1080,17 +1076,18 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
 			 target_wait_flags options)
 {
   {
-    scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
-
     pid_to_prc (&ptid);

-    inferior_ptid = ptid_t (inferior_ptid.pid ());
     ptid = beneath ()->wait (ptid, status, options);
   }

   if (ptid.pid () == -1)
     return ptid_t (-1);

+  /* The target beneath does not deal with threads, so it should only return
+     pid-only ptids.  */
+  gdb_assert (ptid.is_pid ());
+
   /* Check whether libpthdebug might be ready to be initialized.  */
   if (!pd_active && status->kind () == TARGET_WAITKIND_STOPPED
       && status->sig () == GDB_SIGNAL_TRAP)
@@ -1102,10 +1099,10 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,

       if (regcache_read_pc (regcache)
 	  - gdbarch_decr_pc_after_break (gdbarch) == pd_brk_addr)
-	return pd_activate (0);
+	return pd_activate (ptid.pid ());
     }

-  return pd_update (0);
+  return pd_update (ptid.pid ());
 }

 /* Record that the 64-bit general-purpose registers contain VALS.  */
@@ -1765,7 +1762,7 @@ aix_thread_target::pid_to_str (ptid_t ptid)
   if (!PD_TID (ptid))
     return beneath ()->pid_to_str (ptid);

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

 /* Return a printable representation of extra information about
diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index 8563aea313a2..f604f7d503e9 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -523,13 +523,12 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 		      _("Child process unexpectedly missing: %s.\n"),
 		      safe_strerror (save_errno));

-	  /* Claim it exited with unknown signal.  */
-	  ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN);
-	  return inferior_ptid;
+	  ourstatus->set_ignore ();
+	  return minus_one_ptid;
 	}

       /* Ignore terminated detached child processes.  */
-      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
+      if (!WIFSTOPPED (status) && find_inferior_pid (this, pid) == nullptr)
 	pid = -1;
     }
   while (pid == -1);
-- 
2.36.1

RE: Fw: RE: [PATCH] Fix assert pid != 0 assertion failure in AIX

[Aditya Vidyadhar Kamath]

Hi Simon,

I appreciate your patience to read our explanations, understand and suggest us the right path.

I was not aware of minus_one_ptid. I thought since pid will be -1, if a child process in not fetched to send ptid_t(pid).. That change you made looks good. Thank you for your guidance for someone new in this project.

Changing all function parameters will help us in our fork patch coming soon. So that change also looks to good to eliminate depending on inferior_ptid.

Multi thread programs are not seen even from my end. Yes, there is some work for multi thread case. Will work on it finding out a possible solution sometime next week.

Having said that, we can push the changes so that folks using AIX will not see the assertion failure for simple programs soon. It will be great if it can be done.

Have a great day ahead,

Thanks and regards,
Aditya.
________________________________
From: Simon Marchi <simark@simark.ca>
Sent: Wednesday, July 6, 2022 11:20 PM
To: Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com>; Ulrich Weigand <Ulrich.Weigand@de.ibm.com>; Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: [EXTERNAL] Re: Fw: RE: [PATCH] Fix assert pid != 0 assertion failure in AIX

On 7/6/22 00:25, Aditya Vidyadhar Kamath wrote:
>
> Morning Simon.
>
> The reason we were adding one more inferior_ptid!= 0 , condition is the previous condition in the and logic i.e. pid != inferior_ptd.pid() will satisfy as -1 is not equal to 0. [inferior_ptid is set to null before coming into wait]. So, in the next iteration since the process has exited waitpid (), will lead to ERRCHILD error though the current iteration fetched the right pid using waitpid ().
>
> However, we get your point that inferior_ptid should not be used initially [for any condition check till we fetch the pid using waitpid ()] as it is being reset.
>
> Please find attached our modified patch where we do a check of the inferior being in the list. I hope this solution matches to what you suggested.
>
> [See 0001-Fix-gdb_assert-pid-0-assertion-failure-in-AIX.patch file attached to this email]
>
> Have a great day.
>
> Thanks and regards,
> Aditya.
>


> From a1c5ab5338a5d46eab675a85c28a9b00256d395a Mon Sep 17 00:00:00 2001
> From: "aditya@ibm" <aditya.vidyadhar.kamath@ibm.com>
> Date: Tue, 5 Jul 2022 23:05:18 -0500
> Subject: [PATCH] Fix gdb_assert (pid != 0); assertion failure in AIX
>
> ---
>  gdb/aix-thread.c     | 2 ++
>  gdb/rs6000-aix-nat.c | 4 ++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index ecd8200b692..e5c287a3fad 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -1091,6 +1091,8 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
>    if (ptid.pid () == -1)
>      return ptid_t (-1);
>
> +  inferior_ptid = ptid;

I get why you are doing this, because the other functions (pd_update and
friends) then use it.  However, it would be nicer to just change them
all to not use inferior_ptid, but take whatever information is needed
through parameters.  See below.

> +
>    /* Check whether libpthdebug might be ready to be initialized.  */
>    if (!pd_active && status->kind () == TARGET_WAITKIND_STOPPED
>        && status->sig () == GDB_SIGNAL_TRAP)
> diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
> index 8563aea313a..24071a3742f 100644
> --- a/gdb/rs6000-aix-nat.c
> +++ b/gdb/rs6000-aix-nat.c
> @@ -525,11 +525,11 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
>
>          /* Claim it exited with unknown signal.  */
>          ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN);
> -       return inferior_ptid;
> +       return ptid_t(pid);

This is not right, as we return a "signalled" event with a
minus_one_ptid (pid is -1 here).  This is unexpected to the core of GDB,
because a "signalled" event means that some inferior has received a
fatal signal.  So the returned ptid should say which inferior received
the signal.

The code in rs6000_nat_target::wait appears to have been copied from
inf_ptrace_target::wait (the base class of rs6000_nat_target).  In
inf_ptrace_target::wait, that snippet has been changed to return an
"ignore" status in that case, so I suppose we should to that here.  The
change in inf_ptrace_target::wait was done here:

  https://gitlab.com/gnutools/binutils-gdb/-/commit/85e8c48c73a5c39a6980f9b2bd16ec96062fc4c3

See my patch below.

>        }
>
>        /* Ignore terminated detached child processes.  */
> -      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
> +      if (!WIFSTOPPED (status) && find_inferior_pid(this,pid) == NULL)

I think this is correct.  Just make sure to add spaces where appropriate.
And we prefer nullptr over NULL for new code.  See my patch below.

I managed to build GDB on gcc119, on the GCC compile farm and wrote the
patch below that at least fix things enough to be able to debug a simple
program.  I tried a multi-threaded program, and while gdb did not crash,
I was not able to see the multiple threads, so there's more work to do.
But at least, this should be a good starting point.  Please let me know
what you think.

Simon


From e9e35416a45a8454dc87cabf9462e6cf4040d088 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Wed, 6 Jul 2022 13:39:22 -0400
Subject: [PATCH] gdb: fix {rs6000_nat_target,aix_thread_target}::wait to not
 use inferior_ptid

Trying to run a simple program (empty main) on AIX, I get:

    (gdb) run
    Starting program: /scratch/simark/build/gdb/a.out
    Child process unexpectedly missing: There are no child processes..
    ../../src/binutils-gdb/gdb/inferior.c:304: internal-error: find_inferior_pid: Assertion `pid != 0' failed.
    A problem internal to GDB has been detected,
    further debugging may prove unreliable.
    ----- Backtrace -----
    0x10ef12a8 gdb_internal_backtrace_1()
            ../../src/binutils-gdb/gdb/bt-utils.c:122
    0x10ef1470 gdb_internal_backtrace()
            ../../src/binutils-gdb/gdb/bt-utils.c:168
    0x1004d368 internal_vproblem(internal_problem*, char const*, int, char const*, char*)
            ../../src/binutils-gdb/gdb/utils.c:396
    0x1004d8a8 internal_verror(char const*, int, char const*, char*)
            ../../src/binutils-gdb/gdb/utils.c:476
    0x1004c424 internal_error(char const*, int, char const*, ...)
            ../../src/binutils-gdb/gdbsupport/errors.cc:55
    0x102ab344 find_inferior_pid(process_stratum_target*, int)
            ../../src/binutils-gdb/gdb/inferior.c:304
    0x102ab4a4 find_inferior_ptid(process_stratum_target*, ptid_t)
            ../../src/binutils-gdb/gdb/inferior.c:318
    0x1061bae8 find_thread_ptid(process_stratum_target*, ptid_t)
            ../../src/binutils-gdb/gdb/thread.c:519
    0x10319e98 handle_inferior_event(execution_control_state*)
            ../../src/binutils-gdb/gdb/infrun.c:5532
    0x10315544 fetch_inferior_event()
            ../../src/binutils-gdb/gdb/infrun.c:4221
    0x10952e34 inferior_event_handler(inferior_event_type)
            ../../src/binutils-gdb/gdb/inf-loop.c:41
    0x1032640c infrun_async_inferior_event_handler(void*)
            ../../src/binutils-gdb/gdb/infrun.c:9548
    0x10673188 check_async_event_handlers()
            ../../src/binutils-gdb/gdb/async-event.c:335
    0x1066fce4 gdb_do_one_event()
            ../../src/binutils-gdb/gdbsupport/event-loop.cc:214
    0x10001a94 start_event_loop()
            ../../src/binutils-gdb/gdb/main.c:411
    0x10001ca0 captured_command_loop()
            ../../src/binutils-gdb/gdb/main.c:471
    0x10003d74 captured_main(void*)
            ../../src/binutils-gdb/gdb/main.c:1329
    0x10003e48 gdb_main(captured_main_args*)
            ../../src/binutils-gdb/gdb/main.c:1344
    0x10000744 main
            ../../src/binutils-gdb/gdb/gdb.c:32
    ---------------------
    ../../src/binutils-gdb/gdb/inferior.c:304: internal-error: find_inferior_pid: Assertion `pid != 0' failed.
    A problem internal to GDB has been detected,
    further debugging may prove unreliable.
    Quit this debugging session? (y or n)

This is due to some bit-rot in the AIX port, still relying on the entry
value of inferior_ptid in the wait methods.

Problem #1 is in rs6000_nat_target::wait, here:

      /* Ignore terminated detached child processes.  */
      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
        pid = -1;

At this point, waitpid has returned an "exited" status for some pid, so
pid is non-zero.  Since inferior_ptid is set to null_ptid on entry, the
pid returned by wait is not equal to `inferior_ptid.pid ()`, so we reset
pid to -1 and go to waiting again.  Since there are not more children to
wait for, waitpid then returns -1 so we get here:

      if (pid == -1)
        {
          gdb_printf (gdb_stderr,
                      _("Child process unexpectedly missing: %s.\n"),
                      safe_strerror (save_errno));

          /* Claim it exited with unknown signal.  */
          ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN);
          return inferior_ptid;
        }

We therefore return a "signalled" status with a null_ptid (again,
inferior_ptid is null_ptid).  This confuses infrun, because if the
target returns a "signalled" status, it should be coupled with a ptid
for an inferior that exists.

So, the first step is to fix the snippets above to not use
inferior_ptid.  In the first snippet, use find_inferior_pid to see if
we know the event process.  If there is no inferior with that pid, we
assume it's a detached child process to we ignore the event.  That
should be enough to fix the problem, because it should make it so we
won't go into the second snippet.  But still, fix the second snippet to
return an "ignore" status.  This is copied from inf_ptrace_target::wait,
which is where rs6000_nat_target::wait appears to be copied from in the
first place.

These changes, are not sufficient, as the aix_thread_target, which sits
on top of rs6000_nat_target, also relies on inferior_ptid.
aix_thread_target::wait, by calling pd_update, assumes that
rs6000_nat_target has set inferior_ptid to the appropriate value (the
ptid of the event thread), but that's not the case.  pd_update
returns inferior_ptid - null_ptid - and therefore
aix_thread_target::wait returns null_ptid too, and we still hit the
assert shown above.

Fix this by changing pd_activate, pd_update, sync_threadlists and
get_signaled_thread to all avoid using inferior_ptid.  Instead, they
accept as a parameter the pid of the process we are working on.

With this patch, I am able to run the program to completion:

    (gdb) r
    Starting program: /scratch/simark/build/gdb/a.out
    [Inferior 1 (process 11010794) exited normally]

As well as break on main:

    (gdb) b main
    Breakpoint 1 at 0x1000036c
    (gdb) r
    Starting program: /scratch/simark/build/gdb/a.out

    Breakpoint 1, 0x1000036c in main ()
    (gdb) c
    Continuing.
    [Inferior 1 (process 26083688) exited normally]

Change-Id: I7c2613bbefe487d75fa1a0c0994423471d961ee9
---
 gdb/aix-thread.c     | 59 +++++++++++++++++++++-----------------------
 gdb/rs6000-aix-nat.c |  7 +++---
 2 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index ecd8200b6928..d47f5132592a 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -701,14 +701,14 @@ gcmp (const void *t1v, const void *t2v)
    Return 0 if none found.  */

 static pthdb_tid_t
-get_signaled_thread (void)
+get_signaled_thread (int pid)
 {
   struct thrdsinfo64 thrinf;
   tid_t ktid = 0;

   while (1)
     {
-      if (getthrds (inferior_ptid.pid (), &thrinf,
+      if (getthrds (pid, &thrinf,
                     sizeof (thrinf), &ktid, 1) != 1)
         break;

@@ -734,9 +734,9 @@ get_signaled_thread (void)
        have difficulty with certain call patterns */

 static void
-sync_threadlists (void)
+sync_threadlists (int pid)
 {
-  int cmd, status, infpid;
+  int cmd, status;
   int pcount, psize, pi, gcount, gi;
   struct pd_thread *pbuf;
   struct thread_info **gbuf, **g, *thread;
@@ -790,8 +790,6 @@ sync_threadlists (void)
   qsort (gbuf, gcount, sizeof *gbuf, gcmp);

   /* Apply differences between the two arrays to GDB's thread list.  */
-
-  infpid = inferior_ptid.pid ();
   for (pi = gi = 0; pi < pcount || gi < gcount;)
     {
       if (pi == pcount)
@@ -808,7 +806,7 @@ sync_threadlists (void)
           process_stratum_target *proc_target
             = current_inferior ()->process_target ();
           thread = add_thread_with_info (proc_target,
-                                        ptid_t (infpid, 0, pbuf[pi].pthid),
+                                        ptid_t (pid, 0, pbuf[pi].pthid),
                                          priv);

           pi++;
@@ -818,7 +816,7 @@ sync_threadlists (void)
           ptid_t pptid, gptid;
           int cmp_result;

-         pptid = ptid_t (infpid, 0, pbuf[pi].pthid);
+         pptid = ptid_t (pid, 0, pbuf[pi].pthid);
           gptid = gbuf[gi]->ptid;
           pdtid = pbuf[pi].pdtid;
           tid = pbuf[pi].tid;
@@ -872,10 +870,11 @@ iter_tid (struct thread_info *thread, void *tidp)

 /* Synchronize libpthdebug's state with the inferior and with GDB,
    generate a composite process/thread <pid> for the current thread,
-   set inferior_ptid to <pid> if SET_INFPID, and return <pid>.  */
+   Return the ptid of the event thread if one can be found, else
+   return a pid-only ptid with PID.  */

 static ptid_t
-pd_update (int set_infpid)
+pd_update (int pid)
 {
   int status;
   ptid_t ptid;
@@ -883,36 +882,33 @@ pd_update (int set_infpid)
   struct thread_info *thread = NULL;

   if (!pd_active)
-    return inferior_ptid;
+    return ptid_t (pid);

   status = pthdb_session_update (pd_session);
   if (status != PTHDB_SUCCESS)
-    return inferior_ptid;
+    return ptid_t (pid);

-  sync_threadlists ();
+  sync_threadlists (pid);

   /* Define "current thread" as one that just received a trap signal.  */

-  tid = get_signaled_thread ();
+  tid = get_signaled_thread (pid);
   if (tid != 0)
     thread = iterate_over_threads (iter_tid, &tid);
   if (!thread)
-    ptid = inferior_ptid;
+    ptid = ptid_t (pid);
   else
-    {
-      ptid = thread->ptid;
-      if (set_infpid)
-       switch_to_thread (thread);
-    }
+    ptid = thread->ptid;
+
   return ptid;
 }

 /* Try to start debugging threads in the current process.
-   If successful and SET_INFPID, set inferior_ptid to reflect the
-   current thread.  */
+   If successful and there exists and we can find an event thread, return a ptid
+   for that thread.  Otherwise, return a ptid-only ptid using PID.  */

 static ptid_t
-pd_activate (int set_infpid)
+pd_activate (int pid)
 {
   int status;

@@ -921,10 +917,10 @@ pd_activate (int set_infpid)
                                &pd_session);
   if (status != PTHDB_SUCCESS)
     {
-      return inferior_ptid;
+      return ptid_t (pid);
     }
   pd_active = 1;
-  return pd_update (set_infpid);
+  return pd_update (pid);
 }

 /* Undo the effects of pd_activate().  */
@@ -1080,17 +1076,18 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
                          target_wait_flags options)
 {
   {
-    scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
-
     pid_to_prc (&ptid);

-    inferior_ptid = ptid_t (inferior_ptid.pid ());
     ptid = beneath ()->wait (ptid, status, options);
   }

   if (ptid.pid () == -1)
     return ptid_t (-1);

+  /* The target beneath does not deal with threads, so it should only return
+     pid-only ptids.  */
+  gdb_assert (ptid.is_pid ());
+
   /* Check whether libpthdebug might be ready to be initialized.  */
   if (!pd_active && status->kind () == TARGET_WAITKIND_STOPPED
       && status->sig () == GDB_SIGNAL_TRAP)
@@ -1102,10 +1099,10 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,

       if (regcache_read_pc (regcache)
           - gdbarch_decr_pc_after_break (gdbarch) == pd_brk_addr)
-       return pd_activate (0);
+       return pd_activate (ptid.pid ());
     }

-  return pd_update (0);
+  return pd_update (ptid.pid ());
 }

 /* Record that the 64-bit general-purpose registers contain VALS.  */
@@ -1765,7 +1762,7 @@ aix_thread_target::pid_to_str (ptid_t ptid)
   if (!PD_TID (ptid))
     return beneath ()->pid_to_str (ptid);

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

 /* Return a printable representation of extra information about
diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index 8563aea313a2..f604f7d503e9 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -523,13 +523,12 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
                       _("Child process unexpectedly missing: %s.\n"),
                       safe_strerror (save_errno));

-         /* Claim it exited with unknown signal.  */
-         ourstatus->set_signalled (GDB_SIGNAL_UNKNOWN);
-         return inferior_ptid;
+         ourstatus->set_ignore ();
+         return minus_one_ptid;
         }

       /* Ignore terminated detached child processes.  */
-      if (!WIFSTOPPED (status) && pid != inferior_ptid.pid ())
+      if (!WIFSTOPPED (status) && find_inferior_pid (this, pid) == nullptr)
         pid = -1;
     }
   while (pid == -1);
--
2.36.1

Re: Fw: RE: [PATCH] Fix assert pid != 0 assertion failure in AIX

[Simon Marchi]

On 2022-07-07 04:27, Aditya Vidyadhar Kamath wrote:
> Hi Simon,
> 
> I appreciate your patience to read our explanations, understand and suggest us the right path. 
> 
> I was not aware of minus_one_ptid. I thought since pid will be -1, if a child process in not fetched to send ptid_t(pid).. That change you made looks good. Thank you for your guidance for someone new in this project. 
> 
> Changing all function parameters will help us in our fork patch coming soon. So that change also looks to good to eliminate depending on inferior_ptid. 
> 
> Multi thread programs are not seen even from my end. Yes, there is some work for multi thread case. Will work on it finding out a possible solution sometime next week. 
> 
> Having said that, we can push the changes so that folks using AIX will not see the assertion failure for simple programs soon. It will be great if it can be done.   
> 
> Have a great day ahead,

Ok, thanks, I pushed the patch to the master branch.

Simon

Re: [PATCH] Use current_inferior ()->pid for AIX

[Ulrich Weigand]

Aditya Vidyadhar Kamath <ADITYA.VIDYADHAR.KAMATH@ibm.com> wrote:

>AIX is still using inferior_ptid.pid() to get the inferior pid instead
>of the new object current_inferior().With this it is not possible to
>debug any sample program as the it fails with assertion check for
>pid!=0.

I don't quite understand the underlying problem here.  While we may
want to get rid of inferior_ptid at some point in the future, as of
right now inferior_ptid *is* still being used (by many places in the
target stack), and should always be maintained correctly.

Maybe the problem is really that somewhere inferior_ptid isn't being
updated as it should?


As an aside, this part changes semantics of the code:

>@@ -932,11 +932,12 @@
> static void
> pd_deactivate (void)
> {
>+  ptid_t ptdrtn = ptid_t (current_inferior ()->pid);
>   if (!pd_active)
>     return;
>   pthdb_session_destroy (pd_session);
>   
>-  pid_to_prc (&inferior_ptid);
>+  pid_to_prc (&ptdrtn); 
>   pd_active = 0;
> }

The point of this code was to *change* inferior_ptid back from
the per-thread view to the no-threads view as the threading
layer is deactivated.  With your patch, inferior_ptid is no
longer changed at all, which makes the pid_to_prc call useless.

Bye,
Ulrich