PATCH: GDB/MI - implement -exec-abort

PATCH: GDB/MI - implement -exec-abort

[Dennis Brueni]

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


The -exec–abort command has been documented as the MI equivalent to the kill command since GDB 5.1, but it was never implemented.

This patch does that.

gdb/ChangeLog
2014-10-23  Dennis Brueni  <dbrueni@slickedit.com>

* mi/mi-cmds.c Add "exec-abort”
* mi/mi-cmds.h Add prototype for mi_cmd_exec_abort
* mi/mi-main.c Add mi_cmd_exec_abort (identical to kill_command minus prompting)

gdb/doc/ChangeLog
2014-10-23  Dennis Brueni  <dbrueni@slickedit.com>

* doc/gdb.texinfo Revive -exec–abort documentation, add example.



[-- Attachment #2: gdb-7.8-exec-abort.diff --]
[-- Type: application/octet-stream, Size: 3149 bytes --]

diff -urN gdb-7.8-dist/gdb/mi/mi-cmds.c gdb-7.8/gdb/mi/mi-cmds.c
--- gdb-7.8-dist/gdb/mi/mi-cmds.c	2014-06-11 09:34:41.000000000 -0700
+++ gdb-7.8/gdb/mi/mi-cmds.c	2014-10-23 10:59:36.000000000 -0700
@@ -103,6 +103,7 @@
   DEF_MI_CMD_MI ("exec-finish", mi_cmd_exec_finish),
   DEF_MI_CMD_MI ("exec-jump", mi_cmd_exec_jump),
   DEF_MI_CMD_MI ("exec-interrupt", mi_cmd_exec_interrupt),
+  DEF_MI_CMD_MI ("exec-abort", mi_cmd_exec_abort),
   DEF_MI_CMD_MI ("exec-next", mi_cmd_exec_next),
   DEF_MI_CMD_MI ("exec-next-instruction", mi_cmd_exec_next_instruction),
   DEF_MI_CMD_MI ("exec-return", mi_cmd_exec_return),
diff -urN gdb-7.8-dist/gdb/mi/mi-cmds.h gdb-7.8/gdb/mi/mi-cmds.h
--- gdb-7.8-dist/gdb/mi/mi-cmds.h	2014-06-11 09:34:41.000000000 -0700
+++ gdb-7.8/gdb/mi/mi-cmds.h	2014-10-23 10:59:34.000000000 -0700
@@ -61,6 +61,7 @@
 extern mi_cmd_argv_ftype mi_cmd_exec_continue;
 extern mi_cmd_argv_ftype mi_cmd_exec_finish;
 extern mi_cmd_argv_ftype mi_cmd_exec_interrupt;
+extern mi_cmd_argv_ftype mi_cmd_exec_abort;
 extern mi_cmd_argv_ftype mi_cmd_exec_jump;
 extern mi_cmd_argv_ftype mi_cmd_exec_next;
 extern mi_cmd_argv_ftype mi_cmd_exec_next_instruction;
diff -urN gdb-7.8-dist/gdb/mi/mi-main.c gdb-7.8/gdb/mi/mi-main.c
--- gdb-7.8-dist/gdb/mi/mi-main.c	2014-07-29 05:37:42.000000000 -0700
+++ gdb-7.8/gdb/mi/mi-main.c	2014-10-23 11:27:27.000000000 -0700
@@ -405,6 +405,36 @@
     }
 }
 
+/* Stop the execution of the target. */
+
+void
+mi_cmd_exec_abort (char *command, char **argv, int argc)
+{
+  /* FIXME:  This should not really be inferior_ptid (or target_has_execution).
+     It should be a distinct flag that indicates that a target is active, cuz
+     some targets don't have processes!  */
+
+  if (ptid_equal (inferior_ptid, null_ptid))
+    error (_("The program is not being run."));
+  target_kill ();
+
+  /* If we still have other inferiors to debug, then don't mess with
+     with their threads.  */
+  if (!have_inferiors ())
+    {
+      init_thread_list ();		/* Destroy thread info.  */
+
+      /* Killing off the inferior can leave us with a core file.  If
+	 so, print the state we are left in.  */
+      if (target_has_stack)
+	{
+	  printf_filtered (_("In %s,\n"), target_longname);
+	  print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
+	}
+    }
+  bfd_cache_close_all ();
+}
+
 /* Callback for iterate_over_inferiors which starts the execution
    of the given inferior.
 
diff -urN gdb-7.8-dist/gdb/doc/gdb.texinfo gdb-7.8/gdb/doc/gdb.texinfo
--- gdb-7.8-dist/gdb/doc/gdb.texinfo	2014-07-29 05:37:42.000000000 -0700
+++ gdb-7.8/gdb/doc/gdb.texinfo	2014-10-23 13:17:12.000000000 -0700
@@ -31051,7 +31051,6 @@
 @end smallexample
 
 
-@ignore
 @subheading The @code{-exec-abort} Command
 @findex -exec-abort
 
@@ -31068,8 +31067,16 @@
 The corresponding @value{GDBN} command is @samp{kill}.
 
 @subsubheading Example
-N.A.
-@end ignore
+@smallexample
+(gdb) 
+-exec-abort
+=thread-exited,id="9",group-id="i1"
+=thread-exited,id="2",group-id="i1"
+=thread-exited,id="1",group-id="i1"
+=thread-group-exited,id="i1"
+^done
+(gdb) 
+@end smallexample
 
 
 @subheading The @code{-gdb-set} Command

Re: PATCH: GDB/MI - implement -exec-abort

[Eli Zaretskii]

> From: Dennis Brueni <dbrueni@slickedit.com>
> CC: Dennis Brueni <dbrueni@slickedit.com>
> Date: Thu, 23 Oct 2014 20:25:21 +0000
> 
> The -exec–abort command has been documented as the MI equivalent to the kill command since GDB 5.1, but it was never implemented.
> 
> This patch does that.

Thanks.

The documentation part is OK, with 2 comments:

 1) I think this is NEWS-worthy
 2) Please fix the ChangeLog entry:

> gdb/doc/ChangeLog
> 2014-10-23  Dennis Brueni  <dbrueni@slickedit.com>
> 
> * doc/gdb.texinfo Revive -exec–abort documentation, add example.

It should mention the node name as if it were a function.

Re: PATCH: GDB/MI - implement -exec-abort

[Dennis Brueni]

gdb/NEWS:

* MI Changes

  ** The -exec-abort command is now implemented.


gdb/doc/ChangeLog

2014-10-23  Dennis Brueni  <dbrueni@slickedit.com>

	* gdb.texinfo  (GDB/MI Miscellaneous Commands):
	Revive -exec­abort documentation and add example.




On 10/23/14, 1:43 PM, "Eli Zaretskii" <eliz@gnu.org> wrote:

>> From: Dennis Brueni <dbrueni@slickedit.com>
>> CC: Dennis Brueni <dbrueni@slickedit.com>
>> Date: Thu, 23 Oct 2014 20:25:21 +0000
>> 
>> The -exec­abort command has been documented as the MI equivalent to the
>>kill command since GDB 5.1, but it was never implemented.
>> 
>> This patch does that.
>
>Thanks.
>
>The documentation part is OK, with 2 comments:
>
> 1) I think this is NEWS-worthy
> 2) Please fix the ChangeLog entry:
>
>> gdb/doc/ChangeLog
>> 2014-10-23  Dennis Brueni  <dbrueni@slickedit.com>
>> 
>> * doc/gdb.texinfo Revive -exec­abort documentation, add example.
>
>It should mention the node name as if it were a function.

Re: PATCH: GDB/MI - implement -exec-abort

[Eli Zaretskii]

> From: Dennis Brueni <dbrueni@slickedit.com>
> CC: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> Date: Thu, 23 Oct 2014 22:01:39 +0000
> 
> 
> gdb/NEWS:
> 
> * MI Changes
> 
>   ** The -exec-abort command is now implemented.
> 
> 
> gdb/doc/ChangeLog
> 
> 2014-10-23  Dennis Brueni  <dbrueni@slickedit.com>
> 
> 	* gdb.texinfo  (GDB/MI Miscellaneous Commands):
> 	Revive -exec­abort documentation and add example.
> 

Thanks, this is fine.

Re: PATCH: GDB/MI - implement -exec-abort

[Joel Brobecker]

Dennis,

On Thu, Oct 23, 2014 at 08:25:21PM +0000, Dennis Brueni wrote:
> 
> The -exec–abort command has been documented as the MI equivalent to the kill command since GDB 5.1, but it was never implemented.
> 
> This patch does that.
> 
> gdb/ChangeLog
> 2014-10-23  Dennis Brueni  <dbrueni@slickedit.com>
> 
> * mi/mi-cmds.c Add "exec-abort”
> * mi/mi-cmds.h Add prototype for mi_cmd_exec_abort
> * mi/mi-main.c Add mi_cmd_exec_abort (identical to kill_command minus prompting)
> 
> gdb/doc/ChangeLog
> 2014-10-23  Dennis Brueni  <dbrueni@slickedit.com>
> 
> * doc/gdb.texinfo Revive -exec–abort documentation, add example.

Sorry for the delay in reviewing your patch.

First things first, do you have an FSF copyright assignment on file?
I tried looking you up in the FSF records and couldn't find you.
Your patch is too big for us to be able to accept it. If you'd like
to be started on the process, please let me know, as it does take
a little bit of time to get through.

You didn't say how you tested your patch, and the headers of the diff
seem to indicate that the changes were made against GDB 7.8; instead,
we ask that patches be tested on top of our master branch, which is
the branch we use for development.

The formatting of your ChangeLog is not correct. Here is what they should
look like:

gdb/ChangeLog
2014-10-23  Dennis Brueni  <dbrueni@slickedit.com>

	* mi/mi-cmds.c (mi_cmds): Add "exec-abort” command.
	* mi/mi-cmds.h (mi_cmd_exec_abort):  Add prototype.
	* mi/mi-main.c (mi_cmd_exec_abort): New function.

gdb/doc/ChangeLog
2014-10-23  Dennis Brueni  <dbrueni@slickedit.com>

	* doc/gdb.texinfo (GDB/MI Miscellaneous Commands): Revive
        -exec-abort documentation, add example.

More explicitly, I made the following adjustements:

  - the contents should be indented by a tab (warning: _not_ 8 spaces);
  - sentences should start with a capital letter and end with a period;
  - the parts you touch (eg: function name, @node name for the doco,
    should be listed in between parenthesis;
  - there should be a colon before you start explaning what you change;
  - There is max line length soft limit of 74 characters, and a hard
    limit of 80.

I am not sure how you formatted this message to send the patch,
but a recommended way to work is to commit your change with a revision
log that serves both as revision log as well as email to be sent
here for us to review. That way, we see exactly what is proposed,
revision log included, and it also helps archeology to have
the what, where, how and why parts attach to each patch. And, as
a side-effect, the patch will also use diff's "-p" option, which
provides the name of the function each hunk is in, making review
of your patch a bit easier.

I realize that's a lot of info to process for such a small patch,
and I am sorry to hit you with this, but consistency is important
to us.

> +/* Stop the execution of the target. */
> +
> +void
> +mi_cmd_exec_abort (char *command, char **argv, int argc)

First of all, thank you _very much_ for thinking of adding an
introductory comment for the function you're adding! Not everyone
does that, and it's also a rule that we want everyone to follow...

In terms of the function's implementation, however, I would rather
you re-used the code in kill_command rather than reimplement it
entirely like it.

So, can you please do the following instead:

  - Extract the code from kill_command out to a subprogram called
    for instance kill_current_inferior, with one argument used
    to tell the function whether the user should be queried
    or not.

    In the function's command, please make it clear that it uses
    INFERIOR_PTID to determine which inferior to kill.
    "kill_current_inferior" alludes to the "current_inferior" function
    which returns the value of another global. Intuitively, the should
    be the same, but I do not remember why we have two anymore, and
    we don't have to worry about that for your patch.

  - Make both kill_command and mi_cmd_exec_abort both call
    kill_current_inferior.

Thank you,
-- 
Joel