[v5] Exit code of exited inferiors in -list-thread-groups
Commit Message
On 14-05-21 02:09 PM, Pedro Alves wrote:
> On 05/21/2014 06:34 PM, Simon Marchi wrote:
>> On Fri 16 May 2014 04:30:45 PM EDT, Tom Tromey wrote:
>>>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>>>
>>> Simon> This one was also from a year ago, I would like to make sure it is still
>>> Simon> OK.
>>>
>>> Simon> + if (inferior->has_exit_code)
>>> Simon> + ui_out_field_string (uiout, "exit-code",
>>> Simon> + int_string (inferior->exit_code, 8, 0, 0, 1));
>>>
>>> Why not the simpler ui_out_field_int?
>>> Going out of the way to print it in octal seems a bit odd for a machine
>>> interface.
>>>
>>> Tom
>>
>> Agreed. I found that the exit code is often represented in octal (the
>> reason for this probably predates my birth). But for MI, it does not
>> matter.
>
> Though it might be a little less surprising if all places that print
> the exit code print it the same way. That way it's possible that
> frontends just treat the exit code as a string, and present it as
> is to the user. They may already be doing that.
This is not a complex field to parse and output in whatever format. I don't see any
value in leaving it octal just to save that amount of work to frontends. I mean, if a
frontend can't parse a decimal number, how in the world does it use the rest of the MI. ;)
To the doc change suggested by Eli, I added a precision about the base of the value.
Including the doc change and the printing in decimal, does it look good ?
See updated patch below.
Simon
-----------------------
From c746bc18ab8cce63b8c7f7ef16b92535b8cc2a09 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Mon, 12 May 2014 16:18:25 -0400
Subject: [PATCH] Exit code of exited inferiors in -list-thread-groups
Don't reset the exit code at inferior exit and print it in
-list-thread-groups.
gdb/ChangeLog:
2014-06-02 Simon Marchi <simon.marchi@ericsson.com>
* NEWS: Announce new exit-code field in -list-thread-groups
output.
* inferior.c (exit_inferior_1): Don't clear exit code.
(inferior_appeared): Clear exit code.
* mi/mi-main.c (print_one_inferior): Add printing of the exit
code.
gdb/testsuite/ChangeLog:
2014-06-02 Simon Marchi <simon.marchi@ericsson.com>
* gdb.mi/mi-exit-code.exp: New file.
* gdb.mi/mi-exit-code.c: New file.
gdb/doc/ChangeLog:
2014-06-02 Simon Marchi <simon.marchi@ericsson.com>
* gdb.texinfo (Miscellaneous gdb/mi Commands): Document new
exit-code field in -list-thread-groups output.
---
gdb/NEWS | 3 ++
gdb/doc/gdb.texinfo | 5 ++
gdb/inferior.c | 4 +-
gdb/mi/mi-main.c | 2 +
gdb/testsuite/gdb.mi/mi-exit-code.c | 27 +++++++++++
gdb/testsuite/gdb.mi/mi-exit-code.exp | 88 +++++++++++++++++++++++++++++++++++
6 files changed, 127 insertions(+), 2 deletions(-)
create mode 100644 gdb/testsuite/gdb.mi/mi-exit-code.c
create mode 100644 gdb/testsuite/gdb.mi/mi-exit-code.exp
Comments
> Date: Mon, 2 Jun 2014 14:30:37 -0400
> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: <gdb-patches@sourceware.org>
>
> To the doc change suggested by Eli, I added a precision about the base of the value.
> Including the doc change and the printing in decimal, does it look good ?
Yes, thanks.
On 06/02/2014 07:30 PM, Simon Marchi wrote:
> On 14-05-21 02:09 PM, Pedro Alves wrote:
>> On 05/21/2014 06:34 PM, Simon Marchi wrote:
>>> On Fri 16 May 2014 04:30:45 PM EDT, Tom Tromey wrote:
>>>>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>>>>
>>>> Simon> This one was also from a year ago, I would like to make sure it is still
>>>> Simon> OK.
>>>>
>>>> Simon> + if (inferior->has_exit_code)
>>>> Simon> + ui_out_field_string (uiout, "exit-code",
>>>> Simon> + int_string (inferior->exit_code, 8, 0, 0, 1));
>>>>
>>>> Why not the simpler ui_out_field_int?
>>>> Going out of the way to print it in octal seems a bit odd for a machine
>>>> interface.
>>>>
>>>> Tom
>>>
>>> Agreed. I found that the exit code is often represented in octal (the
>>> reason for this probably predates my birth). But for MI, it does not
>>> matter.
>>
>> Though it might be a little less surprising if all places that print
>> the exit code print it the same way. That way it's possible that
>> frontends just treat the exit code as a string, and present it as
>> is to the user. They may already be doing that.
>
> This is not a complex field to parse and output in whatever format. I don't see any
> value in leaving it octal just to save that amount of work to frontends. I mean, if a
> frontend can't parse a decimal number, how in the world does it use the rest of the MI. ;)
Sure, but that's not the point. The point is that frontend might not
be _parsing_ the number _at all_, but just presenting it as a string
to the user as is (*). I don't think we even document anywhere that
this MI field is a number. With that in mind I don't think it's a good
idea to print the same info in different ways in different places.
The "if it ain't broke, don't fix it" principle would then lean on
keeping things octal, for consistency with what the user sees in the
console.
(Note you had missed updating at least one example in the manual.)
(*) - If I were writing a frontend that's what I'd do, as who knows
whether there's some target where the exit code might be best
presented some other way? E.g., on Windows, it makes more sense
to present them in hex.
On Mon 02 Jun 2014 03:23:11 PM EDT, Pedro Alves wrote:
> On 06/02/2014 07:30 PM, Simon Marchi wrote:
>> On 14-05-21 02:09 PM, Pedro Alves wrote:
>>> On 05/21/2014 06:34 PM, Simon Marchi wrote:
>>>> On Fri 16 May 2014 04:30:45 PM EDT, Tom Tromey wrote:
>>>>>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>>>>>
>>>>> Simon> This one was also from a year ago, I would like to make sure it is still
>>>>> Simon> OK.
>>>>>
>>>>> Simon> + if (inferior->has_exit_code)
>>>>> Simon> + ui_out_field_string (uiout, "exit-code",
>>>>> Simon> + int_string (inferior->exit_code, 8, 0, 0, 1));
>>>>>
>>>>> Why not the simpler ui_out_field_int?
>>>>> Going out of the way to print it in octal seems a bit odd for a machine
>>>>> interface.
>>>>>
>>>>> Tom
>>>>
>>>> Agreed. I found that the exit code is often represented in octal (the
>>>> reason for this probably predates my birth). But for MI, it does not
>>>> matter.
>>>
>>> Though it might be a little less surprising if all places that print
>>> the exit code print it the same way. That way it's possible that
>>> frontends just treat the exit code as a string, and present it as
>>> is to the user. They may already be doing that.
>>
>> This is not a complex field to parse and output in whatever format. I don't see any
>> value in leaving it octal just to save that amount of work to frontends. I mean, if a
>> frontend can't parse a decimal number, how in the world does it use the rest of the MI. ;)
>
> Sure, but that's not the point. The point is that frontend might not
> be _parsing_ the number _at all_, but just presenting it as a string
> to the user as is (*). I don't think we even document anywhere that
> this MI field is a number. With that in mind I don't think it's a good
> idea to print the same info in different ways in different places.
>
> The "if it ain't broke, don't fix it" principle would then lean on
> keeping things octal, for consistency with what the user sees in the
> console.
I had forgotten that the exit-code was already displayed somewhere else in the MI. Indeed,
having the same field in different formats in different records would make a very bad interface.
Speaking of which, the documentation for the *stopped record does not mention exit-code at
all. I think it should, since it appears in it when the exit code is not 0.
> (Note you had missed updating at least one example in the manual.)
I am not sure I understand. The examples in the -list-thread-groups command section? If so, I
believe they should not contain a exit-code field, as the process is running.
> (*) - If I were writing a frontend that's what I'd do, as who knows
> whether there's some target where the exit code might be best
> presented some other way? E.g., on Windows, it makes more sense
> to present them in hex.
Ah ok, I didn't realize that.
On 06/02/2014 09:20 PM, Simon Marchi wrote:
>> > keeping things octal, for consistency with what the user sees in the
>> > console.
> I had forgotten that the exit-code was already displayed somewhere else in the MI. Indeed,
> having the same field in different formats in different records would make a very bad interface.
>
> Speaking of which, the documentation for the *stopped record does not mention exit-code at
> all. I think it should, since it appears in it when the exit code is not 0.
Yeah. It only appears in an example of -exec-run:
@smallexample
(gdb)
-exec-run
^running
(gdb)
x = 55
*stopped,reason="exited",exit-code="01"
(gdb)
@end smallexample
>
>> > (Note you had missed updating at least one example in the manual.)
> I am not sure I understand. The examples in the -list-thread-groups command section? If so, I
> believe they should not contain a exit-code field, as the process is running.
I meant the -exec-run example above. But that was kind of me getting
confused, as the patch doesn't change the output in that case. So read
that as me pointing out a place in the manual where we show the result
in octal, that would need updating if we ever want to change the
output format consistently. :-)
Pedro> Sure, but that's not the point. The point is that frontend might
Pedro> not be _parsing_ the number _at all_, but just presenting it as a
Pedro> string to the user as is (*).
It's unfortunate that we can even be in this situation.
I don't object to using octal. I was unaware of the precedent.
However now it seems that it would be an improvement to document that
the exit code is emitted in octal.
Tom
@@ -176,6 +176,9 @@ PowerPC64 GNU/Linux little-endian powerpc64le-*-linux*
Previously "-gdb-set target-async off" affected both MI execution
commands and CLI execution commands.
+ ** The -list-thread-groups command outputs an exit-code field for
+ inferiors that have exited.
+
*** Changes in GDB 7.7
* Improved support for process record-replay and reverse debugging on
@@ -31229,6 +31229,11 @@ valid type.
The target-specific process identifier. This field is only present
for thread groups of type @samp{process} and only if the process exists.
+@item exit-code
+If this thread group is a @samp{process} that exited, this field is the
+exit code of that inferior process in decimal form. Otherwise, the field
+is not present.
+
@item num_children
The number of children this thread group has. This field may be
absent for an available thread group.
@@ -275,8 +275,6 @@ exit_inferior_1 (struct inferior *inftoex, int silent)
inf->vfork_child = NULL;
}
- inf->has_exit_code = 0;
- inf->exit_code = 0;
inf->pending_detach = 0;
}
@@ -322,6 +320,8 @@ void
inferior_appeared (struct inferior *inf, int pid)
{
inf->pid = pid;
+ inf->has_exit_code = 0;
+ inf->exit_code = 0;
observer_notify_inferior_appeared (inf);
}
@@ -659,6 +659,8 @@ print_one_inferior (struct inferior *inferior, void *xdata)
ui_out_field_fmt (uiout, "id", "i%d", inferior->num);
ui_out_field_string (uiout, "type", "process");
+ if (inferior->has_exit_code)
+ ui_out_field_int (uiout, "exit-code", inferior->exit_code);
if (inferior->pid != 0)
ui_out_field_int (uiout, "pid", inferior->pid);
new file mode 100644
@@ -0,0 +1,27 @@
+/* Copyright 1999-2014 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/>. */
+
+int exit_code = 0;
+
+int
+main (int argc, char **argv)
+{
+ if (argc > 1)
+ exit_code = atoi (argv[1]);
+
+ return exit_code;
+}
new file mode 100644
@@ -0,0 +1,88 @@
+# Copyright 1999-2014 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
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+ continue
+}
+
+standard_testfile
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ untested "failed to compile $testfile"
+ return -1
+}
+
+proc test_list_thread_groups { } {
+ global hex
+ global decimal
+
+ # Before any run, exit-code should not be present.
+ mi_gdb_test \
+ "122-list-thread-groups" \
+ "122\\^done,groups=\\\[\{id=\"i1\",type=\"process\"\}\]" \
+ "-list-thread-groups before run shows no exit-code"
+
+ with_test_prefix "first run" {
+ mi_run_to_main
+
+ # During the run, exit-code should not be present.
+ mi_gdb_test \
+ "123-list-thread-groups" \
+ "123\\^done,groups=\\\[\{id=\"i1\",type=\"process\",pid=\"$decimal\",executable=\".*\".*" \
+ "-list-thread-groups during run shows no exit-code"
+
+ # Exit the inferior.
+ mi_send_resuming_command "exec-continue" "continuing to inferior exit"
+ mi_expect_stop "exited-normally" "" "" "" "" "" "exit normally"
+
+ # After the run, exit-code should be present.
+ mi_gdb_test \
+ "124-list-thread-groups" \
+ "124\\^done,groups=\\\[\{id=\"i1\",type=\"process\",exit-code=\"0\",executable=\".*\"\}\]" \
+ "-list-thread-groups after exit shows exit-code"
+ }
+
+ with_test_prefix "second run" {
+ mi_run_to_main
+
+ # Write the exit code we want in the global var
+ mi_gdb_test "set var exit_code = 12" ".*\\^done" "write exit code"
+
+ # During the second run, exit-code should not be present.
+ mi_gdb_test \
+ "125-list-thread-groups" \
+ "125\\^done,groups=\\\[\{id=\"i1\",type=\"process\",pid=\"$decimal\",executable=\".*\".*" \
+ "-list-thread-groups during run shows no exit-code"
+
+ # Exit the inferior.
+ mi_send_resuming_command "exec-continue" "continuing to inferior exit"
+ mi_expect_stop "exited" "" "" "" "" "" "exit with code"
+
+ # After the second run, exit-code should be present.
+ mi_gdb_test \
+ "126-list-thread-groups" \
+ "126\\^done,groups=\\\[\{id=\"i1\",type=\"process\",exit-code=\"12\",executable=\".*\"\}\]" \
+ "-list-thread-groups after exit shows exit-code"
+ }
+}
+
+test_list_thread_groups
+
+mi_gdb_exit
+return 0