Re: [Patch] Fix Assertion pid 0 failure in AIX while running gdb.threads/foll-fork-other-thread

[Aditya Kamath1 <Aditya.Kamath1@ibm.com>]

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

Hi Ulrich and community,

Please find attached the patch.

>Two issues:

>- Please only use current_inferior() in the ::mourn_inferior path.
>  In the ::detach path you still should use the passed-in "inf".
 > (This means pd_enable/disable still should get an "inf" argument.)

This is done. I misunderstood the previous email. Apologise for the same.

So now the inferior argument is passed in pd_enable () and pd_disable () so that we are good and do not cause the memory leak and pid != 0 assertion failure.. Outputs before and after patch remain same as the previous email.

Kindly check in this version.

Have a nice day ahead. Appreciate your patience.

Thanks and regards,
Aditya.


From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Date: Tuesday, 2 May 2023 at 7:36 PM
To: gdb-patches@sourceware.org <gdb-patches@sourceware.org>, Aditya Kamath1 <Aditya.Kamath1@ibm.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: Re: [Patch] Fix Assertion pid 0 failure in AIX while running gdb.threads/foll-fork-other-thread
Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>>Ah, right. There's indeed no inf argument (unfortunately), but you can
>>just use "current_inferior ()" instead.  This is guaranteed to be
>>set correctly at this point.
>
>This makes it easy. Thanks Ulrich. It fixes the issue and no memory leaks.
>Kindly push this small patch if there are no further changes.

Two issues:

- Please only use current_inferior() in the ::mourn_inferior path.
  In the ::detach path you still should use the passed-in "inf".
  (This means pd_enable/disable still should get an "inf" argument.)

- Your patch adds:
    data = get_thread_data_helper_for_pid (current_inferior ()->pid);
  where get_thread_data_helper_for_pid does:
    inferior *inf = find_inferior_pid (current_inferior ()->process_target (),
                                       pid);
    return get_aix_thread_variables_data (inf);
  This seems a bit silly (and inefficient) - if you already have
  an "inf", you should just use get_aix_thread_variables_data directly.

Bye,
Ulrich

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

From f91c70de944e027ee5a093e97cc4eadf239cb3de Mon Sep 17 00:00:00 2001
From: Aditya Kamath <Aditya.Kamath@ibm.com>
Date: Tue, 2 May 2023 09:26:54 -0500
Subject: [PATCH] Fix Assertion pid != 0 failure in AIX.

In AIX if there is a main and a thread created from it , then once the
program completed execution and goes to pd_disable () inferior_ptid
had pid 0 leading to an assertion failure while finding the thread's data
in aix-thread.c file.

This patch is a fix for the same.
---
 gdb/aix-thread.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index c587027fb6d..5a18e2bf30e 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -1049,17 +1049,17 @@ pd_activate (pid_t pid)
    application is pthreaded, and if so, prepare for thread debugging.  */
 
 static void
-pd_enable (void)
+pd_enable (inferior *inf)
 {
   int status;
   char *stub_name;
   struct bound_minimal_symbol ms;
   struct aix_thread_variables *data;
 
-  if (!inferior_ptid.pid ())
+  if (inf == NULL)
     return;
 
-  data = get_thread_data_helper_for_ptid (inferior_ptid);
+  data = get_aix_thread_variables_data (inf);
 
   /* Don't initialize twice.  */
   if (data->pd_able)
@@ -1070,7 +1070,7 @@ pd_enable (void)
 
   /* Check whether the application is pthreaded.  */
   stub_name = NULL;
-  status = pthdb_session_pthreaded (inferior_ptid.pid (), PTHDB_FLAG_REGS,
+  status = pthdb_session_pthreaded (inf->pid, PTHDB_FLAG_REGS,
 				    &pd_callbacks, &stub_name);
   if ((status != PTHDB_SUCCESS
        && status != PTHDB_NOT_PTHREADED) || !stub_name)
@@ -1088,25 +1088,25 @@ pd_enable (void)
   current_inferior ()->push_target (&aix_thread_ops);
   data->pd_able = 1;
 
-  inferior *inf = current_inferior ();
+  inferior *inf_curr = current_inferior ();
   /* When attaching / handling fork child, don't try activating
      thread debugging until we know about all shared libraries.  */
-  if (inf->in_initial_library_scan)
+  if (inf_curr->in_initial_library_scan)
     return;
 
   /* If we're debugging a core file or an attached inferior, the
      pthread library may already have been initialized, so try to
      activate thread debugging.  */
-  pd_activate (inferior_ptid.pid ());
+  pd_activate (inf->pid);
 }
 
 /* Undo the effects of pd_enable().  */
 
 static void
-pd_disable (void)
+pd_disable (inferior *inf)
 {
   struct aix_thread_variables *data;
-  data = get_thread_data_helper_for_ptid (inferior_ptid);
+  data = get_aix_thread_variables_data (inf);
 
   if (!data->pd_able)
     return;
@@ -1129,7 +1129,7 @@ static void
 new_objfile (struct objfile *objfile)
 {
   if (objfile)
-    pd_enable ();
+    pd_enable (NULL);
 }
 
 /* Attach to process specified by ARGS.  */
@@ -1137,7 +1137,7 @@ new_objfile (struct objfile *objfile)
 static void
 aix_thread_inferior_created (inferior *inf)
 {
-  pd_enable ();
+  pd_enable (inf);
 }
 
 /* Detach from the process attached to by aix_thread_attach().  */
@@ -1147,7 +1147,7 @@ aix_thread_target::detach (inferior *inf, int from_tty)
 {
   target_ops *beneath = this->beneath ();
 
-  pd_disable ();
+  pd_disable (inf);
   beneath->detach (inf, from_tty);
 }
 
@@ -2066,7 +2066,7 @@ aix_thread_target::mourn_inferior ()
 {
   target_ops *beneath = this->beneath ();
 
-  pd_disable ();
+  pd_disable (current_inferior ());
   beneath->mourn_inferior ();
 }
 
-- 
2.38.3