Re: [PATCH 2/3] gdb: rename gdb_startup_inferior to inf_child_target::startup_inferior

[Andrew Burgess <aburgess@redhat.com>]

* Andrew Burgess <aburgess@redhat.com> [2021-12-03 10:28:44 +0000]:

> I notice that gdb_startup_inferior is only ever called from within
> classes derived from inf_child_target, which makes sense given what
> the function does.
> 
> This commit makes the function a protected method within
> inf_child_target, and updates all the callers.

I'm dropping this patch from the series.  This patch breaks builds of
targets that don't use fork/exec to start native processes, for
example, configuring with `--enable-targets=all
--host=i686-w64-mingw32` will break with this patch.

To understand what happens, with the above configure options, the
gdb/windows-nat.c file is built into GDB and the files
gdb/fork-child.c and gdb/nat/fork-inferior.c are not built in to GDB.

As windows-nat.c doesn't call gdb_startup_inferior this was fine.

After this patch windows-nat.c still doesn't call
gdb_startup_inferior, but now, the content of that function has moved
into inf-child.c, which is built, and this function does call to a
function in gdb/nat/fork-inferior.c, which still isn't being built.

And so, we now have a missing symbol, and a link failure.

As neither of the other two patches in this series depend on this one,
I plan to push this series without this patch.

Thanks,
Andrew


> 
> There should be no user visible changes after this commit.
> ---
>  gdb/darwin-nat.c |  2 +-
>  gdb/fork-child.c | 17 +----------------
>  gdb/gnu-nat.c    |  2 +-
>  gdb/inf-child.c  | 17 +++++++++++++++++
>  gdb/inf-child.h  | 10 ++++++++++
>  gdb/inf-ptrace.c |  2 +-
>  gdb/inferior.h   |  7 -------
>  gdb/procfs.c     |  2 +-
>  8 files changed, 32 insertions(+), 27 deletions(-)
> 
> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
> index e1aeb69e404..aa7eac1d7fb 100644
> --- a/gdb/darwin-nat.c
> +++ b/gdb/darwin-nat.c
> @@ -1753,7 +1753,7 @@ darwin_nat_target::ptrace_him (int pid)
>  
>    init_thread_list (inf);
>  
> -  gdb_startup_inferior (pid, START_INFERIOR_TRAPS_EXPECTED);
> +  startup_inferior (pid, START_INFERIOR_TRAPS_EXPECTED);
>  }
>  
>  static void
> diff --git a/gdb/fork-child.c b/gdb/fork-child.c
> index 3e995ed6f65..af8dca9ae26 100644
> --- a/gdb/fork-child.c
> +++ b/gdb/fork-child.c
> @@ -29,6 +29,7 @@
>  #include "gdbsupport/filestuff.h"
>  #include "nat/fork-inferior.h"
>  #include "gdbsupport/common-inferior.h"
> +#include "inf-child.h"
>  
>  /* The exec-wrapper, if any, that will be used when starting the
>     inferior.  */
> @@ -118,22 +119,6 @@ postfork_child_hook ()
>    new_tty ();
>  }
>  
> -/* See inferior.h.  */
> -
> -ptid_t
> -gdb_startup_inferior (pid_t pid, int num_traps)
> -{
> -  inferior *inf = current_inferior ();
> -  process_stratum_target *proc_target = inf->process_target ();
> -
> -  ptid_t ptid = startup_inferior (proc_target, pid, num_traps, NULL, NULL);
> -
> -  /* Mark all threads non-executing.  */
> -  set_executing (proc_target, ptid, false);
> -
> -  return ptid;
> -}
> -
>  /* Implement the "unset exec-wrapper" command.  */
>  
>  static void
> diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
> index 13127cd8246..bd09fa94ecc 100644
> --- a/gdb/gnu-nat.c
> +++ b/gdb/gnu-nat.c
> @@ -2137,7 +2137,7 @@ gnu_nat_target::create_inferior (const char *exec_file,
>    thread_change_ptid (this, inferior_ptid,
>  		      ptid_t (inf->pid, inf_pick_first_thread (), 0));
>  
> -  gdb_startup_inferior (pid, START_INFERIOR_TRAPS_EXPECTED);
> +  startup_inferior (pid, START_INFERIOR_TRAPS_EXPECTED);
>  
>    inf->pending_execs = 0;
>    /* Get rid of the old shell threads.  */
> diff --git a/gdb/inf-child.c b/gdb/inf-child.c
> index 5e821f45598..780de98e267 100644
> --- a/gdb/inf-child.c
> +++ b/gdb/inf-child.c
> @@ -34,6 +34,7 @@
>  #include "gdbsupport/agent.h"
>  #include "gdbsupport/gdb_wait.h"
>  #include "gdbsupport/filestuff.h"
> +#include "nat/fork-inferior.h"
>  
>  #include <sys/types.h>
>  #include <fcntl.h>
> @@ -420,6 +421,22 @@ inf_child_target::follow_exec (inferior *follow_inf, ptid_t ptid,
>  
>  /* See inf-child.h.  */
>  
> +ptid_t
> +inf_child_target::startup_inferior (pid_t pid, int num_traps)
> +{
> +  inferior *inf = current_inferior ();
> +  process_stratum_target *proc_target = inf->process_target ();
> +
> +  ptid_t ptid = ::startup_inferior (proc_target, pid, num_traps, NULL, NULL);
> +
> +  /* Mark all threads non-executing.  */
> +  set_executing (proc_target, ptid, false);
> +
> +  return ptid;
> +}
> +
> +/* See inf-child.h.  */
> +
>  void
>  add_inf_child_target (inf_child_target *target)
>  {
> diff --git a/gdb/inf-child.h b/gdb/inf-child.h
> index aa33c538138..0d13d6bc6e8 100644
> --- a/gdb/inf-child.h
> +++ b/gdb/inf-child.h
> @@ -102,6 +102,16 @@ class inf_child_target
>       done by calling either generic_mourn_inferior or
>       detach_inferior.  */
>    void maybe_unpush_target ();
> +
> +  /* Helper function to call the global STARTUP_INFERIOR with PID and
> +     NUM_TRAPS.  PID should correspond to the current inferior.  No threads
> +     in the current inferior should be marked as resumed when calling this
> +     method.
> +
> +     Return the ptid_t from STARTUP_INFERIOR, all the threads in the
> +     process matching the returned ptid_t are marked as non-executing, and
> +     non-resumed.  */
> +  ptid_t startup_inferior (pid_t pid, int num_traps);
>  };
>  
>  /* Functions for helping to write a native target.  */
> diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
> index 852636ba646..e6fe2d04522 100644
> --- a/gdb/inf-ptrace.c
> +++ b/gdb/inf-ptrace.c
> @@ -99,7 +99,7 @@ inf_ptrace_target::create_inferior (const char *exec_file,
>  
>    unpusher.release ();
>  
> -  gdb_startup_inferior (pid, START_INFERIOR_TRAPS_EXPECTED);
> +  startup_inferior (pid, START_INFERIOR_TRAPS_EXPECTED);
>  
>    /* On some targets, there must be some explicit actions taken after
>       the inferior has been started up.  */
> diff --git a/gdb/inferior.h b/gdb/inferior.h
> index e9210b1258d..a99d4afdc17 100644
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -185,13 +185,6 @@ extern void child_pass_ctrlc (struct target_ops *self);
>  
>  extern void child_interrupt (struct target_ops *self);
>  
> -/* From fork-child.c */
> -
> -/* Helper function to call STARTUP_INFERIOR with PID and NUM_TRAPS.
> -   This function already calls set_executing.  Return the ptid_t from
> -   STARTUP_INFERIOR.  */
> -extern ptid_t gdb_startup_inferior (pid_t pid, int num_traps);
> -
>  /* From infcmd.c */
>  
>  /* Initial inferior setup.  Determines the exec file is not yet known,
> diff --git a/gdb/procfs.c b/gdb/procfs.c
> index 2c96919dceb..c4b011da6ce 100644
> --- a/gdb/procfs.c
> +++ b/gdb/procfs.c
> @@ -2704,7 +2704,7 @@ procfs_target::procfs_init_inferior (int pid)
>       about it.  This changes inferior_ptid as well.  */
>    thread_change_ptid (this, ptid_t (pid), ptid_t (pid, lwpid, 0));
>  
> -  gdb_startup_inferior (pid, START_INFERIOR_TRAPS_EXPECTED);
> +  startup_inferior (pid, START_INFERIOR_TRAPS_EXPECTED);
>  }
>  
>  /* When GDB forks to create a new process, this function is called on
> -- 
> 2.25.4
>