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

Thomas Schwinge
2014-01-08T21:21:20Z

[-- 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 --]