[PATCH 1/3] gdb: remove unpush_target free function

[PATCH 1/3] gdb: remove unpush_target free function

[Simon Marchi]

unpush_target unpushes the passed-in target from the current inferior's
target stack.  Calling it is therefore an implicit dependency on the
current global inferior.  Remove that function and make the callers use
the inferior::unpush_target method directly.  This sometimes allows
using the inferior from the context rather than the global current
inferior.

target_unpusher::operator() now needs to be implemented in target.c,
otherwise target.h and inferior.h both need to include each other, and
that wouldn't work.

gdb/ChangeLog:

	* target.h (unpush_target): Remove, update all callers
	to use `inferior::unpush_target` instead.
	(struct target_unpusher) <operator()>: Just declare.
	* target.c (unpush_target): Remove.
	(target_unpusher::operator()): New.

Change-Id: Ia5172dfb3f373e0a75b991885b50322ca2142a8c
---
 gdb/aix-thread.c       |  2 +-
 gdb/bsd-kvm.c          |  2 +-
 gdb/bsd-uthread.c      |  2 +-
 gdb/corelow.c          |  2 +-
 gdb/exec.c             |  2 +-
 gdb/inf-child.c        |  2 +-
 gdb/linux-thread-db.c  |  6 +++---
 gdb/ravenscar-thread.c |  2 +-
 gdb/record-btrace.c    |  2 +-
 gdb/record-full.c      |  2 +-
 gdb/record.c           |  2 +-
 gdb/remote-sim.c       |  4 ++--
 gdb/sol-thread.c       |  4 ++--
 gdb/target.c           | 18 ++++++++----------
 gdb/target.h           |  7 +------
 gdb/tracefile-tfile.c  |  4 ++--
 16 files changed, 28 insertions(+), 35 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index f4b6c1b06ab6..a479d0150bc2 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -993,7 +993,7 @@ pd_disable (void)
   if (pd_active)
     pd_deactivate ();
   pd_able = 0;
-  unpush_target (&aix_thread_ops);
+  current_inferior ()->unpush_target (&aix_thread_ops);
 }
 
 /* new_objfile observer callback.
diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c
index cd25e19a544e..17db2fe1cd60 100644
--- a/gdb/bsd-kvm.c
+++ b/gdb/bsd-kvm.c
@@ -132,7 +132,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
     error (("%s"), errbuf);
 
   bsd_kvm_corefile = filename;
-  unpush_target (&bsd_kvm_ops);
+  current_inferior ()->unpush_target (&bsd_kvm_ops);
   core_kd = temp_kd;
   push_target (&bsd_kvm_ops);
 
diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
index d7dd0a180142..2ee47bfb5c47 100644
--- a/gdb/bsd-uthread.c
+++ b/gdb/bsd-uthread.c
@@ -259,7 +259,7 @@ bsd_uthread_deactivate (void)
   if (!bsd_uthread_active)
     return;
 
-  unpush_target (&bsd_uthread_ops);
+  current_inferior ()->unpush_target (&bsd_uthread_ops);
 }
 
 static void
diff --git a/gdb/corelow.c b/gdb/corelow.c
index a2d2d20afc62..a4c1f6354c6e 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -580,7 +580,7 @@ core_target::detach (inferior *inf, int from_tty)
   /* Note that 'this' is dangling after this call.  unpush_target
      closes the target, and our close implementation deletes
      'this'.  */
-  unpush_target (this);
+  inf->unpush_target (this);
 
   /* Clear the register cache and the frame cache.  */
   registers_changed ();
diff --git a/gdb/exec.c b/gdb/exec.c
index 544a05873f11..bcc54bd966fe 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -671,7 +671,7 @@ program_space::remove_target_sections (void *owner)
 	    continue;
 
 	  switch_to_inferior_no_thread (inf);
-	  unpush_target (&exec_ops);
+	  inf->unpush_target (&exec_ops);
 	}
     }
 }
diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 192cfc336913..b8bc2e2598e6 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -207,7 +207,7 @@ void
 inf_child_target::maybe_unpush_target ()
 {
   if (!inf_child_explicitly_opened)
-    unpush_target (this);
+    current_inferior ()->unpush_target (this);
 }
 
 void
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 4dab64ac3448..3a3d3def6074 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1364,7 +1364,7 @@ thread_db_target::detach (inferior *inf, int from_tty)
   /* NOTE: From this point on, inferior_ptid is null_ptid.  */
 
   /* Detach the thread_db target from this inferior.  */
-  unpush_target (this);
+  inf->unpush_target (this);
 }
 
 ptid_t
@@ -1398,7 +1398,7 @@ thread_db_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
       /* New image, it may or may not end up using thread_db.  Assume
 	 not unless we find otherwise.  */
       delete_thread_db_info (beneath, ptid.pid ());
-      unpush_target (this);
+      current_inferior ()->unpush_target (this);
 
       return ptid;
     }
@@ -1420,7 +1420,7 @@ thread_db_target::mourn_inferior ()
   target_beneath->mourn_inferior ();
 
   /* Detach the thread_db target from this inferior.  */
-  unpush_target (this);
+  current_inferior ()->unpush_target (this);
 }
 
 struct callback_data
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 2cc7bbc0ca36..63aa1d1db843 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -620,7 +620,7 @@ ravenscar_thread_target::mourn_inferior ()
 {
   m_base_ptid = null_ptid;
   target_ops *beneath = this->beneath ();
-  unpush_target (this);
+  current_inferior ()->unpush_target (this);
   beneath->mourn_inferior ();
 }
 
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index d9cc7a3b6d89..1a0cac2f710e 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -428,7 +428,7 @@ record_btrace_target::disconnect (const char *args,
   struct target_ops *beneath = this->beneath ();
 
   /* Do not stop recording, just clean up GDB side.  */
-  unpush_target (this);
+  current_inferior ()->unpush_target (this);
 
   /* Forward disconnect.  */
   beneath->disconnect (args, from_tty);
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 2373741470c4..59e4410c0085 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -2078,7 +2078,7 @@ record_full_core_target::kill ()
   if (record_debug)
     fprintf_unfiltered (gdb_stdlog, "Process record: record_full_core_kill\n");
 
-  unpush_target (this);
+  current_inferior ()->unpush_target (this);
 }
 
 /* "fetch_registers" method for prec over corefile.  */
diff --git a/gdb/record.c b/gdb/record.c
index cd541b56f438..483b906c2d06 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -166,7 +166,7 @@ record_unpush (struct target_ops *t)
 {
   DEBUG ("unpush %s", t->shortname ());
 
-  unpush_target (t);
+  current_inferior ()->unpush_target (t);
 }
 
 /* See record.h.  */
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index d51130562fb1..f72bbd2015e4 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -699,7 +699,7 @@ gdbsim_target_open (const char *args, int from_tty)
      operation until after we complete those operations which could
      error out.  */
   if (gdbsim_is_open)
-    unpush_target (&gdbsim_ops);
+    current_inferior ()->unpush_target (&gdbsim_ops);
 
   len = (7 + 1			/* gdbsim */
 	 + strlen (" -E little")
@@ -834,7 +834,7 @@ gdbsim_target::detach (inferior *inf, int from_tty)
   if (remote_debug)
     fprintf_unfiltered (gdb_stdlog, "gdbsim_detach\n");
 
-  unpush_target (this);		/* calls gdbsim_close to do the real work */
+  inf->unpush_target (this);		/* calls gdbsim_close to do the real work */
   if (from_tty)
     printf_filtered ("Ending simulator %s debugging\n", target_shortname);
 }
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index 7b46a5747877..1458185f414f 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -387,7 +387,7 @@ sol_thread_target::detach (inferior *inf, int from_tty)
 
   sol_thread_active = 0;
   inferior_ptid = ptid_t (main_ph.ptid.pid ());
-  unpush_target (this);
+  inf->unpush_target (this);
   beneath->detach (inf, from_tty);
 }
 
@@ -681,7 +681,7 @@ sol_thread_target::mourn_inferior ()
 
   sol_thread_active = 0;
 
-  unpush_target (this);
+  current_inferior ()->unpush_target (this);
 
   beneath->mourn_inferior ();
 }
diff --git a/gdb/target.c b/gdb/target.c
index 0889da82ea59..236aded0a2e1 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -160,7 +160,7 @@ set_targetdebug  (const char *args, int from_tty, struct cmd_list_element *c)
   if (targetdebug)
     push_target (the_debug_target);
   else
-    unpush_target (the_debug_target);
+    current_inferior ()->unpush_target (the_debug_target);
 }
 
 static void
@@ -589,14 +589,6 @@ push_target (target_ops_up &&t)
 
 /* See target.h.  */
 
-int
-unpush_target (struct target_ops *t)
-{
-  return current_inferior ()->unpush_target (t);
-}
-
-/* See target.h.  */
-
 bool
 target_stack::unpush (target_ops *t)
 {
@@ -640,7 +632,7 @@ target_stack::unpush (target_ops *t)
 static void
 unpush_target_and_assert (struct target_ops *target)
 {
-  if (!unpush_target (target))
+  if (!current_inferior ()->unpush_target (target))
     {
       fprintf_unfiltered (gdb_stderr,
 			  "pop_all_targets couldn't find target %s\n",
@@ -681,6 +673,12 @@ target_is_pushed (target_ops *t)
   return current_inferior ()->target_is_pushed (t);
 }
 
+void
+target_unpusher::operator() (struct target_ops *ops) const
+{
+  current_inferior ()->unpush_target (ops);
+}
+
 /* Default implementation of to_get_thread_local_address.  */
 
 static void
diff --git a/gdb/target.h b/gdb/target.h
index ee93c5cf3959..3a64094ae5b2 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -2390,16 +2390,11 @@ extern void push_target (struct target_ops *);
 /* An overload that deletes the target on failure.  */
 extern void push_target (target_ops_up &&);
 
-extern int unpush_target (struct target_ops *);
-
 /* A unique_ptr helper to unpush a target.  */
 
 struct target_unpusher
 {
-  void operator() (struct target_ops *ops) const
-  {
-    unpush_target (ops);
-  }
+  void operator() (struct target_ops *ops) const;
 };
 
 /* A unique_ptr that unpushes a target on destruction.  */
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index ca6324c03920..ea703643d8f1 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -481,7 +481,7 @@ tfile_target_open (const char *arg, int from_tty)
 
   /* Looks semi-reasonable.  Toss the old trace file and work on the new.  */
 
-  unpush_target (&tfile_ops);
+  current_inferior ()->unpush_target (&tfile_ops);
 
   trace_filename = filename.release ();
   trace_fd = scratch_chan;
@@ -551,7 +551,7 @@ tfile_target_open (const char *arg, int from_tty)
   catch (const gdb_exception &ex)
     {
       /* Remove the partially set up target.  */
-      unpush_target (&tfile_ops);
+      current_inferior ()->unpush_target (&tfile_ops);
       throw;
     }
 
-- 
2.30.1

[PATCH 2/3] gdb: remove push_target free functions

[Simon Marchi]

Same as the previous patch, but for the push_target functions.

The implementation of the move variant is moved to a new overload of
inferior::push_target.

gdb/ChangeLog:

	* target.h (push_target): Remove, update callers to use
	inferior::push_target.
	* target.c (push_target): Remove.
	* inferior.h (class inferior) <push_target>: New overload.

Change-Id: I5a95496666278b8f3965e5e8aecb76f54a97c185
---
 gdb/aix-thread.c          |  2 +-
 gdb/bsd-kvm.c             |  2 +-
 gdb/bsd-uthread.c         |  2 +-
 gdb/corelow.c             |  2 +-
 gdb/darwin-nat.c          |  2 +-
 gdb/exec.c                |  4 ++--
 gdb/gnu-nat.c             |  6 +++---
 gdb/go32-nat.c            |  2 +-
 gdb/inf-child.c           |  2 +-
 gdb/inf-ptrace.c          |  7 ++++---
 gdb/inferior.c            |  2 +-
 gdb/inferior.h            |  7 +++++++
 gdb/infrun.c              |  6 +++---
 gdb/linux-thread-db.c     |  2 +-
 gdb/nto-procfs.c          |  4 ++--
 gdb/procfs.c              |  4 ++--
 gdb/ravenscar-thread.c    |  2 +-
 gdb/record-btrace.c       |  2 +-
 gdb/record-full.c         |  4 ++--
 gdb/regcache.c            |  2 +-
 gdb/remote-sim.c          |  2 +-
 gdb/remote.c              |  4 ++--
 gdb/scoped-mock-context.h |  2 +-
 gdb/sol-thread.c          |  2 +-
 gdb/target.c              | 19 +------------------
 gdb/target.h              |  5 -----
 gdb/tracectf.c            |  2 +-
 gdb/tracefile-tfile.c     |  2 +-
 gdb/windows-nat.c         |  4 ++--
 29 files changed, 47 insertions(+), 61 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index a479d0150bc2..fc34210cf9e2 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -974,7 +974,7 @@ pd_enable (void)
     return;
 
   /* Prepare for thread debugging.  */
-  push_target (&aix_thread_ops);
+  current_inferior ()->push_target (&aix_thread_ops);
   pd_able = 1;
 
   /* If we're debugging a core file or an attached inferior, the
diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c
index 17db2fe1cd60..89da40afcb78 100644
--- a/gdb/bsd-kvm.c
+++ b/gdb/bsd-kvm.c
@@ -134,7 +134,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
   bsd_kvm_corefile = filename;
   current_inferior ()->unpush_target (&bsd_kvm_ops);
   core_kd = temp_kd;
-  push_target (&bsd_kvm_ops);
+  current_inferior ()->push_target (&bsd_kvm_ops);
 
   thread_info *thr = add_thread_silent (&bsd_kvm_ops, bsd_kvm_ptid);
   switch_to_thread (thr);
diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
index 2ee47bfb5c47..3cb8f64c89a7 100644
--- a/gdb/bsd-uthread.c
+++ b/gdb/bsd-uthread.c
@@ -231,7 +231,7 @@ bsd_uthread_activate (struct objfile *objfile)
   bsd_uthread_thread_ctx_offset =
     bsd_uthread_lookup_offset ("_thread_ctx_offset", objfile);
 
-  push_target (&bsd_uthread_ops);
+  current_inferior ()->push_target (&bsd_uthread_ops);
   bsd_uthread_active = 1;
   return 1;
 }
diff --git a/gdb/corelow.c b/gdb/corelow.c
index a4c1f6354c6e..a1943ab2ea6d 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -458,7 +458,7 @@ core_target_open (const char *arg, int from_tty)
   if (!current_program_space->exec_bfd ())
     set_gdbarch_from_file (core_bfd);
 
-  push_target (std::move (target_holder));
+  current_inferior ()->push_target (std::move (target_holder));
 
   switch_to_no_thread ();
 
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index ca95d7b6d385..f8e4443ff408 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1659,7 +1659,7 @@ darwin_attach_pid (struct inferior *inf)
 
   target_ops *darwin_ops = get_native_target ();
   if (!target_is_pushed (darwin_ops))
-    push_target (darwin_ops);
+    inf->push_target (darwin_ops);
 }
 
 /* Get the thread_info object corresponding to this darwin_thread_info.  */
diff --git a/gdb/exec.c b/gdb/exec.c
index bcc54bd966fe..b737bcf6e0f3 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -616,7 +616,7 @@ program_space::add_target_sections (void *owner,
 	    continue;
 
 	  switch_to_inferior_no_thread (inf);
-	  push_target (&exec_ops);
+	  inf->push_target (&exec_ops);
 	}
     }
 }
@@ -682,7 +682,7 @@ void
 exec_on_vfork ()
 {
   if (!current_program_space->target_sections ().empty ())
-    push_target (&exec_ops);
+    current_inferior ()->push_target (&exec_ops);
 }
 
 \f
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index 409d141909b3..9ea4c4089340 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2114,7 +2114,7 @@ gnu_nat_target::create_inferior (const char *exec_file,
   inf_debug (inf, "creating inferior");
 
   if (!target_is_pushed (this))
-    push_target (this);
+    current_inferior ()->push_target (this);
 
   pid = fork_inferior (exec_file, allargs, env, gnu_ptrace_me,
 		       NULL, NULL, NULL, NULL);
@@ -2190,9 +2190,9 @@ gnu_nat_target::attach (const char *args, int from_tty)
 
   inf_attach (inf, pid);
 
-  push_target (this);
-
   inferior = current_inferior ();
+  inferior->push_target (this);
+
   inferior_appeared (inferior, pid);
   inferior->attach_flag = 1;
 
diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index b18a62e9490d..79c31fcd9877 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -757,7 +757,7 @@ go32_nat_target::create_inferior (const char *exec_file,
   inferior_appeared (inf, SOME_PID);
 
   if (!target_is_pushed (this))
-    push_target (this);
+    inf->push_target (this);
 
   thread_info *thr = add_thread_silent (ptid_t (SOME_PID));
   switch_to_thread (thr);
diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index b8bc2e2598e6..0e68a40d7c04 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -166,7 +166,7 @@ inf_child_open_target (const char *arg, int from_tty)
   gdb_assert (dynamic_cast<inf_child_target *> (target) != NULL);
 
   target_preopen (from_tty);
-  push_target (target);
+  current_inferior ()->push_target (target);
   inf_child_explicitly_opened = 1;
   if (from_tty)
     printf_filtered ("Done.  Use the \"run\" command to start a process.\n");
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 7ca02dfd8764..e630ba447f40 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -82,7 +82,7 @@ inf_ptrace_target::create_inferior (const char *exec_file,
   if (! ops_already_pushed)
     {
       /* Clear possible core file with its process_stratum.  */
-      push_target (this);
+      current_inferior ()->push_target (this);
       unpusher.reset (this);
     }
 
@@ -139,12 +139,14 @@ inf_ptrace_target::attach (const char *args, int from_tty)
   if (pid == getpid ())		/* Trying to masturbate?  */
     error (_("I refuse to debug myself!"));
 
+  inf = current_inferior ();
+
   target_unpush_up unpusher;
   if (! ops_already_pushed)
     {
       /* target_pid_to_str already uses the target.  Also clear possible core
 	 file with its process_stratum.  */
-      push_target (this);
+      inf->push_target (this);
       unpusher.reset (this);
     }
 
@@ -169,7 +171,6 @@ inf_ptrace_target::attach (const char *args, int from_tty)
   error (_("This system does not support attaching to a process"));
 #endif
 
-  inf = current_inferior ();
   inferior_appeared (inf, pid);
   inf->attach_flag = 1;
 
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 49f869a4c783..69baee34ce9d 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -771,7 +771,7 @@ switch_to_inferior_and_push_target (inferior *new_inf,
   /* Reuse the target for new inferior.  */
   if (!no_connection && proc_target != NULL)
     {
-      push_target (proc_target);
+      new_inf->push_target (proc_target);
       if (proc_target->connection_string () != NULL)
 	printf_filtered (_("Added inferior %d on connection %d (%s %s)\n"),
 			 new_inf->num,
diff --git a/gdb/inferior.h b/gdb/inferior.h
index b8d5ff94fc56..66fc180ce530 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -352,6 +352,13 @@ class inferior : public refcounted_object
   void push_target (struct target_ops *t)
   { m_target_stack.push (t); }
 
+  /* An overload that deletes the target on failure.  */
+  void push_target (target_ops_up &&t)
+  {
+    m_target_stack.push (t.get ());
+    t.release ();
+  }
+
   /* Unpush T from this inferior's target stack.  */
   int unpush_target (struct target_ops *t)
   { return m_target_stack.unpush (t); }
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3b65a6de9fe2..50340e6edad4 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -477,7 +477,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 	  set_current_inferior (child_inf);
 	  switch_to_no_thread ();
 	  child_inf->symfile_flags = SYMFILE_NO_READ;
-	  push_target (parent_inf->process_target ());
+	  child_inf->push_target (parent_inf->process_target ());
 	  thread_info *child_thr
 	    = add_thread_silent (child_inf->process_target (), child_ptid);
 
@@ -627,7 +627,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 	   informing the solib layer about this new process.  */
 
 	set_current_inferior (child_inf);
-	push_target (target);
+	child_inf->push_target (target);
       }
 
       thread_info *child_thr = add_thread_silent (target, child_ptid);
@@ -1183,7 +1183,7 @@ follow_exec (ptid_t ptid, const char *exec_file_target)
 
       inferior *org_inferior = current_inferior ();
       switch_to_inferior_no_thread (inf);
-      push_target (org_inferior->process_target ());
+      inf->push_target (org_inferior->process_target ());
       thread_info *thr = add_thread (inf->process_target (), ptid);
       switch_to_thread (thr);
     }
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 3a3d3def6074..de8687e99c73 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -944,7 +944,7 @@ try_thread_db_load_1 (struct thread_db_info *info)
 
   /* The thread library was detected.  Activate the thread_db target
      for this process.  */
-  push_target (&the_thread_db_target);
+  current_inferior ()->push_target (&the_thread_db_target);
   return true;
 }
 
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 021230af9c62..b74392fef320 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -718,7 +718,7 @@ nto_procfs_target::attach (const char *args, int from_tty)
   inf->attach_flag = 1;
 
   if (!target_is_pushed (ops))
-    push_target (ops);
+    inf->push_target (ops);
 
   update_thread_list ();
 
@@ -1319,7 +1319,7 @@ nto_procfs_target::create_inferior (const char *exec_file,
 	 errn, safe_strerror(errn) ); */
     }
   if (!target_is_pushed (ops))
-    push_target (ops);
+    inf->push_target (ops);
   target_terminal::init ();
 
   if (current_program_space->exec_bfd () != NULL
diff --git a/gdb/procfs.c b/gdb/procfs.c
index cab29c3cbbcb..b37680923cd8 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -1771,7 +1771,7 @@ procfs_target::attach (const char *args, int from_tty)
   target_unpush_up unpusher;
   if (!target_is_pushed (this))
     {
-      push_target (this);
+      current_inferior ()->push_target (this);
       unpusher.reset (this);
     }
 
@@ -2863,7 +2863,7 @@ procfs_target::create_inferior (const char *exec_file,
     }
 
   if (!target_is_pushed (this))
-    push_target (this);
+    current_inferior ()->push_target (this);
 
   pid = fork_inferior (exec_file, allargs, env, procfs_set_exec_trap,
 		       NULL, procfs_pre_trace, shell_file, NULL);
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 63aa1d1db843..1398d1b7c9e6 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -674,7 +674,7 @@ ravenscar_inferior_created (inferior *inf)
     }
 
   ravenscar_thread_target *rtarget = new ravenscar_thread_target ();
-  push_target (target_ops_up (rtarget));
+  inf->push_target (target_ops_up (rtarget));
   thread_info *thr = rtarget->add_active_thread ();
   if (thr != nullptr)
     switch_to_thread (thr);
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 1a0cac2f710e..b7b3c91f85db 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -337,7 +337,7 @@ record_btrace_push_target (void)
 
   record_btrace_auto_enable ();
 
-  push_target (&record_btrace_ops);
+  current_inferior ()->push_target (&record_btrace_ops);
 
   record_btrace_async_inferior_event_handler
     = create_async_event_handler (record_btrace_handle_async_inferior_event,
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 59e4410c0085..8310b7b4c256 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -924,7 +924,7 @@ record_full_core_open_1 (const char *name, int from_tty)
 
   record_full_core_sections = build_section_table (core_bfd);
 
-  push_target (&record_full_core_ops);
+  current_inferior ()->push_target (&record_full_core_ops);
   record_full_restore ();
 }
 
@@ -947,7 +947,7 @@ record_full_open_1 (const char *name, int from_tty)
     error (_("Process record: the current architecture doesn't support "
 	     "record function."));
 
-  push_target (&record_full_ops);
+  current_inferior ()->push_target (&record_full_ops);
 }
 
 static void record_full_init_record_breakpoints (void);
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 27c0ae5ae239..a419a21f30a0 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1925,7 +1925,7 @@ cooked_write_test (struct gdbarch *gdbarch)
 
   /* Push the process_stratum target so we can mock accessing
      registers.  */
-  push_target (&mock_target);
+  current_inferior ()->push_target (&mock_target);
 
   /* Pop it again on exit (return/exception).  */
   struct on_exit
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index f72bbd2015e4..1746b626fa11 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -763,7 +763,7 @@ gdbsim_target_open (const char *args, int from_tty)
 
   sim_data->gdbsim_desc = gdbsim_desc;
 
-  push_target (&gdbsim_ops);
+  current_inferior ()->push_target (&gdbsim_ops);
   printf_filtered ("Connected to the simulator.\n");
 
   /* There's nothing running after "target sim" or "load"; not until
diff --git a/gdb/remote.c b/gdb/remote.c
index ae15f416153f..a7f35396045b 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -2474,7 +2474,7 @@ remote_target::remote_add_inferior (bool fake_pid_p, int pid, int attached,
 	  inf = add_inferior_with_spaces ();
 	}
       switch_to_inferior_no_thread (inf);
-      push_target (this);
+      inf->push_target (this);
       inferior_appeared (inf, pid);
     }
 
@@ -5673,7 +5673,7 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p)
     }
 
   /* Switch to using the remote target now.  */
-  push_target (std::move (target_holder));
+  current_inferior ()->push_target (std::move (target_holder));
 
   /* Register extra event sources in the event loop.  */
   rs->remote_async_inferior_event_token
diff --git a/gdb/scoped-mock-context.h b/gdb/scoped-mock-context.h
index 8332e462326b..8d295ba1bbe6 100644
--- a/gdb/scoped-mock-context.h
+++ b/gdb/scoped-mock-context.h
@@ -64,7 +64,7 @@ struct scoped_mock_context
     /* Push the process_stratum target so we can mock accessing
        registers.  */
     gdb_assert (mock_target.stratum () == process_stratum);
-    push_target (&mock_target);
+    mock_inferior.push_target (&mock_target);
 
     /* Switch to the mock thread.  */
     switch_to_thread (&mock_thread);
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index 1458185f414f..825f69131fc4 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -641,7 +641,7 @@ check_for_thread_db (void)
       printf_unfiltered (_("[Thread debugging using libthread_db enabled]\n"));
 
       /* The thread library was detected.  Activate the sol_thread target.  */
-      push_target (&sol_thread_ops);
+      current_inferior ()->push_target (&sol_thread_ops);
       sol_thread_active = 1;
 
       main_ph.ptid = inferior_ptid; /* Save for xfer_memory.  */
diff --git a/gdb/target.c b/gdb/target.c
index 236aded0a2e1..fe9731221efd 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -158,7 +158,7 @@ static void
 set_targetdebug  (const char *args, int from_tty, struct cmd_list_element *c)
 {
   if (targetdebug)
-    push_target (the_debug_target);
+    current_inferior ()->push_target (the_debug_target);
   else
     current_inferior ()->unpush_target (the_debug_target);
 }
@@ -572,23 +572,6 @@ target_stack::push (target_ops *t)
 
 /* See target.h.  */
 
-void
-push_target (struct target_ops *t)
-{
-  current_inferior ()->push_target (t);
-}
-
-/* See target.h.  */
-
-void
-push_target (target_ops_up &&t)
-{
-  current_inferior ()->push_target (t.get ());
-  t.release ();
-}
-
-/* See target.h.  */
-
 bool
 target_stack::unpush (target_ops *t)
 {
diff --git a/gdb/target.h b/gdb/target.h
index 3a64094ae5b2..1830d3a8106f 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -2385,11 +2385,6 @@ extern void add_target (const target_info &info,
 extern void add_deprecated_target_alias (const target_info &info,
 					 const char *alias);
 
-extern void push_target (struct target_ops *);
-
-/* An overload that deletes the target on failure.  */
-extern void push_target (target_ops_up &&);
-
 /* A unique_ptr helper to unpush a target.  */
 
 struct target_unpusher
diff --git a/gdb/tracectf.c b/gdb/tracectf.c
index 38c327dd1df0..4a245ee11a5a 100644
--- a/gdb/tracectf.c
+++ b/gdb/tracectf.c
@@ -1165,7 +1165,7 @@ ctf_target_open (const char *dirname, int from_tty)
   gdb_assert (start_pos->type == BT_SEEK_RESTORE);
 
   trace_dirname = xstrdup (dirname);
-  push_target (&ctf_ops);
+  current_inferior ()->push_target (&ctf_ops);
 
   inferior_appeared (current_inferior (), CTF_PID);
 
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index ea703643d8f1..d82cac8d9ddd 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -498,7 +498,7 @@ tfile_target_open (const char *arg, int from_tty)
 	&& (startswith (header + 1, "TRACE0\n"))))
     error (_("File is not a valid trace file."));
 
-  push_target (&tfile_ops);
+  current_inferior ()->push_target (&tfile_ops);
 
   trace_regblock_size = 0;
   ts = current_trace_status ();
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index c8275fc54204..51662896d691 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -2027,8 +2027,9 @@ windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching)
 #endif
   current_event.dwProcessId = pid;
   memset (&current_event, 0, sizeof (current_event));
+  inf = current_inferior ();
   if (!target_is_pushed (this))
-    push_target (this);
+    inf->push_target (this);
   disable_breakpoints_in_shlibs ();
   windows_clear_solib ();
   clear_proceed_status (0);
@@ -2049,7 +2050,6 @@ windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching)
       windows_set_segment_register_p (i386_windows_segment_register_p);
     }
 
-  inf = current_inferior ();
   inferior_appeared (inf, pid);
   inf->attach_flag = attaching;
 
-- 
2.30.1

[PATCH 3/3] gdb: remove target_is_pushed free function

[Simon Marchi]

Same principle as the previous patches.

gdb/ChangeLog:

	* target.h (target_is_pushed): Remove, update callers to use
	inferior::target_is_pushed instead.
	* target.c (target_is_pushed): Remove.

Change-Id: I9862e6205acc65672da807cbe4b46cde009e7b9d
---
 gdb/darwin-nat.c  |  2 +-
 gdb/gnu-nat.c     |  5 +++--
 gdb/go32-nat.c    |  2 +-
 gdb/inf-ptrace.c  | 15 +++++++--------
 gdb/nto-procfs.c  |  4 ++--
 gdb/procfs.c      |  4 ++--
 gdb/target.c      | 11 +----------
 gdb/target.h      |  2 --
 gdb/windows-nat.c |  2 +-
 9 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index f8e4443ff408..587f53174163 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1658,7 +1658,7 @@ darwin_attach_pid (struct inferior *inf)
     }
 
   target_ops *darwin_ops = get_native_target ();
-  if (!target_is_pushed (darwin_ops))
+  if (!inf->target_is_pushed (darwin_ops))
     inf->push_target (darwin_ops);
 }
 
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index 9ea4c4089340..816f901ce23e 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2109,12 +2109,13 @@ gnu_nat_target::create_inferior (const char *exec_file,
 				 int from_tty)
 {
   struct inf *inf = cur_inf ();
+  inferior *inferior = current_inferior ();
   int pid;
 
   inf_debug (inf, "creating inferior");
 
-  if (!target_is_pushed (this))
-    current_inferior ()->push_target (this);
+  if (!inf->target_is_pushed (this))
+    inf->push_target (this);
 
   pid = fork_inferior (exec_file, allargs, env, gnu_ptrace_me,
 		       NULL, NULL, NULL, NULL);
diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index 79c31fcd9877..9899545a1e21 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -756,7 +756,7 @@ go32_nat_target::create_inferior (const char *exec_file,
   inf = current_inferior ();
   inferior_appeared (inf, SOME_PID);
 
-  if (!target_is_pushed (this))
+  if (!inf->target_is_pushed (this))
     inf->push_target (this);
 
   thread_info *thr = add_thread_silent (ptid_t (SOME_PID));
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index e630ba447f40..b6fa71fd2c0b 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -74,15 +74,17 @@ inf_ptrace_target::create_inferior (const char *exec_file,
 				    const std::string &allargs,
 				    char **env, int from_tty)
 {
+  inferior *inf = current_inferior ();
+
   /* Do not change either targets above or the same target if already present.
      The reason is the target stack is shared across multiple inferiors.  */
-  int ops_already_pushed = target_is_pushed (this);
+  int ops_already_pushed = inf->target_is_pushed (this);
 
   target_unpush_up unpusher;
   if (! ops_already_pushed)
     {
       /* Clear possible core file with its process_stratum.  */
-      current_inferior ()->push_target (this);
+      inf->push_target (this);
       unpusher.reset (this);
     }
 
@@ -127,20 +129,17 @@ inf_ptrace_target::mourn_inferior ()
 void
 inf_ptrace_target::attach (const char *args, int from_tty)
 {
-  pid_t pid;
-  struct inferior *inf;
+  inferior *inf = current_inferior ();
 
   /* Do not change either targets above or the same target if already present.
      The reason is the target stack is shared across multiple inferiors.  */
-  int ops_already_pushed = target_is_pushed (this);
+  int ops_already_pushed = inf->target_is_pushed (this);
 
-  pid = parse_pid_to_attach (args);
+  pid_t pid = parse_pid_to_attach (args);
 
   if (pid == getpid ())		/* Trying to masturbate?  */
     error (_("I refuse to debug myself!"));
 
-  inf = current_inferior ();
-
   target_unpush_up unpusher;
   if (! ops_already_pushed)
     {
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index b74392fef320..41b436cfa27f 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -717,7 +717,7 @@ nto_procfs_target::attach (const char *args, int from_tty)
   inferior_appeared (inf, pid);
   inf->attach_flag = 1;
 
-  if (!target_is_pushed (ops))
+  if (!inf->target_is_pushed (ops))
     inf->push_target (ops);
 
   update_thread_list ();
@@ -1318,7 +1318,7 @@ nto_procfs_target::create_inferior (const char *exec_file,
       /* warning( "Failed to set Kill-on-Last-Close flag: errno = %d(%s)\n",
 	 errn, safe_strerror(errn) ); */
     }
-  if (!target_is_pushed (ops))
+  if (!inf->target_is_pushed (ops))
     inf->push_target (ops);
   target_terminal::init ();
 
diff --git a/gdb/procfs.c b/gdb/procfs.c
index b37680923cd8..91d2039e3f9a 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -1769,7 +1769,7 @@ procfs_target::attach (const char *args, int from_tty)
 
   /* Push the target if needed, ensure it gets un-pushed it if attach fails.  */
   target_unpush_up unpusher;
-  if (!target_is_pushed (this))
+  if (!inf->target_is_pushed (this))
     {
       current_inferior ()->push_target (this);
       unpusher.reset (this);
@@ -2862,7 +2862,7 @@ procfs_target::create_inferior (const char *exec_file,
       shell_file = tryname;
     }
 
-  if (!target_is_pushed (this))
+  if (!inf->target_is_pushed (this))
     current_inferior ()->push_target (this);
 
   pid = fork_inferior (exec_file, allargs, env, procfs_set_exec_trap,
diff --git a/gdb/target.c b/gdb/target.c
index fe9731221efd..afc4b2cbbb60 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -647,15 +647,6 @@ pop_all_targets (void)
   pop_all_targets_above (dummy_stratum);
 }
 
-/* Return true if T is now pushed in the current inferior's target
-   stack.  Return false otherwise.  */
-
-bool
-target_is_pushed (target_ops *t)
-{
-  return current_inferior ()->target_is_pushed (t);
-}
-
 void
 target_unpusher::operator() (struct target_ops *ops) const
 {
@@ -3097,7 +3088,7 @@ debug_target::info () const
 void
 target_close (struct target_ops *targ)
 {
-  gdb_assert (!target_is_pushed (targ));
+  gdb_assert (!current_inferior ()->target_is_pushed (targ));
 
   fileio_handles_invalidate_target (targ);
 
diff --git a/gdb/target.h b/gdb/target.h
index 1830d3a8106f..6b71e741f6d7 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -2411,8 +2411,6 @@ extern void pop_all_targets_at_and_above (enum strata stratum);
    strictly above ABOVE_STRATUM.  */
 extern void pop_all_targets_above (enum strata above_stratum);
 
-extern bool target_is_pushed (target_ops *t);
-
 extern CORE_ADDR target_translate_tls_address (struct objfile *objfile,
 					       CORE_ADDR offset);
 
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 51662896d691..00f83536e330 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -2028,7 +2028,7 @@ windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching)
   current_event.dwProcessId = pid;
   memset (&current_event, 0, sizeof (current_event));
   inf = current_inferior ();
-  if (!target_is_pushed (this))
+  if (!inf->target_is_pushed (this))
     inf->push_target (this);
   disable_breakpoints_in_shlibs ();
   windows_clear_solib ();
-- 
2.30.1

RE: [PATCH 1/3] gdb: remove unpush_target free function

[Aktemur, Tankut Baris]

On Monday, March 22, 2021 4:20 AM, Simon Marchi Wrote:
> unpush_target unpushes the passed-in target from the current inferior's
> target stack.  Calling it is therefore an implicit dependency on the
> current global inferior.  Remove that function and make the callers use
> the inferior::unpush_target method directly.  This sometimes allows
> using the inferior from the context rather than the global current
> inferior.
> 
> target_unpusher::operator() now needs to be implemented in target.c,
> otherwise target.h and inferior.h both need to include each other, and
> that wouldn't work.
> 
> gdb/ChangeLog:
> 
> 	* target.h (unpush_target): Remove, update all callers
> 	to use `inferior::unpush_target` instead.
> 	(struct target_unpusher) <operator()>: Just declare.
> 	* target.c (unpush_target): Remove.
> 	(target_unpusher::operator()): New.
> 
> Change-Id: Ia5172dfb3f373e0a75b991885b50322ca2142a8c
> ---
>  gdb/aix-thread.c       |  2 +-
>  gdb/bsd-kvm.c          |  2 +-
>  gdb/bsd-uthread.c      |  2 +-
>  gdb/corelow.c          |  2 +-
>  gdb/exec.c             |  2 +-
>  gdb/inf-child.c        |  2 +-
>  gdb/linux-thread-db.c  |  6 +++---
>  gdb/ravenscar-thread.c |  2 +-
>  gdb/record-btrace.c    |  2 +-
>  gdb/record-full.c      |  2 +-
>  gdb/record.c           |  2 +-
>  gdb/remote-sim.c       |  4 ++--
>  gdb/sol-thread.c       |  4 ++--
>  gdb/target.c           | 18 ++++++++----------
>  gdb/target.h           |  7 +------
>  gdb/tracefile-tfile.c  |  4 ++--
>  16 files changed, 28 insertions(+), 35 deletions(-)
> 
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index f4b6c1b06ab6..a479d0150bc2 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -993,7 +993,7 @@ pd_disable (void)
>    if (pd_active)
>      pd_deactivate ();
>    pd_able = 0;
> -  unpush_target (&aix_thread_ops);
> +  current_inferior ()->unpush_target (&aix_thread_ops);
>  }
> 
>  /* new_objfile observer callback.
> diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c
> index cd25e19a544e..17db2fe1cd60 100644
> --- a/gdb/bsd-kvm.c
> +++ b/gdb/bsd-kvm.c
> @@ -132,7 +132,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
>      error (("%s"), errbuf);
> 
>    bsd_kvm_corefile = filename;
> -  unpush_target (&bsd_kvm_ops);
> +  current_inferior ()->unpush_target (&bsd_kvm_ops);
>    core_kd = temp_kd;
>    push_target (&bsd_kvm_ops);
> 
> diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
> index d7dd0a180142..2ee47bfb5c47 100644
> --- a/gdb/bsd-uthread.c
> +++ b/gdb/bsd-uthread.c
> @@ -259,7 +259,7 @@ bsd_uthread_deactivate (void)
>    if (!bsd_uthread_active)
>      return;
> 
> -  unpush_target (&bsd_uthread_ops);
> +  current_inferior ()->unpush_target (&bsd_uthread_ops);
>  }
> 
>  static void
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index a2d2d20afc62..a4c1f6354c6e 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -580,7 +580,7 @@ core_target::detach (inferior *inf, int from_tty)
>    /* Note that 'this' is dangling after this call.  unpush_target
>       closes the target, and our close implementation deletes
>       'this'.  */
> -  unpush_target (this);
> +  inf->unpush_target (this);
> 
>    /* Clear the register cache and the frame cache.  */
>    registers_changed ();
> diff --git a/gdb/exec.c b/gdb/exec.c
> index 544a05873f11..bcc54bd966fe 100644
> --- a/gdb/exec.c
> +++ b/gdb/exec.c
> @@ -671,7 +671,7 @@ program_space::remove_target_sections (void *owner)
>  	    continue;
> 
>  	  switch_to_inferior_no_thread (inf);
> -	  unpush_target (&exec_ops);
> +	  inf->unpush_target (&exec_ops);
>  	}
>      }
>  }

I think the purpose of the 'switch_to_inferior_no_thread' above
was to unpush_target from the current inferior.  It should be OK to
remove the switch.

Thanks.
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH 2/3] gdb: remove push_target free functions

[Aktemur, Tankut Baris]

On Monday, March 22, 2021 4:20 AM, Simon Marchi wrote:
> Same as the previous patch, but for the push_target functions.
> 
> The implementation of the move variant is moved to a new overload of
> inferior::push_target.
> 
> gdb/ChangeLog:
> 
> 	* target.h (push_target): Remove, update callers to use
> 	inferior::push_target.
> 	* target.c (push_target): Remove.
> 	* inferior.h (class inferior) <push_target>: New overload.
> 
> Change-Id: I5a95496666278b8f3965e5e8aecb76f54a97c185
> ---
>  gdb/aix-thread.c          |  2 +-
>  gdb/bsd-kvm.c             |  2 +-
>  gdb/bsd-uthread.c         |  2 +-
>  gdb/corelow.c             |  2 +-
>  gdb/darwin-nat.c          |  2 +-
>  gdb/exec.c                |  4 ++--
>  gdb/gnu-nat.c             |  6 +++---
>  gdb/go32-nat.c            |  2 +-
>  gdb/inf-child.c           |  2 +-
>  gdb/inf-ptrace.c          |  7 ++++---
>  gdb/inferior.c            |  2 +-
>  gdb/inferior.h            |  7 +++++++
>  gdb/infrun.c              |  6 +++---
>  gdb/linux-thread-db.c     |  2 +-
>  gdb/nto-procfs.c          |  4 ++--
>  gdb/procfs.c              |  4 ++--
>  gdb/ravenscar-thread.c    |  2 +-
>  gdb/record-btrace.c       |  2 +-
>  gdb/record-full.c         |  4 ++--
>  gdb/regcache.c            |  2 +-
>  gdb/remote-sim.c          |  2 +-
>  gdb/remote.c              |  4 ++--
>  gdb/scoped-mock-context.h |  2 +-
>  gdb/sol-thread.c          |  2 +-
>  gdb/target.c              | 19 +------------------
>  gdb/target.h              |  5 -----
>  gdb/tracectf.c            |  2 +-
>  gdb/tracefile-tfile.c     |  2 +-
>  gdb/windows-nat.c         |  4 ++--
>  29 files changed, 47 insertions(+), 61 deletions(-)
> 
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index a479d0150bc2..fc34210cf9e2 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -974,7 +974,7 @@ pd_enable (void)
>      return;
> 
>    /* Prepare for thread debugging.  */
> -  push_target (&aix_thread_ops);
> +  current_inferior ()->push_target (&aix_thread_ops);
>    pd_able = 1;
> 
>    /* If we're debugging a core file or an attached inferior, the
> diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c
> index 17db2fe1cd60..89da40afcb78 100644
> --- a/gdb/bsd-kvm.c
> +++ b/gdb/bsd-kvm.c
> @@ -134,7 +134,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
>    bsd_kvm_corefile = filename;
>    current_inferior ()->unpush_target (&bsd_kvm_ops);
>    core_kd = temp_kd;
> -  push_target (&bsd_kvm_ops);
> +  current_inferior ()->push_target (&bsd_kvm_ops);
> 
>    thread_info *thr = add_thread_silent (&bsd_kvm_ops, bsd_kvm_ptid);
>    switch_to_thread (thr);
> diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
> index 2ee47bfb5c47..3cb8f64c89a7 100644
> --- a/gdb/bsd-uthread.c
> +++ b/gdb/bsd-uthread.c
> @@ -231,7 +231,7 @@ bsd_uthread_activate (struct objfile *objfile)
>    bsd_uthread_thread_ctx_offset =
>      bsd_uthread_lookup_offset ("_thread_ctx_offset", objfile);
> 
> -  push_target (&bsd_uthread_ops);
> +  current_inferior ()->push_target (&bsd_uthread_ops);
>    bsd_uthread_active = 1;
>    return 1;
>  }
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index a4c1f6354c6e..a1943ab2ea6d 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -458,7 +458,7 @@ core_target_open (const char *arg, int from_tty)
>    if (!current_program_space->exec_bfd ())
>      set_gdbarch_from_file (core_bfd);
> 
> -  push_target (std::move (target_holder));
> +  current_inferior ()->push_target (std::move (target_holder));
> 
>    switch_to_no_thread ();
> 
> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
> index ca95d7b6d385..f8e4443ff408 100644
> --- a/gdb/darwin-nat.c
> +++ b/gdb/darwin-nat.c
> @@ -1659,7 +1659,7 @@ darwin_attach_pid (struct inferior *inf)
> 
>    target_ops *darwin_ops = get_native_target ();
>    if (!target_is_pushed (darwin_ops))
> -    push_target (darwin_ops);
> +    inf->push_target (darwin_ops);
>  }
> 
>  /* Get the thread_info object corresponding to this darwin_thread_info.  */
> diff --git a/gdb/exec.c b/gdb/exec.c
> index bcc54bd966fe..b737bcf6e0f3 100644
> --- a/gdb/exec.c
> +++ b/gdb/exec.c
> @@ -616,7 +616,7 @@ program_space::add_target_sections (void *owner,
>  	    continue;
> 
>  	  switch_to_inferior_no_thread (inf);
> -	  push_target (&exec_ops);
> +	  inf->push_target (&exec_ops);

Similar to the previous patch, it should be OK to also remove the 
switch_to_inferior_no_thread here.

>  	}
>      }
>  }
> @@ -682,7 +682,7 @@ void
>  exec_on_vfork ()
>  {
>    if (!current_program_space->target_sections ().empty ())
> -    push_target (&exec_ops);
> +    current_inferior ()->push_target (&exec_ops);
>  }
> 
> 
> 
> 
> diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
> index 409d141909b3..9ea4c4089340 100644
> --- a/gdb/gnu-nat.c
> +++ b/gdb/gnu-nat.c
> @@ -2114,7 +2114,7 @@ gnu_nat_target::create_inferior (const char *exec_file,
>    inf_debug (inf, "creating inferior");
> 
>    if (!target_is_pushed (this))
> -    push_target (this);
> +    current_inferior ()->push_target (this);
> 
>    pid = fork_inferior (exec_file, allargs, env, gnu_ptrace_me,
>  		       NULL, NULL, NULL, NULL);
> @@ -2190,9 +2190,9 @@ gnu_nat_target::attach (const char *args, int from_tty)
> 
>    inf_attach (inf, pid);
> 
> -  push_target (this);
> -
>    inferior = current_inferior ();
> +  inferior->push_target (this);
> +
>    inferior_appeared (inferior, pid);
>    inferior->attach_flag = 1;
> 
> diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
> index b18a62e9490d..79c31fcd9877 100644
> --- a/gdb/go32-nat.c
> +++ b/gdb/go32-nat.c
> @@ -757,7 +757,7 @@ go32_nat_target::create_inferior (const char *exec_file,
>    inferior_appeared (inf, SOME_PID);
> 
>    if (!target_is_pushed (this))
> -    push_target (this);
> +    inf->push_target (this);
> 
>    thread_info *thr = add_thread_silent (ptid_t (SOME_PID));
>    switch_to_thread (thr);
> diff --git a/gdb/inf-child.c b/gdb/inf-child.c
> index b8bc2e2598e6..0e68a40d7c04 100644
> --- a/gdb/inf-child.c
> +++ b/gdb/inf-child.c
> @@ -166,7 +166,7 @@ inf_child_open_target (const char *arg, int from_tty)
>    gdb_assert (dynamic_cast<inf_child_target *> (target) != NULL);
> 
>    target_preopen (from_tty);
> -  push_target (target);
> +  current_inferior ()->push_target (target);
>    inf_child_explicitly_opened = 1;
>    if (from_tty)
>      printf_filtered ("Done.  Use the \"run\" command to start a process.\n");
> diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
> index 7ca02dfd8764..e630ba447f40 100644
> --- a/gdb/inf-ptrace.c
> +++ b/gdb/inf-ptrace.c
> @@ -82,7 +82,7 @@ inf_ptrace_target::create_inferior (const char *exec_file,
>    if (! ops_already_pushed)
>      {
>        /* Clear possible core file with its process_stratum.  */
> -      push_target (this);
> +      current_inferior ()->push_target (this);
>        unpusher.reset (this);
>      }
> 
> @@ -139,12 +139,14 @@ inf_ptrace_target::attach (const char *args, int from_tty)
>    if (pid == getpid ())		/* Trying to masturbate?  */
>      error (_("I refuse to debug myself!"));
> 
> +  inf = current_inferior ();
> +
>    target_unpush_up unpusher;
>    if (! ops_already_pushed)
>      {
>        /* target_pid_to_str already uses the target.  Also clear possible core
>  	 file with its process_stratum.  */
> -      push_target (this);
> +      inf->push_target (this);
>        unpusher.reset (this);
>      }
> 
> @@ -169,7 +171,6 @@ inf_ptrace_target::attach (const char *args, int from_tty)
>    error (_("This system does not support attaching to a process"));
>  #endif
> 
> -  inf = current_inferior ();
>    inferior_appeared (inf, pid);
>    inf->attach_flag = 1;
> 
> diff --git a/gdb/inferior.c b/gdb/inferior.c
> index 49f869a4c783..69baee34ce9d 100644
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -771,7 +771,7 @@ switch_to_inferior_and_push_target (inferior *new_inf,
>    /* Reuse the target for new inferior.  */
>    if (!no_connection && proc_target != NULL)
>      {
> -      push_target (proc_target);
> +      new_inf->push_target (proc_target);
>        if (proc_target->connection_string () != NULL)
>  	printf_filtered (_("Added inferior %d on connection %d (%s %s)\n"),
>  			 new_inf->num,
> diff --git a/gdb/inferior.h b/gdb/inferior.h
> index b8d5ff94fc56..66fc180ce530 100644
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -352,6 +352,13 @@ class inferior : public refcounted_object
>    void push_target (struct target_ops *t)
>    { m_target_stack.push (t); }
> 
> +  /* An overload that deletes the target on failure.  */
> +  void push_target (target_ops_up &&t)
> +  {
> +    m_target_stack.push (t.get ());
> +    t.release ();
> +  }
> +
>    /* Unpush T from this inferior's target stack.  */
>    int unpush_target (struct target_ops *t)
>    { return m_target_stack.unpush (t); }
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 3b65a6de9fe2..50340e6edad4 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -477,7 +477,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
>  	  set_current_inferior (child_inf);
>  	  switch_to_no_thread ();
>  	  child_inf->symfile_flags = SYMFILE_NO_READ;
> -	  push_target (parent_inf->process_target ());
> +	  child_inf->push_target (parent_inf->process_target ());
>  	  thread_info *child_thr
>  	    = add_thread_silent (child_inf->process_target (), child_ptid);
> 
> @@ -627,7 +627,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
>  	   informing the solib layer about this new process.  */
> 
>  	set_current_inferior (child_inf);
> -	push_target (target);
> +	child_inf->push_target (target);
>        }
> 
>        thread_info *child_thr = add_thread_silent (target, child_ptid);
> @@ -1183,7 +1183,7 @@ follow_exec (ptid_t ptid, const char *exec_file_target)
> 
>        inferior *org_inferior = current_inferior ();
>        switch_to_inferior_no_thread (inf);

And here.

Thanks.
-Baris



Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 1/3] gdb: remove unpush_target free function

[Simon Marchi]

On 2021-03-22 12:22 p.m., Aktemur, Tankut Baris wrote:
> On Monday, March 22, 2021 4:20 AM, Simon Marchi Wrote:
>> unpush_target unpushes the passed-in target from the current inferior's
>> target stack.  Calling it is therefore an implicit dependency on the
>> current global inferior.  Remove that function and make the callers use
>> the inferior::unpush_target method directly.  This sometimes allows
>> using the inferior from the context rather than the global current
>> inferior.
>>
>> target_unpusher::operator() now needs to be implemented in target.c,
>> otherwise target.h and inferior.h both need to include each other, and
>> that wouldn't work.
>>
>> gdb/ChangeLog:
>>
>> 	* target.h (unpush_target): Remove, update all callers
>> 	to use `inferior::unpush_target` instead.
>> 	(struct target_unpusher) <operator()>: Just declare.
>> 	* target.c (unpush_target): Remove.
>> 	(target_unpusher::operator()): New.
>>
>> Change-Id: Ia5172dfb3f373e0a75b991885b50322ca2142a8c
>> ---
>>  gdb/aix-thread.c       |  2 +-
>>  gdb/bsd-kvm.c          |  2 +-
>>  gdb/bsd-uthread.c      |  2 +-
>>  gdb/corelow.c          |  2 +-
>>  gdb/exec.c             |  2 +-
>>  gdb/inf-child.c        |  2 +-
>>  gdb/linux-thread-db.c  |  6 +++---
>>  gdb/ravenscar-thread.c |  2 +-
>>  gdb/record-btrace.c    |  2 +-
>>  gdb/record-full.c      |  2 +-
>>  gdb/record.c           |  2 +-
>>  gdb/remote-sim.c       |  4 ++--
>>  gdb/sol-thread.c       |  4 ++--
>>  gdb/target.c           | 18 ++++++++----------
>>  gdb/target.h           |  7 +------
>>  gdb/tracefile-tfile.c  |  4 ++--
>>  16 files changed, 28 insertions(+), 35 deletions(-)
>>
>> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
>> index f4b6c1b06ab6..a479d0150bc2 100644
>> --- a/gdb/aix-thread.c
>> +++ b/gdb/aix-thread.c
>> @@ -993,7 +993,7 @@ pd_disable (void)
>>    if (pd_active)
>>      pd_deactivate ();
>>    pd_able = 0;
>> -  unpush_target (&aix_thread_ops);
>> +  current_inferior ()->unpush_target (&aix_thread_ops);
>>  }
>>
>>  /* new_objfile observer callback.
>> diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c
>> index cd25e19a544e..17db2fe1cd60 100644
>> --- a/gdb/bsd-kvm.c
>> +++ b/gdb/bsd-kvm.c
>> @@ -132,7 +132,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
>>      error (("%s"), errbuf);
>>
>>    bsd_kvm_corefile = filename;
>> -  unpush_target (&bsd_kvm_ops);
>> +  current_inferior ()->unpush_target (&bsd_kvm_ops);
>>    core_kd = temp_kd;
>>    push_target (&bsd_kvm_ops);
>>
>> diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
>> index d7dd0a180142..2ee47bfb5c47 100644
>> --- a/gdb/bsd-uthread.c
>> +++ b/gdb/bsd-uthread.c
>> @@ -259,7 +259,7 @@ bsd_uthread_deactivate (void)
>>    if (!bsd_uthread_active)
>>      return;
>>
>> -  unpush_target (&bsd_uthread_ops);
>> +  current_inferior ()->unpush_target (&bsd_uthread_ops);
>>  }
>>
>>  static void
>> diff --git a/gdb/corelow.c b/gdb/corelow.c
>> index a2d2d20afc62..a4c1f6354c6e 100644
>> --- a/gdb/corelow.c
>> +++ b/gdb/corelow.c
>> @@ -580,7 +580,7 @@ core_target::detach (inferior *inf, int from_tty)
>>    /* Note that 'this' is dangling after this call.  unpush_target
>>       closes the target, and our close implementation deletes
>>       'this'.  */
>> -  unpush_target (this);
>> +  inf->unpush_target (this);
>>
>>    /* Clear the register cache and the frame cache.  */
>>    registers_changed ();
>> diff --git a/gdb/exec.c b/gdb/exec.c
>> index 544a05873f11..bcc54bd966fe 100644
>> --- a/gdb/exec.c
>> +++ b/gdb/exec.c
>> @@ -671,7 +671,7 @@ program_space::remove_target_sections (void *owner)
>>  	    continue;
>>
>>  	  switch_to_inferior_no_thread (inf);
>> -	  unpush_target (&exec_ops);
>> +	  inf->unpush_target (&exec_ops);
>>  	}
>>      }
>>  }
> 
> I think the purpose of the 'switch_to_inferior_no_thread' above
> was to unpush_target from the current inferior.  It should be OK to
> remove the switch.

In fact, unpush_target decrefs the target, which can cause its to be
closed.  There are some target_ops::close methods which reference the
current_inferior (though I'm not sure that it's really appropriate for
them to do so).  I've left the switch_to_inferior calls to be safe for
this reason.  I'll add a note in the commit message to that effect.

But I agree this is the right direction.

Simon

Re: [PATCH 2/3] gdb: remove push_target free functions

[Simon Marchi]

On 2021-03-22 12:30 p.m., Aktemur, Tankut Baris wrote:
> On Monday, March 22, 2021 4:20 AM, Simon Marchi wrote:
>> Same as the previous patch, but for the push_target functions.
>>
>> The implementation of the move variant is moved to a new overload of
>> inferior::push_target.
>>
>> gdb/ChangeLog:
>>
>> 	* target.h (push_target): Remove, update callers to use
>> 	inferior::push_target.
>> 	* target.c (push_target): Remove.
>> 	* inferior.h (class inferior) <push_target>: New overload.
>>
>> Change-Id: I5a95496666278b8f3965e5e8aecb76f54a97c185
>> ---
>>  gdb/aix-thread.c          |  2 +-
>>  gdb/bsd-kvm.c             |  2 +-
>>  gdb/bsd-uthread.c         |  2 +-
>>  gdb/corelow.c             |  2 +-
>>  gdb/darwin-nat.c          |  2 +-
>>  gdb/exec.c                |  4 ++--
>>  gdb/gnu-nat.c             |  6 +++---
>>  gdb/go32-nat.c            |  2 +-
>>  gdb/inf-child.c           |  2 +-
>>  gdb/inf-ptrace.c          |  7 ++++---
>>  gdb/inferior.c            |  2 +-
>>  gdb/inferior.h            |  7 +++++++
>>  gdb/infrun.c              |  6 +++---
>>  gdb/linux-thread-db.c     |  2 +-
>>  gdb/nto-procfs.c          |  4 ++--
>>  gdb/procfs.c              |  4 ++--
>>  gdb/ravenscar-thread.c    |  2 +-
>>  gdb/record-btrace.c       |  2 +-
>>  gdb/record-full.c         |  4 ++--
>>  gdb/regcache.c            |  2 +-
>>  gdb/remote-sim.c          |  2 +-
>>  gdb/remote.c              |  4 ++--
>>  gdb/scoped-mock-context.h |  2 +-
>>  gdb/sol-thread.c          |  2 +-
>>  gdb/target.c              | 19 +------------------
>>  gdb/target.h              |  5 -----
>>  gdb/tracectf.c            |  2 +-
>>  gdb/tracefile-tfile.c     |  2 +-
>>  gdb/windows-nat.c         |  4 ++--
>>  29 files changed, 47 insertions(+), 61 deletions(-)
>>
>> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
>> index a479d0150bc2..fc34210cf9e2 100644
>> --- a/gdb/aix-thread.c
>> +++ b/gdb/aix-thread.c
>> @@ -974,7 +974,7 @@ pd_enable (void)
>>      return;
>>
>>    /* Prepare for thread debugging.  */
>> -  push_target (&aix_thread_ops);
>> +  current_inferior ()->push_target (&aix_thread_ops);
>>    pd_able = 1;
>>
>>    /* If we're debugging a core file or an attached inferior, the
>> diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c
>> index 17db2fe1cd60..89da40afcb78 100644
>> --- a/gdb/bsd-kvm.c
>> +++ b/gdb/bsd-kvm.c
>> @@ -134,7 +134,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
>>    bsd_kvm_corefile = filename;
>>    current_inferior ()->unpush_target (&bsd_kvm_ops);
>>    core_kd = temp_kd;
>> -  push_target (&bsd_kvm_ops);
>> +  current_inferior ()->push_target (&bsd_kvm_ops);
>>
>>    thread_info *thr = add_thread_silent (&bsd_kvm_ops, bsd_kvm_ptid);
>>    switch_to_thread (thr);
>> diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
>> index 2ee47bfb5c47..3cb8f64c89a7 100644
>> --- a/gdb/bsd-uthread.c
>> +++ b/gdb/bsd-uthread.c
>> @@ -231,7 +231,7 @@ bsd_uthread_activate (struct objfile *objfile)
>>    bsd_uthread_thread_ctx_offset =
>>      bsd_uthread_lookup_offset ("_thread_ctx_offset", objfile);
>>
>> -  push_target (&bsd_uthread_ops);
>> +  current_inferior ()->push_target (&bsd_uthread_ops);
>>    bsd_uthread_active = 1;
>>    return 1;
>>  }
>> diff --git a/gdb/corelow.c b/gdb/corelow.c
>> index a4c1f6354c6e..a1943ab2ea6d 100644
>> --- a/gdb/corelow.c
>> +++ b/gdb/corelow.c
>> @@ -458,7 +458,7 @@ core_target_open (const char *arg, int from_tty)
>>    if (!current_program_space->exec_bfd ())
>>      set_gdbarch_from_file (core_bfd);
>>
>> -  push_target (std::move (target_holder));
>> +  current_inferior ()->push_target (std::move (target_holder));
>>
>>    switch_to_no_thread ();
>>
>> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
>> index ca95d7b6d385..f8e4443ff408 100644
>> --- a/gdb/darwin-nat.c
>> +++ b/gdb/darwin-nat.c
>> @@ -1659,7 +1659,7 @@ darwin_attach_pid (struct inferior *inf)
>>
>>    target_ops *darwin_ops = get_native_target ();
>>    if (!target_is_pushed (darwin_ops))
>> -    push_target (darwin_ops);
>> +    inf->push_target (darwin_ops);
>>  }
>>
>>  /* Get the thread_info object corresponding to this darwin_thread_info.  */
>> diff --git a/gdb/exec.c b/gdb/exec.c
>> index bcc54bd966fe..b737bcf6e0f3 100644
>> --- a/gdb/exec.c
>> +++ b/gdb/exec.c
>> @@ -616,7 +616,7 @@ program_space::add_target_sections (void *owner,
>>  	    continue;
>>
>>  	  switch_to_inferior_no_thread (inf);
>> -	  push_target (&exec_ops);
>> +	  inf->push_target (&exec_ops);
> 
> Similar to the previous patch, it should be OK to also remove the 
> switch_to_inferior_no_thread here.

Similarly, pushing a target can cause another (at the same stratum) to
get unpushed, and some target_ops::close implementations refer to the
current inferior.  So out of caution I would leave it.  I'll also add
a note to the commit message here.

> 
>>  	}
>>      }
>>  }
>> @@ -682,7 +682,7 @@ void
>>  exec_on_vfork ()
>>  {
>>    if (!current_program_space->target_sections ().empty ())
>> -    push_target (&exec_ops);
>> +    current_inferior ()->push_target (&exec_ops);
>>  }
>>
>>
>>
>>
>> diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
>> index 409d141909b3..9ea4c4089340 100644
>> --- a/gdb/gnu-nat.c
>> +++ b/gdb/gnu-nat.c
>> @@ -2114,7 +2114,7 @@ gnu_nat_target::create_inferior (const char *exec_file,
>>    inf_debug (inf, "creating inferior");
>>
>>    if (!target_is_pushed (this))
>> -    push_target (this);
>> +    current_inferior ()->push_target (this);
>>
>>    pid = fork_inferior (exec_file, allargs, env, gnu_ptrace_me,
>>  		       NULL, NULL, NULL, NULL);
>> @@ -2190,9 +2190,9 @@ gnu_nat_target::attach (const char *args, int from_tty)
>>
>>    inf_attach (inf, pid);
>>
>> -  push_target (this);
>> -
>>    inferior = current_inferior ();
>> +  inferior->push_target (this);
>> +
>>    inferior_appeared (inferior, pid);
>>    inferior->attach_flag = 1;
>>
>> diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
>> index b18a62e9490d..79c31fcd9877 100644
>> --- a/gdb/go32-nat.c
>> +++ b/gdb/go32-nat.c
>> @@ -757,7 +757,7 @@ go32_nat_target::create_inferior (const char *exec_file,
>>    inferior_appeared (inf, SOME_PID);
>>
>>    if (!target_is_pushed (this))
>> -    push_target (this);
>> +    inf->push_target (this);
>>
>>    thread_info *thr = add_thread_silent (ptid_t (SOME_PID));
>>    switch_to_thread (thr);
>> diff --git a/gdb/inf-child.c b/gdb/inf-child.c
>> index b8bc2e2598e6..0e68a40d7c04 100644
>> --- a/gdb/inf-child.c
>> +++ b/gdb/inf-child.c
>> @@ -166,7 +166,7 @@ inf_child_open_target (const char *arg, int from_tty)
>>    gdb_assert (dynamic_cast<inf_child_target *> (target) != NULL);
>>
>>    target_preopen (from_tty);
>> -  push_target (target);
>> +  current_inferior ()->push_target (target);
>>    inf_child_explicitly_opened = 1;
>>    if (from_tty)
>>      printf_filtered ("Done.  Use the \"run\" command to start a process.\n");
>> diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
>> index 7ca02dfd8764..e630ba447f40 100644
>> --- a/gdb/inf-ptrace.c
>> +++ b/gdb/inf-ptrace.c
>> @@ -82,7 +82,7 @@ inf_ptrace_target::create_inferior (const char *exec_file,
>>    if (! ops_already_pushed)
>>      {
>>        /* Clear possible core file with its process_stratum.  */
>> -      push_target (this);
>> +      current_inferior ()->push_target (this);
>>        unpusher.reset (this);
>>      }
>>
>> @@ -139,12 +139,14 @@ inf_ptrace_target::attach (const char *args, int from_tty)
>>    if (pid == getpid ())		/* Trying to masturbate?  */
>>      error (_("I refuse to debug myself!"));
>>
>> +  inf = current_inferior ();
>> +
>>    target_unpush_up unpusher;
>>    if (! ops_already_pushed)
>>      {
>>        /* target_pid_to_str already uses the target.  Also clear possible core
>>  	 file with its process_stratum.  */
>> -      push_target (this);
>> +      inf->push_target (this);
>>        unpusher.reset (this);
>>      }
>>
>> @@ -169,7 +171,6 @@ inf_ptrace_target::attach (const char *args, int from_tty)
>>    error (_("This system does not support attaching to a process"));
>>  #endif
>>
>> -  inf = current_inferior ();
>>    inferior_appeared (inf, pid);
>>    inf->attach_flag = 1;
>>
>> diff --git a/gdb/inferior.c b/gdb/inferior.c
>> index 49f869a4c783..69baee34ce9d 100644
>> --- a/gdb/inferior.c
>> +++ b/gdb/inferior.c
>> @@ -771,7 +771,7 @@ switch_to_inferior_and_push_target (inferior *new_inf,
>>    /* Reuse the target for new inferior.  */
>>    if (!no_connection && proc_target != NULL)
>>      {
>> -      push_target (proc_target);
>> +      new_inf->push_target (proc_target);
>>        if (proc_target->connection_string () != NULL)
>>  	printf_filtered (_("Added inferior %d on connection %d (%s %s)\n"),
>>  			 new_inf->num,
>> diff --git a/gdb/inferior.h b/gdb/inferior.h
>> index b8d5ff94fc56..66fc180ce530 100644
>> --- a/gdb/inferior.h
>> +++ b/gdb/inferior.h
>> @@ -352,6 +352,13 @@ class inferior : public refcounted_object
>>    void push_target (struct target_ops *t)
>>    { m_target_stack.push (t); }
>>
>> +  /* An overload that deletes the target on failure.  */
>> +  void push_target (target_ops_up &&t)
>> +  {
>> +    m_target_stack.push (t.get ());
>> +    t.release ();
>> +  }
>> +
>>    /* Unpush T from this inferior's target stack.  */
>>    int unpush_target (struct target_ops *t)
>>    { return m_target_stack.unpush (t); }
>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>> index 3b65a6de9fe2..50340e6edad4 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -477,7 +477,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
>>  	  set_current_inferior (child_inf);
>>  	  switch_to_no_thread ();
>>  	  child_inf->symfile_flags = SYMFILE_NO_READ;
>> -	  push_target (parent_inf->process_target ());
>> +	  child_inf->push_target (parent_inf->process_target ());
>>  	  thread_info *child_thr
>>  	    = add_thread_silent (child_inf->process_target (), child_ptid);
>>
>> @@ -627,7 +627,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
>>  	   informing the solib layer about this new process.  */
>>
>>  	set_current_inferior (child_inf);
>> -	push_target (target);
>> +	child_inf->push_target (target);
>>        }
>>
>>        thread_info *child_thr = add_thread_silent (target, child_ptid);
>> @@ -1183,7 +1183,7 @@ follow_exec (ptid_t ptid, const char *exec_file_target)
>>
>>        inferior *org_inferior = current_inferior ();
>>        switch_to_inferior_no_thread (inf);
> 
> And here.

Same.  Although here it's a brand new inferior, so we know there isn't
an existing process target that is going to get unpushed.  But also
here, is it maybe important for the current inferior to be correctly set
for the remainder of the follow_exec function?

Simon

Re: [PATCH 1/3] gdb: remove unpush_target free function

[Simon Marchi]

On 2021-03-22 1:25 p.m., Simon Marchi via Gdb-patches wrote:
>>> diff --git a/gdb/exec.c b/gdb/exec.c
>>> index 544a05873f11..bcc54bd966fe 100644
>>> --- a/gdb/exec.c
>>> +++ b/gdb/exec.c
>>> @@ -671,7 +671,7 @@ program_space::remove_target_sections (void *owner)
>>>  	    continue;
>>>
>>>  	  switch_to_inferior_no_thread (inf);
>>> -	  unpush_target (&exec_ops);
>>> +	  inf->unpush_target (&exec_ops);
>>>  	}
>>>      }
>>>  }
>>
>> I think the purpose of the 'switch_to_inferior_no_thread' above
>> was to unpush_target from the current inferior.  It should be OK to
>> remove the switch.
> 
> In fact, unpush_target decrefs the target, which can cause its to be
> closed.  There are some target_ops::close methods which reference the
> current_inferior (though I'm not sure that it's really appropriate for
> them to do so).  I've left the switch_to_inferior calls to be safe for
> this reason.  I'll add a note in the commit message to that effect.
> 
> But I agree this is the right direction.

Actually, since we know which target's close method will be called (the
exec target) and we know that the exec target's close method doesn't
care about the current inferior, I think it's safe indeed.  I'll update
the patch to remove the call.

Simon

Re: [PATCH 2/3] gdb: remove push_target free functions

[Simon Marchi]

On 2021-03-22 1:31 p.m., Simon Marchi via Gdb-patches wrote:
>>> index bcc54bd966fe..b737bcf6e0f3 100644
>>> --- a/gdb/exec.c
>>> +++ b/gdb/exec.c
>>> @@ -616,7 +616,7 @@ program_space::add_target_sections (void *owner,
>>>  	    continue;
>>>
>>>  	  switch_to_inferior_no_thread (inf);
>>> -	  push_target (&exec_ops);
>>> +	  inf->push_target (&exec_ops);
>>
>> Similar to the previous patch, it should be OK to also remove the 
>> switch_to_inferior_no_thread here.
> 
> Similarly, pushing a target can cause another (at the same stratum) to
> get unpushed, and some target_ops::close implementations refer to the
> current inferior.  So out of caution I would leave it.  I'll also add
> a note to the commit message here.

Here too, it should be safe to remove it, now that I think about it.
We check just before that the exec target isn't pushed on the inferior's
stack, and there isn't any other file_stratum that could be there.  So
I'll remove it.  Thanks for making me double check that.

>>> @@ -1183,7 +1183,7 @@ follow_exec (ptid_t ptid, const char *exec_file_target)
>>>
>>>        inferior *org_inferior = current_inferior ();
>>>        switch_to_inferior_no_thread (inf);
>>
>> And here.
> 
> Same.  Although here it's a brand new inferior, so we know there isn't
> an existing process target that is going to get unpushed.  But also
> here, is it maybe important for the current inferior to be correctly set
> for the remainder of the follow_exec function?

I'll leave this one, because I still think it's important for the
remainder of the follow_exec function.

Simon

RE: [PATCH 2/3] gdb: remove push_target free functions

[Aktemur, Tankut Baris]

On Monday, March 22, 2021 7:21 PM, Simon Marchi wrote:
> On 2021-03-22 1:31 p.m., Simon Marchi via Gdb-patches wrote:
> >>> index bcc54bd966fe..b737bcf6e0f3 100644
> >>> --- a/gdb/exec.c
> >>> +++ b/gdb/exec.c
> >>> @@ -616,7 +616,7 @@ program_space::add_target_sections (void *owner,
> >>>  	    continue;
> >>>
> >>>  	  switch_to_inferior_no_thread (inf);
> >>> -	  push_target (&exec_ops);
> >>> +	  inf->push_target (&exec_ops);
> >>
> >> Similar to the previous patch, it should be OK to also remove the
> >> switch_to_inferior_no_thread here.
> >
> > Similarly, pushing a target can cause another (at the same stratum) to
> > get unpushed, and some target_ops::close implementations refer to the
> > current inferior.  So out of caution I would leave it.  I'll also add
> > a note to the commit message here.
> 
> Here too, it should be safe to remove it, now that I think about it.
> We check just before that the exec target isn't pushed on the inferior's
> stack, and there isn't any other file_stratum that could be there.  So
> I'll remove it.  Thanks for making me double check that.
> 
> >>> @@ -1183,7 +1183,7 @@ follow_exec (ptid_t ptid, const char *exec_file_target)
> >>>
> >>>        inferior *org_inferior = current_inferior ();
> >>>        switch_to_inferior_no_thread (inf);
> >>
> >> And here.
> >
> > Same.  Although here it's a brand new inferior, so we know there isn't
> > an existing process target that is going to get unpushed.  But also
> > here, is it maybe important for the current inferior to be correctly set
> > for the remainder of the follow_exec function?
> 
> I'll leave this one, because I still think it's important for the
> remainder of the follow_exec function.
> 
> Simon

Thanks for checking this and the clarifications.

-Baris



Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 2/3] gdb: remove push_target free functions

[Simon Marchi]

On 2021-03-22 2:21 p.m., Simon Marchi via Gdb-patches wrote:
> On 2021-03-22 1:31 p.m., Simon Marchi via Gdb-patches wrote:
>>>> index bcc54bd966fe..b737bcf6e0f3 100644
>>>> --- a/gdb/exec.c
>>>> +++ b/gdb/exec.c
>>>> @@ -616,7 +616,7 @@ program_space::add_target_sections (void *owner,
>>>>  	    continue;
>>>>
>>>>  	  switch_to_inferior_no_thread (inf);
>>>> -	  push_target (&exec_ops);
>>>> +	  inf->push_target (&exec_ops);
>>>
>>> Similar to the previous patch, it should be OK to also remove the 
>>> switch_to_inferior_no_thread here.
>>
>> Similarly, pushing a target can cause another (at the same stratum) to
>> get unpushed, and some target_ops::close implementations refer to the
>> current inferior.  So out of caution I would leave it.  I'll also add
>> a note to the commit message here.
> 
> Here too, it should be safe to remove it, now that I think about it.
> We check just before that the exec target isn't pushed on the inferior's
> stack, and there isn't any other file_stratum that could be there.  So
> I'll remove it.  Thanks for making me double check that.

Ok, I tried to do this and failed.  Removing the inferior switching
uncovers what I think is a latent bug somewhere in the follow-fork
mechanism.  The follow_fork_inferior function leaves things in an
invalid state, where the current inferior/thread/program space don't
match.  That switch_to_inferior_no_thread call (or rather the restore)
happens to make things right, by chance.

So what I'll do is push the original versions of the patches, and
investigate this issue independently.

Simon