Re: [PATCH] Make STARTUP_WITH_SHELL a runtime toggle -- add new "set/show startup-with-shell" option.

[Thomas Schwinge]

Hi!

On Thu, 24 Oct 2013 16:17:21 +0100, Pedro Alves <palves@redhat.com> wrote:
> Here's what I pushed

..., and what made the Hurd port pretty unhappy.  ;-)

> gdb/
> 2013-10-24  Pedro Alves  <palves@redhat.com>
> 
>       * NEWS (New options): Mention set/show startup-with-shell.
>       * config/alpha/nm-osf3.h (START_INFERIOR_TRAPS_EXPECTED): Set to 2
>       instead of 3.
>       * fork-child.c (fork_inferior, startup_inferior): Handle 'set
>       startup-with-shell'.
>       (show_startup_with_shell): New function.
>       (_initialize_fork_child): Register the set/show startup-with-shell
>       commands.
>       * inf-ptrace.c (inf_ptrace_create_inferior): Remove comment.
>       * inf-ttrace.c (inf_ttrace_him): Remove comment.
>       * procfs.c (procfs_init_inferior): Remove comment.
>       * infcmd.c (startup_with_shell): New global.
>       * inferior.h (startup_with_shell): Declare global.
>       (STARTUP_WITH_SHELL): Delete.
>       (START_INFERIOR_TRAPS_EXPECTED): Set to 1 by default instead of 2.

> --- a/gdb/fork-child.c
> +++ b/gdb/fork-child.c
> @@ -419,6 +419,12 @@ startup_inferior (int ntraps)
>    int terminal_initted = 0;
>    ptid_t resume_ptid;
>  
> +  if (startup_with_shell)
> +    {
> +      /* One trap extra for exec'ing the shell.  */
> +      pending_execs++;
> +    }
> +

> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -346,25 +365,12 @@ struct displaced_step_closure 
> *get_displaced_step_closure_by_addr (CORE_ADDR add
> +/* Number of traps that happen between exec'ing the shell to run an
> +   inferior and when we finally get to the inferior code, not counting
> +   the exec for the shell.  This is 1 on most implementations.
> +   Overridden in nm.h files.  */
>  #if !defined(START_INFERIOR_TRAPS_EXPECTED)
> -#define START_INFERIOR_TRAPS_EXPECTED        2
> +#define START_INFERIOR_TRAPS_EXPECTED        1
>  #endif

I'd like to push the following, to get the Hurd port functional again.

In the thread around
<http://news.gmane.org/find-root.php?message_id=%3C200810110047.39807.pedro%40codesourcery.com%3E>,
and
<http://news.gmane.org/find-root.php?message_id=%3C200810131935.35253.pedro%40codesourcery.com%3E>,
we had already (very) briefly been discussing gnu-nat's local
pending_execs handling.

Is the new approach that I'm posting below (flag instead of counting;
saves us from repeating in gnu_create_inferior the increment in the
startup_with_shell case) OK until we get a clear understanding if and how
we can get rid of it for good?  Or should indeed the current value of
fork-child.c:startup_inferior's pending_execs be made available, say by
moving that variable into inferior.h:struct inferior, and using that in
gnu-nat.c instead of gnu-nat.c:struct inf's local pending_execs
"replica"?  (But apparently there are no other users of this.)

commit 57c9fb3afadab5813d7463dc2393d5affe78849e
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Wed Jan 8 21:42:07 2014 +0100

    Hurd: Adjust to startup-with-shell changes.
    
    In commit 98882a26513e25b2161b41dfd4bed97b59b2c01a, STARTUP_WITH_SHELL was 
made
    a runtime toggle, startup-with-shell.  The Hurd code not adjusted, which 
had a
    value hard-coded instead of using START_INFERIOR_TRAPS_EXPECTED.  Fix that, 
and
    also simplify gnu-nat's pending_execs handling from counting to just a flag.
    
        gdb/
        * gnu-nat.c (struct inf): Change pending_execs member to a 1-bit
        flag.  Adjust all users; in particular...
        (gnu_wait): ..., don't decrement its value in here...
        (gnu_create_inferior): ..., and instead set the flag in here,
        around the startup_inferior call, and call that one with
        START_INFERIOR_TRAPS_EXPECTED.

diff --git gdb/gnu-nat.c gdb/gnu-nat.c
index 26db81a..5538af5 100644
--- gdb/gnu-nat.c
+++ gdb/gnu-nat.c
@@ -210,9 +210,9 @@ struct inf
     unsigned int no_wait:1;
 
     /* When starting a new inferior, we don't try to validate threads until all
-       the proper execs have been done.  This is a count of how many execs we
+       the proper execs have been done, which this flag states we still
        expect to happen.  */
-    unsigned pending_execs;
+    unsigned int pending_execs:1;
 
     /* Fields describing global state.  */
 
@@ -1568,26 +1568,14 @@ rewait:
           while execing.  */
        {
          w->suppress = 1;
-         inf_debug (inf, "pending_execs = %d, ignoring minor event",
-                    inf->pending_execs);
+         inf_debug (inf, "pending_execs, ignoring minor event");
        }
       else if (kind == TARGET_WAITKIND_STOPPED
               && w->status.value.sig == GDB_SIGNAL_TRAP)
        /* Ah hah!  A SIGTRAP from the inferior while starting up probably
           means we've succesfully completed an exec!  */
        {
-         if (--inf->pending_execs == 0)
-           /* We're done!  */
-           {
-#if 0                          /* do we need this?  */
-             prune_threads (1);        /* Get rid of the old shell
-                                          threads.  */
-             renumber_threads (0);     /* Give our threads reasonable
-                                          names.  */
-#endif
-           }
-         inf_debug (inf, "pending exec completed, pending_execs => %d",
-                    inf->pending_execs);
+         inf_debug (inf, "one pending exec completed");
        }
       else if (kind == TARGET_WAITKIND_STOPPED)
        /* It's possible that this signal is because of a crashed process
@@ -2146,7 +2134,7 @@ gnu_create_inferior (struct target_ops *ops,
 
   push_target (ops);
 
-  inf->pending_execs = 2;
+  inf->pending_execs = 1;
   inf->nomsg = 1;
   inf->traced = 1;
 
@@ -2158,7 +2146,8 @@ gnu_create_inferior (struct target_ops *ops,
   thread_change_ptid (inferior_ptid,
                      ptid_build (inf->pid, inf_pick_first_thread (), 0));
 
-  startup_inferior (inf->pending_execs);
+  startup_inferior (START_INFERIOR_TRAPS_EXPECTED);
+  inf->pending_execs = 0;
 
   inf_validate_procinfo (inf);
   inf_update_signal_thread (inf);


Grüße,
 Thomas

pgpsDShO0uO9c.pgp
Description: PGP signature