gdb: on exec, delegate pushing / unpushing target and adding thread to target_ops...
authorSimon Marchi <simon.marchi@efficios.com>
Thu, 13 May 2021 19:28:20 +0000 (15:28 -0400)
committerSimon Marchi <simon.marchi@polymtl.ca>
Thu, 13 May 2021 19:29:00 +0000 (15:29 -0400)
On "exec", some targets need to unpush themselves from the inferior,
and do some bookkeeping, like forgetting the data associated to the
exec'ing inferior.

One such example is the thread-db target.  It does so in
a special case in thread_db_target::wait, just before returning the
TARGET_WAITKIND_EXECD event to its caller.

We have another such case in the context of rocm-gdb [1], where the
"rocm" target is pushed on top of the linux-nat target.  When an exec
happens, we want to unpush the rocm target from the exec'ing inferior to
close some file descriptors that refer to the pre-exec address space and
forget about that inferior.  We then want to push the target on the
inferior in which execution continues, to open the file descriptors for
the post-exec address space.

I think that a good way to address this cleanly is to do all this in the
target_ops::follow_exec implementations.  Make the
process_stratum_target::follow_exec implementation have the default
behavior of pushing itself to the new inferior's target stack (if
execution continues in a new inferior) and add the initial thread.

remote_target::follow_exec is an example of process target that wants to
do a bit more than the default behavior.  So it calls
process_stratum_target::follow_exec first and does the extra work
second.

linux-thread-db (a non-process target) implements follow_exec to do some
bookeeping (forget about that process' data), before handing down the
event down to the process target (which hits
process_stratum_target::follow_exec).

gdb/ChangeLog:

* target.h (struct target_ops) <follow_exec>: Add ptid_t
parameter.
(target_follow_exec): Likewise.
* target.c (target_follow_exec): Add ptid_t parameter.
* infrun.c (follow_exec): Adjust call to target_follow_exec,
don't push target nor create thread.
* linux-thread-db.c (class thread_db_target) <follow_exec>: New.
(thread_db_target::wait): Just return on TARGET_WAITKIND_EXECD.
(thread_db_target::follow_exec): New.
* remote.c (class remote_target) <follow_exec>: Add ptid_t parameter.
(remote_target::follow_exec): Call
process_stratum_target::follow_exec.
* target-delegates.c: Re-generate.

Change-Id: I3f96d0ba3ea0dde6540b7e1b4d5cdb01635088c8

gdb/ChangeLog
gdb/infrun.c
gdb/linux-thread-db.c
gdb/process-stratum-target.c
gdb/process-stratum-target.h
gdb/remote.c
gdb/target-delegates.c
gdb/target.c
gdb/target.h

index f0ae77e12181d1374f8a7a7272e3b5654c1ca5ef..ad6f3081285d44d181870e98f46e97046748f7b1 100644 (file)
@@ -1,3 +1,19 @@
+2021-05-13  Simon Marchi  <simon.marchi@efficios.com>
+
+       * target.h (struct target_ops) <follow_exec>: Add ptid_t
+       parameter.
+       (target_follow_exec): Likewise.
+       * target.c (target_follow_exec): Add ptid_t parameter.
+       * infrun.c (follow_exec): Adjust call to target_follow_exec,
+       don't push target nor create thread.
+       * linux-thread-db.c (class thread_db_target) <follow_exec>: New.
+       (thread_db_target::wait): Just return on TARGET_WAITKIND_EXECD.
+       (thread_db_target::follow_exec): New.
+       * remote.c (class remote_target) <follow_exec>: Add ptid_t parameter.
+       (remote_target::follow_exec): Call
+       process_stratum_target::follow_exec.
+       * target-delegates.c: Re-generate.
+
 2021-05-13  Simon Marchi  <simon.marchi@efficios.com>
 
        * infrun.c (follow_exec): Call target_follow_fork when
index 3e386aa587ca3a27463f176e2330425ddf28b6db..7fc56dc51f027d09a3b81c7bf55d255876043745 100644 (file)
@@ -1064,7 +1064,6 @@ show_follow_exec_mode_string (struct ui_file *file, int from_tty,
 static void
 follow_exec (ptid_t ptid, const char *exec_file_target)
 {
-  struct inferior *inf = current_inferior ();
   int pid = ptid.pid ();
   ptid_t process_ptid;
 
@@ -1167,6 +1166,8 @@ follow_exec (ptid_t ptid, const char *exec_file_target)
      previous incarnation of this process.  */
   no_shared_libraries (NULL, 0);
 
+  struct inferior *inf = current_inferior ();
+
   if (follow_exec_mode_string == follow_exec_mode_new)
     {
       /* The user wants to keep the old inferior and program spaces
@@ -1176,18 +1177,16 @@ follow_exec (ptid_t ptid, const char *exec_file_target)
         inferior's pid.  Having two inferiors with the same pid would confuse
         find_inferior_p(t)id.  Transfer the terminal state and info from the
          old to the new inferior.  */
-      inf = add_inferior_with_spaces ();
-      swap_terminal_info (inf, current_inferior ());
-      exit_inferior_silent (current_inferior ());
+      inferior *new_inferior = add_inferior_with_spaces ();
 
-      inf->pid = pid;
-      target_follow_exec (inf, exec_file_target);
+      swap_terminal_info (new_inferior, inf);
+      exit_inferior_silent (inf);
 
-      inferior *org_inferior = current_inferior ();
-      switch_to_inferior_no_thread (inf);
-      inf->push_target (org_inferior->process_target ());
-      thread_info *thr = add_thread (inf->process_target (), ptid);
-      switch_to_thread (thr);
+      new_inferior->pid = pid;
+      target_follow_exec (new_inferior, ptid, exec_file_target);
+
+      /* We continue with the new inferior.  */
+      inf = new_inferior;
     }
   else
     {
@@ -1198,9 +1197,10 @@ follow_exec (ptid_t ptid, const char *exec_file_target)
         around (its description is later cleared/refetched on
         restart).  */
       target_clear_description ();
-      target_follow_exec (inf, exec_file_target);
+      target_follow_exec (inf, ptid, exec_file_target);
     }
 
+  gdb_assert (current_inferior () == inf);
   gdb_assert (current_program_space == inf->pspace);
 
   /* Attempt to open the exec file.  SYMFILE_DEFER_BP_RESET is used
index 1c6ea4debd8819a96f156abad11a18b4e237588d..2c75cd607949208ff1e4a191709888c590016ed2 100644 (file)
@@ -95,6 +95,7 @@ public:
   ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override;
   void resume (ptid_t, int, enum gdb_signal) override;
   void mourn_inferior () override;
+  void follow_exec (inferior *, ptid_t, const char *) override;
   void update_thread_list () override;
   std::string pid_to_str (ptid_t) override;
   CORE_ADDR get_thread_local_address (ptid_t ptid,
@@ -1384,6 +1385,7 @@ thread_db_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
     case TARGET_WAITKIND_EXITED:
     case TARGET_WAITKIND_THREAD_EXITED:
     case TARGET_WAITKIND_SIGNALLED:
+    case TARGET_WAITKIND_EXECD:
       return ptid;
     }
 
@@ -1393,16 +1395,6 @@ thread_db_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
   if (info == NULL)
     return ptid;
 
-  if (ourstatus->kind == TARGET_WAITKIND_EXECD)
-    {
-      /* 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 ());
-      current_inferior ()->unpush_target (this);
-
-      return ptid;
-    }
-
   /* Fill in the thread's user-level thread id and status.  */
   thread_from_lwp (find_thread_ptid (beneath, ptid), ptid);
 
@@ -1423,6 +1415,19 @@ thread_db_target::mourn_inferior ()
   current_inferior ()->unpush_target (this);
 }
 
+void
+thread_db_target::follow_exec (inferior *follow_inf, ptid_t ptid,
+                              const char *execd_pathname)
+{
+  process_stratum_target *beneath
+    = as_process_stratum_target (this->beneath ());
+
+  delete_thread_db_info (beneath, ptid.pid ());
+
+  current_inferior ()->unpush_target (this);
+  beneath->follow_exec (follow_inf, ptid, execd_pathname);
+}
+
 struct callback_data
 {
   struct thread_db_info *info;
index 719167803fff47f8b9cbf31c49ea5451f6a1db9f..c851090a7f2ea23913cb9e9d15360015f73a2a4c 100644 (file)
@@ -84,6 +84,26 @@ process_stratum_target::has_execution (inferior *inf)
   return inf->pid != 0;
 }
 
+void
+process_stratum_target::follow_exec (inferior *follow_inf, ptid_t ptid,
+                                    const char *execd_pathname)
+{
+  inferior *orig_inf = current_inferior ();
+
+  if (orig_inf != follow_inf)
+    {
+      /* Execution continues in a new inferior, push the original inferior's
+         process target on the new inferior's target stack.  The process target
+        may decide to unpush itself from the original inferior's target stack
+        after that, at its discretion.  */
+      follow_inf->push_target (orig_inf->process_target ());
+      thread_info *t = add_thread (follow_inf->process_target (), ptid);
+
+      /* Leave the new inferior / thread as the current inferior / thread.  */
+      switch_to_thread (t);
+    }
+}
+
 /* See process-stratum-target.h.  */
 
 std::set<process_stratum_target *>
index 6fddbba90c72c61029c5eadbc6145aa13b205a52..31a97753db9c01037db197f276780c7ef1d27765 100644 (file)
@@ -63,6 +63,14 @@ public:
   bool has_registers () override;
   bool has_execution (inferior *inf) override;
 
+  /* Default implementation of follow_exec.
+
+     If the current inferior and FOLLOW_INF are different (execution continues
+     in a new inferior), push this process target to FOLLOW_INF's target stack
+     and add an initial thread to FOLLOW_INF.  */
+  void follow_exec (inferior *follow_inf, ptid_t ptid,
+                   const char *execd_pathname) override;
+
   /* True if any thread is, or may be executing.  We need to track
      this separately because until we fully sync the thread list, we
      won't know whether the target is fully stopped, even if we see
index 8411b29d7345b0e2c07d96bb3a261c41e181e585..4dcc5515cea7b683a414085a4a5b128446a80253 100644 (file)
@@ -683,7 +683,7 @@ public:
   const struct btrace_config *btrace_conf (const struct btrace_target_info *) override;
   bool augmented_libraries_svr4_read () override;
   void follow_fork (bool, bool) override;
-  void follow_exec (struct inferior *, const char *) override;
+  void follow_exec (inferior *, ptid_t, const char *) override;
   int insert_fork_catchpoint (int) override;
   int remove_fork_catchpoint (int) override;
   int insert_vfork_catchpoint (int) override;
@@ -5925,20 +5925,20 @@ remote_target::follow_fork (bool follow_child, bool detach_fork)
 }
 
 /* Target follow-exec function for remote targets.  Save EXECD_PATHNAME
-   in the program space of the new inferior.  On entry and at return the
-   current inferior is the exec'ing inferior.  INF is the new exec'd
-   inferior, which may be the same as the exec'ing inferior unless
-   follow-exec-mode is "new".  */
+   in the program space of the new inferior.  */
 
 void
-remote_target::follow_exec (struct inferior *inf, const char *execd_pathname)
+remote_target::follow_exec (inferior *follow_inf, ptid_t ptid,
+                           const char *execd_pathname)
 {
+  process_stratum_target::follow_exec (follow_inf, ptid, execd_pathname);
+
   /* We know that this is a target file name, so if it has the "target:"
      prefix we strip it off before saving it in the program space.  */
   if (is_target_filename (execd_pathname))
     execd_pathname += strlen (TARGET_SYSROOT_PREFIX);
 
-  set_pspace_remote_exec_file (inf->pspace, execd_pathname);
+  set_pspace_remote_exec_file (follow_inf->pspace, execd_pathname);
 }
 
 /* Same as remote_detach, but don't send the "D" packet; just disconnect.  */
index ef8c94cec8c6a996e0f9433d438d358ab15a1cf0..3e759a2f80ea9948fc11f60ce0d159bcf9520d27 100644 (file)
@@ -59,7 +59,7 @@ struct dummy_target : public target_ops
   void follow_fork (bool arg0, bool arg1) override;
   int insert_exec_catchpoint (int arg0) override;
   int remove_exec_catchpoint (int arg0) override;
-  void follow_exec (struct inferior *arg0, const char *arg1) override;
+  void follow_exec (inferior *arg0, ptid_t arg1, const char *arg2) override;
   int set_syscall_catchpoint (int arg0, bool arg1, int arg2, gdb::array_view<const int> arg3) override;
   void mourn_inferior () override;
   void pass_signals (gdb::array_view<const unsigned char> arg0) override;
@@ -234,7 +234,7 @@ struct debug_target : public target_ops
   void follow_fork (bool arg0, bool arg1) override;
   int insert_exec_catchpoint (int arg0) override;
   int remove_exec_catchpoint (int arg0) override;
-  void follow_exec (struct inferior *arg0, const char *arg1) override;
+  void follow_exec (inferior *arg0, ptid_t arg1, const char *arg2) override;
   int set_syscall_catchpoint (int arg0, bool arg1, int arg2, gdb::array_view<const int> arg3) override;
   void mourn_inferior () override;
   void pass_signals (gdb::array_view<const unsigned char> arg0) override;
@@ -1595,25 +1595,27 @@ debug_target::remove_exec_catchpoint (int arg0)
 }
 
 void
-target_ops::follow_exec (struct inferior *arg0, const char *arg1)
+target_ops::follow_exec (inferior *arg0, ptid_t arg1, const char *arg2)
 {
-  this->beneath ()->follow_exec (arg0, arg1);
+  this->beneath ()->follow_exec (arg0, arg1, arg2);
 }
 
 void
-dummy_target::follow_exec (struct inferior *arg0, const char *arg1)
+dummy_target::follow_exec (inferior *arg0, ptid_t arg1, const char *arg2)
 {
 }
 
 void
-debug_target::follow_exec (struct inferior *arg0, const char *arg1)
+debug_target::follow_exec (inferior *arg0, ptid_t arg1, const char *arg2)
 {
   fprintf_unfiltered (gdb_stdlog, "-> %s->follow_exec (...)\n", this->beneath ()->shortname ());
-  this->beneath ()->follow_exec (arg0, arg1);
+  this->beneath ()->follow_exec (arg0, arg1, arg2);
   fprintf_unfiltered (gdb_stdlog, "<- %s->follow_exec (", this->beneath ()->shortname ());
-  target_debug_print_struct_inferior_p (arg0);
+  target_debug_print_inferior_p (arg0);
   fputs_unfiltered (", ", gdb_stdlog);
-  target_debug_print_const_char_p (arg1);
+  target_debug_print_ptid_t (arg1);
+  fputs_unfiltered (", ", gdb_stdlog);
+  target_debug_print_const_char_p (arg2);
   fputs_unfiltered (")\n", gdb_stdlog);
 }
 
index 438788e6e62de81ed85fb8cff0a89b7dde22c8fb..6babfc562563d6d09dddcbb955ecaa11a9171fb6 100644 (file)
@@ -2718,12 +2718,14 @@ target_follow_fork (bool follow_child, bool detach_fork)
   return target->follow_fork (follow_child, detach_fork);
 }
 
-/* Target wrapper for follow exec hook.  */
+/* See target.h.  */
 
 void
-target_follow_exec (struct inferior *inf, const char *execd_pathname)
+target_follow_exec (inferior *follow_inf, ptid_t ptid,
+                   const char *execd_pathname)
 {
-  current_inferior ()->top_target ()->follow_exec (inf, execd_pathname);
+  current_inferior ()->top_target ()->follow_exec (follow_inf, ptid,
+                                                  execd_pathname);
 }
 
 static void
index b80cf88db8139745ef85789924cce9f48104fbf4..d867a58e2a8b0196d67920934cda3bad6c178484 100644 (file)
@@ -642,7 +642,7 @@ struct target_ops
       TARGET_DEFAULT_RETURN (1);
     virtual int remove_exec_catchpoint (int)
       TARGET_DEFAULT_RETURN (1);
-    virtual void follow_exec (struct inferior *, const char *)
+    virtual void follow_exec (inferior *, ptid_t, const char *)
       TARGET_DEFAULT_IGNORE ();
     virtual int set_syscall_catchpoint (int, bool, int,
                                        gdb::array_view<const int>)
@@ -1715,12 +1715,19 @@ extern int target_remove_vfork_catchpoint (int pid);
 void target_follow_fork (bool follow_child, bool detach_fork);
 
 /* Handle the target-specific bookkeeping required when the inferior makes an
-   exec call.  The current inferior is the inferior that has executed the exec
-   call.  INF is the inferior in which execution continues post-exec.  It is the
-   same inferior as the current one if "follow-exec-mode" is "same" but is a new
-   one if "follow-exec-mode" is "new".  */
+   exec call.
 
-void target_follow_exec (struct inferior *inf, const char *execd_pathname);
+   The current inferior at the time of the call is the inferior that did the
+   exec.  FOLLOW_INF is the inferior in which execution continues post-exec.
+   If "follow-exec-mode" is "same", FOLLOW_INF is the same as the current
+   inferior, meaning that execution continues with the same inferior.  If
+   "follow-exec-mode" is "new", FOLLOW_INF is a different inferior, meaning
+   that execution continues in a new inferior.
+
+   On exit, the target must leave FOLLOW_INF as the current inferior.  */
+
+void target_follow_exec (inferior *follow_inf, ptid_t ptid,
+                        const char *execd_pathname);
 
 /* On some targets, we can catch an inferior exec event when it
    occurs.  These functions insert/remove an already-created