RFC: GDB as a loader 2/3: return child result

RFC: GDB as a loader 2/3: return child result

[Andrew STUBBS]

[-- Attachment #1: Type: text/plain, Size: 833 bytes --]

Hi all,

The attached patch implements a new option --return-child-result. This
option causes GDB to return the return value of the last child
(inferior) program to run. The patch assumes that the batch-silent patch
has already been applied.

Note that 'quit <expr>' still works as expected. Also, any exit through
a mechanism other than quit_force (i.e. errors) gives the same exit code
as it did before. Batch mode has been adjusted to exit through
quit_force in order to ensure it give the right result.

I am not sure that this has been implemented in the best way. The
declaration of extern variables probably ought to be moved to a header
file somewhere, but I'm not sure which is best. It has also been
suggested that it ought to use the value stored in the existing
$_exitcode convenience variable.

Thanks

Andrew Stubbs




[-- Attachment #2: return-child-result.patch --]
[-- Type: text/plain, Size: 3826 bytes --]

2005-10-18  Andrew Stubbs  <andrew.stubbs@st.com>

	* infrun.c (print_stop_reason): Set return_child_result_value on exit.
	* main.c (return_child_result): New variable.
	(return_child_result_value): Likewise.
	(captured_main): Add option --return-child-result.
	Replace call to target_detach and exit (in batch mode) with quit_force.
	(print_gdb_help): Add option --return-child-result.
	* top.c (quit_force): Return child result if appropriate.


Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2005-09-29 21:41:27.000000000 +0100
+++ src/gdb/infrun.c	2005-10-17 18:00:23.000000000 +0100
@@ -2931,6 +2931,11 @@ print_stop_reason (enum inferior_stop_re
 	       async_reason_lookup (EXEC_ASYNC_EXITED_NORMALLY));
 	  ui_out_text (uiout, "\nProgram exited normally.\n");
 	}
+      {
+	/* Support the --return-child-result option.  */
+	extern int return_child_result_value;
+	return_child_result_value = stop_info;
+      }
       break;
     case SIGNAL_RECEIVED:
       /* Signal received. The signal table tells us to print about
Index: src/gdb/main.c
===================================================================
--- src.orig/gdb/main.c	2005-10-17 17:44:35.000000000 +0100
+++ src/gdb/main.c	2005-10-17 18:19:31.000000000 +0100
@@ -76,6 +76,12 @@ struct ui_file *gdb_stdtargerr;
 /* Support for the --batch-silent option.  */
 int batch_silent = 0;
 
+/* Support for --return-child-result option.
+   Set the default to -1 to return error in the case
+   that the program does not run or does not complete.  */
+int return_child_result = 0;
+int return_child_result_value = -1;
+
 /* Whether to enable writing into executable and core files */
 extern int write_files;
 
@@ -303,6 +309,7 @@ captured_main (void *data)
       {"write", no_argument, &write_files, 1},
       {"args", no_argument, &set_args, 1},
      {"l", required_argument, 0, 'l'},
+      {"return-child-result", no_argument, &return_child_result, 1},
       {0, no_argument, 0, 0}
     };
 
@@ -735,15 +742,8 @@ extern int gdbtk_test (char *);
 
   if (batch)
     {
-      if (attach_flag)
-	/* Either there was a problem executing the command in the
-	   batch file aborted early, or the batch file forgot to do an
-	   explicit detach.  Explicitly detach the inferior ensuring
-	   that there are no zombies.  */
-	target_detach (NULL, 0);
-      
       /* We have hit the end of the batch file.  */
-      exit (0);
+      quit_force (NULL, 0);
     }
 
   /* Do any host- or target-specific hacks.  This is used for i960 targets
@@ -843,6 +843,8 @@ Options:\n\n\
   -b BAUDRATE        Set serial port baud rate used for remote debugging.\n\
   --batch            Exit after processing options.\n\
   --batch-silent     As for --batch, but suppress all gdb stdout output.\n\
+  --return-child-result\n\
+                     GDB exit code will be the child's exit code.\n\
   --cd=DIR           Change current directory to DIR.\n\
   --command=FILE     Execute GDB commands from FILE.\n\
   --core=COREFILE    Analyze the core dump COREFILE.\n\
Index: src/gdb/top.c
===================================================================
--- src.orig/gdb/top.c	2005-07-04 14:29:12.000000000 +0100
+++ src/gdb/top.c	2005-10-17 18:00:23.000000000 +0100
@@ -1177,6 +1177,7 @@ quit_force (char *args, int from_tty)
 {
   int exit_code = 0;
   struct qt_args qt;
+  extern int return_child_result, return_child_result_value;
 
   /* An optional expression may be used to cause gdb to terminate with the 
      value of that expression. */
@@ -1186,6 +1187,8 @@ quit_force (char *args, int from_tty)
 
       exit_code = (int) value_as_long (val);
     }
+  else if (return_child_result)
+    exit_code = return_child_result_value;
 
   qt.args = args;
   qt.from_tty = from_tty;

Re: RFC: GDB as a loader 2/3: return child result

[Andrew STUBBS]

[-- Attachment #1: Type: text/plain, Size: 77 bytes --]

Here is a documentation patch to go with the original patch.

Andrew Stubbs


[-- Attachment #2: return-result-docs.patch --]
[-- Type: text/plain, Size: 1051 bytes --]

2005-10-28  Andrew Stubbs  <andrew.stubbs@st.com>

	* gdb.texinfo (Choosing modes): Add --return-child-result.


Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo	2005-10-28 10:37:07.000000000 +0100
+++ src/gdb/doc/gdb.texinfo	2005-10-28 11:35:18.000000000 +0100
@@ -1006,6 +1006,16 @@ This is particularly useful when using t
 Note that targets that give their output via @value{GDBN}, as opposed to
 writing directly to @code{stdout}, will also be made silent.
 
+@item -return-child-result
+@cindex @code{--return-child-result}
+The return code from @value{GDBN} will be the return code from the child
+process (the process being debugged), unless an error occurs or the user
+quits with an explicit value, such as @samp{quit 1}, for example.
+
+This is useful in conjunction with @samp{-batch} or @samp{-batch-silent},
+when @value{GDBN} is being used as a remote program loader or simulator
+interface.
+
 @item -nowindows
 @itemx -nw
 @cindex @code{--nowindows}

Re: RFC: GDB as a loader 2/3: return child result

[Eli Zaretskii]

> Date: Fri, 28 Oct 2005 12:04:06 +0100
> From: Andrew STUBBS <andrew.stubbs@st.com>
> 
> Here is a documentation patch to go with the original patch.

Thanks.  My comments are below.

> +The return code from @value{GDBN} will be the return code from the child
> +process (the process being debugged), unless an error occurs or the user
> +quits with an explicit value, such as @samp{quit 1}, for example.

The "error occurs" part is not clear enough, IMO.  Could you please
state more clearly what kinds of error could cause this ``unless''
clause to kick in?  Also, if such an error does occur, what will GDB
return as its exit status in that case?

I think we should spell out all this information, since this option is
meant to be used by people who write shell scripts that invoke GDB in
batch mode.  If I'd need to write such a script, I'd wish to know
exactly what kinds of exit codes I will see and under what
circumstances.

Otherwise, this patch is fine with me.

Re: RFC: GDB as a loader 2/3: return child result

[Andrew STUBBS]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

Eli Zaretskii wrote:
> The "error occurs" part is not clear enough, IMO.  Could you please
> state more clearly what kinds of error could cause this ``unless''
> clause to kick in?  Also, if such an error does occur, what will GDB
> return as its exit status in that case?
> 
> I think we should spell out all this information, since this option is
> meant to be used by people who write shell scripts that invoke GDB in
> batch mode.  If I'd need to write such a script, I'd wish to know
> exactly what kinds of exit codes I will see and under what
> circumstances.

Ok. How about the attached?

Andrew

[-- Attachment #2: return-result-docs.patch --]
[-- Type: text/plain, Size: 1396 bytes --]

2005-10-28  Andrew Stubbs  <andrew.stubbs@st.com>

	* gdb.texinfo (Choosing modes): Add --return-child-result.


Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo	2005-10-28 14:04:49.000000000 +0100
+++ src/gdb/doc/gdb.texinfo	2005-10-28 14:39:29.000000000 +0100
@@ -1006,6 +1006,27 @@ This is particularly useful when using t
 Note that targets that give their output via @value{GDBN}, as opposed to
 writing directly to @code{stdout}, will also be made silent.
 
+@item -return-child-result
+@cindex @code{--return-child-result}
+The return code from @value{GDBN} will be the return code from the child
+process (the process being debugged), with the following exceptions:
+
+@itemize @bullet
+@item
+@value{GDBN} exits abnormally.  E.g. due to an incorrect argument or an
+internal error.  In this case the exit code is the same as it would have been
+without @samp{-return-child-result}.
+@item
+The user quits with an explicit value.  E.g. @samp{quit 1}.
+@item
+The child process never runs, or is not allowed to terminate, in which case
+the exit code will be -1.
+@end itemize
+
+This option is useful in conjunction with @samp{-batch} or @samp{-batch-silent},
+when @value{GDBN} is being used as a remote program loader or simulator
+interface.
+
 @item -nowindows
 @itemx -nw
 @cindex @code{--nowindows}

Re: RFC: GDB as a loader 2/3: return child result

[Eli Zaretskii]

> Date: Fri, 28 Oct 2005 14:56:58 +0100
> From: Andrew STUBBS <andrew.stubbs@st.com>
> Cc: gdb@sources.redhat.com
> 
> > I think we should spell out all this information, since this option is
> > meant to be used by people who write shell scripts that invoke GDB in
> > batch mode.  If I'd need to write such a script, I'd wish to know
> > exactly what kinds of exit codes I will see and under what
> > circumstances.
> 
> Ok. How about the attached?

The new text is fine, thanks.

> +@value{GDBN} exits abnormally.  E.g. due to an incorrect argument or an

"E.g." should be followed by a comma.

> +The user quits with an explicit value.  E.g. @samp{quit 1}.

Same here.

Re: RFC: GDB as a loader 2/3: return child result

[Andrew STUBBS]

[-- Attachment #1: Type: text/plain, Size: 305 bytes --]

Eli Zaretskii wrote:
>>Ok. How about the attached?
> 
> 
> The new text is fine, thanks.
> 
>>+@value{GDBN} exits abnormally.  E.g. due to an incorrect argument or an
> 
> "E.g." should be followed by a comma.

Patch with commas attached.

Am I ok to submit the code and docs patches now?

Thanks

Andrew

[-- Attachment #2: return-result-docs.patch --]
[-- Type: text/plain, Size: 1380 bytes --]

2005-10-03  Andrew Stubbs  <andrew.stubbs@st.com>

	* gdb.texinfo (Choosing modes): Add --return-child-result.


Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo	2005-10-31 12:18:35.000000000 +0000
+++ src/gdb/doc/gdb.texinfo	2005-10-31 13:04:56.000000000 +0000
@@ -1007,6 +1007,27 @@ messages, for example.
 Note that targets that give their output via @value{GDBN}, as opposed to
 writing directly to @code{stdout}, will also be made silent.
 
+@item -return-child-result
+@cindex @code{--return-child-result}
+The return code from @value{GDBN} will be the return code from the child
+process (the process being debugged), with the following exceptions:
+
+@itemize @bullet
+@item
+@value{GDBN} exits abnormally.  E.g., due to an incorrect argument or an
+internal error.  In this case the exit code is the same as it would have been
+without @samp{-return-child-result}.
+@item
+The user quits with an explicit value.  E.g., @samp{quit 1}.
+@item
+The child process never runs, or is not allowed to terminate, in which case
+the exit code will be -1.
+@end itemize
+
+This option is useful in conjunction with @samp{-batch} or @samp{-batch-silent},
+when @value{GDBN} is being used as a remote program loader or simulator
+interface.
+
 @item -nowindows
 @itemx -nw
 @cindex @code{--nowindows}

Re: RFC: GDB as a loader 2/3: return child result

[Eli Zaretskii]

> Date: Thu, 03 Nov 2005 11:55:32 +0000
> From: Andrew STUBBS <andrew.stubbs@st.com>
> Cc: gdb@sources.redhat.com
> 
> Eli Zaretskii wrote:
> >>Ok. How about the attached?
> > 
> > 
> > The new text is fine, thanks.
> > 
> >>+@value{GDBN} exits abnormally.  E.g. due to an incorrect argument or an
> > 
> > "E.g." should be followed by a comma.
> 
> Patch with commas attached.
> 
> Am I ok to submit the code and docs patches now?

Yes, thanks.

Re: RFC: GDB as a loader 2/3: return child result

[Andrew STUBBS]

Eli Zaretskii wrote:
>>Am I ok to submit the code and docs patches now?
> 
> 
> Yes, thanks.

Done.

Re: RFC: GDB as a loader 2/3: return child result

[Daniel Jacobowitz]

On Tue, Oct 18, 2005 at 12:28:21PM +0100, Andrew STUBBS wrote:
> Hi all,
> 
> The attached patch implements a new option --return-child-result. This
> option causes GDB to return the return value of the last child
> (inferior) program to run. The patch assumes that the batch-silent patch
> has already been applied.
> 
> Note that 'quit <expr>' still works as expected. Also, any exit through
> a mechanism other than quit_force (i.e. errors) gives the same exit code
> as it did before. Batch mode has been adjusted to exit through
> quit_force in order to ensure it give the right result.
> 
> I am not sure that this has been implemented in the best way. The
> declaration of extern variables probably ought to be moved to a header
> file somewhere, but I'm not sure which is best. It has also been
> suggested that it ought to use the value stored in the existing
> $_exitcode convenience variable.

Sorry, I never got around to looking at the code portion of this. 
Would you mind making one cleanup for me?

It doesn't really matter which header the externs go in.  But they have
to go in a header, visible at both the point of definition and the
point of use.  No externs in C files.  top.h or main.h should be fine.

Otherwise it looks great.  Thanks.


-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: RFC: GDB as a loader 2/3: return child result

[Andrew STUBBS]

[-- Attachment #1: Type: text/plain, Size: 422 bytes --]

Daniel Jacobowitz wrote:
> Sorry, I never got around to looking at the code portion of this. 
> Would you mind making one cleanup for me?
> 
> It doesn't really matter which header the externs go in.  But they have
> to go in a header, visible at both the point of definition and the
> point of use.  No externs in C files.  top.h or main.h should be fine.

Ok. How about the attached modification?

Thanks

Andrew Stubbs

[-- Attachment #2: move-externs.patch --]
[-- Type: text/plain, Size: 2314 bytes --]

2005-11-07  Andrew Stubbs  <andrew.stubbs@st.com>

	* main.h (return_child_result, return_child_result_value): Declare.
	* infrun.h: Include main.h.
	(print_stop_reason): Remove declaration of return_child_result_value.
	* top.c; Include main.h.
	(quit_force): Remove declarations of return_child_result_value and
	return_child_result.

Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2005-11-04 11:42:07.000000000 +0000
+++ src/gdb/infrun.c	2005-11-07 12:00:36.000000000 +0000
@@ -46,6 +46,7 @@
 #include "observer.h"
 #include "language.h"
 #include "solib.h"
+#include "main.h"
 
 #include "gdb_assert.h"
 #include "mi/mi-common.h"
@@ -2931,11 +2932,8 @@ print_stop_reason (enum inferior_stop_re
 	       async_reason_lookup (EXEC_ASYNC_EXITED_NORMALLY));
 	  ui_out_text (uiout, "\nProgram exited normally.\n");
 	}
-      {
-	/* Support the --return-child-result option.  */
-	extern int return_child_result_value;
-	return_child_result_value = stop_info;
-      }
+      /* Support the --return-child-result option.  */
+      return_child_result_value = stop_info;
       break;
     case SIGNAL_RECEIVED:
       /* Signal received. The signal table tells us to print about
Index: src/gdb/main.h
===================================================================
--- src.orig/gdb/main.h	2003-02-13 18:07:24.000000000 +0000
+++ src/gdb/main.h	2005-11-07 11:59:07.000000000 +0000
@@ -32,4 +32,8 @@ struct captured_main_args
 
 extern int gdb_main (struct captured_main_args *);
 
+/* From main.c.  */
+extern int return_child_result;
+extern int return_child_result_value;
+
 #endif
Index: src/gdb/top.c
===================================================================
--- src.orig/gdb/top.c	2005-11-07 11:50:21.000000000 +0000
+++ src/gdb/top.c	2005-11-07 11:56:17.000000000 +0000
@@ -46,6 +46,7 @@
 #include "serial.h"
 #include "doublest.h"
 #include "gdb_assert.h"
+#include "main.h"
 
 /* readline include files */
 #include "readline/readline.h"
@@ -1177,7 +1178,6 @@ quit_force (char *args, int from_tty)
 {
   int exit_code = 0;
   struct qt_args qt;
-  extern int return_child_result, return_child_result_value;
 
   /* An optional expression may be used to cause gdb to terminate with the 
      value of that expression. */

Re: RFC: GDB as a loader 2/3: return child result

[Daniel Jacobowitz]

On Mon, Nov 07, 2005 at 12:17:27PM +0000, Andrew STUBBS wrote:
> Daniel Jacobowitz wrote:
> >Sorry, I never got around to looking at the code portion of this. 
> >Would you mind making one cleanup for me?
> >
> >It doesn't really matter which header the externs go in.  But they have
> >to go in a header, visible at both the point of definition and the
> >point of use.  No externs in C files.  top.h or main.h should be fine.
> 
> Ok. How about the attached modification?

Perfect with one addition:

> 	* infrun.h: Include main.h.
> 	* top.c; Include main.h.

These mean you need to update Makefile.in, too.  We use mostly manually
computed dependencies.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: RFC: GDB as a loader 2/3: return child result

[Andrew STUBBS]

[-- Attachment #1: Type: text/plain, Size: 350 bytes --]

Daniel Jacobowitz wrote:
> Perfect with one addition:
> 
> 
>>	* infrun.h: Include main.h.
>>	* top.c; Include main.h.
> 
> 
> These mean you need to update Makefile.in, too.  We use mostly manually
> computed dependencies.

Ok, I didn't know that (but should've guessed).

Updated patch attached. I've also fixed the typos in the changelog.

Andrew

[-- Attachment #2: move-externs.patch --]
[-- Type: text/plain, Size: 3579 bytes --]

2005-11-07  Andrew Stubbs  <andrew.stubbs@st.com>

	* main.h (return_child_result, return_child_result_value): Declare.
	* infrun.c: Include main.h.
	(print_stop_reason): Remove declaration of return_child_result_value.
	* top.c: Include main.h.
	(quit_force): Remove declarations of return_child_result_value and
	return_child_result.
	* Makefile.in (top.o, infrun.o): Add main.h to dependencies.

Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2005-11-07 12:23:30.000000000 +0000
+++ src/gdb/infrun.c	2005-11-07 14:18:29.000000000 +0000
@@ -46,6 +46,7 @@
 #include "observer.h"
 #include "language.h"
 #include "solib.h"
+#include "main.h"
 
 #include "gdb_assert.h"
 #include "mi/mi-common.h"
@@ -2931,11 +2932,8 @@ print_stop_reason (enum inferior_stop_re
 	       async_reason_lookup (EXEC_ASYNC_EXITED_NORMALLY));
 	  ui_out_text (uiout, "\nProgram exited normally.\n");
 	}
-      {
-	/* Support the --return-child-result option.  */
-	extern int return_child_result_value;
-	return_child_result_value = stop_info;
-      }
+      /* Support the --return-child-result option.  */
+      return_child_result_value = stop_info;
       break;
     case SIGNAL_RECEIVED:
       /* Signal received. The signal table tells us to print about
Index: src/gdb/main.h
===================================================================
--- src.orig/gdb/main.h	2005-11-07 12:23:30.000000000 +0000
+++ src/gdb/main.h	2005-11-07 14:18:29.000000000 +0000
@@ -32,4 +32,8 @@ struct captured_main_args
 
 extern int gdb_main (struct captured_main_args *);
 
+/* From main.c.  */
+extern int return_child_result;
+extern int return_child_result_value;
+
 #endif
Index: src/gdb/top.c
===================================================================
--- src.orig/gdb/top.c	2005-11-07 12:23:30.000000000 +0000
+++ src/gdb/top.c	2005-11-07 14:18:29.000000000 +0000
@@ -46,6 +46,7 @@
 #include "serial.h"
 #include "doublest.h"
 #include "gdb_assert.h"
+#include "main.h"
 
 /* readline include files */
 #include "readline/readline.h"
@@ -1177,7 +1178,6 @@ quit_force (char *args, int from_tty)
 {
   int exit_code = 0;
   struct qt_args qt;
-  extern int return_child_result, return_child_result_value;
 
   /* An optional expression may be used to cause gdb to terminate with the 
      value of that expression. */
Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2005-11-01 10:32:11.000000000 +0000
+++ src/gdb/Makefile.in	2005-11-07 14:26:39.000000000 +0000
@@ -2133,7 +2133,7 @@ infrun.o: infrun.c $(defs_h) $(gdb_strin
 	$(gdbcore_h) $(gdbcmd_h) $(cli_script_h) $(target_h) $(gdbthread_h) \
 	$(annotate_h) $(symfile_h) $(top_h) $(inf_loop_h) $(regcache_h) \
 	$(value_h) $(observer_h) $(language_h) $(solib_h) $(gdb_assert_h) \
-	$(mi_common_h)
+	$(mi_common_h) $(main_h)
 inftarg.o: inftarg.c $(defs_h) $(frame_h) $(inferior_h) $(target_h) \
 	$(gdbcore_h) $(command_h) $(gdb_stat_h) $(observer_h) $(gdb_wait_h) \
 	$(inflow_h)
@@ -2689,7 +2689,7 @@ top.o: top.c $(defs_h) $(gdbcmd_h) $(cal
 	$(annotate_h) $(completer_h) $(top_h) $(version_h) $(serial_h) \
 	$(doublest_h) $(gdb_assert_h) $(readline_h) $(readline_history_h) \
 	$(event_top_h) $(gdb_string_h) $(gdb_stat_h) $(ui_out_h) \
-	$(cli_out_h)
+	$(cli_out_h) $(main_h)
 tracepoint.o: tracepoint.c $(defs_h) $(symtab_h) $(frame_h) $(gdbtypes_h) \
 	$(expression_h) $(gdbcmd_h) $(value_h) $(target_h) $(language_h) \
 	$(gdb_string_h) $(inferior_h) $(tracepoint_h) $(remote_h) \

Re: RFC: GDB as a loader 2/3: return child result

[Daniel Jacobowitz]

On Mon, Nov 07, 2005 at 02:37:39PM +0000, Andrew STUBBS wrote:
> Updated patch attached. I've also fixed the typos in the changelog.
> 
> Andrew

> 2005-11-07  Andrew Stubbs  <andrew.stubbs@st.com>
> 
> 	* main.h (return_child_result, return_child_result_value): Declare.
> 	* infrun.c: Include main.h.
> 	(print_stop_reason): Remove declaration of return_child_result_value.
> 	* top.c: Include main.h.
> 	(quit_force): Remove declarations of return_child_result_value and
> 	return_child_result.
> 	* Makefile.in (top.o, infrun.o): Add main.h to dependencies.

Thanks, this is OK.


-- 
Daniel Jacobowitz
CodeSourcery, LLC