public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@polymtl.ca>
To: gdb-patches@sourceware.org
Subject: [PATCH v2 3/9] gdb: replace inferior::waiting_for_vfork_done with inferior::thread_waiting_for_vfork_done
Date: Mon, 17 Jan 2022 23:09:31 -0500	[thread overview]
Message-ID: <20220118040937.730282-4-simon.marchi@polymtl.ca> (raw)
In-Reply-To: <20220118040937.730282-1-simon.marchi@polymtl.ca>

The inferior::waiting_for_vfork_done flag indicates that some thread in
that inferior is waiting for a vfork-done event.  Subsequent patches
will need to know which thread is waiting for that event.

I think there is a latent buglet in that waiting_for_vfork_done is
currently not reset on inferior exec or exit.  I could imagine that if a
thread in the parent process calls exec or exit while another thread of
the parent process is waiting for its vfork child to exec or exit, we
could end up with inferior::waiting_for_vfork_done without a thread
actually waiting for a vfork-done event anymore.  And since that flag is
checked in resume_1, things could misbehave there.

Since the new field points to a thread_info object, and those are
destroyed on exec or exit, it could be worse now since we could try to
access freed memory, if thread_waiting_for_vfork_done were to point to a
stale thread_info.  To avoid this, clear the field in
infrun_inferior_exit and infrun_inferior_execd.

Change-Id: I31b847278613a49ba03fc4915f74d9ceb228fdce
---
 gdb/inferior.h |  8 ++++----
 gdb/infrun.c   | 14 +++++++++-----
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/gdb/inferior.h b/gdb/inferior.h
index ec0fb6e8b16c..7b82703f1470 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -531,10 +531,10 @@ class inferior : public refcounted_object,
      exits or execs.  */
   bool pending_detach = false;
 
-  /* True if this inferior is a vfork parent waiting for a vfork child
-     not under our control to be done with the shared memory region,
-     either by exiting or execing.  */
-  bool waiting_for_vfork_done = false;
+  /* If non-nullptr, points to a thread that called vfork and is now waiting
+     for a vfork child not under our control to be done with the shared memory
+     region, either by exiting or execing.  */
+  thread_info *thread_waiting_for_vfork_done = nullptr;
 
   /* True if we're in the process of detaching from this inferior.  */
   bool detaching = false;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 46e14d720cce..40b4bdd73130 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -507,7 +507,8 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 	     insert breakpoints, so that we can debug it.  A
 	     subsequent child exec or exit is enough to know when does
 	     the child stops using the parent's address space.  */
-	  parent_inf->waiting_for_vfork_done = detach_fork;
+	  parent_inf->thread_waiting_for_vfork_done
+	    = detach_fork ? inferior_thread () : nullptr;
 	  parent_inf->pspace->breakpoints_not_allowed = detach_fork;
 	}
     }
@@ -639,7 +640,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 	  child_inf->pending_detach = 0;
 	  parent_inf->vfork_child = child_inf;
 	  parent_inf->pending_detach = detach_fork;
-	  parent_inf->waiting_for_vfork_done = 0;
+	  parent_inf->thread_waiting_for_vfork_done = nullptr;
 	}
       else if (detach_fork)
 	{
@@ -1484,6 +1485,7 @@ static void
 infrun_inferior_exit (struct inferior *inf)
 {
   inf->displaced_step_state.reset ();
+  inf->thread_waiting_for_vfork_done = nullptr;
 }
 
 static void
@@ -1502,6 +1504,8 @@ infrun_inferior_execd (inferior *inf)
      one in progress at the time of the exec, it must have been the exec'ing
      thread.  */
   clear_step_over_info ();
+
+  inf->thread_waiting_for_vfork_done = nullptr;
 }
 
 /* If ON, and the architecture supports it, GDB will use displaced
@@ -2254,7 +2258,7 @@ resume_1 (enum gdb_signal sig)
   /* Depends on stepped_breakpoint.  */
   step = currently_stepping (tp);
 
-  if (current_inferior ()->waiting_for_vfork_done)
+  if (current_inferior ()->thread_waiting_for_vfork_done != nullptr)
     {
       /* Don't try to single-step a vfork parent that is waiting for
 	 the child to get out of the shared memory region (by exec'ing
@@ -2374,7 +2378,7 @@ resume_1 (enum gdb_signal sig)
       && use_displaced_stepping (tp)
       && !step_over_info_valid_p ()
       && sig == GDB_SIGNAL_0
-      && !current_inferior ()->waiting_for_vfork_done)
+      && current_inferior ()->thread_waiting_for_vfork_done == nullptr)
     {
       displaced_step_prepare_status prepare_status
 	= displaced_step_prepare (tp);
@@ -5618,7 +5622,7 @@ handle_inferior_event (struct execution_control_state *ecs)
 
       context_switch (ecs);
 
-      current_inferior ()->waiting_for_vfork_done = 0;
+      current_inferior ()->thread_waiting_for_vfork_done = nullptr;
       current_inferior ()->pspace->breakpoints_not_allowed = 0;
 
       if (handle_stop_requested (ecs))
-- 
2.34.1


  parent reply	other threads:[~2022-01-18  4:09 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-18  4:09 [PATCH v2 0/9] Some fixes for handling vfork by multi-threaded programs Simon Marchi
2022-01-18  4:09 ` [PATCH v2 1/9] gdb/infrun: add reason parameter to stop_all_threads Simon Marchi
2022-01-18  4:09 ` [PATCH v2 2/9] gdb/linux-nat: remove check based on current_inferior in linux_handle_extended_wait Simon Marchi
2022-01-18  4:09 ` Simon Marchi [this message]
2022-01-18  4:09 ` [PATCH v2 4/9] gdb/infrun: add inferior parameters to stop_all_threads and restart_threads Simon Marchi
2022-01-18  4:09 ` [PATCH v2 5/9] gdb/infrun: add logging statement to do_target_resume Simon Marchi
2022-01-18  4:09 ` [PATCH v2 6/9] gdb: fix handling of vfork by multi-threaded program (follow-fork-mode=parent, detach-on-fork=on) Simon Marchi
2022-02-16  0:28   ` Lancelot SIX
2022-02-16 13:35     ` Simon Marchi
2022-01-18  4:09 ` [PATCH v2 7/9] gdbserver: report correct status in thread stop race condition Simon Marchi
2022-01-18  4:09 ` [PATCH v2 8/9] gdb/remote: remove_new_fork_children don't access target_waitstatus::child_ptid if kind == TARGET_WAITKIND_THREAD_EXITED Simon Marchi
2022-03-31 19:25   ` Pedro Alves
2022-01-18  4:09 ` [PATCH v2 9/9] gdb: resume ongoing step after handling fork or vfork Simon Marchi
2022-03-31 19:28   ` Pedro Alves
2022-03-23 13:02 ` [PATCH v2 0/9] Some fixes for handling vfork by multi-threaded programs Simon Marchi
2022-04-05  2:13 ` Simon Marchi

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=20220118040937.730282-4-simon.marchi@polymtl.ca \
    --to=simon.marchi@polymtl.ca \
    --cc=gdb-patches@sourceware.org \
    /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).