[v5] Exit code of exited inferiors in -list-thread-groups

Message ID 1400018204-29559-1-git-send-email-simon.marchi@ericsson.com
State Superseded
Headers

Commit Message

Simon Marchi May 13, 2014, 9:56 p.m. UTC
  This one was also from a year ago, I would like to make sure it is still
OK.

Original submission: https://sourceware.org/ml/gdb-patches/2013-06/msg00744.html

Don't reset the exit code at inferior exit and print it in
-list-thread-groups.

gdb/ChangeLog:
2014-05-13  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-05-13  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-05-13  Simon Marchi  <simon.marchi@ericsson.com>

	* gdb.texinfo (Miscellaneous gdb/mi Commands): Document new
	exit-code field in -list-thread-groups output.
---
 gdb/NEWS                              |  5 ++
 gdb/doc/gdb.texinfo                   |  5 ++
 gdb/inferior.c                        |  4 +-
 gdb/mi/mi-main.c                      |  3 ++
 gdb/testsuite/gdb.mi/mi-exit-code.c   | 27 +++++++++++
 gdb/testsuite/gdb.mi/mi-exit-code.exp | 88 +++++++++++++++++++++++++++++++++++
 6 files changed, 130 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

Eli Zaretskii May 14, 2014, 2:51 a.m. UTC | #1
> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: Simon Marchi <simon.marchi@ericsson.com>
> Date: Tue, 13 May 2014 17:56:44 -0400
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -108,6 +108,11 @@ PowerPC64 GNU/Linux little-endian	powerpc64le-*-linux*
>    and "assf"), have been deprecated.  Use the "sharedlibrary" command, or
>    its alias "share", instead.
>  
> +* MI changes
> +
> +  ** The -list-thread-groups command outputs an exit-code field for
> +     inferiors that have exited.
> +
>  *** Changes in GDB 7.7

This part is OK.

> +@item exit-code
> +The exit code of this thread group when it last exited.  This field is
> +only present for thread groups of type @samp{process} and only if the
> +process is not running.

This is backwards, and the 1st sentence is confusing until you read
the second one.  I suggest to reword like this:

  If this thread group is a @samp{process} that exited, this field is
  the exit code of that inferior process.  Otherwise, the field is not
  present.

The documentation part is OK with this change.
  
Tom Tromey May 16, 2014, 8:30 p.m. UTC | #2
>>>>> "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
  
Simon Marchi May 21, 2014, 5:34 p.m. UTC | #3
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.

Simon
  
Pedro Alves May 21, 2014, 6:09 p.m. UTC | #4
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.

The =thread-group-exited code has:

mi_inferior_exit (struct inferior *inf)
{
  struct mi_interp *mi = top_level_interpreter_data ();

  target_terminal_ours ();
  if (inf->has_exit_code)
    fprintf_unfiltered (mi->event_channel,
			"thread-group-exited,id=\"i%d\",exit-code=\"%s\"",
			inf->num, int_string (inf->exit_code, 8, 0, 0, 1));

(I bet that's where the new code was copied from.)
  

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index d0c44ea..8c6b536 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -108,6 +108,11 @@  PowerPC64 GNU/Linux little-endian	powerpc64le-*-linux*
   and "assf"), have been deprecated.  Use the "sharedlibrary" command, or
   its alias "share", instead.
 
+* MI changes
+
+  ** 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
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2aff5e5..6c19253 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -31134,6 +31134,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
+The exit code of this thread group when it last exited.  This field is
+only present for thread groups of type @samp{process} and only if the
+process is not running.
+
 @item num_children
 The number of children this thread group has.  This field may be
 absent for an available thread group.
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 90d9649..6bd1d1b 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -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);
 }
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 1598af0..d8d66c4 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -616,6 +616,9 @@  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_string (uiout, "exit-code",
+			     int_string (inferior->exit_code, 8, 0, 0, 1));
       if (inferior->pid != 0)
 	ui_out_field_int (uiout, "pid", inferior->pid);
 
diff --git a/gdb/testsuite/gdb.mi/mi-exit-code.c b/gdb/testsuite/gdb.mi/mi-exit-code.c
new file mode 100644
index 0000000..df711a6
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-exit-code.c
@@ -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;
+}
diff --git a/gdb/testsuite/gdb.mi/mi-exit-code.exp b/gdb/testsuite/gdb.mi/mi-exit-code.exp
new file mode 100644
index 0000000..025879a
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-exit-code.exp
@@ -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 = 8" ".*\\^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=\"010\",executable=\".*\"\}\]" \
+	    "-list-thread-groups after exit shows exit-code"
+    }
+}
+
+test_list_thread_groups
+
+mi_gdb_exit
+return 0