public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
To: "simark@simark.ca" <simark@simark.ca>,
	Aditya Kamath1 <Aditya.Kamath1@ibm.com>,
	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: Re: [PATCH] 0001-Fix-multi-thread-debug-bug-in-AIX.patch
Date: Thu, 16 Feb 2023 19:46:16 +0000	[thread overview]
Message-ID: <c0c44fae47994492ebb08b3055b9f9d79d241c38.camel@de.ibm.com> (raw)
In-Reply-To: <CH2PR15MB35444D2E4EC3340265427F47D6A29@CH2PR15MB3544.namprd15.prod.outlook.com>

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

>So what is happening is that the when after a new process is born,
>its pthread library is getting intialised and we have changed its
>ptid from ptid (pid, 0, 0) to ptid (pid, 0, tid). Since we follow
>fork the code in inferior.c file will switch to the thread child
>where the child is reported as ptid (pid, 0, 0) but exists as
>ptid (pid, 0, tid). This leads to this crash. We did try with two
>variables if you recall in the previous patch. But your point of
>pd_active being there for it was valid. So somehow something isn't
>correct that I did not understand. We have pd_activate () in only
>two places. So is follow_fork () is expecting us to switch to child
>process and then change the ptid of the child?? If yes, how do we go??
>And if not where are we going wrong here. 

Not sure if I follow you exactly here, but my understanding is
indeed that follow_fork should initially create an inferior for
the new process with just a single main thread using (pid, 0, 0).
Subsequently, aix-thread should detect that the new inferior
uses pthreads and then switch its ptid to (pid, 0, tid).

Not sure where exactly this goes wrong for you.  What is the
path leading to this crash you're observing?


>Also this ptid_t (pid, 0, 0) and our main thread being
>ptid_t (pid, 0, tid) might need a smarted way to switch to the
>main thread's process space and set the right current inferior
>process in pdc_read_memory. Kindly check it in this patch and
>let me know if we can do it better. 

So you currently do:

+  thread_info *thread = find_thread_ptid (current_inferior (),
+                                         ptid_t (user_current_pid));
+  /* If the pthread debug library is loaded, then we need the ptid_t (pid, 0 ,tid).
+     Since the main thread in the below for loop will be in the first iteration
+     we will break.  */
+  if (!thread)
+  {
+    for (thread_info *tp: all_threads (current_inferior ()->process_target (),
+                                       ptid_t (user_current_pid)))
+      {
+        thread = tp;
+        break;
+      }
+  }
[...]
+  {
+    scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
+    inferior_ptid = ptid_t (user_current_pid);
+    scoped_restore_current_inferior restore_inferior;
+    set_current_inferior (thread->inf);
+
+    scoped_restore_current_program_space restore_current_progspace;
+    set_current_program_space (thread->inf->pspace);
+    status = target_write_memory (addr, (gdb_byte *) buf, len);
+  }

This is overkill.  Note that at no point do you actually ever
use "thread" itself, only "thread->inf".  But you can determine
the inferior associated with a PID directly via find_inferior_pid
without ever involving any thread.

Bye,
Ulrich


  reply	other threads:[~2023-02-16 19:46 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-25  6:47 Aditya Kamath1
2022-10-28  9:49 ` Ulrich Weigand
2022-11-08 12:00   ` Aditya Kamath1
2022-11-08 12:17     ` Ulrich Weigand
2022-11-13 18:15       ` Aditya Kamath1
2022-11-15 18:16         ` Ulrich Weigand
2022-11-21  8:27           ` Aditya Kamath1
2022-11-23 14:15             ` Ulrich Weigand
2022-11-23 16:03               ` Aditya Kamath1
2022-11-23 17:09                 ` Ulrich Weigand
2022-11-23 18:45                   ` Aditya Kamath1
2022-11-29  8:18                     ` Aditya Kamath1
2022-11-30 14:57                       ` Ulrich Weigand
2022-12-02  7:50                         ` Aditya Kamath1
2022-12-05 18:33                           ` Ulrich Weigand
2022-12-08 10:28                             ` Aditya Kamath1
2022-12-08 10:46                               ` Aditya Kamath1
2022-12-08 16:29                               ` Ulrich Weigand
2022-12-15 12:58                                 ` Aditya Kamath1
2022-12-15 15:53                                   ` Ulrich Weigand
2022-12-19  6:30                                     ` Aditya Kamath1
2022-12-22 12:50                                       ` Ulrich Weigand
2022-12-26 13:18                                         ` Aditya Kamath1
2023-01-09 14:04                                           ` Ulrich Weigand
2023-01-10 12:23                                             ` Aditya Kamath1
2023-01-11 13:31                                               ` Ulrich Weigand
2023-01-13 14:06                                                 ` Aditya Kamath1
2023-01-20 14:44                                                   ` Ulrich Weigand
2023-01-27 14:40                                                     ` Aditya Kamath1
2023-01-30 19:54                                                       ` Tom Tromey
2023-02-02  6:24                                                       ` Aditya Kamath1
2023-02-02  6:35                                                         ` Aditya Kamath1
2023-02-02 17:43                                                           ` Ulrich Weigand
2023-02-03 11:10                                                             ` Aditya Kamath1
2023-02-06 19:07                                                               ` Ulrich Weigand
2023-02-07 11:57                                                                 ` Aditya Kamath1
2023-02-08 18:44                                                                   ` Ulrich Weigand
2023-02-10 16:33                                                                     ` Aditya Kamath1
2023-02-10 16:46                                                                       ` Aditya Kamath1
2023-02-13 19:01                                                                       ` Ulrich Weigand
2023-02-14 14:13                                                                         ` Aditya Kamath1
2023-02-16 19:46                                                                           ` Ulrich Weigand [this message]
2023-02-17 11:26                                                                             ` Aditya Kamath1
2023-02-17 12:04                                                                               ` Ulrich Weigand
2023-02-17 13:22                                                                                 ` Aditya Kamath1
2023-02-17 14:18                                                                                   ` Ulrich Weigand
2023-02-17 15:15                                                                                     ` Aditya Kamath1
2023-02-17 19:14                                                                                       ` Ulrich Weigand
2022-11-08 12:00 Aditya Kamath1

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=c0c44fae47994492ebb08b3055b9f9d79d241c38.camel@de.ibm.com \
    --to=ulrich.weigand@de.ibm.com \
    --cc=Aditya.Kamath1@ibm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=sangamesh.swamy@in.ibm.com \
    --cc=simark@simark.ca \
    /path/to/YOUR_REPLY

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

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