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

[Thomas Schwinge <thomas@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 5884 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]