public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/3] Remove args from target detach
@ 2017-12-31  5:56 Simon Marchi
  2017-12-31  5:58 ` [PATCH 2/3] Pass inferior down to target_detach and to_detach Simon Marchi
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Simon Marchi @ 2017-12-31  5:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

I was looking into adding a parameter to target_detach, and was
wondering what the args parameter was.  It seems like in the distant
past, it was possible to specify a signal number when detaching.  That
signal was injected in the process before it was detached.  There is an
example of code handling this in linux_nat_detach.  With today's GDB, I
can't get this to work.  Doing "detach 15" (15 == SIGTERM) doesn't work,
because detach is a prefix command and doesn't recognize the sub-command
15.  Doing "detach inferiors 15" doesn't work because it expects a list
of inferior id to detach.  Therefore, I don't think there's a way of
invoking detach_command with a non-NULL args.  I also didn't find any
documentation related to this feature.

I assume that this feature stopped working when detach was made a prefix
command, which is in f73adfeb8bae36885e6ea248d12223ab0d5eb9cb (sorry,
there's no commit title) from 2006.  Given that this feature was broken
for such a long time and we haven't heard anything (AFAIK, I did not
find any related bug), I think it's safe to remove it, as well as the
args parameter to target_detach.  If someone wants to re-introduce it, I
would suggest rethinking the user interface, and in particular would
suggest using signal name instead of numbers.

I tried to fix all the impacted code, but I might have forgotten some
spots.  It shouldn't be hard to fix if that's the case.  I also couldn't
build-test everything I changed, especially the nto and solaris stuff.

gdb/ChangeLog:

	* target.h (struct target_ops) <to_detach>: Remove args
	parameter.
	(target_detach): Likewise.
	* target.c (dispose_inferior): Adjust.
	(target_detach): Remove args parameter, adjust.
	* aix-thread.c (aix_thread_detach): Adjust.
	* corefile.c (core_file_command): Adjust.
	* corelow.c (core_detach): Adjust.
	* darwin-nat.c (darwin_detach): Adjust.
	* gnu-nat.c (gnu_detach): Adjust.
	* inf-ptrace.c (inf_ptrace_detach): Adjust.
	* infcmd.c (detach_command): Adjust
	* infrun.c (follow_fork_inferior): Adjust.
	(handle_vfork_child_exec_or_exit): Adjust.
	* linux-fork.c (linux_fork_detach): Remove args parameter.
	* linux-fork.h (linux_fork_detach): Likewise.
	* linux-nat.c (linux_nat_detach): Likewise, and adjust.
	* linux-thread-db.c (thread_db_detach): Likewise.
	* nto-procfs.c (procfs_detach): Likewise.
	* procfs.c (procfs_detach): Likewise.
	* record.c (record_detach): Likewise.
	* record.h (record_detach): Likewise.
	* remote-sim.c (gdbsim_detach): Likewise.
	* remote.c (remote_detach_1): Likewise.
	(remote_detach): Likewise.
	(extended_remote_detach): Likewise.
	* sol-thread.c (sol_thread_detach): Likewise.
	* target-delegates.c: Re-generate.
	* top.c (struct qt_args) <args>: Remove field.
	(kill_or_detach): Don't pass args.
	(quit_force): Don't set args.
	* windows-nat.c (windows_detach): Remove args parameter.
---
 gdb/aix-thread.c       |  4 ++--
 gdb/corefile.c         |  2 +-
 gdb/corelow.c          |  4 +---
 gdb/darwin-nat.c       |  2 +-
 gdb/gnu-nat.c          |  2 +-
 gdb/inf-ptrace.c       | 10 +++-------
 gdb/infcmd.c           |  2 +-
 gdb/infrun.c           |  4 ++--
 gdb/linux-fork.c       |  2 +-
 gdb/linux-fork.h       |  2 +-
 gdb/linux-nat.c        | 15 ++++-----------
 gdb/linux-thread-db.c  |  4 ++--
 gdb/nto-procfs.c       |  8 ++------
 gdb/procfs.c           |  8 ++------
 gdb/record.c           |  4 ++--
 gdb/record.h           |  2 +-
 gdb/remote-sim.c       | 12 +++++-------
 gdb/remote.c           | 13 +++++--------
 gdb/sol-thread.c       |  4 ++--
 gdb/target-delegates.c | 14 ++++++--------
 gdb/target.c           |  8 ++++----
 gdb/target.h           |  9 ++++-----
 gdb/top.c              |  4 +---
 gdb/windows-nat.c      |  2 +-
 24 files changed, 55 insertions(+), 86 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index 1a1d769..adb3a83 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -981,12 +981,12 @@ aix_thread_inferior_created (struct target_ops *ops, int from_tty)
 /* Detach from the process attached to by aix_thread_attach().  */
 
 static void
-aix_thread_detach (struct target_ops *ops, const char *args, int from_tty)
+aix_thread_detach (struct target_ops *ops, int from_tty)
 {
   struct target_ops *beneath = find_target_beneath (ops);
 
   pd_disable ();
-  beneath->to_detach (beneath, args, from_tty);
+  beneath->to_detach (beneath, from_tty);
 }
 
 /* Tell the inferior process to continue running thread PID if != -1
diff --git a/gdb/corefile.c b/gdb/corefile.c
index 686603a..434c64a 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -68,7 +68,7 @@ core_file_command (const char *filename, int from_tty)
   gdb_assert (core_target != NULL);
 
   if (!filename)
-    (core_target->to_detach) (core_target, filename, from_tty);
+    (core_target->to_detach) (core_target, from_tty);
   else
     (core_target->to_open) (filename, from_tty);
 }
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 4214896..2894c96 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -464,10 +464,8 @@ core_open (const char *arg, int from_tty)
 }
 
 static void
-core_detach (struct target_ops *ops, const char *args, int from_tty)
+core_detach (struct target_ops *ops, int from_tty)
 {
-  if (args)
-    error (_("Too many arguments"));
   unpush_target (ops);
   reinit_frame_cache ();
   if (from_tty)
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 5e708c6..1b64536 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1938,7 +1938,7 @@ darwin_attach (struct target_ops *ops, const char *args, int from_tty)
    previously attached.  It *might* work if the program was
    started via fork.  */
 static void
-darwin_detach (struct target_ops *ops, const char *args, int from_tty)
+darwin_detach (struct target_ops *ops, int from_tty)
 {
   pid_t pid = ptid_get_pid (inferior_ptid);
   struct inferior *inf = current_inferior ();
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index 0d500ea..caa8c3d 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2253,7 +2253,7 @@ gnu_attach (struct target_ops *ops, const char *args, int from_tty)
    previously attached.  It *might* work if the program was
    started via fork.  */
 static void
-gnu_detach (struct target_ops *ops, const char *args, int from_tty)
+gnu_detach (struct target_ops *ops, int from_tty)
 {
   int pid;
 
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index e127741..8633108 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -241,18 +241,14 @@ inf_ptrace_post_attach (struct target_ops *self, int pid)
 
 #endif
 
-/* Detach from the inferior, optionally passing it the signal
-   specified by ARGS.  If FROM_TTY is non-zero, be chatty about it.  */
+/* Detach from the inferior.  If FROM_TTY is non-zero, be chatty about it.  */
 
 static void
-inf_ptrace_detach (struct target_ops *ops, const char *args, int from_tty)
+inf_ptrace_detach (struct target_ops *ops, int from_tty)
 {
   pid_t pid = ptid_get_pid (inferior_ptid);
-  int sig = 0;
 
   target_announce_detach (from_tty);
-  if (args)
-    sig = atoi (args);
 
 #ifdef PT_DETACH
   /* We'd better not have left any breakpoints in the program or it'll
@@ -260,7 +256,7 @@ inf_ptrace_detach (struct target_ops *ops, const char *args, int from_tty)
      previously attached to the inferior.  It *might* work if we
      started the process ourselves.  */
   errno = 0;
-  ptrace (PT_DETACH, pid, (PTRACE_TYPE_ARG3)1, sig);
+  ptrace (PT_DETACH, pid, (PTRACE_TYPE_ARG3)1, 0);
   if (errno != 0)
     perror_with_name (("ptrace"));
 #else
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 1b63f9b..3aa2ced 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2976,7 +2976,7 @@ detach_command (const char *args, int from_tty)
 
   disconnect_tracing ();
 
-  target_detach (args, from_tty);
+  target_detach (from_tty);
 
   /* The current inferior process was just detached successfully.  Get
      rid of breakpoints that no longer make sense.  Note we don't do
diff --git a/gdb/infrun.c b/gdb/infrun.c
index d7df3c7..c4d168c 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -606,7 +606,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 				target_pid_to_str (process_ptid));
 	    }
 
-	  target_detach (NULL, 0);
+	  target_detach (0);
 	}
 
       /* Note that the detach above makes PARENT_INF dangling.  */
@@ -976,7 +976,7 @@ handle_vfork_child_exec_or_exit (int exec)
 		}
 	    }
 
-	  target_detach (NULL, 0);
+	  target_detach (0);
 
 	  /* Put it back.  */
 	  inf->pspace = pspace;
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index f55f743..d05cec9 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -407,7 +407,7 @@ linux_fork_mourn_inferior (void)
    the first available.  */
 
 void
-linux_fork_detach (const char *args, int from_tty)
+linux_fork_detach (int from_tty)
 {
   /* OK, inferior_ptid is the one we are detaching from.  We need to
      delete it from the fork_list, and switch to the next available
diff --git a/gdb/linux-fork.h b/gdb/linux-fork.h
index 6211482..8af822a 100644
--- a/gdb/linux-fork.h
+++ b/gdb/linux-fork.h
@@ -22,6 +22,6 @@ extern struct fork_info *add_fork (pid_t);
 extern struct fork_info *find_fork_pid (pid_t);
 extern void linux_fork_killall (void);
 extern void linux_fork_mourn_inferior (void);
-extern void linux_fork_detach (const char *, int);
+extern void linux_fork_detach (int);
 extern int forks_exist_p (void);
 extern int linux_fork_checkpointing_p (int);
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index b8f3108..3290ed5 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1507,7 +1507,7 @@ detach_callback (struct lwp_info *lp, void *data)
 }
 
 static void
-linux_nat_detach (struct target_ops *ops, const char *args, int from_tty)
+linux_nat_detach (struct target_ops *ops, int from_tty)
 {
   int pid;
   struct lwp_info *main_lwp;
@@ -1537,21 +1537,14 @@ linux_nat_detach (struct target_ops *ops, const char *args, int from_tty)
 	 from, but there are other viable forks to debug.  Detach from
 	 the current fork, and context-switch to the first
 	 available.  */
-      linux_fork_detach (args, from_tty);
+      linux_fork_detach (from_tty);
     }
   else
     {
-      int signo;
-
       target_announce_detach (from_tty);
 
-      /* Pass on any pending signal for the last LWP, unless the user
-	 requested detaching with a different signal (most likely 0,
-	 meaning, discard the signal).  */
-      if (args != NULL)
-	signo = atoi (args);
-      else
-	signo = get_detach_signal (main_lwp);
+      /* Pass on any pending signal for the last LWP.  */
+      int signo = get_detach_signal (main_lwp);
 
       detach_one_lwp (main_lwp, &signo);
 
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 7fe6e2d..ed41640 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1090,13 +1090,13 @@ record_thread (struct thread_db_info *info,
 }
 
 static void
-thread_db_detach (struct target_ops *ops, const char *args, int from_tty)
+thread_db_detach (struct target_ops *ops, int from_tty)
 {
   struct target_ops *target_beneath = find_target_beneath (ops);
 
   delete_thread_db_info (ptid_get_pid (inferior_ptid));
 
-  target_beneath->to_detach (target_beneath, args, from_tty);
+  target_beneath->to_detach (target_beneath, from_tty);
 
   /* NOTE: From this point on, inferior_ptid is null_ptid.  */
 
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 04f2213..5094eb4 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -943,18 +943,14 @@ procfs_xfer_partial (struct target_ops *ops, enum target_object object,
    on signals, etc.  We'd better not have left any breakpoints
    in the program or it'll die when it hits one.  */
 static void
-procfs_detach (struct target_ops *ops, const char *args, int from_tty)
+procfs_detach (struct target_ops *ops, int from_tty)
 {
-  int siggnal = 0;
   int pid;
 
   target_announce_detach ();
 
-  if (args)
-    siggnal = atoi (args);
-
   if (siggnal)
-    SignalKill (nto_node (), ptid_get_pid (inferior_ptid), 0, siggnal, 0, 0);
+    SignalKill (nto_node (), ptid_get_pid (inferior_ptid), 0, 0, 0, 0);
 
   close (ctl_fd);
   ctl_fd = -1;
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 888dc69..f6f9ab0 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -1924,14 +1924,10 @@ procfs_attach (struct target_ops *ops, const char *args, int from_tty)
 }
 
 static void
-procfs_detach (struct target_ops *ops, const char *args, int from_tty)
+procfs_detach (struct target_ops *ops, int from_tty)
 {
-  int sig = 0;
   int pid = ptid_get_pid (inferior_ptid);
 
-  if (args)
-    sig = atoi (args);
-
   if (from_tty)
     {
       const char *exec_file;
@@ -1945,7 +1941,7 @@ procfs_detach (struct target_ops *ops, const char *args, int from_tty)
       gdb_flush (gdb_stdout);
     }
 
-  do_detach (sig);
+  do_detach (0);
 
   inferior_ptid = null_ptid;
   detach_inferior (pid);
diff --git a/gdb/record.c b/gdb/record.c
index 5b554d4..6e1af91 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -187,7 +187,7 @@ record_disconnect (struct target_ops *t, const char *args, int from_tty)
 /* See record.h.  */
 
 void
-record_detach (struct target_ops *t, const char *args, int from_tty)
+record_detach (struct target_ops *t, int from_tty)
 {
   gdb_assert (t->to_stratum == record_stratum);
 
@@ -196,7 +196,7 @@ record_detach (struct target_ops *t, const char *args, int from_tty)
   record_stop (t);
   record_unpush (t);
 
-  target_detach (args, from_tty);
+  target_detach (from_tty);
 }
 
 /* See record.h.  */
diff --git a/gdb/record.h b/gdb/record.h
index 3fc17e4..bee1142 100644
--- a/gdb/record.h
+++ b/gdb/record.h
@@ -88,7 +88,7 @@ extern void record_goto (const char *arg);
 extern void record_disconnect (struct target_ops *, const char *, int);
 
 /* The default "to_detach" target method for record targets.  */
-extern void record_detach (struct target_ops *, const char *, int);
+extern void record_detach (struct target_ops *, int);
 
 /* The default "to_mourn_inferior" target method for record targets.  */
 extern void record_mourn_inferior (struct target_ops *);
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index a684d32..8eaadcb 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -77,8 +77,7 @@ static void gdbsim_open (const char *args, int from_tty);
 
 static void gdbsim_close (struct target_ops *self);
 
-static void gdbsim_detach (struct target_ops *ops, const char *args,
-			   int from_tty);
+static void gdbsim_detach (struct target_ops *ops, int from_tty);
 
 static void gdbsim_prepare_to_store (struct target_ops *self,
 				     struct regcache *regcache);
@@ -815,17 +814,16 @@ gdbsim_close (struct target_ops *self)
 /* Takes a program previously attached to and detaches it.
    The program may resume execution (some targets do, some don't) and will
    no longer stop on signals, etc.  We better not have left any breakpoints
-   in the program or it'll die when it hits one.  ARGS is arguments
-   typed by the user (e.g. a signal to send the process).  FROM_TTY
-   says whether to be verbose or not.  */
+   in the program or it'll die when it hits one.  FROM_TTY says whether to be
+   verbose or not.  */
 /* Terminate the open connection to the remote debugger.
    Use this when you want to detach and do something else with your gdb.  */
 
 static void
-gdbsim_detach (struct target_ops *ops, const char *args, int from_tty)
+gdbsim_detach (struct target_ops *ops, int from_tty)
 {
   if (remote_debug)
-    fprintf_unfiltered (gdb_stdlog, "gdbsim_detach: args \"%s\"\n", args);
+    fprintf_unfiltered (gdb_stdlog, "gdbsim_detach\n");
 
   unpush_target (ops);		/* calls gdbsim_close to do the real work */
   if (from_tty)
diff --git a/gdb/remote.c b/gdb/remote.c
index 852fdef..3fcd279 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5125,16 +5125,13 @@ remote_detach_pid (int pid)
    one.  */
 
 static void
-remote_detach_1 (const char *args, int from_tty)
+remote_detach_1 (int from_tty)
 {
   int pid = ptid_get_pid (inferior_ptid);
   struct remote_state *rs = get_remote_state ();
   struct thread_info *tp = find_thread_ptid (inferior_ptid);
   int is_fork_parent;
 
-  if (args)
-    error (_("Argument given to \"detach\" when remotely debugging."));
-
   if (!target_has_execution)
     error (_("No process to detach from."));
 
@@ -5164,15 +5161,15 @@ remote_detach_1 (const char *args, int from_tty)
 }
 
 static void
-remote_detach (struct target_ops *ops, const char *args, int from_tty)
+remote_detach (struct target_ops *ops, int from_tty)
 {
-  remote_detach_1 (args, from_tty);
+  remote_detach_1 (from_tty);
 }
 
 static void
-extended_remote_detach (struct target_ops *ops, const char *args, int from_tty)
+extended_remote_detach (struct target_ops *ops, int from_tty)
 {
-  remote_detach_1 (args, from_tty);
+  remote_detach_1 (from_tty);
 }
 
 /* Target follow-fork function for remote targets.  On entry, and
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index 5f07a3c..4765ff3 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -348,14 +348,14 @@ lwp_to_thread (ptid_t lwp)
    program was started via the normal ptrace (PTRACE_TRACEME).  */
 
 static void
-sol_thread_detach (struct target_ops *ops, const char *args, int from_tty)
+sol_thread_detach (struct target_ops *ops, int from_tty)
 {
   struct target_ops *beneath = find_target_beneath (ops);
 
   sol_thread_active = 0;
   inferior_ptid = pid_to_ptid (ptid_get_pid (main_ph.ptid));
   unpush_target (ops);
-  beneath->to_detach (beneath, args, from_tty);
+  beneath->to_detach (beneath, from_tty);
 }
 
 /* Resume execution of process PTID.  If STEP is nozero, then just
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index aaf11d8..6bb7d7f 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -28,28 +28,26 @@ debug_post_attach (struct target_ops *self, int arg1)
 }
 
 static void
-delegate_detach (struct target_ops *self, const char *arg1, int arg2)
+delegate_detach (struct target_ops *self, int arg1)
 {
   self = self->beneath;
-  self->to_detach (self, arg1, arg2);
+  self->to_detach (self, arg1);
 }
 
 static void
-tdefault_detach (struct target_ops *self, const char *arg1, int arg2)
+tdefault_detach (struct target_ops *self, int arg1)
 {
 }
 
 static void
-debug_detach (struct target_ops *self, const char *arg1, int arg2)
+debug_detach (struct target_ops *self, int arg1)
 {
   fprintf_unfiltered (gdb_stdlog, "-> %s->to_detach (...)\n", debug_target.to_shortname);
-  debug_target.to_detach (&debug_target, arg1, arg2);
+  debug_target.to_detach (&debug_target, arg1);
   fprintf_unfiltered (gdb_stdlog, "<- %s->to_detach (", debug_target.to_shortname);
   target_debug_print_struct_target_ops_p (&debug_target);
   fputs_unfiltered (", ", gdb_stdlog);
-  target_debug_print_const_char_p (arg1);
-  fputs_unfiltered (", ", gdb_stdlog);
-  target_debug_print_int (arg2);
+  target_debug_print_int (arg1);
   fputs_unfiltered (")\n", gdb_stdlog);
 }
 
diff --git a/gdb/target.c b/gdb/target.c
index 767a2ad..728cab8 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2108,7 +2108,7 @@ dispose_inferior (struct inferior *inf, void *args)
       if (target_has_execution)
 	target_kill ();
       else
-	target_detach (NULL, 0);
+	target_detach (0);
     }
 
   return 0;
@@ -2141,10 +2141,10 @@ target_preopen (int from_tty)
   target_pre_inferior (from_tty);
 }
 
-/* Detach a target after doing deferred register stores.  */
+/* See target.h.  */
 
 void
-target_detach (const char *args, int from_tty)
+target_detach (int from_tty)
 {
   if (gdbarch_has_global_breakpoints (target_gdbarch ()))
     /* Don't remove global breakpoints here.  They're removed on
@@ -2157,7 +2157,7 @@ target_detach (const char *args, int from_tty)
 
   prepare_for_detach ();
 
-  current_target.to_detach (&current_target, args, from_tty);
+  current_target.to_detach (&current_target, from_tty);
 }
 
 void
diff --git a/gdb/target.h b/gdb/target.h
index f3774a2..efd0311 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -440,7 +440,7 @@ struct target_ops
     void (*to_attach) (struct target_ops *ops, const char *, int);
     void (*to_post_attach) (struct target_ops *, int)
       TARGET_DEFAULT_IGNORE ();
-    void (*to_detach) (struct target_ops *ops, const char *, int)
+    void (*to_detach) (struct target_ops *ops, int)
       TARGET_DEFAULT_IGNORE ();
     void (*to_disconnect) (struct target_ops *, const char *, int)
       TARGET_DEFAULT_NORETURN (tcomplain ());
@@ -1323,11 +1323,10 @@ extern void target_announce_detach (int from_tty);
 /* Takes a program previously attached to and detaches it.
    The program may resume execution (some targets do, some don't) and will
    no longer stop on signals, etc.  We better not have left any breakpoints
-   in the program or it'll die when it hits one.  ARGS is arguments
-   typed by the user (e.g. a signal to send the process).  FROM_TTY
-   says whether to be verbose or not.  */
+   in the program or it'll die when it hits one.  FROM_TTY says whether to be
+   verbose or not.  */
 
-extern void target_detach (const char *, int);
+extern void target_detach (int from_tty);
 
 /* Disconnect from the current target without resuming it (leaving it
    waiting for a debugger).  */
diff --git a/gdb/top.c b/gdb/top.c
index 3fb113c..61055cf 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1461,7 +1461,6 @@ set_prompt (const char *s)
 
 struct qt_args
 {
-  char *args;
   int from_tty;
 };
 
@@ -1486,7 +1485,7 @@ kill_or_detach (struct inferior *inf, void *args)
       if (target_has_execution)
 	{
 	  if (inf->attach_flag)
-	    target_detach (qt->args, qt->from_tty);
+	    target_detach (qt->from_tty);
 	  else
 	    target_kill ();
 	}
@@ -1577,7 +1576,6 @@ quit_force (int *exit_arg, int from_tty)
   else if (return_child_result)
     exit_code = return_child_result_value;
 
-  qt.args = NULL;
   qt.from_tty = from_tty;
 
   /* We want to handle any quit errors and exit regardless.  */
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index dc6dbdd..6a6cd08 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1931,7 +1931,7 @@ windows_attach (struct target_ops *ops, const char *args, int from_tty)
 }
 
 static void
-windows_detach (struct target_ops *ops, const char *args, int from_tty)
+windows_detach (struct target_ops *ops, int from_tty)
 {
   int detached = 1;
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/3] Pass inferior down to target_detach and to_detach
  2017-12-31  5:56 [PATCH 1/3] Remove args from target detach Simon Marchi
@ 2017-12-31  5:58 ` Simon Marchi
  2018-01-18 16:41   ` Pedro Alves
  2017-12-31  6:01 ` [PATCH 3/3] Make linux_nat_detach/thread_db_detach use the inferior parameter Simon Marchi
  2018-01-18 15:57 ` [PATCH 1/3] Remove args from target detach Pedro Alves
  2 siblings, 1 reply; 8+ messages in thread
From: Simon Marchi @ 2017-12-31  5:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

The to_detach target_ops method implementations are currently expected
to work on current_inferior/inferior_ptid.  In order to make things more
explicit, and remove some "shadow" parameter passing through globals,
this patch adds an "inferior" parameter to to_detach.  Implementations
will be expected to use this instead of relying on the global.  However,
to keep things simple, this patch only does the minimum that is
necessary to add the parameter.  The following patch gives an example of
how one such implementation would be adapted.  If the approach is deemed
good, we can then look into adapting more implementations.  Until then,
they'll continue to work as they do currently.

gdb/ChangeLog:

	* target.h (struct target_ops) <to_detach>: Add inferior
	parameter.
	(target_detach): Likewise.
	* target.c (dispose_inferior): Pass inferior down.
	(target_detach): Likewise.
	* aix-thread.c (aix_thread_detach): Likewise.
	* corefile.c (core_file_command): Pass current_inferior() down.
	* corelow.c (core_detach): Add inferior parameter.
	* darwin-nat.c (darwin_detach): Likewise.
	* gnu-nat.c (gnu_detach): Likewise.
	* inf-ptrace.c (inf_ptrace_detach): Likewise.
	* infcmd.c (detach_command): Pass current_inferior() down to
	target_detach.
	* infrun.c (follow_fork_inferior): Pass parent_inf to
	target_detach.
	(handle_vfork_child_exec_or_exit): Pass inf->vfork_parent to
	target_detach.
	* linux-nat.c (linux_nat_detach): Add inferior parameter.
	* linux-thread-db.c (thread_db_detach): Likewise.
	* nto-procfs.c (procfs_detach): Likewise.
	* procfs.c (procfs_detach): Likewise.
	* record.c (record_detach): Likewise.
	* record.h (struct inferior): Forward-declare.
	(record_detach): Add inferior parameter.
	* remote-sim.c (gdbsim_detach): Likewise.
	* remote.c (remote_detach_1): Likewise.
	(remote_detach): Likewise.
	(extended_remote_detach): Likewise.
	* sol-thread.c (sol_thread_detach): Likewise.
	* target-debug.h (target_debug_print_inferior_p): New macro.
	* target-delegates.c: Re-generate.
	* top.c (kill_or_detach): Pass inferior down to target_detach.
	* windows-nat.c (windows_detach): Add inferior parameter.
---
 gdb/aix-thread.c       |  4 ++--
 gdb/corefile.c         |  2 +-
 gdb/corelow.c          |  2 +-
 gdb/darwin-nat.c       |  3 +--
 gdb/gnu-nat.c          |  2 +-
 gdb/inf-ptrace.c       |  2 +-
 gdb/infcmd.c           |  2 +-
 gdb/infrun.c           |  4 ++--
 gdb/linux-nat.c        |  2 +-
 gdb/linux-thread-db.c  |  4 ++--
 gdb/nto-procfs.c       |  2 +-
 gdb/procfs.c           |  2 +-
 gdb/record.c           |  4 ++--
 gdb/record.h           |  3 ++-
 gdb/remote-sim.c       |  4 ++--
 gdb/remote.c           | 10 +++++-----
 gdb/sol-thread.c       |  4 ++--
 gdb/target-debug.h     |  2 ++
 gdb/target-delegates.c | 14 ++++++++------
 gdb/target.c           |  6 +++---
 gdb/target.h           |  4 ++--
 gdb/top.c              |  2 +-
 gdb/windows-nat.c      |  2 +-
 23 files changed, 45 insertions(+), 41 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index adb3a83..242df11 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -981,12 +981,12 @@ aix_thread_inferior_created (struct target_ops *ops, int from_tty)
 /* Detach from the process attached to by aix_thread_attach().  */
 
 static void
-aix_thread_detach (struct target_ops *ops, int from_tty)
+aix_thread_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   struct target_ops *beneath = find_target_beneath (ops);
 
   pd_disable ();
-  beneath->to_detach (beneath, from_tty);
+  beneath->to_detach (beneath, inf, from_tty);
 }
 
 /* Tell the inferior process to continue running thread PID if != -1
diff --git a/gdb/corefile.c b/gdb/corefile.c
index 434c64a..c4e0bad 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -68,7 +68,7 @@ core_file_command (const char *filename, int from_tty)
   gdb_assert (core_target != NULL);
 
   if (!filename)
-    (core_target->to_detach) (core_target, from_tty);
+    (core_target->to_detach) (core_target, current_inferior (), from_tty);
   else
     (core_target->to_open) (filename, from_tty);
 }
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 2894c96..0f7da4b 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -464,7 +464,7 @@ core_open (const char *arg, int from_tty)
 }
 
 static void
-core_detach (struct target_ops *ops, int from_tty)
+core_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   unpush_target (ops);
   reinit_frame_cache ();
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 1b64536..cf02bad 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1938,10 +1938,9 @@ darwin_attach (struct target_ops *ops, const char *args, int from_tty)
    previously attached.  It *might* work if the program was
    started via fork.  */
 static void
-darwin_detach (struct target_ops *ops, int from_tty)
+darwin_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   pid_t pid = ptid_get_pid (inferior_ptid);
-  struct inferior *inf = current_inferior ();
   darwin_inferior *priv = get_darwin_inferior (inf);
   kern_return_t kret;
   int res;
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index caa8c3d..db79bab 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2253,7 +2253,7 @@ gnu_attach (struct target_ops *ops, const char *args, int from_tty)
    previously attached.  It *might* work if the program was
    started via fork.  */
 static void
-gnu_detach (struct target_ops *ops, int from_tty)
+gnu_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   int pid;
 
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 8633108..766a660 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -244,7 +244,7 @@ inf_ptrace_post_attach (struct target_ops *self, int pid)
 /* Detach from the inferior.  If FROM_TTY is non-zero, be chatty about it.  */
 
 static void
-inf_ptrace_detach (struct target_ops *ops, int from_tty)
+inf_ptrace_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   pid_t pid = ptid_get_pid (inferior_ptid);
 
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 3aa2ced..e5bceb4 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2976,7 +2976,7 @@ detach_command (const char *args, int from_tty)
 
   disconnect_tracing ();
 
-  target_detach (from_tty);
+  target_detach (current_inferior (), from_tty);
 
   /* The current inferior process was just detached successfully.  Get
      rid of breakpoints that no longer make sense.  Note we don't do
diff --git a/gdb/infrun.c b/gdb/infrun.c
index c4d168c..7d71ad5 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -606,7 +606,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 				target_pid_to_str (process_ptid));
 	    }
 
-	  target_detach (0);
+	  target_detach (parent_inf, 0);
 	}
 
       /* Note that the detach above makes PARENT_INF dangling.  */
@@ -976,7 +976,7 @@ handle_vfork_child_exec_or_exit (int exec)
 		}
 	    }
 
-	  target_detach (0);
+	  target_detach (inf->vfork_parent, 0);
 
 	  /* Put it back.  */
 	  inf->pspace = pspace;
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 3290ed5..9a54397 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1507,7 +1507,7 @@ detach_callback (struct lwp_info *lp, void *data)
 }
 
 static void
-linux_nat_detach (struct target_ops *ops, int from_tty)
+linux_nat_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   int pid;
   struct lwp_info *main_lwp;
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index ed41640..ddb010e 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1090,13 +1090,13 @@ record_thread (struct thread_db_info *info,
 }
 
 static void
-thread_db_detach (struct target_ops *ops, int from_tty)
+thread_db_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   struct target_ops *target_beneath = find_target_beneath (ops);
 
   delete_thread_db_info (ptid_get_pid (inferior_ptid));
 
-  target_beneath->to_detach (target_beneath, from_tty);
+  target_beneath->to_detach (target_beneath, inf, from_tty);
 
   /* NOTE: From this point on, inferior_ptid is null_ptid.  */
 
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 5094eb4..a56a7ce 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -943,7 +943,7 @@ procfs_xfer_partial (struct target_ops *ops, enum target_object object,
    on signals, etc.  We'd better not have left any breakpoints
    in the program or it'll die when it hits one.  */
 static void
-procfs_detach (struct target_ops *ops, int from_tty)
+procfs_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   int pid;
 
diff --git a/gdb/procfs.c b/gdb/procfs.c
index f6f9ab0..cc8dce4 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -1924,7 +1924,7 @@ procfs_attach (struct target_ops *ops, const char *args, int from_tty)
 }
 
 static void
-procfs_detach (struct target_ops *ops, int from_tty)
+procfs_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   int pid = ptid_get_pid (inferior_ptid);
 
diff --git a/gdb/record.c b/gdb/record.c
index 6e1af91..fad28cd 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -187,7 +187,7 @@ record_disconnect (struct target_ops *t, const char *args, int from_tty)
 /* See record.h.  */
 
 void
-record_detach (struct target_ops *t, int from_tty)
+record_detach (struct target_ops *t, inferior *inf, int from_tty)
 {
   gdb_assert (t->to_stratum == record_stratum);
 
@@ -196,7 +196,7 @@ record_detach (struct target_ops *t, int from_tty)
   record_stop (t);
   record_unpush (t);
 
-  target_detach (from_tty);
+  target_detach (inf, from_tty);
 }
 
 /* See record.h.  */
diff --git a/gdb/record.h b/gdb/record.h
index bee1142..3f6583d 100644
--- a/gdb/record.h
+++ b/gdb/record.h
@@ -24,6 +24,7 @@
 #include "common/enum-flags.h"
 
 struct cmd_list_element;
+struct inferior;
 
 extern unsigned int record_debug;
 
@@ -88,7 +89,7 @@ extern void record_goto (const char *arg);
 extern void record_disconnect (struct target_ops *, const char *, int);
 
 /* The default "to_detach" target method for record targets.  */
-extern void record_detach (struct target_ops *, int);
+extern void record_detach (struct target_ops *, inferior *, int);
 
 /* The default "to_mourn_inferior" target method for record targets.  */
 extern void record_mourn_inferior (struct target_ops *);
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index 8eaadcb..7e586fa 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -77,7 +77,7 @@ static void gdbsim_open (const char *args, int from_tty);
 
 static void gdbsim_close (struct target_ops *self);
 
-static void gdbsim_detach (struct target_ops *ops, int from_tty);
+static void gdbsim_detach (struct target_ops *ops, inferior *inf, int from_tty);
 
 static void gdbsim_prepare_to_store (struct target_ops *self,
 				     struct regcache *regcache);
@@ -820,7 +820,7 @@ gdbsim_close (struct target_ops *self)
    Use this when you want to detach and do something else with your gdb.  */
 
 static void
-gdbsim_detach (struct target_ops *ops, int from_tty)
+gdbsim_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   if (remote_debug)
     fprintf_unfiltered (gdb_stdlog, "gdbsim_detach\n");
diff --git a/gdb/remote.c b/gdb/remote.c
index 3fcd279..90eabd8 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5125,7 +5125,7 @@ remote_detach_pid (int pid)
    one.  */
 
 static void
-remote_detach_1 (int from_tty)
+remote_detach_1 (int from_tty, inferior *inf)
 {
   int pid = ptid_get_pid (inferior_ptid);
   struct remote_state *rs = get_remote_state ();
@@ -5161,15 +5161,15 @@ remote_detach_1 (int from_tty)
 }
 
 static void
-remote_detach (struct target_ops *ops, int from_tty)
+remote_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
-  remote_detach_1 (from_tty);
+  remote_detach_1 (from_tty, inf);
 }
 
 static void
-extended_remote_detach (struct target_ops *ops, int from_tty)
+extended_remote_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
-  remote_detach_1 (from_tty);
+  remote_detach_1 (from_tty, inf);
 }
 
 /* Target follow-fork function for remote targets.  On entry, and
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index 4765ff3..acf0da2 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -348,14 +348,14 @@ lwp_to_thread (ptid_t lwp)
    program was started via the normal ptrace (PTRACE_TRACEME).  */
 
 static void
-sol_thread_detach (struct target_ops *ops, int from_tty)
+sol_thread_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   struct target_ops *beneath = find_target_beneath (ops);
 
   sol_thread_active = 0;
   inferior_ptid = pid_to_ptid (ptid_get_pid (main_ph.ptid));
   unpush_target (ops);
-  beneath->to_detach (beneath, from_tty);
+  beneath->to_detach (beneath, inf, from_tty);
 }
 
 /* Resume execution of process PTID.  If STEP is nozero, then just
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index 27e5a55..d8d2c48 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -170,6 +170,8 @@
   target_debug_do_print (host_address_to_string (X.get ()))
 #define target_debug_print_gdb_array_view_const_int(X)	\
   target_debug_do_print (host_address_to_string (X.data ()))
+#define target_debug_print_inferior_p(inf) \
+  target_debug_do_print (host_address_to_string (inf))
 
 static void
 target_debug_print_struct_target_waitstatus_p (struct target_waitstatus *status)
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 6bb7d7f..bc84791 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -28,26 +28,28 @@ debug_post_attach (struct target_ops *self, int arg1)
 }
 
 static void
-delegate_detach (struct target_ops *self, int arg1)
+delegate_detach (struct target_ops *self, inferior *arg1, int arg2)
 {
   self = self->beneath;
-  self->to_detach (self, arg1);
+  self->to_detach (self, arg1, arg2);
 }
 
 static void
-tdefault_detach (struct target_ops *self, int arg1)
+tdefault_detach (struct target_ops *self, inferior *arg1, int arg2)
 {
 }
 
 static void
-debug_detach (struct target_ops *self, int arg1)
+debug_detach (struct target_ops *self, inferior *arg1, int arg2)
 {
   fprintf_unfiltered (gdb_stdlog, "-> %s->to_detach (...)\n", debug_target.to_shortname);
-  debug_target.to_detach (&debug_target, arg1);
+  debug_target.to_detach (&debug_target, arg1, arg2);
   fprintf_unfiltered (gdb_stdlog, "<- %s->to_detach (", debug_target.to_shortname);
   target_debug_print_struct_target_ops_p (&debug_target);
   fputs_unfiltered (", ", gdb_stdlog);
-  target_debug_print_int (arg1);
+  target_debug_print_inferior_p (arg1);
+  fputs_unfiltered (", ", gdb_stdlog);
+  target_debug_print_int (arg2);
   fputs_unfiltered (")\n", gdb_stdlog);
 }
 
diff --git a/gdb/target.c b/gdb/target.c
index 728cab8..a9ee175 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2108,7 +2108,7 @@ dispose_inferior (struct inferior *inf, void *args)
       if (target_has_execution)
 	target_kill ();
       else
-	target_detach (0);
+	target_detach (inf, 0);
     }
 
   return 0;
@@ -2144,7 +2144,7 @@ target_preopen (int from_tty)
 /* See target.h.  */
 
 void
-target_detach (int from_tty)
+target_detach (inferior *inf, int from_tty)
 {
   if (gdbarch_has_global_breakpoints (target_gdbarch ()))
     /* Don't remove global breakpoints here.  They're removed on
@@ -2157,7 +2157,7 @@ target_detach (int from_tty)
 
   prepare_for_detach ();
 
-  current_target.to_detach (&current_target, from_tty);
+  current_target.to_detach (&current_target, inf, from_tty);
 }
 
 void
diff --git a/gdb/target.h b/gdb/target.h
index efd0311..3ed6350 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -440,7 +440,7 @@ struct target_ops
     void (*to_attach) (struct target_ops *ops, const char *, int);
     void (*to_post_attach) (struct target_ops *, int)
       TARGET_DEFAULT_IGNORE ();
-    void (*to_detach) (struct target_ops *ops, int)
+    void (*to_detach) (struct target_ops *ops, inferior *, int)
       TARGET_DEFAULT_IGNORE ();
     void (*to_disconnect) (struct target_ops *, const char *, int)
       TARGET_DEFAULT_NORETURN (tcomplain ());
@@ -1326,7 +1326,7 @@ extern void target_announce_detach (int from_tty);
    in the program or it'll die when it hits one.  FROM_TTY says whether to be
    verbose or not.  */
 
-extern void target_detach (int from_tty);
+extern void target_detach (inferior *inf, int from_tty);
 
 /* Disconnect from the current target without resuming it (leaving it
    waiting for a debugger).  */
diff --git a/gdb/top.c b/gdb/top.c
index 61055cf..2c78e4e 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1485,7 +1485,7 @@ kill_or_detach (struct inferior *inf, void *args)
       if (target_has_execution)
 	{
 	  if (inf->attach_flag)
-	    target_detach (qt->from_tty);
+	    target_detach (inf, qt->from_tty);
 	  else
 	    target_kill ();
 	}
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 6a6cd08..266f710 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1931,7 +1931,7 @@ windows_attach (struct target_ops *ops, const char *args, int from_tty)
 }
 
 static void
-windows_detach (struct target_ops *ops, int from_tty)
+windows_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   int detached = 1;
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/3] Make linux_nat_detach/thread_db_detach use the inferior parameter
  2017-12-31  5:56 [PATCH 1/3] Remove args from target detach Simon Marchi
  2017-12-31  5:58 ` [PATCH 2/3] Pass inferior down to target_detach and to_detach Simon Marchi
@ 2017-12-31  6:01 ` Simon Marchi
  2018-01-18 17:06   ` Pedro Alves
  2018-01-18 15:57 ` [PATCH 1/3] Remove args from target detach Pedro Alves
  2 siblings, 1 reply; 8+ messages in thread
From: Simon Marchi @ 2017-12-31  6:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

This patch makes these two functions actually use the inferior parameter
added by the previous patch, instead of reading inferior_ptid.  I chose
these two, because they are the one actually used when I detach on my
GNU/Linux system, so they were easy to test.

I took the opportunity to pass the inferior being detached to
inf_ptrace_detach_success, so it could use it too.  From there, it made
sense to add an overload of detach_inferior that takes the inferior
directly rather than the pid, to avoid having to pass inf->pid only for
the callee to look up the inferior structure by pid.

gdb/ChangeLog:

	* inf-ptrace.c (inf_ptrace_detach): Adjust call to
	inf_ptrace_detach_success.
	(inf_ptrace_detach_success): Add inferior parameter, use it
	instead of inferior_ptid, pass it to detach_inferior.
	* inf-ptrace.h (inf_ptrace_detach_success): Add inferior
	parameter.
	* inferior.c (detach_inferior): Add overload that takes an
	inferior object.
	* inferior.h (detach_inferior): Likewise.
	* linux-nat.c (linux_nat_detach): Use the inf parameter, don't
	use inferior_ptid, adjust call to inf_ptrace_detach_success.
	* linux-thread-db.c (thread_db_detach): Use inf parameter.
---
 gdb/inf-ptrace.c      |  8 +++-----
 gdb/inf-ptrace.h      |  2 +-
 gdb/inferior.c        | 15 +++++++++++++--
 gdb/inferior.h        |  3 +++
 gdb/linux-nat.c       |  8 +++-----
 gdb/linux-thread-db.c |  2 +-
 6 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 766a660..c6318d4 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -263,18 +263,16 @@ inf_ptrace_detach (struct target_ops *ops, inferior *inf, int from_tty)
   error (_("This system does not support detaching from a process"));
 #endif
 
-  inf_ptrace_detach_success (ops);
+  inf_ptrace_detach_success (ops, inf);
 }
 
 /* See inf-ptrace.h.  */
 
 void
-inf_ptrace_detach_success (struct target_ops *ops)
+inf_ptrace_detach_success (struct target_ops *ops, inferior *inf)
 {
-  pid_t pid = ptid_get_pid (inferior_ptid);
-
   inferior_ptid = null_ptid;
-  detach_inferior (pid);
+  detach_inferior (inf);
 
   inf_child_maybe_unpush_target (ops);
 }
diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h
index 3f27efe..61dc091 100644
--- a/gdb/inf-ptrace.h
+++ b/gdb/inf-ptrace.h
@@ -40,6 +40,6 @@ extern pid_t get_ptrace_pid (ptid_t);
 
 
 /* Cleanup the inferior after a successful ptrace detach.  */
-extern void inf_ptrace_detach_success (struct target_ops *ops);
+extern void inf_ptrace_detach_success (struct target_ops *ops, inferior *inf);
 
 #endif
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 9b3043d..2db358e 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -253,10 +253,13 @@ exit_inferior_num_silent (int num)
   exit_inferior_1 (inf, 1);
 }
 
+/* See inferior.h.  */
+
 void
-detach_inferior (int pid)
+detach_inferior (inferior *inf)
 {
-  struct inferior *inf = find_inferior_pid (pid);
+  /* Save the pid, since exit_inferior_1 will reset it.  */
+  int pid = inf->pid;
 
   exit_inferior_1 (inf, 0);
 
@@ -264,6 +267,14 @@ detach_inferior (int pid)
     printf_unfiltered (_("[Inferior %d detached]\n"), pid);
 }
 
+/* See inferior.h.  */
+
+void
+detach_inferior (int pid)
+{
+  detach_inferior (find_inferior_pid (pid));
+}
+
 void
 inferior_appeared (struct inferior *inf, int pid)
 {
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 0705dd9..dd4addf 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -458,6 +458,9 @@ extern struct inferior *add_inferior_silent (int pid);
 extern void delete_inferior (struct inferior *todel);
 
 /* Delete an existing inferior list entry, due to inferior detaching.  */
+extern void detach_inferior (inferior *inf);
+
+/* Same as the above, but with the inferior specified by PID.  */
 extern void detach_inferior (int pid);
 
 extern void exit_inferior (int pid);
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 9a54397..a8793d8 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1509,10 +1509,8 @@ detach_callback (struct lwp_info *lp, void *data)
 static void
 linux_nat_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
-  int pid;
   struct lwp_info *main_lwp;
-
-  pid = ptid_get_pid (inferior_ptid);
+  int pid = inf->pid;
 
   /* Don't unregister from the event loop, as there may be other
      inferiors running. */
@@ -1527,7 +1525,7 @@ linux_nat_detach (struct target_ops *ops, inferior *inf, int from_tty)
   iterate_over_lwps (pid_to_ptid (pid), detach_callback, NULL);
 
   /* Only the initial process should be left right now.  */
-  gdb_assert (num_lwps (ptid_get_pid (inferior_ptid)) == 1);
+  gdb_assert (num_lwps (pid) == 1);
 
   main_lwp = find_lwp_pid (pid_to_ptid (pid));
 
@@ -1548,7 +1546,7 @@ linux_nat_detach (struct target_ops *ops, inferior *inf, int from_tty)
 
       detach_one_lwp (main_lwp, &signo);
 
-      inf_ptrace_detach_success (ops);
+      inf_ptrace_detach_success (ops, inf);
     }
 }
 
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index ddb010e..eb5e49b 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1094,7 +1094,7 @@ thread_db_detach (struct target_ops *ops, inferior *inf, int from_tty)
 {
   struct target_ops *target_beneath = find_target_beneath (ops);
 
-  delete_thread_db_info (ptid_get_pid (inferior_ptid));
+  delete_thread_db_info (inf->pid);
 
   target_beneath->to_detach (target_beneath, inf, from_tty);
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] Remove args from target detach
  2017-12-31  5:56 [PATCH 1/3] Remove args from target detach Simon Marchi
  2017-12-31  5:58 ` [PATCH 2/3] Pass inferior down to target_detach and to_detach Simon Marchi
  2017-12-31  6:01 ` [PATCH 3/3] Make linux_nat_detach/thread_db_detach use the inferior parameter Simon Marchi
@ 2018-01-18 15:57 ` Pedro Alves
  2 siblings, 0 replies; 8+ messages in thread
From: Pedro Alves @ 2018-01-18 15:57 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 12/31/2017 05:50 AM, Simon Marchi wrote:
> I was looking into adding a parameter to target_detach, and was
> wondering what the args parameter was.  It seems like in the distant
> past, it was possible to specify a signal number when detaching.  That
> signal was injected in the process before it was detached.  There is an
> example of code handling this in linux_nat_detach.  With today's GDB, I
> can't get this to work.  Doing "detach 15" (15 == SIGTERM) doesn't work,
> because detach is a prefix command and doesn't recognize the sub-command
> 15.  Doing "detach inferiors 15" doesn't work because it expects a list
> of inferior id to detach.  Therefore, I don't think there's a way of
> invoking detach_command with a non-NULL args.  I also didn't find any
> documentation related to this feature.
> 
> I assume that this feature stopped working when detach was made a prefix
> command, which is in f73adfeb8bae36885e6ea248d12223ab0d5eb9cb (sorry,
> there's no commit title) from 2006.  Given that this feature was broken
> for such a long time and we haven't heard anything (AFAIK, I did not
> find any related bug), I think it's safe to remove it, as well as the
> args parameter to target_detach.  If someone wants to re-introduce it, I
> would suggest rethinking the user interface, and in particular would
> suggest using signal name instead of numbers.
> 
> I tried to fix all the impacted code, but I might have forgotten some
> spots.  It shouldn't be hard to fix if that's the case.  I also couldn't
> build-test everything I changed, especially the nto and solaris stuff.
> 

Eh.  I knew about "detach SIG", from touching the detach-related
code many times before.  Had no idea that it was broken.

I think the main use cases are/were:

 - To be able to detach without passing the just-intercepted signal
   to the program.  I.e., "detach 0" was to "detach", like "signal 0"
   is to "continue".

 - To be able to attach to a program (maybe job-stopped), debug it
   a bit, and then detach it, leaving it job-stopped.  I.e., queue
   a SIGSTOP when you detach, with "detach SIGTOP".

I think you can do the former today with either:
  "handle SIGFOO nopass" + "detach"
  "queue-signal 0" + "detach"

and the latter with either:
  "queue-signal SIGSTOP" + "detach"
  "shell kill -SIGSTOP $PID" + "detach"

The "shell kill" variant is a bit more cumbersome, of
course, and only works with native debugging.

We're probably missing testcases for the above scenarios.
IWBN to add them, of course.

The patch looks fine to me.  One nit:

>  static void
> -procfs_detach (struct target_ops *ops, const char *args, int from_tty)
> +procfs_detach (struct target_ops *ops, int from_tty)
>  {
> -  int sig = 0;
>    int pid = ptid_get_pid (inferior_ptid);
>  
> -  if (args)
> -    sig = atoi (args);
> -
>    if (from_tty)
>      {
>        const char *exec_file;
> @@ -1945,7 +1941,7 @@ procfs_detach (struct target_ops *ops, const char *args, int from_tty)
>        gdb_flush (gdb_stdout);
>      }
>  
> -  do_detach (sig);
> +  do_detach (0);

I think the 'signo' parameter of do_detach is now always
0, so it could be removed.  I understand that you might
want to avoid breaking the port (too much), but it looks
very easy to remove.

>  
>    inferior_ptid = null_ptid;
>    detach_inferior (pid);

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] Pass inferior down to target_detach and to_detach
  2017-12-31  5:58 ` [PATCH 2/3] Pass inferior down to target_detach and to_detach Simon Marchi
@ 2018-01-18 16:41   ` Pedro Alves
  2018-01-19  3:17     ` Simon Marchi
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2018-01-18 16:41 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 12/31/2017 05:50 AM, Simon Marchi wrote:
> The to_detach target_ops method implementations are currently expected
> to work on current_inferior/inferior_ptid.  In order to make things more
> explicit, and remove some "shadow" parameter passing through globals,
> this patch adds an "inferior" parameter to to_detach.  Implementations
> will be expected to use this instead of relying on the global.  However,
> to keep things simple, this patch only does the minimum that is
> necessary to add the parameter.  The following patch gives an example of
> how one such implementation would be adapted.  If the approach is deemed
> good, we can then look into adapting more implementations.  Until then,
> they'll continue to work as they do currently.

On the multi-target work/branch, I ended up hanging the current
target stack to the current inferior.  Which means that in practice
we have to switch the current inferior/thread before calling any target
method.  I can't see any other practical way.  I've pondered making
every target method take an inferior or some kind of "execution
context" object as parameter, but that's a massive undertaking.
The result is still better for relying more on thread pointers
instead of inferior_ptid / ptid_t, though, IMO.

Note that in practice, even with your patch (in master) we still have
to switch the current inferior before calling target_detach anyway,
for making sure things like the current program space is set correctly,
target_gdbarch(), target description, removing breakpoints, accessing memory,
registers, etc., like here:

>        /* Note that the detach above makes PARENT_INF dangling.  */
> @@ -976,7 +976,7 @@ handle_vfork_child_exec_or_exit (int exec)
>  		}
>  	    }
>  
> -	  target_detach (0);
> +	  target_detach (inf->vfork_parent, 0);

... this still relies on the switch_to_thread call a bit above.

Or look at the prepare_to_detach call in target_detach.

All that said, I'm totally fine with incremental progress.  Actually,
it's probably the only way to go!

So I'm fine with the patch, though, I think we should add a
temporary assertion in target_detach, like:

  gdb_assert (inf == current_inferior ());

until everything beneath either uses an explicit inferior,
or switches the current inferior temporarily.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] Make linux_nat_detach/thread_db_detach use the inferior parameter
  2017-12-31  6:01 ` [PATCH 3/3] Make linux_nat_detach/thread_db_detach use the inferior parameter Simon Marchi
@ 2018-01-18 17:06   ` Pedro Alves
  2018-01-19  3:26     ` Simon Marchi
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2018-01-18 17:06 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 12/31/2017 05:50 AM, Simon Marchi wrote:
> This patch makes these two functions actually use the inferior parameter
> added by the previous patch, instead of reading inferior_ptid.  I chose
> these two, because they are the one actually used when I detach on my
> GNU/Linux system, so they were easy to test.
> 
> I took the opportunity to pass the inferior being detached to
> inf_ptrace_detach_success, so it could use it too.  From there, it made
> sense to add an overload of detach_inferior that takes the inferior
> directly rather than the pid, to avoid having to pass inf->pid only for
> the callee to look up the inferior structure by pid.

Looks fine to me.

(FWIW, the 'detach_inferior(int)' overload disappears in
my multi-target branch).

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] Pass inferior down to target_detach and to_detach
  2018-01-18 16:41   ` Pedro Alves
@ 2018-01-19  3:17     ` Simon Marchi
  0 siblings, 0 replies; 8+ messages in thread
From: Simon Marchi @ 2018-01-19  3:17 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Simon Marchi, gdb-patches

On 2018-01-18 11:41, Pedro Alves wrote:
> On 12/31/2017 05:50 AM, Simon Marchi wrote:
>> The to_detach target_ops method implementations are currently expected
>> to work on current_inferior/inferior_ptid.  In order to make things 
>> more
>> explicit, and remove some "shadow" parameter passing through globals,
>> this patch adds an "inferior" parameter to to_detach.  Implementations
>> will be expected to use this instead of relying on the global.  
>> However,
>> to keep things simple, this patch only does the minimum that is
>> necessary to add the parameter.  The following patch gives an example 
>> of
>> how one such implementation would be adapted.  If the approach is 
>> deemed
>> good, we can then look into adapting more implementations.  Until 
>> then,
>> they'll continue to work as they do currently.
> 
> On the multi-target work/branch, I ended up hanging the current
> target stack to the current inferior.  Which means that in practice
> we have to switch the current inferior/thread before calling any target
> method.  I can't see any other practical way.  I've pondered making
> every target method take an inferior or some kind of "execution
> context" object as parameter, but that's a massive undertaking.
> The result is still better for relying more on thread pointers
> instead of inferior_ptid / ptid_t, though, IMO.

Ok.

> Note that in practice, even with your patch (in master) we still have
> to switch the current inferior before calling target_detach anyway,
> for making sure things like the current program space is set correctly,
> target_gdbarch(), target description, removing breakpoints, accessing 
> memory,
> registers, etc., like here:
> 
>>        /* Note that the detach above makes PARENT_INF dangling.  */
>> @@ -976,7 +976,7 @@ handle_vfork_child_exec_or_exit (int exec)
>>  		}
>>  	    }
>> 
>> -	  target_detach (0);
>> +	  target_detach (inf->vfork_parent, 0);
> 
> ... this still relies on the switch_to_thread call a bit above.
> 
> Or look at the prepare_to_detach call in target_detach.
> 
> All that said, I'm totally fine with incremental progress.  Actually,
> it's probably the only way to go!

Yes this is the idea.  The ramifications go very deep, so it's hard to 
know what still relies on the current inferior being set.  This is just 
one step in the right direction, and it's easier to take smaller steps.

> So I'm fine with the patch, though, I think we should add a
> temporary assertion in target_detach, like:
> 
>   gdb_assert (inf == current_inferior ());
> 
> until everything beneath either uses an explicit inferior,
> or switches the current inferior temporarily.

That's a good idea, this way it will also check if I passed the wrong 
thing to target_detach.

Simon

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] Make linux_nat_detach/thread_db_detach use the inferior parameter
  2018-01-18 17:06   ` Pedro Alves
@ 2018-01-19  3:26     ` Simon Marchi
  0 siblings, 0 replies; 8+ messages in thread
From: Simon Marchi @ 2018-01-19  3:26 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Simon Marchi, gdb-patches

On 2018-01-18 12:06, Pedro Alves wrote:
> On 12/31/2017 05:50 AM, Simon Marchi wrote:
>> This patch makes these two functions actually use the inferior 
>> parameter
>> added by the previous patch, instead of reading inferior_ptid.  I 
>> chose
>> these two, because they are the one actually used when I detach on my
>> GNU/Linux system, so they were easy to test.
>> 
>> I took the opportunity to pass the inferior being detached to
>> inf_ptrace_detach_success, so it could use it too.  From there, it 
>> made
>> sense to add an overload of detach_inferior that takes the inferior
>> directly rather than the pid, to avoid having to pass inf->pid only 
>> for
>> the callee to look up the inferior structure by pid.
> 
> Looks fine to me.
> 
> (FWIW, the 'detach_inferior(int)' overload disappears in
> my multi-target branch).

Ah, good to know it goes in the same direction.

I'm sending the series through the buildbot again (see if that new 
assert is hit somewhere), and will send a v2 tomorrow if all goes well.

Thanks for reviewing.

Simon

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-01-19  3:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-31  5:56 [PATCH 1/3] Remove args from target detach Simon Marchi
2017-12-31  5:58 ` [PATCH 2/3] Pass inferior down to target_detach and to_detach Simon Marchi
2018-01-18 16:41   ` Pedro Alves
2018-01-19  3:17     ` Simon Marchi
2017-12-31  6:01 ` [PATCH 3/3] Make linux_nat_detach/thread_db_detach use the inferior parameter Simon Marchi
2018-01-18 17:06   ` Pedro Alves
2018-01-19  3:26     ` Simon Marchi
2018-01-18 15:57 ` [PATCH 1/3] Remove args from target detach Pedro Alves

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).