[RFC] Allow disabling the default run target.
Commit Message
Wonder what people think of this. This is practically done, but
misses tests. I'll try writing some if people agree with the
approach.
Sometimes it's useful to be able to disable the default run target.
E.g., sometimes GDB disconnects from the extended-remote target I was
debugging, without me noticing it, and then I do "run". That starts
the program locally, and only after a little head scratch session do I
figure out the program is running locally instead of remotely as
intended. Same thing with "attach", "info os", etc.
With the patch, we now can have this instead:
(gdb) set default-run-target off
(gdb) target extended-remote :9999
...
*gdb disconnects*
(gdb) run
Don't know how to run. Try "help target".
To still be able to connect to the native target with
default-run-target set to off, I've made "target child" work instead
of erroring out as today.
Before:
(gdb) target child
Use the "run" command to start a child process.
After:
(gdb) target child
(gdb) maint print target-stack
The current target stack is:
- child (Child process)
- exec (Local exec file)
- None (None)
(gdb) run
Starting program: ./a.out
...
I've also wanted this for the testsuite, when running against the
native-extended-gdbserver.exp board (runs against gdbserver in
extended-remote mode). With that board, it's always a bug to launch a
program with the native target. Turns out we still have one such
case this patch catches:
(gdb) break main
Breakpoint 1 at 0x4009e5: file ../../../src/gdb/testsuite/gdb.base/coremaker.c, line 138.
(gdb) run
Don't know how to run. Try "help target".
(gdb) FAIL: gdb.base/corefile.exp: run: with core
Tested on x86_64 Fedora 17, native, and also with the extended-gdbserver board.
gdb/
2014-03-13 Pedro Alves <palves@redhat.com>
* inf-child.c (inf_child_ops, inf_child_explicitly_opened): New
globals.
(inf_child_open): Push the target instead of erroring out.
(inf_child_disconnect, inf_child_close, inf_child_maybe_unpush):
New functions.
(inf_child_target): Install inf_child_disconnect and
inf_child_close. Store a pointer to the returned object.
* inf-child.h (inf_child_maybe_unpush): New declaration.
* inf-ptrace.c (inf_ptrace_mourn_inferior, inf_ptrace_detach): Use
inf_child_maybe_unpush.
* linux-nat.c (super_close): New global.
(linux_nat_close): Call super_close.
(linux_nat_add_target): Store a pointer to the base class's
to_close method.
* target.c (default_run_target): New global.
(show_default_run_target): New function.
(find_default_run_target): Return NULL if falling back to the
default run target is disabled.
(_initialize_target): Install set/show default-run-target.
* NEWS: Mention "set default-run-target", and target child.
gdb/doc/
2014-03-13 Pedro Alves <palves@redhat.com>
* gdb.texinfo (Starting): Document "set/show default-run-target".
(Target Commands): Document "target child".
gdb/testsuite/
2014-03-13 Pedro Alves <palves@redhat.com>
* boards/native-extended-gdbserver.exp (GDBFLAGS): Set to "set
default-run-target off".
---
gdb/NEWS | 11 ++++
gdb/doc/gdb.texinfo | 58 ++++++++++++++++++++++
gdb/inf-child.c | 44 +++++++++++++++-
gdb/inf-child.h | 2 +
gdb/inf-ptrace.c | 6 +--
gdb/linux-nat.c | 8 +++
gdb/target.c | 48 ++++++++++++++----
gdb/testsuite/boards/native-extended-gdbserver.exp | 2 +
8 files changed, 164 insertions(+), 15 deletions(-)
Comments
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 13 Mar 2014 19:02:48 +0000
>
> Wonder what people think of this.
FWIW, the names of commands and options confused me a lot.
> +By default, if not connected to any target yet (e.g., with
> +@code{target remote}), the @code{run} command starts your program
> +under @value{GDBN}, on your local machine. When you're sure you don't
^^^^
I think you meant "If", not "When".
> +In case you connected explicitly to the @code{target child} target,
^^^^^^^^^^^^^^^^^^^^
This doesn't look right.
The rest of the docs parts look OK to me.
Thanks.
> With the patch, we now can have this instead:
>
> (gdb) set default-run-target off
> (gdb) target extended-remote :9999
> ...
> *gdb disconnects*
> (gdb) run
> Don't know how to run. Try "help target".
No strong opinion, but this does sound like a useful enhancement indeed,
especially considering the fact that the implementation seems fairly
light.
On 03/13/2014 08:16 PM, Eli Zaretskii wrote:
>> From: Pedro Alves <palves@redhat.com>
>> Date: Thu, 13 Mar 2014 19:02:48 +0000
>>
>> Wonder what people think of this.
>
> FWIW, the names of commands and options confused me a lot.
Thanks Eli. I battled with several different namings, and
all the others seemed worse. :-) But I do think we should
try to come up with something better -- always a bad sign
to me when a GDB maintainer is confused. Probably users
will be even more.
Can you point out specifically what confused you?
I've been pondering renaming "target child" to something else.
"child" is a little lie in case of "attach".
By best suggestion so far is "target native". I worry a little
that "native" might be a lie too if some port comes up with
a default target that can run but is not really native. But
I think that's a very minor issue, we can consider that native
means the default built in target that gdb supports, instead of
saying that's the target that debugs host native processes.
BTW, not all native targets are called "child" yet. djgpp is
one that isn't (it's "target djgpp"). Would you mind if that
target's name was changed? Hmm, I now see that go32-nat.c
implements a to_open hook:
static void
go32_open (char *name, int from_tty)
{
printf_unfiltered ("Done. Use the \"run\" command to run the program.\n");
}
It says "Done", kind of implying that has connected to the
target, but it really hasn't, since a push_target call is
missing. Given this prints instead of erroring out like
"target child" does currently (and with a similar message), it looks
like "target djgpp"'s ends up having a similar effect my patch has
(expect it doesn't make sure the target stays connected after the
program exits). (Not sure whether djgpp even supports remote
debugging, or who would want to use it that way, so I don't see
the point of "target djgpp" as is today). If my patch goes in,
I'd suggest just removing go32_open, and letting inf-child.c's
to_open handle pushing the target.
For the toggle option itself, I considered putting "enable" in the name:
set enable-default-run-target
But disabling this isn't really disabling the target, only
the falling back is disabled. Adding "fallback" and/to "to"
seems to help. Like:
set enable-fallback-to-default-run-target
set enable-fallback-default-run-target
set fallback-to-default-run-target
set fallback-default-run-target
Seemed a little too long, so I ended up dropping the "fallback".
Dunno, maybe tab completion makes that a non-issue.
Assuming using "native" instead would be OK, I think we could
dispense with "default", like:
set enable-fallback-to-native-target
set enable-fallback-native-target
set fallback-to-native-target
set fallback-native-target
set native-target-fallback
(and then "target child" renamed to "target native").
>
>> +By default, if not connected to any target yet (e.g., with
>> +@code{target remote}), the @code{run} command starts your program
>> +under @value{GDBN}, on your local machine. When you're sure you don't
> ^^^^
> I think you meant "If", not "When".
>
>> +In case you connected explicitly to the @code{target child} target,
> ^^^^^^^^^^^^^^^^^^^^
> This doesn't look right.
>
> The rest of the docs parts look OK to me.
Thanks, I've fixed these locally.
> Date: Fri, 14 Mar 2014 11:06:32 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
>
> On 03/13/2014 08:16 PM, Eli Zaretskii wrote:
> >> From: Pedro Alves <palves@redhat.com>
> >> Date: Thu, 13 Mar 2014 19:02:48 +0000
> >>
> >> Wonder what people think of this.
> >
> > FWIW, the names of commands and options confused me a lot.
>
> Thanks Eli. I battled with several different namings, and
> all the others seemed worse. :-) But I do think we should
> try to come up with something better -- always a bad sign
> to me when a GDB maintainer is confused. Probably users
> will be even more.
>
> Can you point out specifically what confused you?
"target child" itself, and then the apparent disconnect between that
and the option name, although NEWS says that "'target child' [...]
connects to the default run target". Also, the fact that setting
default-run-target to OFF actually _enables_ something (AFAIU).
> I've been pondering renaming "target child" to something else.
> "child" is a little lie in case of "attach".
> By best suggestion so far is "target native".
"target native" is a much better name, IMO.
> Not sure whether djgpp even supports remote debugging
AFAIK, it does, see ser-go32.c. But if you meant remote debugging of
DJGPP programs, then no, this isn't supported, since gdbserver
doesn't, and there's no other stub that could do that instead.
> I'd suggest just removing go32_open, and letting inf-child.c's
> to_open handle pushing the target.
Fine with me. I don't think "target djgpp" was ever important anyway,
I think it's there mostly for completeness.
> For the toggle option itself, I considered putting "enable" in the name:
>
> set enable-default-run-target
>
> But disabling this isn't really disabling the target, only
> the falling back is disabled. Adding "fallback" and/to "to"
> seems to help. Like:
>
> set enable-fallback-to-default-run-target
> set enable-fallback-default-run-target
> set fallback-to-default-run-target
> set fallback-default-run-target
>
> Seemed a little too long, so I ended up dropping the "fallback".
> Dunno, maybe tab completion makes that a non-issue.
>
> Assuming using "native" instead would be OK, I think we could
> dispense with "default", like:
>
> set enable-fallback-to-native-target
> set enable-fallback-native-target
> set fallback-to-native-target
> set fallback-native-target
> set native-target-fallback
>
> (and then "target child" renamed to "target native").
Maybe we should take a step back and discuss why this fallback is
useful. Is it only so native debugging works by default without a
need to say "target native"?
On 03/14/2014 11:47 AM, Eli Zaretskii wrote:
>> Date: Fri, 14 Mar 2014 11:06:32 +0000
>> From: Pedro Alves <palves@redhat.com>
>> CC: gdb-patches@sourceware.org
>>
>> On 03/13/2014 08:16 PM, Eli Zaretskii wrote:
>>>> From: Pedro Alves <palves@redhat.com>
>>>> Date: Thu, 13 Mar 2014 19:02:48 +0000
>>>>
>>>> Wonder what people think of this.
>>>
>>> FWIW, the names of commands and options confused me a lot.
>>
>> Thanks Eli. I battled with several different namings, and
>> all the others seemed worse. :-) But I do think we should
>> try to come up with something better -- always a bad sign
>> to me when a GDB maintainer is confused. Probably users
>> will be even more.
>>
>> Can you point out specifically what confused you?
>
> "target child" itself, and then the apparent disconnect between that
> and the option name, although NEWS says that "'target child' [...]
> connects to the default run target". Also, the fact that setting
> default-run-target to OFF actually _enables_ something (AFAIU).
No, it disables the fallback to the default run target if
no target is pushed yet.
>> I've been pondering renaming "target child" to something else.
>> "child" is a little lie in case of "attach".
>> By best suggestion so far is "target native".
>
> "target native" is a much better name, IMO.
OK, I'll see if others have comments, and then propose
that with a patch.
>> I'd suggest just removing go32_open, and letting inf-child.c's
>> to_open handle pushing the target.
>
> Fine with me. I don't think "target djgpp" was ever important anyway,
> I think it's there mostly for completeness.
OK.
> Maybe we should take a step back and discuss why this fallback is
> useful. Is it only so native debugging works by default without a
> need to say "target native"?
Yes. It's what makes
$ gdb program
(gdb) run
Just Work.
@@ -43,6 +43,12 @@ maint ada show ignore-descriptive-types
the user manual for more details on descriptive types and the intended
usage of this option.
+set default-run-target
+ Control whether GDB is allowed to fall back to the default run
+ target (usually the native target) for the run, attach,
+ etc. commands when not connected to any target yet. See also
+ "target child" below.
+
* New features in the GDB remote stub, GDBserver
** New option --debug-format=option1[,option2,...] allows one to add
@@ -76,6 +82,11 @@ maint ada show ignore-descriptive-types
* The "catch syscall" command now works on s390*-linux* targets.
+* The "target child" command now connects to the default run target
+ (usually the native target) instead of erroring out. This can be
+ used to launch native programs when "set default-run-target" is set
+ to off.
+
* New remote packets
qXfer:btrace:read's annex
@@ -2146,6 +2146,57 @@ initialization file---such as @file{.cshrc} for C-shell,
$@file{.zshenv} for the Z shell, or the file specified in the
@samp{BASH_ENV} environment variable for BASH.
+@anchor{set default-run-target}
+@kindex set default-run-target
+@item set default-run-target
+@itemx set default-run-target on
+@itemx set default-run-target off
+@itemx show default-run-target
+
+By default, if not connected to any target yet (e.g., with
+@code{target remote}), the @code{run} command starts your program
+under @value{GDBN}, on your local machine. When you're sure you don't
+want to debug programs on your local machine, you can tell
+@value{GDBN} to not fall back to the default native target with the
+@code{set default-run-target off} command.
+
+If @code{on}, which is the default, and if @value{GDBN} is not
+connected to a target already, the @code{run} command uses the default
+native target, if one is available.
+
+If @code{off}, and if @value{GDBN} is not connected to a target
+already, the @code{run} command fail with an error:
+
+@smallexample
+(@value{GDBP}) run
+Don't know how to run. Try "help target".
+@end smallexample
+
+If @value{GDBN} is already connected to a target, @value{GDBN} always
+uses it with the @code{run} command.
+
+In any case, you can explicitly connect to the default native target
+with the @code{target child} command. For example,
+
+@smallexample
+(@value{GDBP}) set default-run-target off
+(@value{GDBP}) run
+Don't know how to run. Try "help target".
+(@value{GDBP}) target child
+(@value{GDBP}) run
+Starting program: ./a.out
+[Inferior 1 (process 10421) exited normally]
+@end smallexample
+
+In case you connected explicitly to the @code{target child} target,
+@value{GDBN} remains connected even if all inferiors exit, ready for
+the next @code{run} command. Use the @code{disconnect} command to
+disconnect from the @code{child} target.
+
+Examples of other commands that likewise respect the
+@code{default-run-target} setting: @code{run}, @code{attach},
+@code{info proc}, @code{info osdata}.
+
@kindex set disable-randomization
@item set disable-randomization
@itemx set disable-randomization on
@@ -18050,6 +18101,13 @@ provide these. For info about any processor-specific simulator details,
see the appropriate section in @ref{Embedded Processors, ,Embedded
Processors}.
+@item target child
+@cindex native target
+Setup for local/native process debugging. Useful to make the
+@code{run}, @code{attach}, etc.@: commands spawn native processes even
+when @code{set default-run-target} is @code{off} (@pxref{set
+default-run-target}).
+
@end table
Different targets are available on different configurations of @value{GDBN};
@@ -40,6 +40,10 @@
#include <fcntl.h>
#include <unistd.h>
+/* A pointer to what is returned by inf_child_target. Used to be able
+ to push the most-derived target in reaction to "target child". */
+static struct target_ops *inf_child_ops = NULL;
+
/* Helper function for child_wait and the derivatives of child_wait.
HOSTSTATUS is the waitstatus from wait() or the equivalent; store our
translation of that in OURSTATUS. */
@@ -109,10 +113,41 @@ inf_child_prepare_to_store (struct target_ops *self,
{
}
+/* True if the user did "target child". In that case, we won't unpush
+ the child target automatically when the last inferior is gone. */
+static int inf_child_explicitly_opened;
+
static void
inf_child_open (char *arg, int from_tty)
{
- error (_("Use the \"run\" command to start a child process."));
+ target_preopen (from_tty);
+ push_target (inf_child_ops);
+ inf_child_explicitly_opened = 1;
+}
+
+static void
+inf_child_disconnect (struct target_ops *target, char *args, int from_tty)
+{
+ if (args != NULL)
+ error (_("Argument given to \"disconnect\"."));
+
+ /* This offers to detach/kill current inferiors, and then pops all
+ targets. */
+ target_preopen (from_tty);
+}
+
+static void
+inf_child_close (struct target_ops *target)
+{
+ /* In case we were forcibly closed. */
+ inf_child_explicitly_opened = 0;
+}
+
+void
+inf_child_maybe_unpush (struct target_ops *ops)
+{
+ if (!inf_child_explicitly_opened && !have_inferiors ())
+ unpush_target (ops);
}
static void
@@ -410,6 +445,8 @@ inf_child_target (void)
t->to_longname = "Child process";
t->to_doc = "Child process (started by the \"run\" command).";
t->to_open = inf_child_open;
+ t->to_close = inf_child_close;
+ t->to_disconnect = inf_child_disconnect;
t->to_post_attach = inf_child_post_attach;
t->to_fetch_registers = inf_child_fetch_inferior_registers;
t->to_store_registers = inf_child_store_inferior_registers;
@@ -445,5 +482,10 @@ inf_child_target (void)
t->to_magic = OPS_MAGIC;
t->to_use_agent = inf_child_use_agent;
t->to_can_use_agent = inf_child_can_use_agent;
+
+ /* Store a pointer so we can push the target from
+ inf_child_open. */
+ inf_child_ops = t;
+
return t;
}
@@ -25,6 +25,8 @@
extern struct target_ops *inf_child_target (void);
+extern void inf_child_maybe_unpush (struct target_ops *ops);
+
/* Functions for helping to write a native target. */
/* This is for native targets which use a unix/POSIX-style waitstatus. */
@@ -176,8 +176,7 @@ inf_ptrace_mourn_inferior (struct target_ops *ops)
generic_mourn_inferior ();
- if (!have_inferiors ())
- unpush_target (ops);
+ inf_child_maybe_unpush (ops);
}
/* Attach to the process specified by ARGS. If FROM_TTY is non-zero,
@@ -297,8 +296,7 @@ inf_ptrace_detach (struct target_ops *ops, const char *args, int from_tty)
inferior_ptid = null_ptid;
detach_inferior (pid);
- if (!have_inferiors ())
- unpush_target (ops);
+ inf_child_maybe_unpush (ops);
}
/* Kill the inferior. */
@@ -201,6 +201,10 @@ static int (*linux_nat_siginfo_fixup) (siginfo_t *,
Called by our to_xfer_partial. */
static target_xfer_partial_ftype *super_xfer_partial;
+/* The saved to_close method, inherited from inf-ptrace.c.
+ Called by our to_close. */
+static void (*super_close) (struct target_ops *);
+
static unsigned int debug_linux_nat;
static void
show_debug_linux_nat (struct ui_file *file, int from_tty,
@@ -4776,6 +4780,8 @@ linux_nat_close (struct target_ops *self)
if (linux_ops->to_close)
linux_ops->to_close (linux_ops);
+
+ super_close (self);
}
/* When requests are passed down from the linux-nat layer to the
@@ -4857,6 +4863,8 @@ linux_nat_add_target (struct target_ops *t)
t->to_async = linux_nat_async;
t->to_terminal_inferior = linux_nat_terminal_inferior;
t->to_terminal_ours = linux_nat_terminal_ours;
+
+ super_close = t->to_close;
t->to_close = linux_nat_close;
/* Methods for non-stop support. */
@@ -2574,6 +2574,20 @@ target_require_runnable (void)
internal_error (__FILE__, __LINE__, _("No targets found"));
}
+/* Whether GDB is allowed to fall back to the default run target for
+ "run", "attach", etc. when no target is connected yet. */
+static int default_run_target = 1;
+
+static void
+show_default_run_target (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c, const char *value)
+{
+ fprintf_filtered (file,
+ _("Whether GDB may fall back to the "
+ "default run target is %s.\n"),
+ value);
+}
+
/* Look through the list of possible targets for a target that can
execute a run or attach command without any other data. This is
used to locate the default process stratum.
@@ -2584,23 +2598,28 @@ target_require_runnable (void)
static struct target_ops *
find_default_run_target (char *do_mesg)
{
- struct target_ops **t;
struct target_ops *runable = NULL;
- int count;
- count = 0;
-
- for (t = target_structs; t < target_structs + target_struct_size;
- ++t)
+ if (default_run_target)
{
- if ((*t)->to_can_run != delegate_can_run && target_can_run (*t))
+ struct target_ops **t;
+ int count = 0;
+
+ for (t = target_structs; t < target_structs + target_struct_size;
+ ++t)
{
- runable = *t;
- ++count;
+ if ((*t)->to_can_run != delegate_can_run && target_can_run (*t))
+ {
+ runable = *t;
+ ++count;
+ }
}
+
+ if (count != 1)
+ runable = NULL;
}
- if (count != 1)
+ if (runable == NULL)
{
if (do_mesg)
error (_("Don't know how to %s. Try \"help target\"."), do_mesg);
@@ -4369,4 +4388,13 @@ When this permission is on, GDB may interrupt/stop the target's execution.\n\
Otherwise, any attempt to interrupt or stop will be ignored."),
set_target_permissions, NULL,
&setlist, &showlist);
+
+ add_setshow_boolean_cmd ("default-run-target", class_support,
+ &default_run_target, _("\
+Set whether GDB may fall back to the default run target."), _("\
+Show whether GDB may fall back to the default run target."), _("\
+When on, and GDB is not connected to a target yet, GDB\n\
+attempts \"run\" and other commands with the default run target."),
+ NULL, show_default_run_target,
+ &setlist, &showlist);
}
@@ -36,6 +36,8 @@ set_board_info gdb_protocol "extended-remote"
send_user "configuring for gdbserver local testing (extended-remote)\n"
+set GDBFLAGS "${GDBFLAGS} -ex \"set default-run-target off\""
+
# We must load this explicitly here, and rename the procedures we want
# to override. If we didn't do this, given that mi-support.exp is
# loaded later in the test files, the procedures loaded then would