[2/2] mi: Add launch-type={run,attach} in =thread-group-started
Commit Message
For Eclipse CDT to properly handle processes attached or started from
the console, it would be useful to have a hint of whether the
thread-group was started by gdb or attached. In particular, it helps
for inferior pty handling, since a process that is "ran" requires some
special handling for its output to appear in a console in CDT, whereas
an "attached" process doesn't require any intervention.
This patch adds launch-type="run" or launch-type="attach" to the
=thread-group-started event. "launch" comes from the Eclipse
terminology, so it's perhaps not the best term. It would be nice to
have something that fits better in the gdb terminology. Suggestions are
welcome. I also thought of using attached="{1,0}", but I think the
other approach is more easily extensible if we ever need to.
Note that launch-type="run" will be present even "starting" a core file.
That's because we display "run" if inferior->attach_flag is 0 and
"attach" otherwise. If that's a problem, we could find a way to omit
the field completely when it's irrelevant. For example, when none of
the target present on the target stack know how to run or attach (which
is the case when inspecting a core file I believe), then it's probably
irrelevant.
I needed to do some little changes in various targets, so that struct
inferior's attach_flag field is set before inferior_appeared is called.
I can't test all of them, but the changes seem very low risk to me.
I have added a test for an attach in MI (it seems like there was none),
in which I test =thread-group-started with launch-type="attach".
Testing launch-type="run" could probably be done as well, but would be a
bit more involved (we could perhaps test it as part of
mi_run_cmd_full...).
Regtested on x86, unix/native-gdbserver/native-extended-gdbserver.
gdb/ChangeLog:
* NEWS: Announce new "launch-type" field in =thread-group-started.
* mi/mi-interp.c (mi_inferior_appeared): Output "launch-type"
attribute in =thread-group-started event.
* darwin-nat.c (darwin_attach): Move setting of attach_flag
before call to inferior_appeared.
* gnu-nat.c (gnu_attach): Likewise.
* inf-ptrace.c (inf_ptrace_attach): Likewise.
* nto-procfs.c (procfs_attach): Likewise.
* procfs.c (do_attach): Likewise.
* remote.c (remote_add_inferior): Likewise.
* windows-nat.c (do_initial_windows_stuff): Likewise.
gdb/doc/ChangeLog:
* gdb.texinfo (GDB/MI Async Records): Document new "launch-type" field
in =thread-group-started.
gdb/testsuite/ChangeLog:
* gdb.mi/mi-attach.exp: New file.
* gdb.mi/mi-attach.c: New file.
---
gdb/NEWS | 3 +++
gdb/darwin-nat.c | 2 +-
gdb/doc/gdb.texinfo | 6 +++--
gdb/gnu-nat.c | 2 +-
gdb/inf-ptrace.c | 2 +-
gdb/mi/mi-interp.c | 6 +++--
gdb/nto-procfs.c | 4 ++--
gdb/procfs.c | 2 +-
gdb/remote.c | 3 ++-
gdb/testsuite/gdb.mi/mi-attach.c | 31 +++++++++++++++++++++++++
gdb/testsuite/gdb.mi/mi-attach.exp | 46 ++++++++++++++++++++++++++++++++++++++
gdb/windows-nat.c | 2 +-
12 files changed, 97 insertions(+), 12 deletions(-)
create mode 100644 gdb/testsuite/gdb.mi/mi-attach.c
create mode 100644 gdb/testsuite/gdb.mi/mi-attach.exp
Comments
> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: Simon Marchi <simon.marchi@ericsson.com>
> Date: Mon, 1 Aug 2016 17:14:01 -0400
>
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -26437,12 +26437,14 @@ group is added, it generally might not be associated with a running
> process. When a thread group is removed, its id becomes invalid and
> cannot be used in any way.
>
> -@item =thread-group-started,id="@var{id}",pid="@var{pid}"
> +@item =thread-group-started,id="@var{id}",pid="@var{pid}",launch-type="@var{launch-type}"
> A thread group became associated with a running program,
> either because the program was just started or the thread group
> was attached to a program. The @var{id} field contains the
> @value{GDBN} identifier of the thread group. The @var{pid} field
> -contains process identifier, specific to the operating system.
> +contains process identifier, specific to the operating system. The
> +@var{launch-type} field has the value @code{"run"} if the program was started
> +by gdb and @code{"attach"} if it was attached.
^^^
@value{GDBN}
Also, @code{"foo"} looks bad in Info due to too many quotes. Suggest
to use just @code{foo} instead.
The documentation parts are okay with these fixed.
Thanks.
On 16-08-02 10:49 AM, Eli Zaretskii wrote:
>> From: Simon Marchi <simon.marchi@ericsson.com>
>> CC: Simon Marchi <simon.marchi@ericsson.com>
>> Date: Mon, 1 Aug 2016 17:14:01 -0400
>>
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -26437,12 +26437,14 @@ group is added, it generally might not be associated with a running
>> process. When a thread group is removed, its id becomes invalid and
>> cannot be used in any way.
>>
>> -@item =thread-group-started,id="@var{id}",pid="@var{pid}"
>> +@item =thread-group-started,id="@var{id}",pid="@var{pid}",launch-type="@var{launch-type}"
>> A thread group became associated with a running program,
>> either because the program was just started or the thread group
>> was attached to a program. The @var{id} field contains the
>> @value{GDBN} identifier of the thread group. The @var{pid} field
>> -contains process identifier, specific to the operating system.
>> +contains process identifier, specific to the operating system. The
>> +@var{launch-type} field has the value @code{"run"} if the program was started
>> +by gdb and @code{"attach"} if it was attached.
> ^^^
> @value{GDBN}
>
> Also, @code{"foo"} looks bad in Info due to too many quotes. Suggest
> to use just @code{foo} instead.
>
> The documentation parts are okay with these fixed.
>
> Thanks.
>
Thanks! Updated locally.
On 16-08-01 05:14 PM, Simon Marchi wrote:
> For Eclipse CDT to properly handle processes attached or started from
> the console, it would be useful to have a hint of whether the
> thread-group was started by gdb or attached. In particular, it helps
> for inferior pty handling, since a process that is "ran" requires some
> special handling for its output to appear in a console in CDT, whereas
> an "attached" process doesn't require any intervention.
>
> This patch adds launch-type="run" or launch-type="attach" to the
> =thread-group-started event. "launch" comes from the Eclipse
> terminology, so it's perhaps not the best term. It would be nice to
> have something that fits better in the gdb terminology. Suggestions are
> welcome. I also thought of using attached="{1,0}", but I think the
> other approach is more easily extensible if we ever need to.
>
> Note that launch-type="run" will be present even "starting" a core file.
> That's because we display "run" if inferior->attach_flag is 0 and
> "attach" otherwise. If that's a problem, we could find a way to omit
> the field completely when it's irrelevant. For example, when none of
> the target present on the target stack know how to run or attach (which
> is the case when inspecting a core file I believe), then it's probably
> irrelevant.
>
> I needed to do some little changes in various targets, so that struct
> inferior's attach_flag field is set before inferior_appeared is called.
> I can't test all of them, but the changes seem very low risk to me.
>
> I have added a test for an attach in MI (it seems like there was none),
> in which I test =thread-group-started with launch-type="attach".
> Testing launch-type="run" could probably be done as well, but would be a
> bit more involved (we could perhaps test it as part of
> mi_run_cmd_full...).
>
> Regtested on x86, unix/native-gdbserver/native-extended-gdbserver.
>
> gdb/ChangeLog:
>
> * NEWS: Announce new "launch-type" field in =thread-group-started.
> * mi/mi-interp.c (mi_inferior_appeared): Output "launch-type"
> attribute in =thread-group-started event.
> * darwin-nat.c (darwin_attach): Move setting of attach_flag
> before call to inferior_appeared.
> * gnu-nat.c (gnu_attach): Likewise.
> * inf-ptrace.c (inf_ptrace_attach): Likewise.
> * nto-procfs.c (procfs_attach): Likewise.
> * procfs.c (do_attach): Likewise.
> * remote.c (remote_add_inferior): Likewise.
> * windows-nat.c (do_initial_windows_stuff): Likewise.
>
> gdb/doc/ChangeLog:
>
> * gdb.texinfo (GDB/MI Async Records): Document new "launch-type" field
> in =thread-group-started.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.mi/mi-attach.exp: New file.
> * gdb.mi/mi-attach.c: New file.
I am abandoning this for now.
Based on this conversation [0], we concluded that the solution we had planned
might not be an appropriate user experience. And even then, it had its
technical flaws, so it's not clear that this new field will be needed.
[0] https://sourceware.org/ml/gdb/2016-08/msg00036.html
@@ -3,6 +3,9 @@
*** Changes since GDB 7.12
+* MI async record =thread-group-started now includes a
+ launch-type={run,attach} attribute.
+
*** Changes in GDB 7.12
* GDBserver now supports recording btrace without maintaining an active
@@ -1704,8 +1704,8 @@ darwin_attach (struct target_ops *ops, const char *args, int from_tty)
inferior_ptid = pid_to_ptid (pid);
inf = current_inferior ();
- inferior_appeared (inf, pid);
inf->attach_flag = 1;
+ inferior_appeared (inf, pid);
/* Always add a main thread. */
add_thread_silent (inferior_ptid);
@@ -26437,12 +26437,14 @@ group is added, it generally might not be associated with a running
process. When a thread group is removed, its id becomes invalid and
cannot be used in any way.
-@item =thread-group-started,id="@var{id}",pid="@var{pid}"
+@item =thread-group-started,id="@var{id}",pid="@var{pid}",launch-type="@var{launch-type}"
A thread group became associated with a running program,
either because the program was just started or the thread group
was attached to a program. The @var{id} field contains the
@value{GDBN} identifier of the thread group. The @var{pid} field
-contains process identifier, specific to the operating system.
+contains process identifier, specific to the operating system. The
+@var{launch-type} field has the value @code{"run"} if the program was started
+by gdb and @code{"attach"} if it was attached.
@item =thread-group-exited,id="@var{id}"[,exit-code="@var{code}"]
A thread group is no longer associated with a running program,
@@ -2200,8 +2200,8 @@ gnu_attach (struct target_ops *ops, const char *args, int from_tty)
push_target (ops);
inferior = current_inferior ();
- inferior_appeared (inferior, pid);
inferior->attach_flag = 1;
+ inferior_appeared (inferior, pid);
inf_update_procs (inf);
@@ -203,8 +203,8 @@ inf_ptrace_attach (struct target_ops *ops, const char *args, int from_tty)
#endif
inf = current_inferior ();
- inferior_appeared (inf, pid);
inf->attach_flag = 1;
+ inferior_appeared (inf, pid);
inferior_ptid = pid_to_ptid (pid);
/* Always add a main thread. If some target extends the ptrace
@@ -481,6 +481,7 @@ static void
mi_inferior_appeared (struct inferior *inf)
{
struct switch_thru_all_uis state;
+ const char *launch_type = inf->attach_flag ? "attach" : "run";
SWITCH_THRU_ALL_UIS (state)
{
@@ -494,8 +495,9 @@ mi_inferior_appeared (struct inferior *inf)
target_terminal_ours_for_output ();
fprintf_unfiltered (mi->event_channel,
- "thread-group-started,id=\"i%d\",pid=\"%d\"",
- inf->num, inf->pid);
+ "thread-group-started,id=\"i%d\",pid=\"%d\","
+ "launch-type=\"%s\"",
+ inf->num, inf->pid, launch_type);
gdb_flush (mi->event_channel);
do_cleanups (old_chain);
}
@@ -672,8 +672,8 @@ procfs_attach (struct target_ops *ops, const char *args, int from_tty)
}
inferior_ptid = do_attach (pid_to_ptid (pid));
inf = current_inferior ();
- inferior_appeared (inf, pid);
inf->attach_flag = 1;
+ inferior_appeared (inf, pid);
if (!target_is_pushed (ops))
push_target (ops);
@@ -1270,8 +1270,8 @@ procfs_create_inferior (struct target_ops *ops, char *exec_file,
procfs_update_thread_list (ops);
inf = current_inferior ();
- inferior_appeared (inf, pid);
inf->attach_flag = 0;
+ inferior_appeared (inf, pid);
flags = _DEBUG_FLAG_KLC; /* Kill-on-Last-Close flag. */
errn = devctl (ctl_fd, DCMD_PROC_SET_FLAG, &flags, sizeof (flags), 0);
@@ -3135,9 +3135,9 @@ do_attach (ptid_t ptid)
dead_procinfo (pi, "do_attach: failed in procfs_debug_inferior", NOKILL);
inf = current_inferior ();
- inferior_appeared (inf, pi->pid);
/* Let GDB know that the inferior was attached. */
inf->attach_flag = 1;
+ inferior_appeared (inf, pi->pid);
/* Create a procinfo for the current lwp. */
lwpid = proc_get_current_thread (pi);
@@ -1783,6 +1783,7 @@ remote_add_inferior (int fake_pid_p, int pid, int attached,
space. */
inf->aspace = maybe_new_address_space ();
inf->pspace = current_program_space;
+ inf->attach_flag = attached;
}
else
{
@@ -1790,10 +1791,10 @@ remote_add_inferior (int fake_pid_p, int pid, int attached,
between program/address spaces. We simply bind the inferior
to the program space's address space. */
inf = current_inferior ();
+ inf->attach_flag = attached;
inferior_appeared (inf, pid);
}
- inf->attach_flag = attached;
inf->fake_pid_p = fake_pid_p;
/* If no main executable is currently open then attempt to
new file mode 100644
@@ -0,0 +1,31 @@
+/* Copyright 2016 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <unistd.h>
+
+int
+main ()
+{
+ int i;
+
+ for (i = 0; i < 30; i++)
+ {
+ sleep (1);
+ }
+
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,46 @@
+# Copyright 2016 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+
+standard_testfile
+
+if {![can_spawn_for_attach]} {
+ untested ${testfile}.exp
+ return
+}
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != ""} {
+ untested ${testfile}.exp
+ return -1
+}
+
+# Test an attach in MI.
+
+proc test_mi_attach {} {
+ global binfile
+
+ set exec_spawn_id [spawn_wait_for_attach $binfile]
+ set exec_pid [spawn_id_get_pid $exec_spawn_id]
+
+ mi_gdb_start
+
+ mi_gdb_test "123-target-attach $exec_pid" \
+ [multi_line "=thread-group-started,id=\"i1\",pid=\"$exec_pid\",launch-type=\"attach\"" \
+ "=thread-created,id=\"1\",group-id=\"i1\".*" ] \
+ "attach to process"
+}
+
+test_mi_attach
@@ -1697,8 +1697,8 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
init_wait_for_inferior ();
inf = current_inferior ();
- inferior_appeared (inf, pid);
inf->attach_flag = attaching;
+ inferior_appeared (inf, pid);
/* Make the new process the current inferior, so terminal handling
can rely on it. When attaching, we don't know about any thread