Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH 1/3] gdb: split inferior and thread setup when opening a core file
Date: Mon,  5 Jun 2023 10:11:07 +0100	[thread overview]
Message-ID: <a99db75d461e1d414b49f7c781096658bfb73680.1685956034.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1685956034.git.aburgess@redhat.com>

I noticed that in corelow.c, when a core file is opened, both the
thread and inferior setup is done in add_to_thread_list.  In this
patch I propose hoisting the inferior setup out of add_to_thread_list
into core_target_open.

The only thing about this change that gave me cause for concern is
that in add_to_thread_list, we only setup the inferior after finding
the first section with a name like ".reg/NN".  If we find no such
section then the inferior will never be setup.

Is this important?

Well, I don't think so.  Back in core_target_open, if there is no
current thread (which there will not be if no ".reg/NN" section was
found), then we look for a thread in the current inferior.  If there
are no threads (which there will not be if no ".reg/NN" is found),
then we once again setup the current inferior.

What I think this means, is that, in all cases, the current inferior
will end up being setup.  By moving the inferior setup code earlier in
core_target_open and making it non-conditional, we can remove the
later code that sets up the inferior, we now know this will always
have been done.

There should be no user visible changes after this commit.
---
 gdb/corelow.c | 62 ++++++++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 33 deletions(-)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index db489b4280e..7312d40374f 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -351,40 +351,24 @@ core_target::close ()
 /* Look for sections whose names start with `.reg/' so that we can
    extract the list of threads in a core file.  */
 
+/* If ASECT is a section whose name begins with '.reg/' then extract the
+   lwpid after the '/' and create a new thread in INF.
+
+   If REG_SECT is not nullptr, and the both ASECT and REG_SECT point at the
+   same position in the parent bfd object then switch to the newly created
+   thread, otherwise, the selected thread is left unchanged.  */
+
 static void
-add_to_thread_list (asection *asect, asection *reg_sect)
+add_to_thread_list (asection *asect, asection *reg_sect, inferior *inf)
 {
-  int core_tid;
-  int pid, lwpid;
-  bool fake_pid_p = false;
-  struct inferior *inf;
-
   if (!startswith (bfd_section_name (asect), ".reg/"))
     return;
 
-  core_tid = atoi (bfd_section_name (asect) + 5);
-
-  pid = bfd_core_file_pid (core_bfd);
-  if (pid == 0)
-    {
-      fake_pid_p = true;
-      pid = CORELOW_PID;
-    }
-
-  lwpid = core_tid;
-
-  inf = current_inferior ();
-  if (inf->pid == 0)
-    {
-      inferior_appeared (inf, pid);
-      inf->fake_pid_p = fake_pid_p;
-    }
-
-  ptid_t ptid (pid, lwpid);
-
+  int lwpid = atoi (bfd_section_name (asect) + 5);
+  ptid_t ptid (inf->pid, lwpid);
   thread_info *thr = add_thread (inf->process_target (), ptid);
 
-/* Warning, Will Robinson, looking at BFD private data! */
+  /* Warning, Will Robinson, looking at BFD private data! */
 
   if (reg_sect != NULL
       && asect->filepos == reg_sect->filepos)	/* Did we find .reg?  */
@@ -541,12 +525,27 @@ core_target_open (const char *arg, int from_tty)
      previous session, and the frame cache being stale.  */
   registers_changed ();
 
+  /* Find (or fake) the pid for the process in this core file, and
+     initialise the current inferior with that pid.  */
+  bool fake_pid_p = false;
+  int pid = bfd_core_file_pid (core_bfd);
+  if (pid == 0)
+    {
+      fake_pid_p = true;
+      pid = CORELOW_PID;
+    }
+
+  inferior *inf = current_inferior ();
+  gdb_assert (inf->pid == 0);
+  inferior_appeared (inf, pid);
+  inf->fake_pid_p = fake_pid_p;
+
   /* Build up thread list from BFD sections, and possibly set the
      current thread to the .reg/NN section matching the .reg
      section.  */
   asection *reg_sect = bfd_get_section_by_name (core_bfd, ".reg");
   for (asection *sect : gdb_bfd_sections (core_bfd))
-    add_to_thread_list (sect, reg_sect);
+    add_to_thread_list (sect, reg_sect, inf);
 
   if (inferior_ptid == null_ptid)
     {
@@ -556,13 +555,10 @@ core_target_open (const char *arg, int from_tty)
 	 which was the "main" thread.  The latter case shouldn't
 	 usually happen, but we're dealing with input here, which can
 	 always be broken in different ways.  */
-      thread_info *thread = first_thread_of_inferior (current_inferior ());
+      thread_info *thread = first_thread_of_inferior (inf);
 
       if (thread == NULL)
-	{
-	  inferior_appeared (current_inferior (), CORELOW_PID);
-	  thread = add_thread_silent (target, ptid_t (CORELOW_PID));
-	}
+	thread = add_thread_silent (target, ptid_t (CORELOW_PID));
 
       switch_to_thread (thread);
     }
-- 
2.25.4


  reply	other threads:[~2023-06-05  9:12 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-05  9:11 [PATCH 0/3] Improve vmcore loading Andrew Burgess via Gdb-patches
2023-06-05  9:11 ` Andrew Burgess via Gdb-patches [this message]
2023-06-10  0:04   ` [PATCH 1/3] gdb: split inferior and thread setup when opening a core file Kevin Buettner via Gdb-patches
2023-06-05  9:11 ` [PATCH 2/3] gdb/testsuite: add test for core file with a 0 pid Andrew Burgess via Gdb-patches
2023-06-10  0:16   ` Kevin Buettner via Gdb-patches
2023-07-06 14:49   ` Pedro Alves
2023-07-07  9:56     ` Andrew Burgess via Gdb-patches
2023-07-10 11:10   ` Andrew Burgess via Gdb-patches
2023-06-05  9:11 ` [PATCH 3/3] gdb: handle core files with .reg/0 section names Andrew Burgess via Gdb-patches
2023-06-10  0:36   ` Kevin Buettner via Gdb-patches
2023-07-03 17:03 ` [PATCH 0/3] Improve vmcore loading Andrew Burgess via Gdb-patches

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=a99db75d461e1d414b49f7c781096658bfb73680.1685956034.git.aburgess@redhat.com \
    --to=gdb-patches@sourceware.org \
    --cc=aburgess@redhat.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