public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Aditya Kamath1 <Aditya.Kamath1@ibm.com>
To: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>,
	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
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
Date: Tue, 2 May 2023 11:49:59 +0000	[thread overview]
Message-ID: <CH2PR15MB3544035B06D680B6CFC4DDDCD66F9@CH2PR15MB3544.namprd15.prod.outlook.com> (raw)
In-Reply-To: <7397ecb838f4a65cbdde67a869d5ac4788ced334.camel@de.ibm.com>


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

Hi Ulrich and community,

Please find attached the patch. {See: : 0001-Fix-Assertion-pid-0-failure-in-AIX.patch}

Ideally this patch should have worked and it is. But I see some problems.

>I'm not sure this is quite right.  Can this now cause leaks e.g.
>because we did not properly call pthdb_session_destroy?

>Also, I'm starting to wonder if using inferior_ptid is right here
>in the first place.  For example, aix_thread_target::detach
>actually gets an "inferior *" argument, which it then completely
>ignores and instead tries to re-create an inferior from
>inferior_ptid - this doesn't look right.

>I guess pd_enable and pd_disable should be getting inferior *
>arguments and use those, whenever available.

Yes it does cause leaks. Currently there are two places from where pd_disable () is called. One from aix_thread_tarhet::detach () where we have the inferior. Unfortunately this is not getting called at all and hence we are never actually detaching. We can take the case of the program pasted in this mail below. But even in complex programs detach has never been called. This makes me worried that I am missing something major. Am I??

And when pd_disable () is called it is from the mourn inferior (). Here we do not have the inferior_ptid.pid nor the inf to catch hold of the exited thread/process.. So in AIX, thread exits are not captured properly. I have missed something in my analysis. This patch is only an adjustment that fixes the problem, but not correctly.

Also I begin to see these warnings…

Starting program: /home/aditya/latest_gdb/binutils-gdb/gdb/foll-fork-other-thread
[New Thread 258]
[New inferior 2 (process 30278140)]
warning: "/usr/lib/libpthreads.a": member "shr_comm.o" missing.
warning: "/usr/lib/libpthread.a": member "shr_xpg5.o" missing.
warning: "/usr/lib/libc.a": member "shr.o" missing.
warning: Could not load shared library symbols for 3 libraries, e.g. /usr/lib/libpthreads.a(shr_comm.o).
Use the "info sharedlibrary" command to see the complete listing.
Do you need "set solib-search-path" or "set sysroot"?
[New process 29229422]


This indicates my shared libraries are not loaded.

When I checked in my solib-aix.c I figured out that the function
gdb_bfd_ref_ptr object_bfd
    (gdb_bfd_openr_next_archived_file (archive_bfd.get (), NULL));

is returning NULL for object_bfd and hence it does not enter that while loop in search of the library and this happens for every new forked process that is attached. And what’s worse is libc is getting loaded properly. The other three are not.  You can see it in the ouput pasted below named as problematic output.

Did this API change in the last few days?? Until Thursday all was fine and I was not seeing these warnings. I updated my GDB to the latest development branch today.

So I have these problems and I am missing a trick. I have shared the details of my analysis. Kindly let me know where I have gone wrong and where we can correct this.

Have a nice day ahead.

Thanks and regards,
Aditya.

----------------------------------------------
Code:- { Program Credits: GDB threads testsuite}

#include <pthread.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <errno.h>
#include <assert.h>
#include <limits.h>

/* Set by GDB.  */
volatile int stop_looping = 0;

static void *
gdb_forker_thread (void *arg)
{
  int ret;
  int stat;
  pid_t pid = FORK_FUNC ();

  if (pid == 0)
    _exit (0);

  assert (pid > 0);

  /* Wait for child to exit.  */
  do
    {
      ret = waitpid (pid, &stat, 0);
    }
  while (ret == -1 && errno == EINTR);

  assert (ret == pid);
  assert (WIFEXITED (stat));
  assert (WEXITSTATUS (stat) == 0);

  stop_looping = 1;

  return NULL;
}

static void
sleep_a_bit (void)
{
  usleep (1000 * 50);
}

int
main (void)
{
  int i;
  int ret;
  pthread_t thread;

  alarm (60);

  ret = pthread_create (&thread, NULL, gdb_forker_thread, NULL);
  assert (ret == 0);

  while (!stop_looping)  /* while loop */
    {
      sleep_a_bit ();    /* break here */
      sleep_a_bit ();    /* other line */
    }

  pthread_join (thread, NULL);

  return 0; /* exiting here */
}

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

Output before patch:-

./gdb ~/gdb_tests/foll-fork-other-thread
GNU gdb (GDB) 14.0.50.20230327-git
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "powerpc64-ibm-aix7.2.0.0".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /home/aditya/gdb_tests/foll-fork-other-thread...
(gdb) r
Starting program: /home/aditya/gdb_tests/foll-fork-other-thread
[New Thread 258]
[Detaching after fork from child process 10944778]
[Inferior 1 (process 7209300) exited normally]
inferior.c:350: internal-error: find_inferior_pid: Assertion `pid != 0' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
0x100f7d307 ???
0x100f7d4cf ???

--------------------------------------------------------
Output after applying patch:-

bash-5.1$ ./gdb ~/gdb_tests/foll-fork-other-thread
GNU gdb (GDB) 14.0.50.20230327-git
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "powerpc64-ibm-aix7.2.0.0".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /home/aditya/gdb_tests/foll-fork-other-thread...
(gdb) r
Starting program: /home/aditya/gdb_tests/foll-fork-other-thread
[New Thread 258]
[Detaching after fork from child process 7209320]
[Inferior 1 (process 10944912) exited normally]
(gdb)


Problematic case:-

bash-5.1$ ./gdb foll-fork-other-thread
GNU gdb (GDB) 14.0.50.20230502-git
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "powerpc64-ibm-aix7.2.0.0".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
https://www.gnu.org/software/gdb/bugs/.
Find the GDB manual and other documentation resources online at:
    http://www.gnu.org/software/gdb/documentation/.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from foll-fork-other-thread...
(gdb) set detach-on-fork off
(gdb) r
Starting program: /home/aditya/latest_gdb/binutils-gdb/gdb/foll-fork-other-thread
[New Thread 258]
[New inferior 2 (process 29295030)]
warning: "/usr/lib/libpthreads.a": member "shr_comm.o" missing.
warning: "/usr/lib/libpthread.a": member "shr_xpg5.o" missing.
warning: "/usr/lib/libc.a": member "shr.o" missing.
warning: Could not load shared library symbols for 3 libraries, e.g. /usr/lib/libpthreads.a(shr_comm.o).
Use the "info sharedlibrary" command to see the complete listing.
Do you need "set solib-search-path" or "set sysroot"?
[New process 20251106]

Thread 1.3 received signal SIGINT, Interrupt.
[Switching to process 20251106]
0xd02390e0 in waitpid () from /usr/lib/libc.a(shr.o)
(gdb) inferior 2
[Switching to inferior 2 [process 29295030] (/home/aditya/latest_gdb/binutils-gdb/gdb/foll-fork-other-thread)]
[Switching to thread 2.1 (process 29295030)]
#0  0xd058f948 in ?? ()
(gdb) info sharedlibrary
From        To          Syms Read   Shared Object Library
                        No          /usr/lib/libpthreads.a(shr_comm.o)
0xd05b5240  0xd05b59a1  Yes (*)     /usr/lib/libcrypt.a(shr.o)
                        No          /usr/lib/libpthread.a(shr_xpg5.o)
                        No          /usr/lib/libc.a(shr.o)
(*): Shared library is missing debugging information.
(gdb)

From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Date: Thursday, 27 April 2023 at 5:50 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:

>Once the thread completes execution and goes to pd_disable () in
>aix-thread.c , since the process exits, the inferior_ptid.pid () is 0.
>Hence in pd_disable () when GDB goes to fetch the aix thread data for
>process 0, this assertion failure occurs.

I'm not sure this is quite right.  Can this now cause leaks e.g.
because we did not properly call pthdb_session_destroy?

Also, I'm starting to wonder if using inferior_ptid is right here
in the first place.  For example, aix_thread_target::detach
actually gets an "inferior *" argument, which it then completely
ignores and instead tries to re-create an inferior from
inferior_ptid - this doesn't look right.

I guess pd_enable and pd_disable should be getting inferior *
arguments and use those, whenever available.

Bye,
Ulrich

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

From 088891db606db4d93ff3354ae5c096663c359fdd Mon Sep 17 00:00:00 2001
From: Aditya Kamath <Aditya.Kamath@ibm.com>
Date: Tue, 2 May 2023 06:20:56 -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 | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index c587027fb6d..4a61d252afc 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_thread_data_helper_for_pid (inf->pid);;
 
   /* 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,27 @@ 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);
+  if (inf == NULL)
+    return;
+  data = get_thread_data_helper_for_pid (inf->pid);
 
   if (!data->pd_able)
     return;
@@ -1129,7 +1131,7 @@ static void
 new_objfile (struct objfile *objfile)
 {
   if (objfile)
-    pd_enable ();
+    pd_enable (NULL);
 }
 
 /* Attach to process specified by ARGS.  */
@@ -1137,7 +1139,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 +1149,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 +2068,7 @@ aix_thread_target::mourn_inferior ()
 {
   target_ops *beneath = this->beneath ();
 
-  pd_disable ();
+  pd_disable (NULL);
   beneath->mourn_inferior ();
 }
 
-- 
2.38.3


  reply	other threads:[~2023-05-02 11:50 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-27  9:45 Aditya Kamath1
2023-04-27 12:20 ` Ulrich Weigand
2023-05-02 11:49   ` Aditya Kamath1 [this message]
2023-05-02 12:53     ` Ulrich Weigand
2023-05-02 13:55       ` Aditya Kamath1
2023-05-02 14:06         ` Ulrich Weigand
2023-05-02 14:40           ` Aditya Kamath1
2023-05-02 14:52             ` Ulrich Weigand
2023-05-02 15:22               ` Aditya Kamath1
2023-05-02 15:33                 ` Ulrich Weigand

Reply instructions:

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

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

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

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

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

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

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