* [PATCH] Use current_inferior ()->pid for AIX @ 2022-03-29 6:58 Aditya Vidyadhar Kamath 2022-03-29 13:01 ` Simon Marchi 0 siblings, 1 reply; 21+ messages in thread From: Aditya Vidyadhar Kamath @ 2022-03-29 6:58 UTC (permalink / raw) To: Joel Brobecker via Gdb-patches; +Cc: Sangamesh Mallayya [-- 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); ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Use current_inferior ()->pid for AIX 2022-03-29 6:58 [PATCH] Use current_inferior ()->pid for AIX Aditya Vidyadhar Kamath @ 2022-03-29 13:01 ` Simon Marchi 2022-03-30 13:04 ` Aditya Vidyadhar Kamath 0 siblings, 1 reply; 21+ messages in thread From: Simon Marchi @ 2022-03-29 13:01 UTC (permalink / raw) To: Aditya Vidyadhar Kamath, Joel Brobecker via Gdb-patches Cc: Sangamesh Mallayya 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] Use current_inferior ()->pid for AIX 2022-03-29 13:01 ` Simon Marchi @ 2022-03-30 13:04 ` Aditya Vidyadhar Kamath 2022-04-05 12:15 ` Aditya Vidyadhar Kamath 2022-04-05 12:47 ` Simon Marchi 0 siblings, 2 replies; 21+ messages in thread From: Aditya Vidyadhar Kamath @ 2022-03-30 13:04 UTC (permalink / raw) To: Simon Marchi, Joel Brobecker via Gdb-patches; +Cc: Sangamesh Mallayya [-- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] Use current_inferior ()->pid for AIX 2022-03-30 13:04 ` Aditya Vidyadhar Kamath @ 2022-04-05 12:15 ` Aditya Vidyadhar Kamath 2022-04-05 12:47 ` Simon Marchi 1 sibling, 0 replies; 21+ messages in thread From: Aditya Vidyadhar Kamath @ 2022-04-05 12:15 UTC (permalink / raw) To: Simon Marchi, Joel Brobecker via Gdb-patches; +Cc: Sangamesh Mallayya 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Use current_inferior ()->pid for AIX 2022-03-30 13:04 ` Aditya Vidyadhar Kamath 2022-04-05 12:15 ` Aditya Vidyadhar Kamath @ 2022-04-05 12:47 ` Simon Marchi 2022-04-12 13:32 ` Aditya Vidyadhar Kamath 1 sibling, 1 reply; 21+ messages in thread From: Simon Marchi @ 2022-04-05 12:47 UTC (permalink / raw) To: Aditya Vidyadhar Kamath, Joel Brobecker via Gdb-patches Cc: Sangamesh Mallayya 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] Use current_inferior ()->pid for AIX 2022-04-05 12:47 ` Simon Marchi @ 2022-04-12 13:32 ` Aditya Vidyadhar Kamath 2022-04-18 6:33 ` Aditya Vidyadhar Kamath ` (2 more replies) 0 siblings, 3 replies; 21+ messages in thread From: Aditya Vidyadhar Kamath @ 2022-04-12 13:32 UTC (permalink / raw) To: Simon Marchi, Joel Brobecker via Gdb-patches; +Cc: Sangamesh Mallayya [-- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] Use current_inferior ()->pid for AIX 2022-04-12 13:32 ` Aditya Vidyadhar Kamath @ 2022-04-18 6:33 ` Aditya Vidyadhar Kamath 2022-04-21 11:41 ` Aditya Vidyadhar Kamath 2022-04-21 14:51 ` Simon Marchi 2 siblings, 0 replies; 21+ messages in thread From: Aditya Vidyadhar Kamath @ 2022-04-18 6:33 UTC (permalink / raw) To: Simon Marchi, Joel Brobecker via Gdb-patches, Aditya Vidyadhar Kamath Cc: Sangamesh Mallayya 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] Use current_inferior ()->pid for AIX 2022-04-12 13:32 ` Aditya Vidyadhar Kamath 2022-04-18 6:33 ` Aditya Vidyadhar Kamath @ 2022-04-21 11:41 ` Aditya Vidyadhar Kamath 2022-04-21 14:51 ` Simon Marchi 2 siblings, 0 replies; 21+ messages in thread From: Aditya Vidyadhar Kamath @ 2022-04-21 11:41 UTC (permalink / raw) To: Simon Marchi, Joel Brobecker via Gdb-patches, Aditya Vidyadhar Kamath Cc: Sangamesh Mallayya 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Use current_inferior ()->pid for AIX 2022-04-12 13:32 ` Aditya Vidyadhar Kamath 2022-04-18 6:33 ` Aditya Vidyadhar Kamath 2022-04-21 11:41 ` Aditya Vidyadhar Kamath @ 2022-04-21 14:51 ` Simon Marchi [not found] ` <BN8PR15MB2867D6D625DD0B353C99D3A3B5DD9@BN8PR15MB2867.namprd15.prod.outlook.com> 2 siblings, 1 reply; 21+ messages in thread From: Simon Marchi @ 2022-04-21 14:51 UTC (permalink / raw) To: Aditya Vidyadhar Kamath, Joel Brobecker via Gdb-patches Cc: Sangamesh Mallayya > 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <BN8PR15MB2867D6D625DD0B353C99D3A3B5DD9@BN8PR15MB2867.namprd15.prod.outlook.com>]
* Re: [PATCH] Use current_inferior ()->pid for AIX [not found] ` <BN8PR15MB2867D6D625DD0B353C99D3A3B5DD9@BN8PR15MB2867.namprd15.prod.outlook.com> @ 2022-05-30 12:45 ` Simon Marchi 2022-06-10 14:47 ` Aditya Vidyadhar Kamath 0 siblings, 1 reply; 21+ messages in thread From: Simon Marchi @ 2022-05-30 12:45 UTC (permalink / raw) To: Aditya Vidyadhar Kamath; +Cc: Sangamesh Mallayya, Simon Marchi via Gdb-patches 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] Use current_inferior ()->pid for AIX 2022-05-30 12:45 ` Simon Marchi @ 2022-06-10 14:47 ` Aditya Vidyadhar Kamath 2022-06-15 4:03 ` Aditya Vidyadhar Kamath 2022-06-27 12:55 ` Fw: " Aditya Vidyadhar Kamath 0 siblings, 2 replies; 21+ messages in thread From: Aditya Vidyadhar Kamath @ 2022-06-10 14:47 UTC (permalink / raw) To: Simon Marchi Cc: Sangamesh Mallayya, Simon Marchi via Gdb-patches, Simon Marchi via Gdb-patches [-- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] Use current_inferior ()->pid for AIX 2022-06-10 14:47 ` Aditya Vidyadhar Kamath @ 2022-06-15 4:03 ` Aditya Vidyadhar Kamath 2022-06-23 20:40 ` Aditya Vidyadhar Kamath 2022-06-27 12:55 ` Fw: " Aditya Vidyadhar Kamath 1 sibling, 1 reply; 21+ messages in thread From: Aditya Vidyadhar Kamath @ 2022-06-15 4:03 UTC (permalink / raw) To: Simon Marchi, Aditya Vidyadhar Kamath Cc: Sangamesh Mallayya, Simon Marchi via Gdb-patches 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] Use current_inferior ()->pid for AIX 2022-06-15 4:03 ` Aditya Vidyadhar Kamath @ 2022-06-23 20:40 ` Aditya Vidyadhar Kamath 0 siblings, 0 replies; 21+ messages in thread From: Aditya Vidyadhar Kamath @ 2022-06-23 20:40 UTC (permalink / raw) To: Simon Marchi; +Cc: Sangamesh Mallayya, Simon Marchi via Gdb-patches 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Fw: RE: [PATCH] Use current_inferior ()->pid for AIX 2022-06-10 14:47 ` Aditya Vidyadhar Kamath 2022-06-15 4:03 ` Aditya Vidyadhar Kamath @ 2022-06-27 12:55 ` Aditya Vidyadhar Kamath 2022-06-27 15:11 ` Simon Marchi 2022-07-04 19:28 ` Simon Marchi 1 sibling, 2 replies; 21+ messages in thread From: Aditya Vidyadhar Kamath @ 2022-06-27 12:55 UTC (permalink / raw) To: Ulrich Weigand, Joel Brobecker via Gdb-patches; +Cc: Sangamesh Mallayya [-- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Fw: RE: [PATCH] Use current_inferior ()->pid for AIX 2022-06-27 12:55 ` Fw: " Aditya Vidyadhar Kamath @ 2022-06-27 15:11 ` Simon Marchi 2022-07-04 19:28 ` Simon Marchi 1 sibling, 0 replies; 21+ messages in thread From: Simon Marchi @ 2022-06-27 15:11 UTC (permalink / raw) To: Aditya Vidyadhar Kamath, Ulrich Weigand, Joel Brobecker via Gdb-patches Cc: Sangamesh Mallayya 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Fw: RE: [PATCH] Use current_inferior ()->pid for AIX 2022-06-27 12:55 ` Fw: " Aditya Vidyadhar Kamath 2022-06-27 15:11 ` Simon Marchi @ 2022-07-04 19:28 ` Simon Marchi 2022-07-06 4:25 ` Fw: RE: [PATCH] Fix assert pid != 0 assertion failure in AIX Aditya Vidyadhar Kamath 1 sibling, 1 reply; 21+ messages in thread From: Simon Marchi @ 2022-07-04 19:28 UTC (permalink / raw) To: Aditya Vidyadhar Kamath, Ulrich Weigand, Joel Brobecker via Gdb-patches Cc: Sangamesh Mallayya 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: Fw: RE: [PATCH] Fix assert pid != 0 assertion failure in AIX 2022-07-04 19:28 ` Simon Marchi @ 2022-07-06 4:25 ` Aditya Vidyadhar Kamath 2022-07-06 17:50 ` Simon Marchi 0 siblings, 1 reply; 21+ messages in thread From: Aditya Vidyadhar Kamath @ 2022-07-06 4:25 UTC (permalink / raw) To: Simon Marchi, Ulrich Weigand, Joel Brobecker via Gdb-patches Cc: Sangamesh Mallayya [-- 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Fw: RE: [PATCH] Fix assert pid != 0 assertion failure in AIX 2022-07-06 4:25 ` Fw: RE: [PATCH] Fix assert pid != 0 assertion failure in AIX Aditya Vidyadhar Kamath @ 2022-07-06 17:50 ` Simon Marchi 2022-07-07 8:27 ` Aditya Vidyadhar Kamath 0 siblings, 1 reply; 21+ messages in thread From: Simon Marchi @ 2022-07-06 17:50 UTC (permalink / raw) To: Aditya Vidyadhar Kamath, Ulrich Weigand, Joel Brobecker via Gdb-patches Cc: Sangamesh Mallayya 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: Fw: RE: [PATCH] Fix assert pid != 0 assertion failure in AIX 2022-07-06 17:50 ` Simon Marchi @ 2022-07-07 8:27 ` Aditya Vidyadhar Kamath 2022-07-07 13:56 ` Simon Marchi 0 siblings, 1 reply; 21+ messages in thread From: Aditya Vidyadhar Kamath @ 2022-07-07 8:27 UTC (permalink / raw) To: Simon Marchi, Ulrich Weigand, Joel Brobecker via Gdb-patches Cc: Sangamesh Mallayya 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Fw: RE: [PATCH] Fix assert pid != 0 assertion failure in AIX 2022-07-07 8:27 ` Aditya Vidyadhar Kamath @ 2022-07-07 13:56 ` Simon Marchi 0 siblings, 0 replies; 21+ messages in thread From: Simon Marchi @ 2022-07-07 13:56 UTC (permalink / raw) To: Aditya Vidyadhar Kamath, Ulrich Weigand, Joel Brobecker via Gdb-patches Cc: Sangamesh Mallayya 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <BN6PR15MB13130BF943A019871F8F4E0EB5119@BN6PR15MB1313.namprd15.prod.outlook.com>]
* Re: [PATCH] Use current_inferior ()->pid for AIX [not found] <BN6PR15MB13130BF943A019871F8F4E0EB5119@BN6PR15MB1313.namprd15.prod.outlook.com> @ 2022-05-02 14:50 ` Ulrich Weigand 0 siblings, 0 replies; 21+ messages in thread From: Ulrich Weigand @ 2022-05-02 14:50 UTC (permalink / raw) To: gdb-patches, Aditya Vidyadhar Kamath 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2022-07-07 13:56 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-29 6:58 [PATCH] Use current_inferior ()->pid for AIX Aditya Vidyadhar Kamath 2022-03-29 13:01 ` Simon Marchi 2022-03-30 13:04 ` Aditya Vidyadhar Kamath 2022-04-05 12:15 ` Aditya Vidyadhar Kamath 2022-04-05 12:47 ` Simon Marchi 2022-04-12 13:32 ` Aditya Vidyadhar Kamath 2022-04-18 6:33 ` Aditya Vidyadhar Kamath 2022-04-21 11:41 ` Aditya Vidyadhar Kamath 2022-04-21 14:51 ` Simon Marchi [not found] ` <BN8PR15MB2867D6D625DD0B353C99D3A3B5DD9@BN8PR15MB2867.namprd15.prod.outlook.com> 2022-05-30 12:45 ` Simon Marchi 2022-06-10 14:47 ` Aditya Vidyadhar Kamath 2022-06-15 4:03 ` Aditya Vidyadhar Kamath 2022-06-23 20:40 ` Aditya Vidyadhar Kamath 2022-06-27 12:55 ` Fw: " Aditya Vidyadhar Kamath 2022-06-27 15:11 ` Simon Marchi 2022-07-04 19:28 ` Simon Marchi 2022-07-06 4:25 ` Fw: RE: [PATCH] Fix assert pid != 0 assertion failure in AIX Aditya Vidyadhar Kamath 2022-07-06 17:50 ` Simon Marchi 2022-07-07 8:27 ` Aditya Vidyadhar Kamath 2022-07-07 13:56 ` Simon Marchi [not found] <BN6PR15MB13130BF943A019871F8F4E0EB5119@BN6PR15MB1313.namprd15.prod.outlook.com> 2022-05-02 14:50 ` [PATCH] Use current_inferior ()->pid for AIX Ulrich Weigand
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).